Changeset 102894 in webkit

Timestamp:

Dec 14, 2011 10:51:20 PM (12 years ago)

Author:

levin@chromium.org

Message:

[chromium] DatabaseObserver needs threadsafe fixes and other clean-up.
​https://bugs.webkit.org/show_bug.cgi?id=74558

Reviewed by Dmitry Titov.

Source/WebKit/chromium:

The important part of this fix is the removal of AllowCrossThreadAccess so
that ref counting happens appropriately.

Minor clean up throughout: Removed unnecessary WTF prefix in many
of these places and unnecessary String().

• src/DatabaseObserver.cpp:

(WebKit::AllowDatabaseMainThreadBridge::create):
(WebKit::AllowDatabaseMainThreadBridge::signalCompleted): Get rid of
AllowCrossThreadAccess so that "this" get ref counted and remove mode from being
a member variable since a String in a ThreadSafeRefCounted class is bad.
(WebKit::AllowDatabaseMainThreadBridge::AllowDatabaseMainThreadBridge): Ditto.
(WebKit::AllowDatabaseMainThreadBridge::allowDatabaseTask): Pass through mode
and minor code cleaning to do only one call to signalCompleted.
(WebKit::allowDatabaseForWorker): Just removed unnecessary String()'s.

LayoutTests:

Added a test which would fail without some of these fixes.

• http/tests/workers/interrupt-database-sync-open-crash.html: Added.
• http/tests/workers/resources/open-database-sync.js: Added.

(onmessage):

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/http/tests/workers/interrupt-database-sync-open-crash.html (added)
• LayoutTests/http/tests/workers/resources/open-database-sync.js (added)
• Source/WebKit/chromium/ChangeLog (modified) (1 diff)
• Source/WebKit/chromium/src/DatabaseObserver.cpp (modified) (4 diffs)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2011-12-14  David Levin  <levin@chromium.org>
+
+        [chromium] DatabaseObserver needs threadsafe fixes and other clean-up.
+        https://bugs.webkit.org/show_bug.cgi?id=74558
+
+        Reviewed by Dmitry Titov.
+
+        Added a test which would fail without some of these fixes.
+
+        * http/tests/workers/interrupt-database-sync-open-crash.html: Added.
+        * http/tests/workers/resources/open-database-sync.js: Added.
+        (onmessage):
+
 2011-12-14  Kent Tamura  <tkent@chromium.org>
 

trunk/Source/WebKit/chromium/ChangeLog

@@ +1 @@
+2011-12-14  David Levin  <levin@chromium.org>
+
+        [chromium] DatabaseObserver needs threadsafe fixes and other clean-up.
+        https://bugs.webkit.org/show_bug.cgi?id=74558
+
+        Reviewed by Dmitry Titov.
+
+        The important part of this fix is the removal of AllowCrossThreadAccess so
+        that ref counting happens appropriately.
+
+        Minor clean up throughout: Removed unnecessary WTF prefix in many
+        of these places and unnecessary String().
+
+        * src/DatabaseObserver.cpp:
+        (WebKit::AllowDatabaseMainThreadBridge::create):
+        (WebKit::AllowDatabaseMainThreadBridge::signalCompleted): Get rid of
+        AllowCrossThreadAccess so that "this" get ref counted and remove mode from being
+        a member variable since a String in a ThreadSafeRefCounted class is bad.
+        (WebKit::AllowDatabaseMainThreadBridge::AllowDatabaseMainThreadBridge): Ditto.
+        (WebKit::AllowDatabaseMainThreadBridge::allowDatabaseTask): Pass through mode
+        and minor code cleaning to do only one call to signalCompleted.
+        (WebKit::allowDatabaseForWorker): Just removed unnecessary String()'s.
+
 2011-12-14  Tony Chang  <tony@chromium.org>
 

trunk/Source/WebKit/chromium/src/DatabaseObserver.cpp

