Attachment 340181 Details for Bug 157068 – Patch

[patch] Patch

bug-157068-20180511113901.patch (text/plain), 7.48 KB, created by youenn fablet on 2018-05-11 02:39:03 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: youenn fablet

Created: 2018-05-11 02:39:03 PDT

Size: 7.48 KB

patch

obsolete

>Subversion Revision: 231552
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index 59054656b423898737048596640c23a9b137fcc3..2895887912decc6116a1d78fa067b82cf30ec095 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,23 @@
>+2018-05-11  Youenn Fablet  <youenn@apple.com>
>+
>+        imported/w3c/web-platform-tests/fetch/nosniff/importscripts.html is crashing on debug builds
>+        https://bugs.webkit.org/show_bug.cgi?id=157068
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Make Worker uncollectable if it has an onmessage event handler.
>+        Remove previous code that was making the Worker uncollectable if messages were in the pipe to be sent.
>+        Covered by unflaked test.
>+
>+        * workers/WorkerMessagingProxy.cpp:
>+        (WebCore::WorkerMessagingProxy::postMessageToWorkerGlobalScope):
>+        (WebCore::WorkerMessagingProxy::workerThreadCreated):
>+        (WebCore::WorkerMessagingProxy::confirmMessageFromWorkerObject):
>+        (WebCore::WorkerMessagingProxy::reportPendingActivity):
>+        (WebCore::WorkerMessagingProxy::hasPendingActivity const):
>+        (WebCore::WorkerMessagingProxy::reportPendingActivityInternal): Deleted.
>+        * workers/WorkerMessagingProxy.h:
>+
> 2018-05-09  Youenn Fablet  <youenn@apple.com>
> 
>         Allow WebResourceLoader to cancel a load served from a service worker
>diff --git a/Source/WebCore/workers/WorkerMessagingProxy.cpp b/Source/WebCore/workers/WorkerMessagingProxy.cpp
>index 1540d77d5be78ef7b1b69b4e2158a771c6633adc..f70eaa699dc37756fca03219459ad2992d33c828 100644
>--- a/Source/WebCore/workers/WorkerMessagingProxy.cpp
>+++ b/Source/WebCore/workers/WorkerMessagingProxy.cpp
>@@ -122,10 +122,10 @@ void WorkerMessagingProxy::postMessageToWorkerGlobalScope(MessageWithMessagePort
>     });
> 
>     if (m_workerThread) {
>-        ++m_unconfirmedMessageCount;
>         m_workerThread->runLoop().postTask(WTFMove(task));
>-    } else
>-        m_queuedEarlyTasks.append(std::make_unique<ScriptExecutionContext::Task>(WTFMove(task)));
>+        return;
>+    }
>+    m_queuedEarlyTasks.append(std::make_unique<ScriptExecutionContext::Task>(WTFMove(task)));
> }
> 
> void WorkerMessagingProxy::postTaskToLoader(ScriptExecutionContext::Task&& task)
>@@ -194,8 +194,6 @@ void WorkerMessagingProxy::workerThreadCreated(DedicatedWorkerThread& workerThre
>         // Worker.terminate() could be called from JS before the thread was created.
>         m_workerThread->stop(nullptr);
>     } else {
>-        ASSERT(!m_unconfirmedMessageCount);
>-        m_unconfirmedMessageCount = m_queuedEarlyTasks.size();
>         m_workerThreadHadPendingActivity = true; // Worker initialization means a pending activity.
> 
>         auto queuedEarlyTasks = WTFMove(m_queuedEarlyTasks);
>@@ -274,30 +272,26 @@ void WorkerMessagingProxy::terminateWorkerGlobalScope()
> void WorkerMessagingProxy::confirmMessageFromWorkerObject(bool hasPendingActivity)
> {
>     m_scriptExecutionContext->postTask([this, hasPendingActivity] (ScriptExecutionContext&) {
>-        reportPendingActivityInternal(true, hasPendingActivity);
>+        m_workerThreadHadPendingActivity = hasPendingActivity;
>     });
> }
> 
> void WorkerMessagingProxy::reportPendingActivity(bool hasPendingActivity)
> {
>     m_scriptExecutionContext->postTask([this, hasPendingActivity] (ScriptExecutionContext&) {
>-        reportPendingActivityInternal(false, hasPendingActivity);
>+        m_workerThreadHadPendingActivity = hasPendingActivity;
>     });
> }
> 
>-void WorkerMessagingProxy::reportPendingActivityInternal(bool confirmingMessage, bool hasPendingActivity)
>+bool WorkerMessagingProxy::hasPendingActivity() const
> {
>-    if (confirmingMessage && !m_askedToTerminate) {
>-        ASSERT(m_unconfirmedMessageCount);
>-        --m_unconfirmedMessageCount;
>-    }
>+    if (m_askedToTerminate)
>+        return false;
> 
>-    m_workerThreadHadPendingActivity = hasPendingActivity;
>-}
>+    if (m_workerObject && m_workerObject->hasEventListeners(eventNames().messageEvent))
>+        return true;
> 
>-bool WorkerMessagingProxy::hasPendingActivity() const
>-{
>-    return (m_unconfirmedMessageCount || m_workerThreadHadPendingActivity) && !m_askedToTerminate;
>+    return m_workerThreadHadPendingActivity;
> }
> 
> } // namespace WebCore
>diff --git a/Source/WebCore/workers/WorkerMessagingProxy.h b/Source/WebCore/workers/WorkerMessagingProxy.h
>index 4eff2fa4a243ecfc14f4645ead6cff610ea44bec..a16b0be1fb7d21a2af31c5e7ebe770de2fac2f66 100644
>--- a/Source/WebCore/workers/WorkerMessagingProxy.h
>+++ b/Source/WebCore/workers/WorkerMessagingProxy.h
>@@ -80,7 +80,6 @@ private:
>     bool askedToTerminate() const { return m_askedToTerminate; }
> 
>     void workerGlobalScopeDestroyedInternal();
>-    void reportPendingActivityInternal(bool confirmingMessage, bool hasPendingActivity);
>     Worker* workerObject() const { return m_workerObject; }
> 
>     RefPtr<ScriptExecutionContext> m_scriptExecutionContext;
>@@ -89,7 +88,6 @@ private:
>     bool m_mayBeDestroyed { false };
>     RefPtr<DedicatedWorkerThread> m_workerThread;
> 
>-    unsigned m_unconfirmedMessageCount { 0 }; // Unconfirmed messages from worker object to worker thread.
>     bool m_workerThreadHadPendingActivity { false }; // The latest confirmation from worker thread reported that it was still active.
> 
>     bool m_askedToTerminate { false };
>diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
>index 738688140fa32b745b533373daa077b90e613f02..a73fd608f1ed9f914671d6b56a640d5f4329d46f 100644
>--- a/LayoutTests/ChangeLog
>+++ b/LayoutTests/ChangeLog
>@@ -1,3 +1,13 @@
>+2018-05-11  Youenn Fablet  <youenn@apple.com>
>+
>+        imported/w3c/web-platform-tests/fetch/nosniff/importscripts.html is crashing on debug builds
>+        https://bugs.webkit.org/show_bug.cgi?id=157068
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        * TestExpectations:
>+        * platform/mac-wk1/TestExpectations:
>+
> 2018-05-11  Youenn Fablet  <youenn@apple.com>
> 
>         Rebase some fetch API tests
>diff --git a/LayoutTests/TestExpectations b/LayoutTests/TestExpectations
>index 8473ca5afc6856d718ee980021097c207f3409ac..eb0cf5aaa37d4ec83c4a128a537b2402ed3af770 100644
>--- a/LayoutTests/TestExpectations
>+++ b/LayoutTests/TestExpectations
>@@ -1857,8 +1857,6 @@ webkit.org/b/183848 imported/mozilla/css-animations/test_keyframeeffect-getkeyfr
> 
> webkit.org/b/177440 imported/w3c/web-platform-tests/html/browsers/origin/relaxing-the-same-origin-restriction/document_domain_setter_null.tentative.html [ Pass Failure ]
> 
>-webkit.org/b/157068 [ Debug ] imported/w3c/web-platform-tests/fetch/nosniff/importscripts.html [ Pass Crash ]
>-
> # Application Manifest tests
> webkit.org/b/153152 http/tests/security/contentSecurityPolicy/manifest-src-allowed.html [ Skip ]
> webkit.org/b/153152 http/tests/security/contentSecurityPolicy/manifest-src-blocked.html [ Skip ]
>diff --git a/LayoutTests/platform/mac-wk1/TestExpectations b/LayoutTests/platform/mac-wk1/TestExpectations
>index df0f144c9b130c01a16dd411d159c014aff942a1..490dfc817c362fed326565bf2b1161a7b41b184c 100644
>--- a/LayoutTests/platform/mac-wk1/TestExpectations
>+++ b/LayoutTests/platform/mac-wk1/TestExpectations
>@@ -450,8 +450,6 @@ webkit.org/b/175195 media/modern-media-controls/css/webkit-cursor-visibility-aut
> webkit.org/b/168265 media/modern-media-controls/forward-button/forward-button.html [ Pass Timeout ]
> webkit.org/b/182571 media/modern-media-controls/tracks-support/tracks-support-show-panel-fullscreen.html [ Pass Timeout ]
> 
>-webkit.org/b/173432 [ Debug ] imported/w3c/web-platform-tests/fetch/nosniff/importscripts.html [ Pass Crash ]
>-
> # Requires WK2 loading support
> http/wpt/fetch/dnt-header-after-redirection.html [ Skip ]
>

Actions: View | Formatted Diff | Diff

Attachments on bug 157068: 329570 | 330374 | 330379 | 330399 | 330402 | 330403 | 330406 | 330410 | 330412 | 340181 | 340188 | 340699 | 340701 | 344632 | 344676