[webkit-changes] [255953] trunk/Source/WebCore

Thu, 06 Feb 2020 07:56:14 -0800

Title: [255953] trunk/Source/WebCore
Revision
255953
Author
[email protected]
Date
2020-02-06 07:56:02 -0800 (Thu, 06 Feb 2020)

Log Message

[Web Animations] Ensure all timelines are detached from their document
https://bugs.webkit.org/show_bug.cgi?id=207331
<rdar://problem/59210306>

Patch by Antoine Quint <[email protected]> on 2020-02-06
Reviewed by Dean Jackson.

We recently added a WeakHashSet<DocumentTimeline> m_timelines member to Document and added code to ~DocumentTimeline
to remove themselves from their Document's m_timelines. However, Document::prepareForDestruction() would call
DocumentTimeline::detachFromDocument() only for the main timeline and neglect to do the same for any additional
timelines that may have been created with the DocumentTimeline constructor.

We now cleanly call DocumentTimeline::detachFromDocument() for all items in a Document's m_timelines, which has the
effect of clearing the Document <> DocumentTimeline relationship since DocumentTimeline::detachFromDocument() now
calls Document::removeTimeline().

Finally, we now call DocumentTimeline::detachFromDocument() from the DocumentTimeline destructor to ensure that timelines
that were created purely in JS but got garbage-collected are no longer referenced from the Document still.

* animation/DocumentTimeline.cpp:
(WebCore::DocumentTimeline::~DocumentTimeline):
(WebCore::DocumentTimeline::detachFromDocument):
(WebCore::DocumentTimeline::cacheCurrentTime):
* dom/Document.cpp:
(WebCore::Document::prepareForDestruction):

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (255952 => 255953)


--- trunk/Source/WebCore/ChangeLog	2020-02-06 15:12:19 UTC (rev 255952)
+++ trunk/Source/WebCore/ChangeLog	2020-02-06 15:56:02 UTC (rev 255953)
@@ -1,3 +1,30 @@
+2020-02-06  Antoine Quint  <[email protected]>
+
+        [Web Animations] Ensure all timelines are detached from their document
+        https://bugs.webkit.org/show_bug.cgi?id=207331
+        <rdar://problem/59210306>
+
+        Reviewed by Dean Jackson.
+
+        We recently added a WeakHashSet<DocumentTimeline> m_timelines member to Document and added code to ~DocumentTimeline
+        to remove themselves from their Document's m_timelines. However, Document::prepareForDestruction() would call
+        DocumentTimeline::detachFromDocument() only for the main timeline and neglect to do the same for any additional
+        timelines that may have been created with the DocumentTimeline constructor.
+
+        We now cleanly call DocumentTimeline::detachFromDocument() for all items in a Document's m_timelines, which has the
+        effect of clearing the Document <> DocumentTimeline relationship since DocumentTimeline::detachFromDocument() now
+        calls Document::removeTimeline().
+
+        Finally, we now call DocumentTimeline::detachFromDocument() from the DocumentTimeline destructor to ensure that timelines
+        that were created purely in JS but got garbage-collected are no longer referenced from the Document still.
+
+        * animation/DocumentTimeline.cpp:
+        (WebCore::DocumentTimeline::~DocumentTimeline):
+        (WebCore::DocumentTimeline::detachFromDocument):
+        (WebCore::DocumentTimeline::cacheCurrentTime):
+        * dom/Document.cpp:
+        (WebCore::Document::prepareForDestruction):
+
 2020-02-06  Adrian Perez de Castro  <[email protected]>
 
         [Cairo] Do not use old-style GNU field initializers

Modified: trunk/Source/WebCore/animation/DocumentTimeline.cpp (255952 => 255953)


--- trunk/Source/WebCore/animation/DocumentTimeline.cpp	2020-02-06 15:12:19 UTC (rev 255952)
+++ trunk/Source/WebCore/animation/DocumentTimeline.cpp	2020-02-06 15:56:02 UTC (rev 255953)
@@ -75,12 +75,14 @@
 
 DocumentTimeline::~DocumentTimeline()
 {
-    if (m_document)
-        m_document->removeTimeline(*this);
+    detachFromDocument();
 }
 
 void DocumentTimeline::detachFromDocument()
 {
+    if (m_document)
+        m_document->removeTimeline(*this);
+
     m_currentTimeClearingTaskQueue.close();
     m_elementsWithRunningAcceleratedAnimations.clear();
 
@@ -280,6 +282,8 @@
 
 void DocumentTimeline::cacheCurrentTime(DOMHighResTimeStamp newCurrentTime)
 {
+    ASSERT(m_document);
+
     m_cachedCurrentTime = Seconds(newCurrentTime);
     // We want to be sure to keep this time cached until we've both finished running JS and finished updating
     // animations, so we schedule the invalidation task and register a whenIdle callback on the VM, which will

Modified: trunk/Source/WebCore/dom/Document.cpp (255952 => 255953)


--- trunk/Source/WebCore/dom/Document.cpp	2020-02-06 15:12:19 UTC (rev 255952)
+++ trunk/Source/WebCore/dom/Document.cpp	2020-02-06 15:56:02 UTC (rev 255953)
@@ -2578,10 +2578,9 @@
 
     detachFromFrame();
 
-    if (m_timeline) {
-        m_timeline->detachFromDocument();
-        m_timeline = nullptr;
-    }
+    while (!m_timelines.computesEmpty())
+        m_timelines.begin()->detachFromDocument();
+    m_timeline = nullptr;
 
 #if ENABLE(CSS_PAINTING_API)
     for (auto& scope : m_paintWorkletGlobalScopes.values())

_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to