New issue
Advanced search Search tips

Issue 771722 link

Starred by 2 users
Status: Available
Owner: ----
Cc:
flackr@chromium.org
smcgruer@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug

Blocking:
issue 772407



Sign in to add a comment

Web Animations: blink::Animation::setStartTime does not handle null values

Project Member Reported by smcgruer@chromium.org, Oct 4 2017 
Currently blink::Animation::setStartTime ignores null values (is_null == true), instead treating them as setting a start time of 0:

void Animation::setStartTime(double start_time, bool is_null) {
  PlayStateUpdateScope update_scope(*this, kTimingUpdateOnDemand);

  if (start_time == start_time_)
    return;

  current_time_pending_ = false;
  play_state_ = kUnset;
  paused_ = false;
  SetStartTimeInternal(start_time / 1000);
}

This causes us to fail external/wpt/web-animations/timing-model/animations/current-time.html, as well as being incorrect by the spec (https://w3c.github.io/web-animations/#setting-the-start-time-of-an-animation).

Comment 1 by smcgruer@chromium.org, Oct 4 2017 

This also causes us to fail:

external/wpt/web-animations/timing-model/animations/set-the-animation-start-time.html:
  - Setting an unresolved start time sets the hold time

The reason is that instead of running the procedure for a null start time, we set a zero start time instead and get an incorrect current time from that.

Comment 2 by shans@chromium.org, Oct 4 2017

Labels: Hotlist-Interop

Comment 3 by shend@chromium.org, Oct 5 2017

Labels: Update-Quarterly

Comment 4 by smcgruer@chromium.org, Oct 6 2017

Blocking: 772407
Project Member

Comment 5 by bugdroid1@chromium.org, Oct 12 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/c48db31689105ce81dcbe640416e45860299c248

commit c48db31689105ce81dcbe640416e45860299c248
Author: Stephen McGruer <smcgruer@chromium.org>
Date: Thu Oct 12 20:34:35 2017

Update crbug links for some failing WPT web-animations tests

Bug:  600248 , 771722, 771751, 771977,  771985 ,  772014 , 772048, 772060, 772076 
Change-Id: Ie08474f751fd45484627c8a52d84db13ca6b39ac
Reviewed-on: https://chromium-review.googlesource.com/702536
Commit-Queue: Stephen McGruer <smcgruer@chromium.org>
Reviewed-by: Robert Flack <flackr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#508447}
[modify] https://crrev.com/c48db31689105ce81dcbe640416e45860299c248/third_party/WebKit/LayoutTests/TestExpectations

Comment 6 by smcgruer@chromium.org, Feb 28 2018 

Incidentally, this allows you to crash devtools in DCHECK-on mode.

https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/inspector/InspectorAnimationAgent.cpp?sq=package:chromium&l=326 sets the clone start time to the original animation start time, so by pausing an animation from javascript and then clicking on it in devtools you will hit the isfinite DCHECK in SetStartTimeInternal.

Comment 7 by smcgruer@chromium.org, Apr 25 2018

Cc: smcgruer@chromium.org
Owner: ----
Status: Available (was: Assigned)
Not planning to work on this in the near future, dropping to Available.
►

Sign in to add a comment