Diff
Modified: trunk/LayoutTests/ChangeLog (233348 => 233349)
--- trunk/LayoutTests/ChangeLog 2018-06-29 03:45:47 UTC (rev 233348)
+++ trunk/LayoutTests/ChangeLog 2018-06-29 05:45:34 UTC (rev 233349)
@@ -1,3 +1,18 @@
+2018-06-27 Antoine Quint <[email protected]>
+
+ [Web Animations] Using a Web Animation leaks the Document
+ https://bugs.webkit.org/show_bug.cgi?id=187088
+ <rdar://problem/41392046>
+
+ Reviewed by Dean Jackson.
+
+ Add a new test that creates an Animation object in JS within an iframe and checks that removing
+ the iframe clears its Document.
+
+ * webanimations/leak-document-with-web-animation-expected.txt: Added.
+ * webanimations/leak-document-with-web-animation.html: Added.
+ * webanimations/resources/web-animation-leak-iframe.html: Added.
+
2018-06-28 Olivia Barnett <[email protected]>
Find in page for typographic quotes does not find low (German) quotes
Added: trunk/LayoutTests/webanimations/leak-document-with-web-animation-expected.txt (0 => 233349)
--- trunk/LayoutTests/webanimations/leak-document-with-web-animation-expected.txt (rev 0)
+++ trunk/LayoutTests/webanimations/leak-document-with-web-animation-expected.txt 2018-06-29 05:45:34 UTC (rev 233349)
@@ -0,0 +1,13 @@
+This test asserts that Document doesn't leak when a Web Animation is created.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+The iframe has finished loading.
+The iframe has been destroyed.
+PASS internals.numberOfLiveDocuments() is numberOfLiveDocumentsAfterIframeLoaded - 1
+
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
Added: trunk/LayoutTests/webanimations/leak-document-with-web-animation.html (0 => 233349)
--- trunk/LayoutTests/webanimations/leak-document-with-web-animation.html (rev 0)
+++ trunk/LayoutTests/webanimations/leak-document-with-web-animation.html 2018-06-29 05:45:34 UTC (rev 233349)
@@ -0,0 +1,48 @@
+<!DOCTYPE html>
+<html>
+<body _onload_="runTest()">
+<script src=""
+<script>
+description("This test asserts that Document doesn't leak when a Web Animation is created.");
+
+if (window.internals)
+ jsTestIsAsync = true;
+
+gc();
+
+var numberOfLiveDocumentsAfterIframeLoaded = 0;
+
+function runTest() {
+ if (!window.internals)
+ return;
+
+ var frame = document.body.appendChild(document.createElement("iframe"));
+
+ frame._onload_ = function() {
+ if (frame.src ="" 'about:blank')
+ return true;
+
+ numberOfLiveDocumentsAfterIframeLoaded = internals.numberOfLiveDocuments();
+ debug("The iframe has finished loading.");
+
+ frame.remove();
+ frame = null;
+
+ setTimeout(() => {
+ gc();
+ setTimeout(function () {
+ debug("The iframe has been destroyed.");
+ shouldBe("internals.numberOfLiveDocuments()", "numberOfLiveDocumentsAfterIframeLoaded - 1");
+ debug("");
+ finishJSTest();
+ });
+ });
+ }
+
+ frame.src = '';
+}
+
+</script>
+<script src=""
+</body>
+</html>
\ No newline at end of file
Added: trunk/LayoutTests/webanimations/resources/web-animation-leak-iframe.html (0 => 233349)
--- trunk/LayoutTests/webanimations/resources/web-animation-leak-iframe.html (rev 0)
+++ trunk/LayoutTests/webanimations/resources/web-animation-leak-iframe.html 2018-06-29 05:45:34 UTC (rev 233349)
@@ -0,0 +1,4 @@
+<div></div>
+<script type="text/_javascript_">
+document.querySelector("div").animate({ marginLeft: ["0", "100px"] }, 1000);
+</script>
Modified: trunk/Source/WebCore/ChangeLog (233348 => 233349)
--- trunk/Source/WebCore/ChangeLog 2018-06-29 03:45:47 UTC (rev 233348)
+++ trunk/Source/WebCore/ChangeLog 2018-06-29 05:45:34 UTC (rev 233349)
@@ -1,3 +1,35 @@
+2018-06-27 Antoine Quint <[email protected]>
+
+ [Web Animations] Using a Web Animation leaks the Document
+ https://bugs.webkit.org/show_bug.cgi?id=187088
+ <rdar://problem/41392046>
+
+ Reviewed by Dean Jackson.
+
+ Test: webanimations/leak-document-with-web-animation.html
+
+ We need to ensure that any remaining animation is cleared when the DocumentTimeline is detached from its Document.
+ We rename WebAnimation::prepareAnimationForRemoval() to WebAnimation::remove() since it really actively disassociates
+ the animation from its timeline.
+
+ * animation/AnimationTimeline.cpp:
+ (WebCore::AnimationTimeline::removeAnimationsForElement): We no longer need the call to removeAnimation()
+ since the new WebAnimation::remove() method will also set the timeline to null which will eventually call
+ removeAnimation() on the disassociated timeline.
+ * animation/DeclarativeAnimation.cpp:
+ (WebCore::DeclarativeAnimation::remove):
+ (WebCore::DeclarativeAnimation::prepareAnimationForRemoval): Deleted.
+ * animation/DeclarativeAnimation.h:
+ * animation/DocumentTimeline.cpp:
+ (WebCore::DocumentTimeline::detachFromDocument): Call remove() on all known animations.
+ * animation/WebAnimation.cpp:
+ (WebCore::WebAnimation::remove): Set the timeline to null to fully disassociate this animation from its timeline.
+ (WebCore::WebAnimation::setTimeline): Factor the internal timeline-association code out of this JS API method so
+ that we can call this code without any JS-facing implications.
+ (WebCore::WebAnimation::setTimelineInternal):
+ (WebCore::WebAnimation::prepareAnimationForRemoval): Deleted.
+ * animation/WebAnimation.h:
+
2018-06-28 Zalan Bujtas <[email protected]>
[LFC] Out-of-flow positioned height does not necessarily equal to "bottom - top".
Modified: trunk/Source/WebCore/animation/AnimationTimeline.cpp (233348 => 233349)
--- trunk/Source/WebCore/animation/AnimationTimeline.cpp 2018-06-29 03:45:47 UTC (rev 233348)
+++ trunk/Source/WebCore/animation/AnimationTimeline.cpp 2018-06-29 05:45:34 UTC (rev 233349)
@@ -176,10 +176,8 @@
void AnimationTimeline::removeAnimationsForElement(Element& element)
{
- for (auto& animation : animationsForElement(element)) {
- animation->prepareAnimationForRemoval();
- removeAnimation(animation.releaseNonNull());
- }
+ for (auto& animation : animationsForElement(element))
+ animation->remove();
}
void AnimationTimeline::cancelDeclarativeAnimationsForElement(Element& element)
Modified: trunk/Source/WebCore/animation/DeclarativeAnimation.cpp (233348 => 233349)
--- trunk/Source/WebCore/animation/DeclarativeAnimation.cpp 2018-06-29 03:45:47 UTC (rev 233348)
+++ trunk/Source/WebCore/animation/DeclarativeAnimation.cpp 2018-06-29 05:45:34 UTC (rev 233349)
@@ -49,10 +49,10 @@
{
}
-void DeclarativeAnimation::prepareAnimationForRemoval()
+void DeclarativeAnimation::remove()
{
- WebAnimation::prepareAnimationForRemoval();
m_eventQueue.close();
+ WebAnimation::remove();
}
void DeclarativeAnimation::setBackingAnimation(const Animation& backingAnimation)
Modified: trunk/Source/WebCore/animation/DeclarativeAnimation.h (233348 => 233349)
--- trunk/Source/WebCore/animation/DeclarativeAnimation.h 2018-06-29 03:45:47 UTC (rev 233348)
+++ trunk/Source/WebCore/animation/DeclarativeAnimation.h 2018-06-29 05:45:34 UTC (rev 233349)
@@ -45,10 +45,10 @@
const Animation& backingAnimation() const { return m_backingAnimation; }
void setBackingAnimation(const Animation&);
void invalidateDOMEvents(Seconds elapsedTime = 0_s);
- void prepareAnimationForRemoval() final;
void setTimeline(RefPtr<AnimationTimeline>&&) final;
void cancel() final;
+ void remove() final;
protected:
DeclarativeAnimation(Element&, const Animation&);
Modified: trunk/Source/WebCore/animation/DocumentTimeline.cpp (233348 => 233349)
--- trunk/Source/WebCore/animation/DocumentTimeline.cpp 2018-06-29 03:45:47 UTC (rev 233348)
+++ trunk/Source/WebCore/animation/DocumentTimeline.cpp 2018-06-29 05:45:34 UTC (rev 233349)
@@ -63,6 +63,10 @@
m_invalidationTaskQueue.close();
m_eventDispatchTaskQueue.close();
m_animationScheduleTimer.stop();
+
+ for (const auto& animation : animations())
+ animation->remove();
+
m_document = nullptr;
}
Modified: trunk/Source/WebCore/animation/WebAnimation.cpp (233348 => 233349)
--- trunk/Source/WebCore/animation/WebAnimation.cpp 2018-06-29 03:45:47 UTC (rev 233348)
+++ trunk/Source/WebCore/animation/WebAnimation.cpp 2018-06-29 05:45:34 UTC (rev 233349)
@@ -70,9 +70,10 @@
{
}
-void WebAnimation::prepareAnimationForRemoval()
+void WebAnimation::remove()
{
setEffectInternal(nullptr);
+ setTimelineInternal(nullptr);
}
void WebAnimation::suspendEffectInvalidation()
@@ -177,12 +178,6 @@
if (m_startTime)
setHoldTime(std::nullopt);
- if (m_timeline)
- m_timeline->removeAnimation(*this);
-
- if (timeline)
- timeline->addAnimation(*this);
-
if (is<KeyframeEffectReadOnly>(m_effect)) {
auto* keyframeEffect = downcast<KeyframeEffectReadOnly>(m_effect.get());
auto* target = keyframeEffect->target();
@@ -197,7 +192,7 @@
}
}
- m_timeline = WTFMove(timeline);
+ setTimelineInternal(WTFMove(timeline));
setSuspended(is<DocumentTimeline>(m_timeline) && downcast<DocumentTimeline>(*m_timeline).animationsAreSuspended());
@@ -208,6 +203,17 @@
updateFinishedState(DidSeek::No, SynchronouslyNotify::No);
}
+void WebAnimation::setTimelineInternal(RefPtr<AnimationTimeline>&& timeline)
+{
+ if (m_timeline)
+ m_timeline->removeAnimation(*this);
+
+ if (timeline)
+ timeline->addAnimation(*this);
+
+ m_timeline = WTFMove(timeline);
+}
+
void WebAnimation::effectTargetDidChange(Element* previousTarget, Element* newTarget)
{
if (!m_timeline)
Modified: trunk/Source/WebCore/animation/WebAnimation.h (233348 => 233349)
--- trunk/Source/WebCore/animation/WebAnimation.h 2018-06-29 03:45:47 UTC (rev 233348)
+++ trunk/Source/WebCore/animation/WebAnimation.h 2018-06-29 05:45:34 UTC (rev 233349)
@@ -63,7 +63,6 @@
AnimationEffectReadOnly* effect() const { return m_effect.get(); }
void setEffect(RefPtr<AnimationEffectReadOnly>&&);
- void setEffectInternal(RefPtr<AnimationEffectReadOnly>&&, bool = false);
AnimationTimeline* timeline() const { return m_timeline.get(); }
virtual void setTimeline(RefPtr<AnimationTimeline>&&);
@@ -117,7 +116,7 @@
void unsuspendEffectInvalidation();
void setSuspended(bool);
bool isSuspended() const { return m_isSuspended; }
- virtual void prepareAnimationForRemoval();
+ virtual void remove();
String description();
@@ -156,7 +155,9 @@
void runPendingPauseTask();
void runPendingPlayTask();
void resetPendingTasks();
-
+ void setEffectInternal(RefPtr<AnimationEffectReadOnly>&&, bool = false);
+ void setTimelineInternal(RefPtr<AnimationTimeline>&&);
+
String m_id;
RefPtr<AnimationEffectReadOnly> m_effect;
RefPtr<AnimationTimeline> m_timeline;
