Devtools Animations: Pause button based on groups not global

Devtools Animations: Pause button based on groups not global

This change moves the pause button from a global toggle to one that
only locally affects the selected group.

BUG=447083
Committed: https://crrev.com/06fec0f5518c839857b295e0bf6e65de22e64500
Cr-Commit-Position: refs/heads/master@{#356219}

[samli, 2015-10-21 22:21:20]

samli@chromium.org changed reviewers:
+ pfeldman@chromium.org

[pfeldman, 2015-10-21 22:53:10]

https://codereview.chromium.org/1417173002/diff/20001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/inspector-protocol/animation/animation-pause.html
(right):

https://codereview.chromium.org/1417173002/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/inspector-protocol/animation/animation-pause.html:41:
setTimeout(InspectorTest.evaluateInPage.bind(InspectorTest, "getWidth()",
checkWidth), 50);
timeout is in most cases a source of the flake. there is no guarantee you get
frames, etc. What are you trying to simulate here?

[samli, 2015-10-21 22:54:21]

https://codereview.chromium.org/1417173002/diff/20001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/inspector-protocol/animation/animation-pause.html
(right):

https://codereview.chromium.org/1417173002/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/inspector-protocol/animation/animation-pause.html:41:
setTimeout(InspectorTest.evaluateInPage.bind(InspectorTest, "getWidth()",
checkWidth), 50);
On 2015/10/21 at 22:53:10, pfeldman_slow wrote:
> timeout is in most cases a source of the flake. there is no guarantee you get
frames, etc. What are you trying to simulate here?

That it is indeed truly paused.

[samli, 2015-10-21 23:46:29]

On 2015/10/21 at 22:54:21, samli wrote:
>
https://codereview.chromium.org/1417173002/diff/20001/third_party/WebKit/Layo...
> File
third_party/WebKit/LayoutTests/inspector-protocol/animation/animation-pause.html
(right):
> 
>
https://codereview.chromium.org/1417173002/diff/20001/third_party/WebKit/Layo...
>
third_party/WebKit/LayoutTests/inspector-protocol/animation/animation-pause.html:41:
setTimeout(InspectorTest.evaluateInPage.bind(InspectorTest, "getWidth()",
checkWidth), 50);
> On 2015/10/21 at 22:53:10, pfeldman_slow wrote:
> > timeout is in most cases a source of the flake. there is no guarantee you
get frames, etc. What are you trying to simulate here?
> 
> That it is indeed truly paused.

Trying to simulate the passing of time, where frames would have been painted
that if the animation was /not/ paused, it would have moved the element.

[samli, 2015-10-22 18:30:28]

Description was changed from

==========
Devtools Animations: Pause button based on groups not global

This change moves the pause button from a global toggle to one that
only locally affects the selected group.

BUG=447082
==========

to

==========
Devtools Animations: Pause button based on groups not global

This change moves the pause button from a global toggle to one that
only locally affects the selected group.

BUG=447083
==========

[samli, 2015-10-23 18:32:19]

ping

[pfeldman, 2015-10-26 18:20:50]

https://codereview.chromium.org/1417173002/diff/20001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/inspector-protocol/animation/animation-pause.html
(right):

https://codereview.chromium.org/1417173002/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/inspector-protocol/animation/animation-pause.html:41:
setTimeout(InspectorTest.evaluateInPage.bind(InspectorTest, "getWidth()",
checkWidth), 50);
But 50 gives you nothing, it could result in 0 or 10000. And that is the
front-end timer that is not in sync with the page. Wouldn't setPaused pause
synchronously? If so, should you call getWidth right after? Where is asynchrony
coming from?

[samli, 2015-10-26 18:30:49]

https://codereview.chromium.org/1417173002/diff/20001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/inspector-protocol/animation/animation-pause.html
(right):

https://codereview.chromium.org/1417173002/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/inspector-protocol/animation/animation-pause.html:41:
setTimeout(InspectorTest.evaluateInPage.bind(InspectorTest, "getWidth()",
checkWidth), 50);
On 2015/10/26 at 18:20:50, pfeldman wrote:
> But 50 gives you nothing, it could result in 0 or 10000. And that is the
front-end timer that is not in sync with the page. Wouldn't setPaused pause
synchronously? If so, should you call getWidth right after? Where is asynchrony
coming from?

If its correctly paused, it doesn't matter if I measure 0 or 10000ms later.

setPaused does pause synchronously. I do call getWidth() immediately, and that
just tells me what the width is, but I have no guarantee when I initially called
setPaused(). What I'm testing is that if the animation did NOT pause, then with
some change in time, the element would move. No frames are required to be
painted, just the passing of time. We don't have any other public API to test it
is synchronously paused, and since I'm using clones on top of original
animations, testing the actual element is the best sanity check I can perform.

asynchrony = passing of time for the animation in the fail case.

[pfeldman, 2015-10-26 20:21:24]

As per our offline discussion, lets use raf.

[samli, 2015-10-26 21:30:23]

https://codereview.chromium.org/1417173002/diff/40001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/http/tests/inspector-protocol/resources/protocol-test.html
(right):

https://codereview.chromium.org/1417173002/diff/40001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/http/tests/inspector-protocol/resources/protocol-test.html:332:
testRunner.evaluateInWebInspector(2,
"InspectorTest.didInvokePageFunctionPromise(" + callId + ", " +
JSON.stringify(result) + ", true);");
2 was evalCallbackCallId in inspector-test.js, but when I tried to add "var
evalCallbackCallId = 2;
" it said evalCallbackCallId was undefined. Not sure why, if they're being run
in different contexts?

[pfeldman, 2015-10-26 21:35:48]

lgtm

https://codereview.chromium.org/1417173002/diff/40001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/http/tests/inspector-protocol/resources/protocol-test.html
(right):

https://codereview.chromium.org/1417173002/diff/40001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/http/tests/inspector-protocol/resources/protocol-test.html:332:
testRunner.evaluateInWebInspector(2,
"InspectorTest.didInvokePageFunctionPromise(" + callId + ", " +
JSON.stringify(result) + ", true);");
I don't think this number matters at all.

[samli, 2015-10-27 00:46:55]

The CQ bit was checked by samli@chromium.org

[commit-bot: I haz the power, 2015-10-27 00:47:41]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1417173002/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1417173002/40001

[commit-bot: I haz the power, 2015-10-27 00:57:29]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-10-27 00:57:30]

Try jobs failed on following builders:
  mac_chromium_gn_rel on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_gn_r...)

[samli, 2015-10-27 01:32:16]

The CQ bit was checked by samli@chromium.org

[commit-bot: I haz the power, 2015-10-27 01:33:45]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1417173002/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1417173002/40001

[commit-bot: I haz the power, 2015-10-27 02:44:16]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2015-10-27 02:45:01]

Patchset 3 (id:??) landed as
https://crrev.com/06fec0f5518c839857b295e0bf6e65de22e64500
Cr-Commit-Position: refs/heads/master@{#356219}