promise returned by animation.finished() remains pending forever
Reported by
l446240525@gmail.com,
Jun 6 2016
|
||||||||||||||||||
Issue descriptionhttp://a1.alicdn.com/oss/uploads/2016/06/06/b0596cd0-2bbe-11e6-b06b-4bace99b6af1.html The animation should loop forever but actually it stops after several loops. see also: issue 617535 Components: Blink>Animation
,
Jun 7 2016
Yes, I still see the issue in canary.
,
Jun 7 2016
I had '#enable-experimental-web-platform-features' that's why couldn't repro. Issue is reproducible on the latest canary(53.0.2761.2) on Mac OS 10.11.5 with all the flags set to default. Marking this Untriaged for further investigation. Note: Seeing the same behavior on older chrome version: 35.0.1849.0/45.0.2413.0 as well.
,
Jun 7 2016
Please cc dstockwell@chromium.org who implemented .finished() promise https://codereview.chromium.org/832713004
,
Jun 7 2016
Cc'ing dstockwell@ for more inputs on this.
,
Jun 8 2016
On Canary without enable-experimental-web-platform-features, it just halts after one run (animation.finished is undefined). With enable-experimental-web-platform-features it runs and then stops as described.
,
Jun 8 2016
Thanks for the report. We will look into this, but since it is a bug behind the enable-experimental-web-platform-features flag, it's a lower priority for us at the moment.
,
Jun 9 2016
The test URL isn't loading for me, would you be able to attach it here?
,
Jul 7 2016
Moving this nonessential bug to the next milestone. For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Sep 12 2016
,
Sep 13 2016
,
Oct 26 2016
In Animation.cpp, the promise is being resolved (m_animation->m_finishedPromise->resolve(m_animation) executes in Animation::PlayStateUpdateScope::~PlayStateUpdateScope()), before ~Animation() is being called.
We added a registerPreFinalizer call to Animation's constructor, and saw Animation::dispose also being called after the promise was resolved.
Unfortunately, if garbage collection occurs, the callback never gets called. We verified this by adding setInterval(gc, 0) and now the bug occurs immediately.
This seems to be a bug in our Promise implementation.
Minified test:
<body>
<div id="target"></div>
<div id="output">Test running</div>
<script>
// Chrome must be run with --enable-experimental-web-platform-features --js-flags="--expose-gc"
// Test passes without this line
setInterval(gc, 0);
var promise = target.animate(null, 1).finished;
var timeout = setTimeout(function() {
output.textContent = 'Test fails';
}, 1000);
promise.then(() => {
output.textContent = 'Test passes';
clearTimeout(timeout);
});
</script>
</body>
,
Nov 14 2016
Hmm, isn't Animation::hasPendingActivity() a bit off? i.e., if it is in a non-finished state but with a pending "finished" promise, it (Animation) should stay around until the finished play state has been reached & promise resolved.
,
Nov 14 2016
TIL about hasPendingActivity(), it looks like updating that should fix this bug. Thanks!
,
Nov 15 2016
,
Nov 23 2016
,
Nov 24 2016
620160 is not related (verified).
,
Nov 25 2016
,
Nov 28 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/bfbc065caf6187d00c0e549f07ad264bb1614873 commit bfbc065caf6187d00c0e549f07ad264bb1614873 Author: loyso <loyso@chromium.org> Date: Mon Nov 28 06:34:28 2016 Blink Animation: Extend Animation's GC lifetime if it has a finish promise. BUG= 617539 Review-Url: https://codereview.chromium.org/2536523002 Cr-Commit-Position: refs/heads/master@{#434612} [add] https://crrev.com/bfbc065caf6187d00c0e549f07ad264bb1614873/third_party/WebKit/LayoutTests/web-animations-api/animation-finish-promise-gc.html [modify] https://crrev.com/bfbc065caf6187d00c0e549f07ad264bb1614873/third_party/WebKit/Source/core/animation/Animation.cpp
,
Nov 28 2016
,
Nov 28 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/ed16c60bdef2a6fca694e312d5a77e5b93d538d2 commit ed16c60bdef2a6fca694e312d5a77e5b93d538d2 Author: magjed <magjed@chromium.org> Date: Mon Nov 28 13:35:34 2016 Revert of Blink Animation: Extend Animation's GC lifetime if it has a finish promise. (patchset #2 id:20001 of https://codereview.chromium.org/2536523002/ ) Reason for revert: Causes unexpected leaks in webkit_tests: web-animations-api/animation-finish-event-cancelled.html web-animations-api/animation-cancel-ready-finished-ordering.html imported/wpt/web-animations/interfaces/Animation/finished.html web-animations-api/animation-onfinish.html imported/wpt/web-animations/interfaces/Animation/oncancel.html http/tests/security/mixedContent/websocket/insecure-websocket-in-secure-page-worker.html https://uberchromegw.corp.google.com/i/chromium.webkit/builders/WebKit%20Linux%20Trusty%20Leak/builds/342 Original issue's description: > Blink Animation: Extend Animation's GC lifetime if it has a finish promise. > > BUG= 617539 > > Committed: https://crrev.com/bfbc065caf6187d00c0e549f07ad264bb1614873 > Cr-Commit-Position: refs/heads/master@{#434612} TBR=alancutter@chromium.org,loyso@chromium.org # Skipping CQ checks because original CL landed less than 1 days ago. NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true BUG= 617539 Review-Url: https://codereview.chromium.org/2535793002 Cr-Commit-Position: refs/heads/master@{#434646} [delete] https://crrev.com/ab1f3f78f9d9e9f26302d11d0024adec60b07ea2/third_party/WebKit/LayoutTests/web-animations-api/animation-finish-promise-gc.html [modify] https://crrev.com/ed16c60bdef2a6fca694e312d5a77e5b93d538d2/third_party/WebKit/Source/core/animation/Animation.cpp
,
Nov 29 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/0d1a666647b23289500c395a9fe5cca2011d9bc7 commit 0d1a666647b23289500c395a9fe5cca2011d9bc7 Author: loyso <loyso@chromium.org> Date: Tue Nov 29 03:31:01 2016 Blink Animation: Extend Animation's GC lifetime if it has a finish promise. BUG= 617539 Committed: https://crrev.com/bfbc065caf6187d00c0e549f07ad264bb1614873 Review-Url: https://codereview.chromium.org/2536523002 Cr-Original-Commit-Position: refs/heads/master@{#434612} Cr-Commit-Position: refs/heads/master@{#434869} [add] https://crrev.com/0d1a666647b23289500c395a9fe5cca2011d9bc7/third_party/WebKit/LayoutTests/web-animations-api/animation-finish-promise-gc.html [modify] https://crrev.com/0d1a666647b23289500c395a9fe5cca2011d9bc7/third_party/WebKit/Source/core/animation/Animation.cpp |
||||||||||||||||||
►
Sign in to add a comment |
||||||||||||||||||
Comment 1 by ajha@chromium.org
, Jun 7 2016Components: Blink>Animation
Labels: Needs-Feedback