Title: [269275] trunk/Source/WebCore
- Revision
- 269275
- Author
- [email protected]
- Date
- 2020-11-02 14:24:13 -0800 (Mon, 02 Nov 2020)
Log Message
REGRESSION (r269227): Crash in WebCore::WorkerOrWorkletGlobalScope::prepareForDestruction
https://bugs.webkit.org/show_bug.cgi?id=218455
<rdar://problem/70963191>
Reviewed by Geoffrey Garen.
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:
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (269274 => 269275)
--- trunk/Source/WebCore/ChangeLog 2020-11-02 22:07:00 UTC (rev 269274)
+++ trunk/Source/WebCore/ChangeLog 2020-11-02 22:24:13 UTC (rev 269275)
@@ -1,3 +1,28 @@
+2020-11-02 Chris Dumez <[email protected]>
+
+ REGRESSION (r269227): Crash in WebCore::WorkerOrWorkletGlobalScope::prepareForDestruction
+ https://bugs.webkit.org/show_bug.cgi?id=218455
+ <rdar://problem/70963191>
+
+ Reviewed by Geoffrey Garen.
+
+ 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-02 Alan Bujtas <[email protected]>
Unreviewed, reverting r269245.
Modified: trunk/Source/WebCore/dom/EventLoop.h (269274 => 269275)
--- trunk/Source/WebCore/dom/EventLoop.h 2020-11-02 22:07:00 UTC (rev 269274)
+++ trunk/Source/WebCore/dom/EventLoop.h 2020-11-02 22:24:13 UTC (rev 269275)
@@ -133,7 +133,7 @@
// until all groups in this event loop are ready to stop.
void markAsReadyToStop()
{
- if (isReadyToStop())
+ if (isReadyToStop() || isStoppedPermanently())
return;
m_state = State::ReadyToStop;
Modified: trunk/Source/WebCore/worklets/PaintWorkletGlobalScope.h (269274 => 269275)
--- trunk/Source/WebCore/worklets/PaintWorkletGlobalScope.h 2020-11-02 22:07:00 UTC (rev 269274)
+++ trunk/Source/WebCore/worklets/PaintWorkletGlobalScope.h 2020-11-02 22:24:13 UTC (rev 269275)
@@ -65,6 +65,10 @@
void prepareForDestruction() final
{
+ if (m_hasPreparedForDestruction)
+ return;
+ m_hasPreparedForDestruction = true;
+
{
auto locker = holdLock(paintDefinitionLock());
paintDefinitionMap().clear();
@@ -87,6 +91,7 @@
HashMap<String, std::unique_ptr<PaintDefinition>> m_paintDefinitionMap;
Lock m_paintDefinitionLock;
+ bool m_hasPreparedForDestruction { false };
};
} // namespace WebCore
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