@@ -65 +65 @@
 class AllowDatabaseMainThreadBridge : public ThreadSafeRefCounted<AllowDatabaseMainThreadBridge> {
 public:
-    static PassRefPtr<AllowDatabaseMainThreadBridge> create(WebCore::WorkerLoaderProxy* workerLoaderProxy, const WTF::String& mode, NewWebCommonWorkerClient* commonClient, WebFrame* frame, const WTF::String& name, const WTF::String& displayName, unsigned long estimatedSize)
+    static PassRefPtr<AllowDatabaseMainThreadBridge> create(WebCore::WorkerLoaderProxy* workerLoaderProxy, const String& mode, NewWebCommonWorkerClient* commonClient, WebFrame* frame, const String& name, const String& displayName, unsigned long estimatedSize)
     {
         return adoptRef(new AllowDatabaseMainThreadBridge(workerLoaderProxy, mode, commonClient, frame, name, displayName, estimatedSize));
@@ -83 +83 @@
 
     // This method is invoked on the main thread.
-    void signalCompleted(bool result)
+    void signalCompleted(const String& mode, bool result)
     {
         MutexLocker locker(m_mutex);
-        if (m_workerLoaderProxy)
-            m_workerLoaderProxy->postTaskForModeToWorkerContext(
-                createCallbackTask(&didComplete, WebCore::AllowCrossThreadAccess(this), result), 
-                m_mode);
+        if (!m_workerLoaderProxy)
+            return;
+        m_workerLoaderProxy->postTaskForModeToWorkerContext(createCallbackTask(&didComplete, this, result), mode);
     }
 
 private:
-    AllowDatabaseMainThreadBridge(WebCore::WorkerLoaderProxy* workerLoaderProxy, const WTF::String& mode, NewWebCommonWorkerClient* commonClient, WebFrame* frame, const WTF::String& name, const WTF::String& displayName, unsigned long estimatedSize)
+    AllowDatabaseMainThreadBridge(WebCore::WorkerLoaderProxy* workerLoaderProxy, const String& mode, NewWebCommonWorkerClient* commonClient, WebFrame* frame, const String& name, const String& displayName, unsigned long estimatedSize)
         : m_workerLoaderProxy(workerLoaderProxy)
-        , m_mode(mode)
     {
         WebWorkerBase::dispatchTaskToMainThread(
-            createCallbackTask(&allowDatabaseTask, WebCore::AllowCrossThreadAccess(commonClient),
+            createCallbackTask(&allowDatabaseTask, mode, WebCore::AllowCrossThreadAccess(commonClient),
                                WebCore::AllowCrossThreadAccess(frame),
-                               String(name), String(displayName), estimatedSize,
-                               WebCore::AllowCrossThreadAccess(this)));
-    }
-
-    static void allowDatabaseTask(WebCore::ScriptExecutionContext* context, NewWebCommonWorkerClient* commonClient, WebFrame* frame, const WTF::String name, const WTF::String displayName, unsigned long estimatedSize, PassRefPtr<AllowDatabaseMainThreadBridge> bridge)
-    {
-        if (commonClient)
-            bridge->signalCompleted(commonClient->allowDatabase(frame, name, displayName, estimatedSize));
-        else
-            bridge->signalCompleted(false);
+                               name, displayName, estimatedSize,
+                               this));
+    }
+
+    static void allowDatabaseTask(WebCore::ScriptExecutionContext* context, const String mode, NewWebCommonWorkerClient* commonClient, WebFrame* frame, const String name, const String displayName, unsigned long estimatedSize, PassRefPtr<AllowDatabaseMainThreadBridge> bridge)
+    {
+        bool allowDatabase = commonClient ? commonClient->allowDatabase(frame, name, displayName, estimatedSize) : false;
+        bridge->signalCompleted(mode, allowDatabase);
     }
 
@@ -120 +116 @@
     Mutex m_mutex;
     WebCore::WorkerLoaderProxy* m_workerLoaderProxy;
-    WTF::String m_mode;
 };
 
@@ -135 +130 @@
     mode.append(String::number(runLoop.createUniqueId()));
 
-    RefPtr<AllowDatabaseMainThreadBridge> bridge = AllowDatabaseMainThreadBridge::create(workerLoaderProxy, mode, commonClient, frame, String(name), String(displayName), estimatedSize);
+    RefPtr<AllowDatabaseMainThreadBridge> bridge = AllowDatabaseMainThreadBridge::create(workerLoaderProxy, mode, commonClient, frame, name, displayName, estimatedSize);
 
     // Either the bridge returns, or the queue gets terminated.