Diff
Modified: trunk/LayoutTests/ChangeLog (233360 => 233361)
--- trunk/LayoutTests/ChangeLog 2018-06-29 17:15:08 UTC (rev 233360)
+++ trunk/LayoutTests/ChangeLog 2018-06-29 17:42:00 UTC (rev 233361)
@@ -1,3 +1,15 @@
+2018-06-29 David Fenton <[email protected]>
+
+ Unreviewed, rolling out r233349.
+
+ caused 42 crashes on iOS GuardMalloc and iOS ASan tests
+
+ Reverted changeset:
+
+ "[Web Animations] Using a Web Animation leaks the Document"
+ https://bugs.webkit.org/show_bug.cgi?id=187088
+ https://trac.webkit.org/changeset/233349
+
2018-06-29 Jer Noble <[email protected]>
Returning PiP'd video to fullscreen while playing leaves video muted.
Deleted: trunk/LayoutTests/webanimations/leak-document-with-web-animation-expected.txt (233360 => 233361)
--- trunk/LayoutTests/webanimations/leak-document-with-web-animation-expected.txt 2018-06-29 17:15:08 UTC (rev 233360)
+++ trunk/LayoutTests/webanimations/leak-document-with-web-animation-expected.txt 2018-06-29 17:42:00 UTC (rev 233361)
@@ -1,13 +0,0 @@
-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
-
Deleted: trunk/LayoutTests/webanimations/leak-document-with-web-animation.html (233360 => 233361)
--- trunk/LayoutTests/webanimations/leak-document-with-web-animation.html 2018-06-29 17:15:08 UTC (rev 233360)
+++ trunk/LayoutTests/webanimations/leak-document-with-web-animation.html 2018-06-29 17:42:00 UTC (rev 233361)
@@ -1,48 +0,0 @@
-<!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
Modified: trunk/Source/WebCore/ChangeLog (233360 => 233361)
--- trunk/Source/WebCore/ChangeLog 2018-06-29 17:15:08 UTC (rev 233360)
+++ trunk/Source/WebCore/ChangeLog 2018-06-29 17:42:00 UTC (rev 233361)
@@ -1,3 +1,15 @@
+2018-06-29 David Fenton <[email protected]>
+
+ Unreviewed, rolling out r233349.
+
+ caused 42 crashes on iOS GuardMalloc and iOS ASan tests
+
+ Reverted changeset:
+
+ "[Web Animations] Using a Web Animation leaks the Document"
+ https://bugs.webkit.org/show_bug.cgi?id=187088
+ https://trac.webkit.org/changeset/233349
+
2018-06-29 Jer Noble <[email protected]>
Returning PiP'd video to fullscreen while playing leaves video muted.
Modified: trunk/Source/WebCore/animation/AnimationTimeline.cpp (233360 => 233361)
--- trunk/Source/WebCore/animation/AnimationTimeline.cpp 2018-06-29 17:15:08 UTC (rev 233360)
+++ trunk/Source/WebCore/animation/AnimationTimeline.cpp 2018-06-29 17:42:00 UTC (rev 233361)
@@ -176,8 +176,10 @@
void AnimationTimeline::removeAnimationsForElement(Element& element)
{
- for (auto& animation : animationsForElement(element))
- animation->remove();
+ for (auto& animation : animationsForElement(element)) {
+ animation->prepareAnimationForRemoval();
+ removeAnimation(animation.releaseNonNull());
+ }
}
void AnimationTimeline::cancelDeclarativeAnimationsForElement(Element& element)
Modified: trunk/Source/WebCore/animation/DeclarativeAnimation.cpp (233360 => 233361)
--- trunk/Source/WebCore/animation/DeclarativeAnimation.cpp 2018-06-29 17:15:08 UTC (rev 233360)
+++ trunk/Source/WebCore/animation/DeclarativeAnimation.cpp 2018-06-29 17:42:00 UTC (rev 233361)
@@ -49,10 +49,10 @@
{
}
-void DeclarativeAnimation::remove()
+void DeclarativeAnimation::prepareAnimationForRemoval()
{
+ WebAnimation::prepareAnimationForRemoval();
m_eventQueue.close();
- WebAnimation::remove();
}
void DeclarativeAnimation::setBackingAnimation(const Animation& backingAnimation)
Modified: trunk/Source/WebCore/animation/DeclarativeAnimation.h (233360 => 233361)
--- trunk/Source/WebCore/animation/DeclarativeAnimation.h 2018-06-29 17:15:08 UTC (rev 233360)
+++ trunk/Source/WebCore/animation/DeclarativeAnimation.h 2018-06-29 17:42:00 UTC (rev 233361)
@@ -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 (233360 => 233361)
--- trunk/Source/WebCore/animation/DocumentTimeline.cpp 2018-06-29 17:15:08 UTC (rev 233360)
+++ trunk/Source/WebCore/animation/DocumentTimeline.cpp 2018-06-29 17:42:00 UTC (rev 233361)
@@ -63,10 +63,6 @@
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 (233360 => 233361)
--- trunk/Source/WebCore/animation/WebAnimation.cpp 2018-06-29 17:15:08 UTC (rev 233360)
+++ trunk/Source/WebCore/animation/WebAnimation.cpp 2018-06-29 17:42:00 UTC (rev 233361)
@@ -70,10 +70,9 @@
{
}
-void WebAnimation::remove()
+void WebAnimation::prepareAnimationForRemoval()
{
setEffectInternal(nullptr);
- setTimelineInternal(nullptr);
}
void WebAnimation::suspendEffectInvalidation()
@@ -178,6 +177,12 @@
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();
@@ -192,7 +197,7 @@
}
}
- setTimelineInternal(WTFMove(timeline));
+ m_timeline = WTFMove(timeline);
setSuspended(is<DocumentTimeline>(m_timeline) && downcast<DocumentTimeline>(*m_timeline).animationsAreSuspended());
@@ -203,17 +208,6 @@
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 (233360 => 233361)
--- trunk/Source/WebCore/animation/WebAnimation.h 2018-06-29 17:15:08 UTC (rev 233360)
+++ trunk/Source/WebCore/animation/WebAnimation.h 2018-06-29 17:42:00 UTC (rev 233361)
@@ -63,6 +63,7 @@
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>&&);
@@ -116,7 +117,7 @@
void unsuspendEffectInvalidation();
void setSuspended(bool);
bool isSuspended() const { return m_isSuspended; }
- virtual void remove();
+ virtual void prepareAnimationForRemoval();
String description();
@@ -155,9 +156,7 @@
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;
