Devtools Animations: Support multiple frames in the animation timeline

Devtools Animations: Support multiple frames in the animation timeline

Currently, the animation timeline only works for animations attached to
the main document frame. This change ensures the inspector agent is
properly attached to all frames within a document. All start times sent
to the front end are normalised to the main frame timeline. When seeking
a timeline, the main frame timeline always understands these time values.
This information is then used to seek all other local frames.

BUG=466827
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=193855

[samli, 2015-04-13 01:16:50]

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

[samli, 2015-04-13 04:04:49]

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

[samli, 2015-04-13 04:04:50]

PTAL.

[dstockwell, 2015-04-13 22:50:01]

https://codereview.chromium.org/1081753002/diff/20001/Source/core/inspector/I...
File Source/core/inspector/InspectorAnimationAgent.cpp (right):

https://codereview.chromium.org/1081753002/diff/20001/Source/core/inspector/I...
Source/core/inspector/InspectorAnimationAgent.cpp:314: if
(normalizedStartTime(*player) - latestStartTime > 1000) {
While we're here, extract 1000 out to a named variable. I assume this is the
threshold for tracking a new set of animations.

https://codereview.chromium.org/1081753002/diff/20001/Source/core/inspector/I...
Source/core/inspector/InspectorAnimationAgent.cpp:348: return player.startTime()
+ (player.timeline()->zeroTime() - referenceTimeline().zeroTime()) * 1000 *
referenceTimeline().playbackRate();
I'm not sure this handles playbackRate == 0, Should we special case that and use
the delta between the timeline's current-times?

https://codereview.chromium.org/1081753002/diff/20001/Source/core/inspector/I...
File Source/core/inspector/InspectorAnimationAgent.h (right):

https://codereview.chromium.org/1081753002/diff/20001/Source/core/inspector/I...
Source/core/inspector/InspectorAnimationAgent.h:64: double
normalizedStartTime(AnimationPlayer&);
can't the parameter be const?

[samli, 2015-04-14 01:11:27]

ptal

https://codereview.chromium.org/1081753002/diff/20001/Source/core/inspector/I...
File Source/core/inspector/InspectorAnimationAgent.cpp (right):

https://codereview.chromium.org/1081753002/diff/20001/Source/core/inspector/I...
Source/core/inspector/InspectorAnimationAgent.cpp:314: if
(normalizedStartTime(*player) - latestStartTime > 1000) {
On 2015/04/13 at 22:50:00, dstockwell wrote:
> While we're here, extract 1000 out to a named variable. I assume this is the
threshold for tracking a new set of animations.

Yeah, done.

https://codereview.chromium.org/1081753002/diff/20001/Source/core/inspector/I...
Source/core/inspector/InspectorAnimationAgent.cpp:348: return player.startTime()
+ (player.timeline()->zeroTime() - referenceTimeline().zeroTime()) * 1000 *
referenceTimeline().playbackRate();
On 2015/04/13 at 22:50:00, dstockwell wrote:
> I'm not sure this handles playbackRate == 0, Should we special case that and
use the delta between the timeline's current-times?

Done.

https://codereview.chromium.org/1081753002/diff/20001/Source/core/inspector/I...
File Source/core/inspector/InspectorAnimationAgent.h (right):

https://codereview.chromium.org/1081753002/diff/20001/Source/core/inspector/I...
Source/core/inspector/InspectorAnimationAgent.h:64: double
normalizedStartTime(AnimationPlayer&);
On 2015/04/13 at 22:50:00, dstockwell wrote:
> can't the parameter be const?

zeroTime() isn't const.

[pfeldman, 2015-04-14 09:37:54]

https://codereview.chromium.org/1081753002/diff/60001/Source/devtools/front_e...
File Source/devtools/front_end/elements/AnimationTimeline.js (right):

https://codereview.chromium.org/1081753002/diff/60001/Source/devtools/front_e...
Source/devtools/front_end/elements/AnimationTimeline.js:181:
target.animationModel.setPlaybackRate(this._animationsPlaybackRate);
You want to instrument it on the backend and set current time / playback rate
upon frameAdded.

[samli, 2015-04-15 03:19:16]

https://codereview.chromium.org/1081753002/diff/60001/Source/devtools/front_e...
File Source/devtools/front_end/elements/AnimationTimeline.js (right):

https://codereview.chromium.org/1081753002/diff/60001/Source/devtools/front_e...
Source/devtools/front_end/elements/AnimationTimeline.js:181:
target.animationModel.setPlaybackRate(this._animationsPlaybackRate);
On 2015/04/14 09:37:53, pfeldman wrote:
> You want to instrument it on the backend and set current time / playback rate
> upon frameAdded.

Done. PTAL, couldn't use the regular frame added instrumentation event. At the
time the LocalFrame is created, there is no DOMWindow attached. When the
DOMWindow is attached, no document has been loaded into the DOMWindow. So I've
instrumented when the document is loaded into the DOMWindow attached to a frame.
This is the only way to retrieve the timeline.

[pfeldman, 2015-04-15 17:56:33]

https://codereview.chromium.org/1081753002/diff/80001/Source/core/frame/Local...
File Source/core/frame/LocalDOMWindow.cpp (right):

https://codereview.chromium.org/1081753002/diff/80001/Source/core/frame/Local...
Source/core/frame/LocalDOMWindow.cpp:380:
InspectorInstrumentation::documentAttachedToFrame(frame());
I wonder if we could (ab)use the didClearDocumentOfWindowObject signal for this
purpose. Could you check if it does the trick?

[samli, 2015-04-16 01:37:37]

ptal

https://codereview.chromium.org/1081753002/diff/80001/Source/core/frame/Local...
File Source/core/frame/LocalDOMWindow.cpp (right):

https://codereview.chromium.org/1081753002/diff/80001/Source/core/frame/Local...
Source/core/frame/LocalDOMWindow.cpp:380:
InspectorInstrumentation::documentAttachedToFrame(frame());
On 2015/04/15 17:56:33, pfeldman wrote:
> I wonder if we could (ab)use the didClearDocumentOfWindowObject signal for
this
> purpose. Could you check if it does the trick?

Worked.

[pfeldman, 2015-04-16 05:45:40]

Awesome! lgtm.

[samli, 2015-04-16 07:32:53]

The CQ bit was checked by samli@chromium.org

[commit-bot: I haz the power, 2015-04-16 07:33:02]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1081753002/100001

[samli, 2015-04-16 09:31:45]

The CQ bit was unchecked by samli@chromium.org

[samli, 2015-04-16 09:31:49]

The CQ bit was checked by samli@chromium.org

[commit-bot: I haz the power, 2015-04-16 09:32:47]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1081753002/100001

[commit-bot: I haz the power, 2015-04-16 11:20:31]

Committed patchset #6 (id:100001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=193855