Title: [243868] trunk
- Revision
- 243868
- Author
- [email protected]
- Date
- 2019-04-04 07:16:05 -0700 (Thu, 04 Apr 2019)
Log Message
[Web Animations] JS wrapper may be deleted while animation is yet to dispatch its finish event
https://bugs.webkit.org/show_bug.cgi?id=196118
<rdar://problem/46614137>
Patch by Antoine Quint <[email protected]> on 2019-04-04
Reviewed by Ryosuke Niwa.
Source/WebCore:
Test: webanimations/js-wrapper-kept-alive.html
We need to teach WebAnimation to keep its JS wrapper alive if it's relevant or could become relevant again by virtue of having a timeline.
* animation/WebAnimation.cpp:
(WebCore::WebAnimation::stop): Drive-by fix for the missing superclass method call.
(WebCore::WebAnimation::hasPendingActivity const):
* animation/WebAnimation.h:
LayoutTests:
Add a test that starts a short animation, sets a custom property on it, registers a "finish" event listener on it and deletes
the sole reference to it in the JS world before triggering garbage collection. Prior to this fix, this test would time out
because the JS wrapper would be garbage-collected prior to the animation completing and thus the event listener would not
be called. To complete successfully, this test checks that it receives the event and its target is the same animation object
that was originally created by checking the custom property is still set.
* legacy-animation-engine/animations/resume-after-page-cache.html:
* webanimations/js-wrapper-kept-alive-expected.txt: Added.
* webanimations/js-wrapper-kept-alive.html: Added.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (243867 => 243868)
--- trunk/LayoutTests/ChangeLog 2019-04-04 14:15:20 UTC (rev 243867)
+++ trunk/LayoutTests/ChangeLog 2019-04-04 14:16:05 UTC (rev 243868)
@@ -1,3 +1,21 @@
+2019-04-04 Antoine Quint <[email protected]>
+
+ [Web Animations] JS wrapper may be deleted while animation is yet to dispatch its finish event
+ https://bugs.webkit.org/show_bug.cgi?id=196118
+ <rdar://problem/46614137>
+
+ Reviewed by Ryosuke Niwa.
+
+ Add a test that starts a short animation, sets a custom property on it, registers a "finish" event listener on it and deletes
+ the sole reference to it in the JS world before triggering garbage collection. Prior to this fix, this test would time out
+ because the JS wrapper would be garbage-collected prior to the animation completing and thus the event listener would not
+ be called. To complete successfully, this test checks that it receives the event and its target is the same animation object
+ that was originally created by checking the custom property is still set.
+
+ * legacy-animation-engine/animations/resume-after-page-cache.html:
+ * webanimations/js-wrapper-kept-alive-expected.txt: Added.
+ * webanimations/js-wrapper-kept-alive.html: Added.
+
2019-04-03 Timothy Hatcher <[email protected]>
Update AutoFill field icons to be SVG instead of PNG images.
Modified: trunk/LayoutTests/legacy-animation-engine/animations/resume-after-page-cache.html (243867 => 243868)
--- trunk/LayoutTests/legacy-animation-engine/animations/resume-after-page-cache.html 2019-04-04 14:15:20 UTC (rev 243867)
+++ trunk/LayoutTests/legacy-animation-engine/animations/resume-after-page-cache.html 2019-04-04 14:16:05 UTC (rev 243868)
@@ -1,3 +1,4 @@
+<!-- webkit-test-runner [ experimental:WebAnimationsCSSIntegrationEnabled=false ] -->
<style>
@-webkit-keyframes bounce {
from {
Added: trunk/LayoutTests/webanimations/js-wrapper-kept-alive-expected.txt (0 => 243868)
--- trunk/LayoutTests/webanimations/js-wrapper-kept-alive-expected.txt (rev 0)
+++ trunk/LayoutTests/webanimations/js-wrapper-kept-alive-expected.txt 2019-04-04 14:16:05 UTC (rev 243868)
@@ -0,0 +1,10 @@
+This test checks that registering an event listener on an animation whose JS wrapper would otherwise be garbage-collected still fires registered event listeners.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS event.target._isMyAnimation is true
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
Added: trunk/LayoutTests/webanimations/js-wrapper-kept-alive.html (0 => 243868)
--- trunk/LayoutTests/webanimations/js-wrapper-kept-alive.html (rev 0)
+++ trunk/LayoutTests/webanimations/js-wrapper-kept-alive.html 2019-04-04 14:16:05 UTC (rev 243868)
@@ -0,0 +1,33 @@
+<!DOCTYPE html>
+<html>
+<body>
+<div id="target"></div>
+<script src=""
+<script>
+description("This test checks that registering an event listener on an animation whose JS wrapper would otherwise be garbage-collected still fires registered event listeners.");
+
+if (window.internals)
+ jsTestIsAsync = true;
+
+// A longer animation that could not be garbage-collected under any circumstance allows us to finish the test
+// with a reasonable delay without hard-coding a timeout.
+const timeoutAnimation = document.getElementById("target").animate({ marginRight: ["0px", "100px"] }, 1000);
+timeoutAnimation.addEventListener("finish", finishJSTest);
+
+function runTest() {
+ const animation = document.getElementById("target").animate({ marginLeft: ["0px", "100px"] }, 100);
+ animation._isMyAnimation = true;
+ animation.addEventListener("finish", event => {
+ shouldBeTrue("event.target._isMyAnimation");
+ finishJSTest();
+ });
+}
+
+gc();
+runTest();
+gc();
+
+</script>
+<script src=""
+</body>
+</html>
\ No newline at end of file
Modified: trunk/Source/WebCore/ChangeLog (243867 => 243868)
--- trunk/Source/WebCore/ChangeLog 2019-04-04 14:15:20 UTC (rev 243867)
+++ trunk/Source/WebCore/ChangeLog 2019-04-04 14:16:05 UTC (rev 243868)
@@ -1,3 +1,20 @@
+2019-04-04 Antoine Quint <[email protected]>
+
+ [Web Animations] JS wrapper may be deleted while animation is yet to dispatch its finish event
+ https://bugs.webkit.org/show_bug.cgi?id=196118
+ <rdar://problem/46614137>
+
+ Reviewed by Ryosuke Niwa.
+
+ Test: webanimations/js-wrapper-kept-alive.html
+
+ We need to teach WebAnimation to keep its JS wrapper alive if it's relevant or could become relevant again by virtue of having a timeline.
+
+ * animation/WebAnimation.cpp:
+ (WebCore::WebAnimation::stop): Drive-by fix for the missing superclass method call.
+ (WebCore::WebAnimation::hasPendingActivity const):
+ * animation/WebAnimation.h:
+
2019-04-04 Miguel Gomez <[email protected]>
[GTK][WPE] Use a timer to request the creation of pending tiles
Modified: trunk/Source/WebCore/animation/WebAnimation.cpp (243867 => 243868)
--- trunk/Source/WebCore/animation/WebAnimation.cpp 2019-04-04 14:15:20 UTC (rev 243867)
+++ trunk/Source/WebCore/animation/WebAnimation.cpp 2019-04-04 14:16:05 UTC (rev 243868)
@@ -1159,10 +1159,16 @@
void WebAnimation::stop()
{
+ ActiveDOMObject::stop();
m_isStopped = true;
removeAllEventListeners();
}
+bool WebAnimation::hasPendingActivity() const
+{
+ return m_timeline || m_isRelevant || ActiveDOMObject::hasPendingActivity();
+}
+
void WebAnimation::updateRelevance()
{
m_isRelevant = computeRelevance();
Modified: trunk/Source/WebCore/animation/WebAnimation.h (243867 => 243868)
--- trunk/Source/WebCore/animation/WebAnimation.h 2019-04-04 14:15:20 UTC (rev 243867)
+++ trunk/Source/WebCore/animation/WebAnimation.h 2019-04-04 14:16:05 UTC (rev 243868)
@@ -117,6 +117,8 @@
bool isSuspended() const { return m_isSuspended; }
virtual void remove();
+ bool hasPendingActivity() const final;
+
using RefCounted::ref;
using RefCounted::deref;
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
