Diff
Modified: trunk/LayoutTests/ChangeLog (251741 => 251742)
--- trunk/LayoutTests/ChangeLog 2019-10-29 23:04:41 UTC (rev 251741)
+++ trunk/LayoutTests/ChangeLog 2019-10-29 23:07:16 UTC (rev 251742)
@@ -1,3 +1,17 @@
+2019-10-29 Antoine Quint <grao...@apple.com>
+
+ WebAnimation should never prevent entering the back/forward cache
+ https://bugs.webkit.org/show_bug.cgi?id=203088
+ <rdar://problem/56374249>
+
+ Reviewed by Antti Koivisto.
+
+ Add a new test that checks that an Animation that would run past a page's navigation is correctly suspended
+ and resumed as it enters and leaves the back/forward cache.
+
+ * webanimations/animation-page-cache-expected.txt: Added.
+ * webanimations/animation-page-cache.html: Added.
+
2019-10-29 Megan Gardner <megan_gard...@apple.com>
Update autocorrect test to have correctly focused contenteditable
Added: trunk/LayoutTests/webanimations/animation-page-cache-expected.txt (0 => 251742)
--- trunk/LayoutTests/webanimations/animation-page-cache-expected.txt (rev 0)
+++ trunk/LayoutTests/webanimations/animation-page-cache-expected.txt 2019-10-29 23:07:16 UTC (rev 251742)
@@ -0,0 +1,2 @@
+This test verifies that the page cache suspends and resumes Web Animations from the page cache. The test starts an animation, then navigates away, waits a bit, navigates back, confirming that the timeline froze at a given time and resumed in the same spot, advancing the animation. If successful, it outputs 'PASS' below.
+PASS.
Added: trunk/LayoutTests/webanimations/animation-page-cache.html (0 => 251742)
--- trunk/LayoutTests/webanimations/animation-page-cache.html (rev 0)
+++ trunk/LayoutTests/webanimations/animation-page-cache.html 2019-10-29 23:07:16 UTC (rev 251742)
@@ -0,0 +1,58 @@
+<!doctype html><!-- webkit-test-runner [ enableBackForwardCache=true ] -->
+<html>
+<script>
+
+let suspended = false;
+let restored = false;
+
+function finish(msg)
+{
+ document.getElementById("result").textContent = msg;
+ if (window.testRunner)
+ testRunner.notifyDone();
+}
+
+window.addEventListener("pagehide", event => {
+ suspended = event.persisted;
+ if (!suspended)
+ finish("FAIL: event.persisted was false for pagehide event.")
+});
+
+window.addEventListener("pageshow", event => {
+ // If we haven't been suspended, then this is the initial page load, not the back navigation.
+ if (!suspended)
+ return;
+
+ restored = event.persisted;
+ if (!restored)
+ finish("FAIL: event.persisted was false for pageshow event.")
+});
+
+window.addEventListener("DOMContentLoaded", event => {
+ if (window.testRunner) {
+ testRunner.dumpAsText();
+ testRunner.waitUntilDone();
+ }
+
+ // We start an animation that is just long enough that it would finish while the page is hidden.
+ document.body.animate({ backgroundColor: "red" }, 500).finished.then(() => {
+ if (!suspended)
+ finish("FAIL: Animation finished but prevented the page from being suspended.");
+ else if (!restored)
+ finish("FAIL: Animation finished but prevented the page from being restored.");
+ else
+ finish("PASS.");
+ });
+
+ requestAnimationFrame(() => {
+ // Load a new page, and let it go back after 250ms.
+ window.location.href = "" _onload_='setTimeout(() => history.back(), 250)'></body>";
+ });
+});
+
+</script>
+<body>
+This test verifies that the page cache suspends and resumes Web Animations from the page cache. The test starts an animation, then navigates away, waits a bit, navigates back, confirming that the timeline froze at a given time and resumed in the same spot, advancing the animation. If successful, it outputs 'PASS' below.
+<div id="result"></div>
+</body>
+</html>
Modified: trunk/LayoutTests/webanimations/leak-document-with-web-animation-expected.txt (251741 => 251742)
--- trunk/LayoutTests/webanimations/leak-document-with-web-animation-expected.txt 2019-10-29 23:04:41 UTC (rev 251741)
+++ trunk/LayoutTests/webanimations/leak-document-with-web-animation-expected.txt 2019-10-29 23:07:16 UTC (rev 251742)
@@ -4,9 +4,7 @@
The iframe has finished loading.
-The iframe has been destroyed.
-PASS internals.numberOfLiveDocuments() is numberOfLiveDocumentsAfterIframeLoaded - 1
-
+PASS The document was destroyed
PASS successfullyParsed is true
TEST COMPLETE
Modified: trunk/LayoutTests/webanimations/leak-document-with-web-animation.html (251741 => 251742)
--- trunk/LayoutTests/webanimations/leak-document-with-web-animation.html 2019-10-29 23:04:41 UTC (rev 251741)
+++ trunk/LayoutTests/webanimations/leak-document-with-web-animation.html 2019-10-29 23:07:16 UTC (rev 251742)
@@ -1,7 +1,7 @@
<!DOCTYPE html>
<html>
<body _onload_="runTest()">
-<script src=""
+<script src=""
<script>
description("This test asserts that Document doesn't leak when a Web Animation is created.");
@@ -8,10 +8,6 @@
if (window.internals)
jsTestIsAsync = true;
-gc();
-
-var numberOfLiveDocumentsAfterIframeLoaded = 0;
-
function runTest() {
if (!window.internals)
return;
@@ -22,21 +18,30 @@
if (frame.src ="" 'about:blank')
return true;
- numberOfLiveDocumentsAfterIframeLoaded = internals.numberOfLiveDocuments();
+ documentIdentifier = internals.documentIdentifier(frame.contentDocument);
debug("The iframe has finished loading.");
frame.remove();
frame = null;
- setTimeout(() => {
+ gc();
+ timeout = 0;
+ handle = setInterval(() => {
+ if (!internals.isDocumentAlive(documentIdentifier)) {
+ clearInterval(handle);
+ testPassed("The document was destroyed");
+ finishJSTest();
+ return;
+ }
+ timeout++;
+ if (timeout == 500) {
+ clearInterval(handle);
+ testFailed("The document was leaked");
+ finishJSTest();
+ return;
+ }
gc();
- setTimeout(function () {
- debug("The iframe has been destroyed.");
- shouldBe("internals.numberOfLiveDocuments()", "numberOfLiveDocumentsAfterIframeLoaded - 1");
- debug("");
- finishJSTest();
- });
- });
+ }, 10);
}
frame.src = '';
@@ -43,6 +48,5 @@
}
</script>
-<script src=""
</body>
-</html>
\ No newline at end of file
+</html>
Modified: trunk/Source/WebCore/ChangeLog (251741 => 251742)
--- trunk/Source/WebCore/ChangeLog 2019-10-29 23:04:41 UTC (rev 251741)
+++ trunk/Source/WebCore/ChangeLog 2019-10-29 23:07:16 UTC (rev 251742)
@@ -1,3 +1,37 @@
+2019-10-29 Antoine Quint <grao...@apple.com>
+
+ WebAnimation should never prevent entering the back/forward cache
+ https://bugs.webkit.org/show_bug.cgi?id=203088
+ <rdar://problem/56374249>
+
+ Reviewed by Antti Koivisto.
+
+ Test: webanimations/animation-page-cache.html
+
+ We remove the Web Animation override of the deprecated method ActiveDOMObject::shouldPreventEnteringBackForwardCache_DEPRECATED()
+ and instead ensure event dispatch is suspended along with the WebAnimation object through the adoption of a SuspendableTaskQueue.
+
+ We also ensure an animation correctly suspends itself when ActiveDOMObject::suspend() and ActiveDOMObject::resume() are called.
+ Implementing these methods showed that we have some placeholders in DeclarativeAnimation that were not necessary, so we remove those.
+
+ Finally, we no longer need to track the stopped state since the SuspendableTaskQueue will close itself when ActiveDOMObject::stop()
+ is called.
+
+ * animation/DeclarativeAnimation.cpp:
+ (WebCore::DeclarativeAnimation::stop): Deleted.
+ (WebCore::DeclarativeAnimation::suspend): Deleted.
+ (WebCore::DeclarativeAnimation::resume): Deleted.
+ * animation/DeclarativeAnimation.h:
+ * animation/WebAnimation.cpp:
+ (WebCore::WebAnimation::WebAnimation):
+ (WebCore::WebAnimation::enqueueAnimationPlaybackEvent):
+ (WebCore::WebAnimation::suspend):
+ (WebCore::WebAnimation::resume):
+ (WebCore::WebAnimation::stop):
+ (WebCore::WebAnimation::hasPendingActivity):
+ (WebCore::WebAnimation::shouldPreventEnteringBackForwardCache_DEPRECATED const): Deleted.
+ * animation/WebAnimation.h:
+
2019-10-07 Jer Noble <jer.no...@apple.com>
Implement the Remote Playback API.
Modified: trunk/Source/WebCore/animation/DeclarativeAnimation.cpp (251741 => 251742)
--- trunk/Source/WebCore/animation/DeclarativeAnimation.cpp 2019-10-29 23:04:41 UTC (rev 251741)
+++ trunk/Source/WebCore/animation/DeclarativeAnimation.cpp 2019-10-29 23:07:16 UTC (rev 251742)
@@ -346,19 +346,4 @@
m_eventQueue->enqueueEvent(TransitionEvent::create(eventType, downcast<CSSTransition>(this)->transitionProperty(), time, PseudoElement::pseudoElementNameForEvents(m_owningElement->pseudoId())));
}
-void DeclarativeAnimation::stop()
-{
- WebAnimation::stop();
-}
-
-void DeclarativeAnimation::suspend(ReasonForSuspension reason)
-{
- WebAnimation::suspend(reason);
-}
-
-void DeclarativeAnimation::resume()
-{
- WebAnimation::resume();
-}
-
} // namespace WebCore
Modified: trunk/Source/WebCore/animation/DeclarativeAnimation.h (251741 => 251742)
--- trunk/Source/WebCore/animation/DeclarativeAnimation.h 2019-10-29 23:04:41 UTC (rev 251741)
+++ trunk/Source/WebCore/animation/DeclarativeAnimation.h 2019-10-29 23:07:16 UTC (rev 251742)
@@ -81,11 +81,6 @@
void enqueueDOMEvent(const AtomString&, Seconds);
void remove() final;
- // ActiveDOMObject.
- void suspend(ReasonForSuspension) final;
- void resume() final;
- void stop() final;
-
bool m_wasPending { false };
AnimationEffectPhase m_previousPhase { AnimationEffectPhase::Idle };
Modified: trunk/Source/WebCore/animation/WebAnimation.cpp (251741 => 251742)
--- trunk/Source/WebCore/animation/WebAnimation.cpp 2019-10-29 23:04:41 UTC (rev 251741)
+++ trunk/Source/WebCore/animation/WebAnimation.cpp 2019-10-29 23:07:16 UTC (rev 251742)
@@ -67,6 +67,7 @@
: ActiveDOMObject(document)
, m_readyPromise(makeUniqueRef<ReadyPromise>(*this, &WebAnimation::readyPromiseResolve))
, m_finishedPromise(makeUniqueRef<FinishedPromise>(*this, &WebAnimation::finishedPromiseResolve))
+ , m_taskQueue(SuspendableTaskQueue::create(document))
{
m_readyPromise->resolve(*this);
suspendIfNeeded();
@@ -613,9 +614,8 @@
downcast<DocumentTimeline>(*m_timeline).enqueueAnimationPlaybackEvent(WTFMove(event));
} else {
// Otherwise, queue a task to dispatch event at animation. The task source for this task is the DOM manipulation task source.
- callOnMainThread([this, pendingActivity = makePendingActivity(*this), event = WTFMove(event)]() {
- if (!m_isStopped)
- this->dispatchEvent(event);
+ m_taskQueue->enqueueTask([this, event = WTFMove(event)] {
+ dispatchEvent(event);
});
}
}
@@ -1157,19 +1157,20 @@
return "Animation";
}
-// FIXME: This should never prevent entering the back/forward cache.
-bool WebAnimation::shouldPreventEnteringBackForwardCache_DEPRECATED() const
+void WebAnimation::suspend(ReasonForSuspension)
{
- // Use the base class's implementation of hasPendingActivity() since we wouldn't want the custom implementation
- // in this class designed to keep JS wrappers alive to interfere with the ability for a page using animations
- // to enter the back/forward cache.
- return ActiveDOMObject::hasPendingActivity();
+ setSuspended(true);
}
+void WebAnimation::resume()
+{
+ setSuspended(false);
+}
+
void WebAnimation::stop()
{
+ m_taskQueue->cancelAllTasks();
ActiveDOMObject::stop();
- m_isStopped = true;
removeAllEventListeners();
}
@@ -1176,7 +1177,7 @@
bool WebAnimation::hasPendingActivity() const
{
// Keep the JS wrapper alive if the animation is considered relevant or could become relevant again by virtue of having a timeline.
- return m_timeline || m_isRelevant || ActiveDOMObject::hasPendingActivity();
+ return m_timeline || m_isRelevant || m_taskQueue->hasPendingTasks() || ActiveDOMObject::hasPendingActivity();
}
void WebAnimation::updateRelevance()
Modified: trunk/Source/WebCore/animation/WebAnimation.h (251741 => 251742)
--- trunk/Source/WebCore/animation/WebAnimation.h 2019-10-29 23:04:41 UTC (rev 251741)
+++ trunk/Source/WebCore/animation/WebAnimation.h 2019-10-29 23:07:16 UTC (rev 251742)
@@ -29,6 +29,7 @@
#include "EventTarget.h"
#include "ExceptionOr.h"
#include "IDLTypes.h"
+#include "SuspendableTaskQueue.h"
#include "WebAnimationUtilities.h"
#include <wtf/Markable.h>
#include <wtf/RefCounted.h>
@@ -140,8 +141,6 @@
protected:
explicit WebAnimation(Document&);
- void stop() override;
-
private:
enum class DidSeek : uint8_t { Yes, No };
enum class SynchronouslyNotify : uint8_t { Yes, No };
@@ -176,6 +175,7 @@
RefPtr<AnimationTimeline> m_timeline;
UniqueRef<ReadyPromise> m_readyPromise;
UniqueRef<FinishedPromise> m_finishedPromise;
+ UniqueRef<SuspendableTaskQueue> m_taskQueue;
Markable<Seconds, Seconds::MarkableTraits> m_previousCurrentTime;
Markable<Seconds, Seconds::MarkableTraits> m_startTime;
Markable<Seconds, Seconds::MarkableTraits> m_holdTime;
@@ -185,7 +185,6 @@
int m_suspendCount { 0 };
- bool m_isStopped { false };
bool m_isSuspended { false };
bool m_finishNotificationStepsMicrotaskPending;
bool m_isRelevant;
@@ -197,7 +196,9 @@
// ActiveDOMObject.
const char* activeDOMObjectName() const final;
- bool shouldPreventEnteringBackForwardCache_DEPRECATED() const final;
+ void suspend(ReasonForSuspension) final;
+ void resume() final;
+ void stop() final;
// EventTarget
EventTargetInterface eventTargetInterface() const final { return WebAnimationEventTargetInterfaceType; }
Modified: trunk/Source/WebCore/platform/SuspendableTaskQueue.h (251741 => 251742)
--- trunk/Source/WebCore/platform/SuspendableTaskQueue.h 2019-10-29 23:04:41 UTC (rev 251741)
+++ trunk/Source/WebCore/platform/SuspendableTaskQueue.h 2019-10-29 23:07:16 UTC (rev 251742)
@@ -58,7 +58,7 @@
void close();
bool isClosed() const { return m_isClosed; }
- bool hasPendingTasks() const { return m_pendingTasks.isEmpty(); }
+ bool hasPendingTasks() const { return !m_pendingTasks.isEmpty(); }
private:
friend UniqueRef<SuspendableTaskQueue> WTF::makeUniqueRefWithoutFastMallocCheck<SuspendableTaskQueue, WebCore::ScriptExecutionContext*&>(WebCore::ScriptExecutionContext*&);
