Attachment 412966 Details for Bug 218455 – Patch

[patch] Patch

bug-218455-20201102134253.patch (text/plain), 2.93 KB, created by Chris Dumez on 2020-11-02 13:42:53 PST

(hide)

Description:

Filename:

MIME Type:

Creator: Chris Dumez

Created: 2020-11-02 13:42:53 PST

Size: 2.93 KB

patch

obsolete

>Subversion Revision: 269268
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index 4f3468d2471e054bf3b43aedb457b6c7aa2e13c9..9bf7af52180721bffdb43db1ef3eee4d416777e5 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,28 @@
>+2020-11-02  Chris Dumez  <cdumez@apple.com>
>+
>+        REGRESSION (r269227): Crash in WebCore::WorkerOrWorkletGlobalScope::prepareForDestruction
>+        https://bugs.webkit.org/show_bug.cgi?id=218455
>+        <rdar://problem/70963191>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Document::willBeRemovedFromFrame() may have the same PaintWorkletGlobalScope instance
>+        more than once in its m_paintWorkletGlobalScopes HashMap, under different keys. As a
>+        result, it may call PaintWorkletGlobalScope::prepareForDestruction() more than once
>+        on the same instance. This was causing issues because
>+        EventLoopTaskGroup::markAsReadyToStop(), when called the second time, would move the
>+        state from Stopped and back to ReadyToStop, which is unexpected.
>+
>+        To address the issue, EventLoopTaskGroup::markAsReadyToStop() now returns early if
>+        the state is "Stopped". Also, I added a boolean check in PaintWorkletGlobalScope's
>+        prepareForDestruction() to make sure we only do the work once per instance.
>+
>+        No new tests, covered by existing tests.
>+
>+        * dom/EventLoop.h:
>+        (WebCore::EventLoopTaskGroup::markAsReadyToStop):
>+        * worklets/PaintWorkletGlobalScope.h:
>+
> 2020-11-01  Darin Adler  <darin@apple.com>
> 
>         Start removing functions that implicitly use composed tree
>diff --git a/Source/WebCore/dom/EventLoop.h b/Source/WebCore/dom/EventLoop.h
>index c3312ba10b90525eaceb787e912b0231ab087689..b6183071d9a23bddcedbd37c5c3fa22021af3105 100644
>--- a/Source/WebCore/dom/EventLoop.h
>+++ b/Source/WebCore/dom/EventLoop.h
>@@ -133,7 +133,7 @@ public:
>     // until all groups in this event loop are ready to stop.
>     void markAsReadyToStop()
>     {
>-        if (isReadyToStop())
>+        if (isReadyToStop() || isStoppedPermanently())
>             return;
> 
>         m_state = State::ReadyToStop;
>diff --git a/Source/WebCore/worklets/PaintWorkletGlobalScope.h b/Source/WebCore/worklets/PaintWorkletGlobalScope.h
>index 3b713dc3f4edc1ba742de3c9ead0b500c6f742c6..42a75b4582476eeda28e9a42b96b11cdbac6e53a 100644
>--- a/Source/WebCore/worklets/PaintWorkletGlobalScope.h
>+++ b/Source/WebCore/worklets/PaintWorkletGlobalScope.h
>@@ -65,6 +65,10 @@ public:
> 
>     void prepareForDestruction() final
>     {
>+        if (m_hasPreparedForDestruction)
>+            return;
>+        m_hasPreparedForDestruction = true;
>+
>         {
>             auto locker = holdLock(paintDefinitionLock());
>             paintDefinitionMap().clear();
>@@ -87,6 +91,7 @@ private:
> 
>     HashMap<String, std::unique_ptr<PaintDefinition>> m_paintDefinitionMap;
>     Lock m_paintDefinitionLock;
>+    bool m_hasPreparedForDestruction { false };
> };
> 
> } // namespace WebCore

Actions: View | Formatted Diff | Diff

Attachments on bug 218455: 412966