Attachment 119324 Details for Bug 74558 – Patch

[patch] Patch

bug-74558-20111214163642.patch (text/plain), 9.73 KB, created by David Levin on 2011-12-14 16:36:43 PST

(hide)

Description:

Filename:

MIME Type:

Creator: David Levin

Created: 2011-12-14 16:36:43 PST

Size: 9.73 KB

patch

obsolete

>Subversion Revision: 102483
>diff --git a/Source/WebKit/chromium/ChangeLog b/Source/WebKit/chromium/ChangeLog
>index 5750c688193d85ac5edb39138ba57d97ab2b28aa..0857db2eb8138ed31b3cf516a03e966efae40e1f 100644
>--- a/Source/WebKit/chromium/ChangeLog
>+++ b/Source/WebKit/chromium/ChangeLog
>@@ -1,3 +1,23 @@
>+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 NOBODY (OOPS!).
>+
>+        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 unneccessary String().
>+
> 2011-12-09  Dirk Pranke  <dpranke@chromium.org>
> 
>         chromium: visual studio projects are busted when build_webkit_exes_from_webkit_gyp==0
>diff --git a/Source/WebKit/chromium/src/DatabaseObserver.cpp b/Source/WebKit/chromium/src/DatabaseObserver.cpp
>index e5f990906847e82bf4fbf007338d80785f79c561..0dff2fb4ee2f51ddfbb6b103c4825f17a60cddac 100644
>--- a/Source/WebKit/chromium/src/DatabaseObserver.cpp
>+++ b/Source/WebKit/chromium/src/DatabaseObserver.cpp
>@@ -64,7 +64,7 @@ static const char allowDatabaseMode[] = "allowDatabaseMode";
> // call back to the worker context.
> 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));
>     }
>@@ -82,33 +82,31 @@ public:
>     }
> 
>     // 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)));
>+                               name, displayName, estimatedSize,
>+                               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)
>+    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)
>     {
>-        if (commonClient)
>-            bridge->signalCompleted(commonClient->allowDatabase(frame, name, displayName, estimatedSize));
>-        else
>-            bridge->signalCompleted(false);
>+        bool allowDatabase = commonClient ? commonClient->allowDatabase(frame, name, displayName, estimatedSize) : false;
>+        bridge->signalCompleted(mode, allowDatabase);
>     }
> 
>     static void didComplete(WebCore::ScriptExecutionContext* context, PassRefPtr<AllowDatabaseMainThreadBridge> bridge, bool result)
>@@ -119,7 +117,6 @@ private:
>     bool m_result;
>     Mutex m_mutex;
>     WebCore::WorkerLoaderProxy* m_workerLoaderProxy;
>-    WTF::String m_mode;
> };
> 
> bool allowDatabaseForWorker(NewWebCommonWorkerClient* commonClient, WebFrame* frame, const WebString& name, const WebString& displayName, unsigned long estimatedSize)
>@@ -134,7 +131,7 @@ bool allowDatabaseForWorker(NewWebCommonWorkerClient* commonClient, WebFrame* fr
>     String mode = allowDatabaseMode;
>     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.
>     if (runLoop.runInMode(workerContext, mode) == MessageQueueTerminated) {
>diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
>index ef7e4ffb80603b9b55cb788e356a4e5cad02d14e..dff569e986c0b38308430373ecc1ffb339bd50b7 100644
>--- a/LayoutTests/ChangeLog
>+++ b/LayoutTests/ChangeLog
>@@ -1,3 +1,16 @@
>+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 NOBODY (OOPS!).
>+
>+        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-09  Adam Klein  <adamk@chromium.org>
> 
>         [MutationObservers] Add test for deliver-at-end-of-task semantics
>diff --git a/LayoutTests/http/tests/workers/interrupt-database-sync-open-crash.html b/LayoutTests/http/tests/workers/interrupt-database-sync-open-crash.html
>new file mode 100644
>index 0000000000000000000000000000000000000000..088dd73e46e6434beed604df718a2ec05c5513aa
>--- /dev/null
>+++ b/LayoutTests/http/tests/workers/interrupt-database-sync-open-crash.html
>@@ -0,0 +1,72 @@
>+<html>
>+<head>
>+<script src='resources/worker-util.js'></script>
>+<script>
>+var remainingIterations = 10;
>+var workersStarted;
>+var workersClosed;
>+var workers = new Array(30);
>+
>+function nextIteration()
>+{
>+    log('Remaining iterations: ' +  remainingIterations);
>+    waitUntilWorkerThreadsExit(startWorkers)
>+}
>+
>+function startWorkers()
>+{
>+    if (!remainingIterations) {
>+        done();
>+        return;
>+    }
>+    remainingIterations--;
>+
>+    workersStarted = 0;
>+    workersClosed = 0;
>+    for (var i = 0; i < workers.length; ++i) {
>+        workers[i] = new Worker('resources/open-database-sync.js?arg=' + i)
>+        workers[i].onmessage = waitForAllWorkersToStart;
>+    }
>+}
>+
>+// Do our best to try to interrupt the databse open
>+// call by waiting for the worker to start and then
>+// telling it to do the open database call (and
>+// then terminate the worker).
>+function waitForAllWorkersToStart()
>+{
>+    workersStarted++;
>+    if (workersStarted < workers.length)
>+        return;
>+
>+    for (var i = 0; i < workers.length; ++i)
>+        workers[i].postMessage('openDatabaseSync');
>+
>+    setTimeout('closeWorker()', 0);
>+}
>+
>+function closeWorker()
>+{
>+    workers[workersClosed].terminate();
>+    workersClosed++;
>+    if (workersClosed < workers.length)
>+        setTimeout('closeWorker()', 3);
>+    else
>+        nextIteration();
>+}
>+
>+function runTest()
>+{
>+    if (window.layoutTestController) {
>+        layoutTestController.dumpAsText();
>+        layoutTestController.waitUntilDone();
>+    }
>+    nextIteration();
>+}
>+</script>
>+</head>
>+<body onload='runTest()'>
>+<div id='result'>
>+</div>
>+</body>
>+</html>
>diff --git a/LayoutTests/http/tests/workers/resources/open-database-sync.js b/LayoutTests/http/tests/workers/resources/open-database-sync.js
>new file mode 100644
>index 0000000000000000000000000000000000000000..1e7be941be94320a6f3a9f73c72220360c66655d
>--- /dev/null
>+++ b/LayoutTests/http/tests/workers/resources/open-database-sync.js
>@@ -0,0 +1,7 @@
>+postMessage('started');
>+
>+onmessage = function(evt)
>+{
>+    if (evt.data == 'openDatabaseSync')
>+        openDatabaseSync('', '', '', 1);
>+}

Actions: View | Formatted Diff | Diff

Attachments on bug 74558: 119324 | 119360 | 119361