Changeset 141431 in webkit

Timestamp:

Jan 31, 2013 10:12:23 AM (11 years ago)

Author:

mark.lam@apple.com

Message:

DatabaseContext needs to outlive the ScriptExecutionContext.
​https://bugs.webkit.org/show_bug.cgi?id=108355.

Reviewed by Geoffrey Garen.

Added a RefPtr<DatabaseContext> in ScriptExecutionContext to keep the
DatabaseContext alive until after ScriptExecutionContext destructs.

No new tests.

• Modules/webdatabase/DatabaseContext.cpp:

(WebCore::DatabaseContext::DatabaseContext):

• dom/ScriptExecutionContext.cpp:

(WebCore::ScriptExecutionContext::setDatabaseContext):

• dom/ScriptExecutionContext.h:

(ScriptExecutionContext):

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• Modules/webdatabase/DatabaseContext.cpp (modified) (2 diffs)
• dom/ScriptExecutionContext.cpp (modified) (2 diffs)
• dom/ScriptExecutionContext.h (modified) (3 diffs)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2013-01-30  Mark Lam  <mark.lam@apple.com>
+
+        DatabaseContext needs to outlive the ScriptExecutionContext.
+        https://bugs.webkit.org/show_bug.cgi?id=108355.
+
+        Reviewed by Geoffrey Garen.
+
+        Added a RefPtr<DatabaseContext> in ScriptExecutionContext to keep the
+        DatabaseContext alive until after ScriptExecutionContext destructs.
+
+        No new tests.
+
+        * Modules/webdatabase/DatabaseContext.cpp:
+        (WebCore::DatabaseContext::DatabaseContext):
+        * dom/ScriptExecutionContext.cpp:
+        (WebCore::ScriptExecutionContext::setDatabaseContext):
+        * dom/ScriptExecutionContext.h:
+        (ScriptExecutionContext):
+
 2013-01-31  Grzegorz Czajkowski  <g.czajkowski@samsung.com>
 

trunk/Source/WebCore/Modules/webdatabase/DatabaseContext.cpp

@@ -40 +40 @@
 #include "Page.h"
 #include "SchemeRegistry.h"
+#include "ScriptExecutionContext.h"
 #include "SecurityOrigin.h"
 #include "Settings.h"
 
 namespace WebCore {
+
+// How the DatabaseContext Life-Cycle works?
+// ========================================
+// ... in other words, who's keeping the DatabaseContext alive and how long does
+// it need to stay alive?
+//
+// The DatabaseContext is referenced from RefPtrs in:
+// 1. ScriptExecutionContext
+// 2. Database
+//
+// At Birth:
+// ========
+// We create a DatabaseContext only when there is a need i.e. the script tries to
+// open a Database via DatabaseManager::openDatabase().
+//
+// The DatabaseContext constructor will call setDatabaseContext() on the
+// the ScriptExecutionContext. This sets the RefPtr in the ScriptExecutionContext
+// for keeping the DatabaseContext alive. Since the DatabaseContext is only
+// created from the script thread, it is safe for the constructor to call
+// ScriptExecutionContext::setDatabaseContext().
+//
+// Once a DatabaseContext is associated with a ScriptExecutionContext, it will
+// live until after the ScriptExecutionContext destructs. This is true even if
+// we don't succeed in opening any Databases for that context. When we do
+// succeed in opening Databases for this ScriptExecutionContext, the Database
+// will re-use the same DatabaseContext.
+//
+// At Shutdown:
+// ===========
+// During shutdown, the DatabaseContext needs to:
+// 1. "outlive" the ScriptExecutionContext.
+//    - This is needed because the DatabaseContext needs to remove itself from the
+//      ScriptExecutionContext's ActiveDOMObject list and ContextDestructionObserver
+//      list. This removal needs to be executed on the script's thread. Hence, we
+//      rely on the ScriptExecutionContext's shutdown process to call
+//      stop() and contextDestroyed() to give us a chance to clean these up from
+//      the script thread.
+//
+// 2. "outlive" the Databases.
+//    - This is because they may make use of the DatabaseContext to execute a close
+//      task and shutdown in an orderly manner. When the Databases are destructed,
+//      they will deref the DatabaseContext from the DatabaseThread.
+//
+// During shutdown, the ScriptExecutionContext is shutting down on the script thread
+// while the Databases are shutting down on the DatabaseThread. Hence, there can be
+// a race condition as to whether the ScriptExecutionContext or the Databases
+// destruct first.
+//
+// The RefPtrs in the Databases and ScriptExecutionContext will ensure that the
+// DatabaseContext will outlive both regardless of which of the 2 destructs first.
+
 
 DatabaseContext::DatabaseContext(ScriptExecutionContext* context)
@@ -53 +105 @@
     // ActiveDOMObject expects this to be called to set internal flags.
     suspendIfNeeded();
+
+    context->setDatabaseContext(this);
 
     // For debug accounting only. We must do this before we register the

trunk/Source/WebCore/dom/ScriptExecutionContext.cpp

@@ -54 +54 @@
 #endif
 
+#if ENABLE(SQL_DATABASE)
+#include "DatabaseContext.h"
+#endif
+
 namespace WTF {
 
@@ -443 +447 @@
 #endif
 
+#if ENABLE(SQL_DATABASE)
+void ScriptExecutionContext::setDatabaseContext(DatabaseContext* databaseContext)
+{
+    ASSERT(!m_databaseContext);
+    m_databaseContext = databaseContext;
+}
+#endif
+
 } // namespace WebCore

trunk/Source/WebCore/dom/ScriptExecutionContext.h

@@ -52 +52 @@
 
 class CachedScript;
+class DatabaseContext;
 class DOMTimer;
 class EventListener;
@@ -171 +172 @@
 
     virtual void reportMemoryUsage(MemoryObjectInfo*) const OVERRIDE;
+
+#if ENABLE(SQL_DATABASE)
+    void setDatabaseContext(DatabaseContext*);
+#endif
 
 protected:
@@ -228 +233 @@
     RefPtr<FileThread> m_fileThread;
 #endif
+
+#if ENABLE(SQL_DATABASE)
+    RefPtr<DatabaseContext> m_databaseContext;
+#endif
 };