Closed Bug 1406285 Opened 7 years ago Closed 6 years ago

React-ify animation item

Categories

(DevTools :: Inspector: Animations, enhancement, P3)

Product:
DevTools
Component:
Inspector: Animations
Type:
enhancement

Tracking

(firefox59 fixed)

Status:
RESOLVED FIXED
Milestone:
Firefox 59
Tracking Flags:
Tracking Status
firefox59 --- fixed

People

(Reporter: daisuke, Assigned: daisuke)

References

(Blocks 1 open bug)

Details

Attachments

(21 files, 1 obsolete file)

Bug 1406285 - Part 1: Implement animation target node.
59 bytes, text/x-review-board-request
gl
: review+
Details
Bug 1406285 - Part 2: Add test for animation target node.
59 bytes, text/x-review-board-request
gl
: review+
Details
Bug 1406285 - Part 3: Implement summary graph base.
59 bytes, text/x-review-board-request
gl
: review+
Details
Bug 1406285 - Part 4: Implement getting tracks(keyframes) from server.
59 bytes, text/x-review-board-request
gl
: review+
Details
Bug 1406285 - Part 5: Implement computed timing graph of summary graph.
59 bytes, text/x-review-board-request
gl
: review+
Details
Bug 1406285 - Part 6: Apply computed timing graph color by animation type.
59 bytes, text/x-review-board-request
gl
: review+
Details
Bug 1406285 - Part 7: Implement effect timing graph.
59 bytes, text/x-review-board-request
gl
: review+
Details
Bug 1406285 - Part 8: Implement delay component.
59 bytes, text/x-review-board-request
gl
: review+
Details
Bug 1406285 - Part 9: Implement endDelay component.
59 bytes, text/x-review-board-request
gl
: review+
Details
Bug 1406285 - Part 10: Implement negative delay path.
59 bytes, text/x-review-board-request
gl
: review+
Details
Bug 1406285 - Part 11: Implement negative endDelay path.
59 bytes, text/x-review-board-request
gl
: review+
Details
Bug 1406285 - Part 12: Implement animation name label.
59 bytes, text/x-review-board-request
gl
: review+
Details
Bug 1406285 - Part 13: Implement tooltip.
59 bytes, text/x-review-board-request
gl
: review+
Details
Bug 1406285 - Part 14: Implement compositor sign.
59 bytes, text/x-review-board-request
gl
: review+
Details
Bug 1406285 - Part 15: Implement scrollable.
59 bytes, text/x-review-board-request
gl
: review+
Details
Bug 1406285 - Part 16: Do async rendering.
59 bytes, text/x-review-board-request
gl
: review+
Details
Bug 1406285 - Part 17: Add tests.
59 bytes, text/x-review-board-request
gl
: review+
Details
sync.gif
1.11 MB, image/gif
Details
async-only-getting-node-front.gif
1.10 MB, image/gif
Details
async.gif
1.17 MB, image/gif
Details
Bug 1406285 - Part 18: Rename tests for consistency.
59 bytes, text/x-review-board-request
gl
: review+
Details 
In this bug, implement following features:

* summary graph
* target node
Severity: normal → enhancement
Priority: -- → P3
Comment on attachment 8927173 [details]
Bug 1406285 - Part 1: Implement animation target node.

https://reviewboard.mozilla.org/r/198418/#review204360

::: devtools/client/inspector/animation/animation.js:12
(Diff revision 2)
>  const { AnimationsFront } = require("devtools/shared/fronts/animation");
>  const { createElement, createFactory } = require("devtools/client/shared/vendor/react");
>  const { Provider } = require("devtools/client/shared/vendor/react-redux");
>  
> +const EventEmitter = require("devtools/shared/event-emitter");
> +const { DomNodePreview } = require("devtools/client/inspector/shared/dom-node-preview");

We should stop using DomNodePreview and actually take on using Rep. You can see how this was done in the Grid Inspector which has the same essential dom-node-preview but doesn't require the same amount of code.

See https://searchfox.org/mozilla-central/source/devtools/client/inspector/grids/components/GridItem.js#140 and https://bugzilla.mozilla.org/show_bug.cgi?id=1338298

I have to stop reviewing Part 1 and 2 since this will set you back a bit.
Attachment #8927173 - Flags: review?(gl)
Comment on attachment 8927173 [details]
Bug 1406285 - Part 1: Implement animation target node.

https://reviewboard.mozilla.org/r/198418/#review204384

::: devtools/client/inspector/animation/animation.js:12
(Diff revision 2)
>  const { AnimationsFront } = require("devtools/shared/fronts/animation");
>  const { createElement, createFactory } = require("devtools/client/shared/vendor/react");
>  const { Provider } = require("devtools/client/shared/vendor/react-redux");
>  
> +const EventEmitter = require("devtools/shared/event-emitter");
> +const { DomNodePreview } = require("devtools/client/inspector/shared/dom-node-preview");

I see thanks!
Comment on attachment 8927173 [details]
Bug 1406285 - Part 1: Implement animation target node.

https://reviewboard.mozilla.org/r/198418/#review207666

::: devtools/client/inspector/animation/animation.js:54
(Diff revision 3)
>          key: "newanimationinspector",
>          store: this.inspector.store
>        },
>        App(
>          {
> +          getNodeFromActor: this.getNodeFromActor,

You can destructure 

{ getNodeFromActor, toggleElementPicker } = this

above to avoid setting these.

::: devtools/client/inspector/animation/components/AnimationTarget.js:79
(Diff revision 3)
> +
> +  async updateNodeFront(animation) {
> +    const { getNodeFromActor } = this.props;
> +
> +    // Try and get it from the playerFront directly.
> +    let nodeFront = animation.animationTargetNodeFront;

We should avoid getting the node from the actor in the React component here. We can do this on update() in animation.js when we got all animations from getAnimationPlayersForNode and mapping all the animations to include the nodeFront and then store it in our Redux store..
Attachment #8927173 - Flags: review?(gl)
Comment on attachment 8927174 [details]
Bug 1406285 - Part 2: Add test for animation target node.

https://reviewboard.mozilla.org/r/198420/#review207668

::: devtools/client/inspector/animation/components/AnimationTarget.js:96
(Diff revision 3)
>          return;
>        }
>      }
>  
>      this.setState({ nodeFront });
> +    emitEventForTest("animation-target-rendered");

I don't see any usage of this emitted event for the test. So, I assume we don't need this. We also using waitUntilState if necessary.
Attachment #8927174 - Flags: review?(gl)
Comment on attachment 8927173 [details]
Bug 1406285 - Part 1: Implement animation target node.

https://reviewboard.mozilla.org/r/198418/#review207666

> We should avoid getting the node from the actor in the React component here. We can do this on update() in animation.js when we got all animations from getAnimationPlayersForNode and mapping all the animations to include the nodeFront and then store it in our Redux store..

Thank you the reviewing and sorry for delay.

If we get nodeFront by synchronous before dispatching, felt slowly a bit. Although I mesured the time till all nodeFronts was loaded using this[1], it took 678ms average in my local environment. So, it might be better to draw by asynchronous, I think. ( Also, patch 16[2] corresponds to get keyframes from server by asynchronous since similar reason )

The approachings except the way of this patch and emitter for testing are,,

1. after dispatch the updateAnimations, load all nodeFronts, then map to each animation, then updateAnimations again.
2. after dispatch the updateAnimations, load all nodeFronts, but dispatch each nodeFront as new action when it was loaded.

Which is better you think?? Or do you have any other ideas?
( I'll proceed approach 1 till got your answer. )

[1] https://bug1210796.bmoattachments.org/attachment.cgi?id=8892259
[2] https://reviewboard.mozilla.org/r/198448/diff/3#index_header
Ok, I think based on your performance testing async rendering would be the way to go. I would assume #1 would cause re-rendering so #2 is probably better based on what you told me. I know you said 678 ms average (for synchronous before dispatch I assume), how did that compare to async rendering?
Attached image sync.gif — 
No, I did not compare to async one.
I measured the time to get all node front synchronously.

I attach a short movie of doing all synchronously here.
It took almost 700ms until starting to draw.
The source code is as follows.

```
for (const animation of animations) {
  const nodeFront =
    await this.inspector.walker.getNodeFromActor(animation.actorID, ["node"]);
  animation.nodeFront = nodeFront;
}

this.inspector.store.dispatch(updateAnimations(animations));
```

I'll attach other version in next comment.
This attachment is asynchronous only for getting node front.
The source code is as follows:

```
const nodeFronts = await Promise.all(animations.map(animation => {
  return this.inspector.walker.getNodeFromActor(animation.actorID, ["node"]);
}));

for (const [index, animation] of animations.entries()) {
  animation.nodeFront = nodeFronts[index];
}

this.inspector.store.dispatch(updateAnimations(animations));
```

This took almost 150ms until starting to draw. But this is better than full sync version.
Attached image async.gif — 
Finally, this is full async version.
The source code is as follows:

```
this.inspector.store.dispatch(updateAnimations(animations));

for (const animation of animations) {
  const { actorID } = animation;
  this.inspector.walker.getNodeFromActor(actorID, ["node"]).then(nodeFront => {
    this.inspector.store.dispatch(updateNodeFront(actorID, nodeFront));
  });
}
```

This does not take a time to wait to start to draw.
And, feels like a bit faster than other.
(In reply to Daisuke Akatsuka (:daisuke) from comment #66)
> Created attachment 8932730 [details]
> async.gif
> 
> Finally, this is full async version.
> The source code is as follows:
> 
> ```
> this.inspector.store.dispatch(updateAnimations(animations));
> 
> for (const animation of animations) {
>   const { actorID } = animation;
>   this.inspector.walker.getNodeFromActor(actorID, ["node"]).then(nodeFront
> => {
>     this.inspector.store.dispatch(updateNodeFront(actorID, nodeFront));
>   });
> }
> ```
> 
> This does not take a time to wait to start to draw.
> And, feels like a bit faster than other.

In terms of the gifs, this async rendering looks like the best. I am wondering if we really need to dispatch(updateNodeFront(actorID, nodeFront)) and update the store or can we just let the React component handle the getNodeFromActor and async rendering.

Also, where is this bit of the code happening?
for (const animation of animations) {
  const { actorID } = animation;
  this.inspector.walker.getNodeFromActor(actorID, ["node"]).then(nodeFront => {
    this.inspector.store.dispatch(updateNodeFront(actorID, nodeFront));
  });
}
I would imagine what I am describing is already what you had implemented in Part 1.
Yeah, we can use getNodeFromActor, however as your comment 60, we should avoid getting the node front in React component, right?

I had implemented "2. after dispatch the updateAnimations, load all nodeFronts, but dispatch each nodeFront as new action when it was loaded." which is in comment 62. ( The source code of comment 66 is that one. )
Also, it almost done, so could you review this version??
(In reply to Daisuke Akatsuka (:daisuke) from comment #69)
> Yeah, we can use getNodeFromActor, however as your comment 60, we should
> avoid getting the node front in React component, right?
> 
> I had implemented "2. after dispatch the updateAnimations, load all
> nodeFronts, but dispatch each nodeFront as new action when it was loaded."
> which is in comment 62. ( The source code of comment 66 is that one. )
> Also, it almost done, so could you review this version??

Yes, I originally said that without knowing about the advantages of doing async rendering. I think I would be happy with your original solution now. Sorry!
Let me take a look at your latest Part 1 and we can chat tonight.
Do you know what the performance difference is between #2 and what you implemented originally with doing getNodeFromActor in the React component?
I have measured total rendering (not only animation target node) time for both original and latest patches.

The average times of measurement with reloading in 10 times are,
original: 701ms
latest  : 1039ms

As you said, will revert to the original in this time. Thanks!
Comment on attachment 8927173 [details]
Bug 1406285 - Part 1: Implement animation target node.

https://reviewboard.mozilla.org/r/198418/#review209628

::: devtools/client/inspector/animation/animation.js:106
(Diff revision 4)
>      }
>  
>      done();
>    }
>  
> +  getNodeFronts() {

Use async await

::: devtools/client/inspector/animation/components/AnimationTarget.js:128
(Diff revision 5)
> +          defaultRep: ElementNode,
> +          mode: MODE.TINY,
> +          object: this.translateNodeFrontToGrip(nodeFront),
> +          onDOMNodeMouseOut: () => onHideBoxModelHighlighter(),
> +          onDOMNodeMouseOver: () => onShowBoxModelHighlighterForNode(nodeFront),
> +          onInspectIconClick: () => setSelectedNode(nodeFront, "layout-panel"),

s/layout-panel/animation-panel

::: devtools/client/inspector/animation/test/head.js:129
(Diff revision 5)
>  };
> +
> +/**
> + * Wait for all AnimationTarget components to be fully loaded
> + * (fetched their related actor and rendered).
> + * @param {Inspector} inspector

Add a new line before @param

::: devtools/client/themes/animation.css:10
(Diff revision 5)
>  /* Animation-inspector specific theme variables */
>  
>  :root {
>    --animation-even-background-color: rgba(0, 0, 0, 0.05);
>    --command-pick-image: url(chrome://devtools/skin/images/command-pick.svg);
> +  --sidebar-width: 200px;

Does this need to be a variable or can it just be placed directly in .animation-target?
Attachment #8927173 - Flags: review?(gl) → review+
Comment on attachment 8927174 [details]
Bug 1406285 - Part 2: Add test for animation target node.

https://reviewboard.mozilla.org/r/198420/#review210332

::: devtools/client/inspector/animation/test/browser_animation_AnimationTarget.js:10
(Diff revision 5)
> +
> +// Test existance and content of animation target.
> +
> +add_task(async function () {
> +  await addTab(URL_ROOT + "doc_simple_animation.html");
> +

Remove this blank line.

::: devtools/client/inspector/animation/test/browser_animation_AnimationTarget.js:14
(Diff revision 5)
> +  await addTab(URL_ROOT + "doc_simple_animation.html");
> +
> +  const { inspector, panel } = await openAnimationInspector();
> +
> +  info("Checking the animation target elements existance");
> +  const animationItemEls = panel.querySelectorAll(".animation-list .animation-item");

Maybe also check the number of expected animation items you expected.
Attachment #8927174 - Flags: review?(gl) → review+
Comment on attachment 8927175 [details]
Bug 1406285 - Part 3: Implement summary graph base.

https://reviewboard.mozilla.org/r/198422/#review210334
Attachment #8927175 - Flags: review?(gl) → review+
Status: NEW → ASSIGNED
Comment on attachment 8927173 [details]
Bug 1406285 - Part 1: Implement animation target node.

https://reviewboard.mozilla.org/r/198418/#review209628

> Does this need to be a variable or can it just be placed directly in .animation-target?

Oh, sorry. I had wanted to use to .animation-timeline-tick-list as well..
Comment on attachment 8927176 [details]
Bug 1406285 - Part 4: Implement getting tracks(keyframes) from server.

https://reviewboard.mozilla.org/r/198424/#review210440

::: devtools/client/inspector/animation/animation.js:113
(Diff revision 5)
>        selection.isConnected() && selection.isElementNode()
>        ? await this.animationsFront.getAnimationPlayersForNode(selection.nodeFront)
>        : [];
>  
>      if (!this.animations || !isAllAnimationEqual(animations, this.animations)) {
> +      const keyframesMaps = await Promise.all(animations.map(animation => {

s/keyframesMaps/keyframesMap

::: devtools/client/inspector/animation/animation.js:118
(Diff revision 5)
> +      const keyframesMaps = await Promise.all(animations.map(animation => {
> +        return this.getKeyframesMap(animation);
> +      }));
> +
> +      for (const [index, animation] of animations.entries()) {
> +        animation.keyframesMap = await keyframesMaps[index];

Do we need the await here? I am seeing mostly Objects and Arrays being pushed into the keyframesMap

::: devtools/client/inspector/animation/animation.js:141
(Diff revision 5)
> +   *
> +   * @param {Object} animation
> +   * @return {Object} A map of keyframes for per animated property
> +   */
> +  async getKeyframesMap(animation) {
> +    const tracks = {};

tracks is not quite clear to me as a naming for a map. I would call this keyframesMap

::: devtools/client/inspector/animation/animation.js:143
(Diff revision 5)
> +   * @return {Object} A map of keyframes for per animated property
> +   */
> +  async getKeyframesMap(animation) {
> +    const tracks = {};
> +
> +    if (!this.serverTraits) {

Do we still need to check for backward compatiblity? I assume these server features have been implemented and in the server for at least a year now. There was some work to remove really old traits since we were very unlikely to need to debug something that far back. If the last update to the server was FF48, I would assume it is now safe to remove them or at least not have to check it in 59.

::: devtools/client/inspector/animation/animation.js:166
(Diff revision 5)
> +      let properties = [];
> +      try {
> +        properties = await animation.getProperties();
> +      } catch (e) {
> +        // Expected if we've already been destroyed in the meantime.
> +        throw e;

I don't usually see us throwing errors. I would at least also do console.error(e)

::: devtools/client/inspector/animation/animation.js:224
(Diff revision 5)
> +   * features should be enabled/disabled.
> +   *
> +   * @param {Target} target The current toolbox target.
> +   * @return {Object} An object with boolean properties.
> +   */
> +  async getServerTraits() {

Same question as above if we really need this anymore.

::: devtools/client/inspector/animation/components/graph/SummaryGraphPath.js:23
(Diff revision 5)
>        timeScale: PropTypes.object.isRequired,
>      };
>    }
>  
> +  componentDidMount() {
> +    this.forceUpdate();

Normally, forceUpdate should be avoided. I am not seeing anything in particular that should be done before rendering. So, I am also wondering about the mounting condition when we render.

::: devtools/client/inspector/animation/components/graph/SummaryGraphPath.js:62
(Diff revision 5)
> +  }
> +
>    render() {
> +    const thisEl = ReactDOM.findDOMNode(this);
> +
> +    if (!thisEl) {

I am wondering if this is needed same as above. I would imagine we can just check if we have any animation props and do the same thing of rendering an unmounted dom.svg

::: devtools/client/inspector/animation/utils/utils.js:21
(Diff revision 5)
> + * @param {String} jsPropertyName
> + *                 A camelcased CSS property name. Typically something that comes
> + *                 out of computed styles. E.g. borderBottomColor
> + * @return {String} The corresponding CSS property name: e.g. border-bottom-color
> + */
> +function getCssPropertyName(jsPropertyName) {

Move this below findOptimalTimeInterval() function

::: devtools/client/inspector/animation/utils/utils.js:102
(Diff revision 5)
>           stateA.fill === stateB.fill &&
>           stateA.iterationCount === stateB.iterationCount &&
>           stateA.iterationStart === stateB.iterationStart;
>  }
>  
> +exports.getCssPropertyName = getCssPropertyName;

Same as above.
Attachment #8927176 - Flags: review?(gl)
Comment on attachment 8927177 [details]
Bug 1406285 - Part 5: Implement computed timing graph of summary graph.

https://reviewboard.mozilla.org/r/198426/#review210790

::: devtools/client/inspector/animation/components/graph/ComputedTimingPath.js:47
(Diff revision 5)
> +    // The reason why we use computed value instead of computed timing progress is to
> +    // include the easing in keyframes as well. Although the computed timing progress
> +    // is not affected by the easing in keyframes at all, computed value reflects that.
> +    const frames = keyframes.map(keyframe => {
> +      return {
> +        opacity: keyframe.offset,

Did  you mean keyframe.opacity instead of keyframe.offset here?

::: devtools/client/inspector/animation/components/graph/ComputedTimingPath.js:54
(Diff revision 5)
> +        easing: keyframe.easing
> +      };
> +    });
> +
> +    const win = getWindow();
> +    const dummyAnimationEl = win.document.createElement("div");

I would personally like to see us stop using dummy elements to sample rules/animations. Is this possible?

::: devtools/client/inspector/animation/components/graph/ComputedTimingPath.js:87
(Diff revision 5)
> +        return pathString;
> +      },
> +      totalDuration: totalDisplayedDuration,
> +    };
> +
> +    const element = dom.g(

We can simply return dom.g and not have to set this to a constant.
Attachment #8927177 - Flags: review?(gl)
Comment on attachment 8927176 [details]
Bug 1406285 - Part 4: Implement getting tracks(keyframes) from server.

https://reviewboard.mozilla.org/r/198424/#review210440

> s/keyframesMaps/keyframesMap

I will changed to keyframesMapList. Thanks.

> Do we still need to check for backward compatiblity? I assume these server features have been implemented and in the server for at least a year now. There was some work to remove really old traits since we were very unlikely to need to debug something that far back. If the last update to the server was FF48, I would assume it is now safe to remove them or at least not have to check it in 59.

Nice to hear. I'll remove checking serverTraits.
Comment on attachment 8927177 [details]
Bug 1406285 - Part 5: Implement computed timing graph of summary graph.

https://reviewboard.mozilla.org/r/198426/#review210888

::: devtools/client/inspector/animation/components/graph/ComputedTimingPath.js:47
(Diff revision 5)
> +    // The reason why we use computed value instead of computed timing progress is to
> +    // include the easing in keyframes as well. Although the computed timing progress
> +    // is not affected by the easing in keyframes at all, computed value reflects that.
> +    const frames = keyframes.map(keyframe => {
> +      return {
> +        opacity: keyframe.offset,

Since computed style of 'opacity' is used as graph value, it is set here.

::: devtools/client/inspector/animation/components/graph/ComputedTimingPath.js:54
(Diff revision 5)
> +        easing: keyframe.easing
> +      };
> +    });
> +
> +    const win = getWindow();
> +    const dummyAnimationEl = win.document.createElement("div");

I don't think so. Although we can inquiry the sample data to server, it takes a lot time. ( I had tried once. )
Comment on attachment 8927176 [details]
Bug 1406285 - Part 4: Implement getting tracks(keyframes) from server.

https://reviewboard.mozilla.org/r/198424/#review212206

::: devtools/client/inspector/animation/animation.js:108
(Diff revision 6)
>        selection.isConnected() && selection.isElementNode()
>        ? await this.animationsFront.getAnimationPlayersForNode(selection.nodeFront)
>        : [];
>  
>      if (!this.animations || !isAllAnimationEqual(animations, this.animations)) {
> +      const keyframesMapList = await Promise.all(animations.map(animation => {

s/keyframesMapList/keyframesMap

We want to be clear about what the object is - just a Map in this case. It is really a Map of Animation to a List of Animation Properties.

::: devtools/client/inspector/animation/animation.js:133
(Diff revision 6)
>  
> +  /**
> +   * Get a map of the keyframes of the animation actor
> +   *
> +   * @param {Object} animation
> +   * @return {Object} A map of keyframes for per animated property

We should be more descriptive about the return Object - Comment on the key:value pair and what each key, value object represent. Also update the JSDoc with anything you add to this @return description.

::: devtools/client/inspector/animation/animation.js:145
(Diff revision 6)
> +    } catch (e) {
> +      // Expected if we've already been destroyed in the meantime.
> +      console.error(e);
> +    }
> +
> +    const keyframesMap = {};

Can this just be a JS Map()?

::: devtools/client/inspector/animation/components/graph/SummaryGraphPath.js:34
(Diff revision 6)
> +
> +  componentDidMount() {
> +    this.updateDurationPerPixel();
> +  }
> +
> +  getOffsetAndEasingOnlyKeyframesList(keyframesMap) {

s/getOffsetAndEasingOnlyKeyframesList/getOffsetAndEasingOnlyKeyframes

Add a JSDoc to note that the @return {Array}

::: devtools/client/inspector/animation/components/graph/SummaryGraphPath.js:35
(Diff revision 6)
> +  componentDidMount() {
> +    this.updateDurationPerPixel();
> +  }
> +
> +  getOffsetAndEasingOnlyKeyframesList(keyframesMap) {
> +    return Object.keys(keyframesMap).reduce((result, name) => {

I think we should use a Map for keyframesMap and avoid using Object.keys/Object.reduce since this really makes the code harder to understand when it doesn't have to be.
Attachment #8927176 - Flags: review?(gl)
Comment on attachment 8927185 [details]
Bug 1406285 - Part 13: Implement tooltip.

https://reviewboard.mozilla.org/r/198442/#review210796

::: devtools/client/inspector/animation/components/graph/SummaryGraph.js:28
(Diff revision 5)
>        getWindow: PropTypes.func.isRequired,
>        timeScale: PropTypes.object.isRequired,
>      };
>    }
>  
> +  getTooltipText(state) {

s/getTooltipText/getTitleText

::: devtools/client/inspector/animation/utils/l10n.js:13
(Diff revision 5)
>  const L10N =
>    new LocalizationHelper("devtools/client/locales/animationinspector.properties");
>  
> +/**
> + * Get a formatted title for this animation. This will be either:
> + * "some-name", "some-name : CSS Transition", "some-name : CSS Animation",

s/some-name/%S

::: devtools/client/inspector/animation/utils/l10n.js:17
(Diff revision 5)
> + * Get a formatted title for this animation. This will be either:
> + * "some-name", "some-name : CSS Transition", "some-name : CSS Animation",
> + * "some-name : Script Animation", or "Script Animation", depending
> + * if the server provides the type, what type it is and if the animation
> + * has a name
> + * @param {Object} state

Add a new line before @param

::: devtools/client/inspector/animation/utils/l10n.js:19
(Diff revision 5)
> + * "some-name : Script Animation", or "Script Animation", depending
> + * if the server provides the type, what type it is and if the animation
> + * has a name
> + * @param {Object} state
> + */
> +function getFormattedTitle(state) {

I would move this into SummaryGraph.js since the getFormatStr is very specific for that component only and cannot be reused
Attachment #8927185 - Flags: review?(gl)
Comment on attachment 8927177 [details]
Bug 1406285 - Part 5: Implement computed timing graph of summary graph.

https://reviewboard.mozilla.org/r/198426/#review212208

::: devtools/client/inspector/animation/components/graph/ComputedTimingPath.js:54
(Diff revision 6)
> +        easing: keyframe.easing
> +      };
> +    });
> +
> +    const win = getWindow();
> +    const dummyAnimationEl = win.document.createElement("div");

Let's put all this dummyAnimation/opacity logic inside of animation.js and add a getter to get the opacity. That way we can just add the dummy animation element once and only once and removed when it is destroyed. I don't think this should be inside of the component, and we should perhaps avoid passing the window object to the components.
Attachment #8927177 - Flags: review?(gl)
Comment on attachment 8927178 [details]
Bug 1406285 - Part 6: Apply computed timing graph color by animation type.

https://reviewboard.mozilla.org/r/198428/#review212210

::: devtools/client/themes/animation.css:59
(Diff revision 6)
>  
>  .animation-item:nth-child(2n+1) {
>    background-color: var(--animation-even-background-color);
>  }
>  
> +.animation-item.cssanimation {

s/cssanimation/css-animation

::: devtools/client/themes/animation.css:60
(Diff revision 6)
>  .animation-item:nth-child(2n+1) {
>    background-color: var(--animation-even-background-color);
>  }
>  
> +.animation-item.cssanimation {
> +  --computed-timing-graph-color: var(--theme-contrast-background);

--theme-contrast-background is not variable that has been updated to be a Photon color. I would add a note that this needs to be fixed in the future and use a Photon color variable or find a new color.

::: devtools/client/themes/animation.css:63
(Diff revision 6)
>  
> +.animation-item.cssanimation {
> +  --computed-timing-graph-color: var(--theme-contrast-background);
> +}
> +
> +.animation-item.csstransition {

s/csstransition/css-animation

::: devtools/client/themes/animation.css:67
(Diff revision 6)
> +
> +.animation-item.csstransition {
> +  --computed-timing-graph-color: var(--theme-highlight-blue);
> +}
> +
> +.animation-item.scriptanimation {

s/scriptanimation/script-animation

::: devtools/client/themes/animation.css:68
(Diff revision 6)
> +.animation-item.csstransition {
> +  --computed-timing-graph-color: var(--theme-highlight-blue);
> +}
> +
> +.animation-item.scriptanimation {
> +  --computed-timing-graph-color: var(--theme-graphs-green);

Same as above. If you can use --theme-highlight-green, I would do that.
Comment on attachment 8927179 [details]
Bug 1406285 - Part 7: Implement effect timing graph.

https://reviewboard.mozilla.org/r/198430/#review212212

::: devtools/client/inspector/animation/components/graph/EffectTimingPath.js:38
(Diff revision 6)
> +    const effectTiming = Object.assign({}, state, {
> +      iterations: state.iterationCount ? state.iterationCount : Infinity
> +    });
> +
> +    const win = getWindow();
> +    const dummyAnimation =

Same comment in Part 5 about moving the dummy animation logic into animation.js so we don't have to use getWindow inside of React components and just call a getter to get what you needed.

::: devtools/client/themes/animation.css:61
(Diff revision 6)
>    background-color: var(--animation-even-background-color);
>  }
>  
>  .animation-item.cssanimation {
>    --computed-timing-graph-color: var(--theme-contrast-background);
> +  --effect-timing-graph-color: var(--theme-highlight-lightorange);

This color variable has not been photonized. Add a note that this variable need to be changed or pick a new color.

::: devtools/client/themes/animation.css:66
(Diff revision 6)
> +  --effect-timing-graph-color: var(--theme-highlight-lightorange);
>  }
>  
>  .animation-item.csstransition {
>    --computed-timing-graph-color: var(--theme-highlight-blue);
> +  --effect-timing-graph-color: var(--theme-highlight-bluegrey);

Same as above.
Attachment #8927179 - Flags: review?(gl)
Comment on attachment 8927180 [details]
Bug 1406285 - Part 8: Implement delay component.

https://reviewboard.mozilla.org/r/198432/#review212214

::: devtools/client/themes/animation.css:123
(Diff revision 6)
>  .animation-effect-timing-path path.infinity:nth-child(n+2) {
>    opacity: .3;
>  }
>  
> +.animation-delay-sign {
> +  background-color: var(--theme-graphs-grey);

This is not a photonized color. Add a note that this color variable needs to be changed or find a new color variable.
Attachment #8927180 - Flags: review?(gl) → review+
Comment on attachment 8927178 [details]
Bug 1406285 - Part 6: Apply computed timing graph color by animation type.

https://reviewboard.mozilla.org/r/198428/#review212216
Attachment #8927178 - Flags: review?(gl) → review+
Comment on attachment 8927181 [details]
Bug 1406285 - Part 9: Implement endDelay component.

https://reviewboard.mozilla.org/r/198434/#review212218

::: devtools/client/inspector/animation/components/graph/EndDelaySign.js:37
(Diff revision 6)
> +    const endDelayClass = state.endDelay < 0 ? "negative" : "";
> +    const fillClass = state.fill === "both" || state.fill === "forwards" ? "fill" : "";
> +
> +    return dom.div(
> +      {
> +        className: `animation-enddelay-sign ${ endDelayClass } ${ fillClass }`,

s/enddelay/end-delay

::: devtools/client/themes/animation.css:123
(Diff revision 6)
>  .animation-effect-timing-path path.infinity:nth-child(n+2) {
>    opacity: .3;
>  }
>  
> -.animation-delay-sign {
> +.animation-delay-sign,
> +.animation-enddelay-sign {

s/enddelay/end-delay for all of them.
Attachment #8927181 - Flags: review?(gl) → review+
Comment on attachment 8927182 [details]
Bug 1406285 - Part 10: Implement negative delay path.

https://reviewboard.mozilla.org/r/198436/#review212220

::: devtools/client/inspector/animation/components/graph/NegativePath.js:55
(Diff revision 6)
> +        easing: keyframe.easing
> +      };
> +    });
> +
> +    const win = getWindow();
> +    const dummyAnimationEl = win.document.createElement("div");

Same about not using getWindow in React components.
Attachment #8927182 - Flags: review?(gl)
Comment on attachment 8927176 [details]
Bug 1406285 - Part 4: Implement getting tracks(keyframes) from server.

https://reviewboard.mozilla.org/r/198424/#review212206

> We should be more descriptive about the return Object - Comment on the key:value pair and what each key, value object represent. Also update the JSDoc with anything you add to this @return description.

I discussed with :gl on IRC, we rename this method to getAnimatedPropertyMap.
Accompanied by this change, we rename the variables.
Comment on attachment 8927178 [details]
Bug 1406285 - Part 6: Apply computed timing graph color by animation type.

https://reviewboard.mozilla.org/r/198428/#review212210

> s/cssanimation/css-animation

I added animation type which is from the actor[1] as class name as it is.
Even so, is it better we change the class name?

[1] https://searchfox.org/mozilla-central/source/devtools/server/actors/animation.js#36

> --theme-contrast-background is not variable that has been updated to be a Photon color. I would add a note that this needs to be fixed in the future and use a Photon color variable or find a new color.

Okay, thanks.
try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=7da4c08d7ac26d4cb5e80b88b0fae3df029a5da1

Also, in this time, I modified patches to reflect a changing for bug 1422218.
Comment on attachment 8927173 [details]
Bug 1406285 - Part 1: Implement animation target node.

https://reviewboard.mozilla.org/r/198418/#review215308

::: devtools/client/inspector/animation/animation.js:14
(Diff revision 7)
>  const { Provider } = require("devtools/client/shared/vendor/react-redux");
>  
> +const EventEmitter = require("devtools/shared/event-emitter");
> +
>  const App = createFactory(require("./components/App"));
> -const { isAllTimingEffectEqual } = require("./utils/utils");
> +const { isAllAnimationEqual } = require("./utils/utils");

Move this down below { updateSidebarSize }

::: devtools/client/inspector/animation/components/AnimationTarget.js:27
(Diff revision 7)
> +      onShowBoxModelHighlighterForNode: PropTypes.func.isRequired,
> +      setSelectedNode: PropTypes.func.isRequired,
> +    };
> +  }
> +
> +  constructor() {

constructor(props) and super(props)

::: devtools/client/inspector/animation/components/AnimationTarget.js:30
(Diff revision 7)
> +  }
> +
> +  constructor() {
> +    super();
> +
> +    this.state = {};

We should still set the initial state. 

this.state = {
  nodeFront: null,
};

So, we know what the React state will be populated with.

::: devtools/client/inspector/animation/utils/utils.js:48
(Diff revision 7)
>      multiplier *= 10;
>    }
>  }
>  
>  /**
> - * Check the equality timing effects from given animations.
> + * Check the equality the given animations.

Check the equality of the given animations.

::: devtools/client/inspector/animation/utils/utils.js:51
(Diff revision 7)
>  
>  /**
> - * Check the equality timing effects from given animations.
> + * Check the equality the given animations.
>   *
>   * @param {Array} animations.
>   * @param {Array} same to avobe.

same as above.
Comment on attachment 8927176 [details]
Bug 1406285 - Part 4: Implement getting tracks(keyframes) from server.

https://reviewboard.mozilla.org/r/198424/#review215310

::: devtools/client/inspector/animation/animation.js:135
(Diff revisions 6 - 7)
>    /**
> -   * Get a map of the keyframes of the animation actor
> +   * Return a map of animated property from given animation actor.
>     *
>     * @param {Object} animation
> -   * @return {Object} A map of keyframes for per animated property
> +   * @return {Map} A map of animated property
> +   *               key: {String} property name

I would only indent up to the "{" of {Map}

::: devtools/client/inspector/animation/animation.js:135
(Diff revisions 6 - 7)
>    /**
> -   * Get a map of the keyframes of the animation actor
> +   * Return a map of animated property from given animation actor.
>     *
>     * @param {Object} animation
> -   * @return {Object} A map of keyframes for per animated property
> +   * @return {Map} A map of animated property
> +   *               key: {String} property name

Animated property name

::: devtools/client/inspector/animation/animation.js:145
(Diff revision 7)
> +   *                 offset: offset of keyframe,
> +   *                 easing:  easing from this keyframe to next keyframe,
> +   *                 distance: use as y coordinate in graph,
> +   *               }
> +   */
> +  async getAnimatedPropertyMap(animation) {

Move these getX methods below destroy() so it's all alphabetically ordered again.

::: devtools/client/inspector/animation/components/graph/SummaryGraphPath.js:39
(Diff revision 7)
> +  /**
> +   * Return animatable keyframes list which has only offset and easing.
> +   * Also, this method remove duplicate offset keyframe by prioritize backward.
> +   *
> +   * @param {Map} animated property map
> +   *              which can get form getAnimatedPropertyMap in animation.js

Indent this below the {Map} or see how the @param are style in https://searchfox.org/mozilla-central/source/devtools/client/inspector/grids/grid-inspector.js#150.
Attachment #8927176 - Flags: review?(gl) → review+
Comment on attachment 8927177 [details]
Bug 1406285 - Part 5: Implement computed timing graph of summary graph.

https://reviewboard.mozilla.org/r/198426/#review215642

::: devtools/client/inspector/animation/animation.js:60
(Diff revision 7)
>      } = this;
>  
>      const target = this.inspector.target;
>      this.animationsFront = new AnimationsFront(target.client, target.form);
>  
> +    const simulatedElement = this.win.document.createElement("div");

Can we lazily create this element and simulatedAnimation only when simulateAnimation is called.

::: devtools/client/inspector/animation/animation.js:101
(Diff revision 7)
>      this.inspector.toolbox.off("picker-started", this.onElementPickerStarted);
>      this.inspector.toolbox.off("picker-stopped", this.onElementPickerStopped);
>  
> +    this.simulatedAnimation.cancel();
> +    this.simulatedElement.remove();
> +    this.simulatedAnimation = null;

Move this.simulatedAnimation and this.simulatedElement with this.inspector/win.

::: devtools/client/inspector/animation/animation.js:184
(Diff revision 7)
>      }
>  
>      return animatedPropertyMap;
>    }
>  
> +  simulateAnimation(keyframes, effectTiming, isElementNeeded) {

Add a JSDoc.

::: devtools/client/inspector/animation/components/graph/SummaryGraphPath.js:14
(Diff revision 7)
>  const dom = require("devtools/client/shared/vendor/react-dom-factories");
>  const ReactDOM = require("devtools/client/shared/vendor/react-dom");
>  
>  const ComputedTimingPath = createFactory(require("./ComputedTimingPath"));
> +// Minimum opacity for semitransparent fill color for keyframes's easing graph.
> +const MIN_KEYFRAMES_EASING_OPACITY = .5;

s/.5/0.5

::: devtools/client/inspector/animation/components/graph/TimingPath.js:15
(Diff revision 7)
> +// Show max 10 iterations for infinite animations
> +// to give users a clue that the animation does repeat.
> +const MAX_INFINITE_ANIMATIONS_ITERATIONS = 10;
> +
> +class TimingPath extends PureComponent {
> +  renderGraph(state, helper) {

Add JSDoc for each of these render functions so someone else can easily figure out what is being rendered as well.

::: devtools/client/inspector/animation/utils/graph-helper.js:212
(Diff revision 7)
> +    pathString += `L${ segment.x },${ segment.y } `;
> +  });
> +  return pathString;
> +}
> +
> +module.exports.SummaryGraphHelper = SummaryGraphHelper;

s/module.exports/exports
Attachment #8927177 - Flags: review?(gl)
Comment on attachment 8927178 [details]
Bug 1406285 - Part 6: Apply computed timing graph color by animation type.

https://reviewboard.mozilla.org/r/198428/#review212210

> I added animation type which is from the actor[1] as class name as it is.
> Even so, is it better we change the class name?
> 
> [1] https://searchfox.org/mozilla-central/source/devtools/server/actors/animation.js#36

How shall we do?
try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=01f53c5d23e428b3e0c9de34874cd1561d41b870

Hi Gabriel,
Sorry, I modified getOffsetAndEasingOnlyKeyframes in patch 4 to fit to original behavior.
https://reviewboard.mozilla.org/r/198424/diff/8#index_header
If possible, could you review again?

Thanks.
Comment on attachment 8927176 [details]
Bug 1406285 - Part 4: Implement getting tracks(keyframes) from server.

https://reviewboard.mozilla.org/r/198424/#review219118

::: devtools/client/inspector/animation/animation.js:104
(Diff revisions 7 - 8)
> -   *               value: {Array} array of keyframe object
> -   *               Also, the keyframe object is consisted as following.
> -   *               {
> -   *                 value: style,
> -   *                 offset: offset of keyframe,
> -   *                 easing:  easing from this keyframe to next keyframe,
> +   *         value: {Array} Array of keyframe object
> +   *         Also, the keyframe object is consisted as following.
> +   *         {
> +   *           value: style,
> +   *           offset: offset of keyframe,
> +   *           easing:  easing from this keyframe to next keyframe,

NIT 2 spaces after :

::: devtools/client/inspector/animation/components/graph/SummaryGraphPath.js:69
(Diff revisions 7 - 8)
> +   *       },
> +   *     ],
> +   *   },
> +   * ]
> +   *
> +   * then this method returns,

Is this return object correct in the example?

::: devtools/client/inspector/animation/animation.js:102
(Diff revision 8)
> +   * @return {Map} A map of animated property
> +   *         key: {String} Animated property name
> +   *         value: {Array} Array of keyframe object
> +   *         Also, the keyframe object is consisted as following.
> +   *         {
> +   *           value: style,

I would also specify the type for each property in this keyframe object.

value: {String} style,
Comment on attachment 8927177 [details]
Bug 1406285 - Part 5: Implement computed timing graph of summary graph.

https://reviewboard.mozilla.org/r/198426/#review219126

::: devtools/client/inspector/animation/components/graph/TimingPath.js:18
(Diff revision 8)
> +
> +class TimingPath extends PureComponent {
> +  /**
> +   * Render a graph of given parameters and return as <path> element list.
> +   *
> +   * @param {Object} state - State of animation.

The description of these @param should be on the next line.

::: devtools/client/inspector/animation/utils/graph-helper.js:30
(Diff revision 8)
> +class SummaryGraphHelper {
> +  /**
> +   * Constructor.
> +   *
> +   * @param {Object} state - animation state.
> +   * @param {Array} keyframes - keyframes.

@param description should be on the next line.
Attachment #8927177 - Flags: review?(gl) → review+
Comment on attachment 8927179 [details]
Bug 1406285 - Part 7: Implement effect timing graph.

https://reviewboard.mozilla.org/r/198430/#review219154
Attachment #8927179 - Flags: review?(gl) → review+
Comment on attachment 8927182 [details]
Bug 1406285 - Part 10: Implement negative delay path.

https://reviewboard.mozilla.org/r/198436/#review219156

::: devtools/client/inspector/animation/components/graph/NegativeDelayPath.js:32
(Diff revision 8)
> +
> +  renderGraph(state, helper) {
> +    const startTime = state.delay;
> +    const endTime = 0;
> +    const segments = helper.createPathSegments(startTime, endTime);
> +    return dom.path(

Add a new line before.

::: devtools/client/inspector/animation/components/graph/SummaryGraphPath.js:186
(Diff revision 8)
>              totalDuration,
>            }
>          )
>        ),
>        animation.state.easing !== "linear" ?
>        EffectTimingPath(

Indent this.

::: devtools/client/inspector/animation/components/graph/SummaryGraphPath.js:197
(Diff revision 8)
>          }
>        )
>        :
> +      null,
> +      animation.state.delay < 0 ?
> +      keyframesList.map(keyframes => {

Indent this to make it easier to follow.
Attachment #8927182 - Flags: review?(gl) → review+
Comment on attachment 8927183 [details]
Bug 1406285 - Part 11: Implement negative endDelay path.

https://reviewboard.mozilla.org/r/198438/#review219158

::: devtools/client/inspector/animation/components/graph/NegativeEndDelayPath.js:32
(Diff revision 8)
> +
> +  renderGraph(state, helper) {
> +    const endTime = state.delay + state.iterationCount * state.duration;
> +    const startTime = endTime + state.endDelay;
> +    const segments = helper.createPathSegments(startTime, endTime);
> +    return dom.path(

Add a new line before

::: devtools/client/inspector/animation/components/graph/SummaryGraphPath.js:212
(Diff revision 8)
>          );
>        })
>        :
> +      null,
> +      animation.state.iterationCount && animation.state.endDelay < 0 ?
> +      keyframesList.map(keyframes => {

Indent this.
Attachment #8927183 - Flags: review?(gl) → review+
Comment on attachment 8927184 [details]
Bug 1406285 - Part 12: Implement animation name label.

https://reviewboard.mozilla.org/r/198440/#review219160

::: devtools/client/inspector/animation/components/graph/SummaryGraph.js:62
(Diff revision 8)
>          }
>        )
>        :
> +      null,
> +      animation.state.name ?
> +      AnimationName(

Indent this.
Attachment #8927184 - Flags: review?(gl) → review+
Comment on attachment 8927185 [details]
Bug 1406285 - Part 13: Implement tooltip.

https://reviewboard.mozilla.org/r/198442/#review219164
Attachment #8927185 - Flags: review?(gl) → review+
Comment on attachment 8927186 [details]
Bug 1406285 - Part 14: Implement compositor sign.

https://reviewboard.mozilla.org/r/198444/#review219168

::: devtools/client/inspector/animation/components/graph/SummaryGraph.js:133
(Diff revision 8)
>      } = this.props;
> +    const compositorClass = animation.state.isRunningOnCompositor ? "compositor" : "";
>  
>      return dom.div(
>        {
> -        className: "animation-summary-graph",
> +        className: `animation-summary-graph ${ compositorClass }`,

I think I prefer seeing

className: "animation-summary-graph" +
           (isRunningOnComposition ? " compositor" : "")
Attachment #8927186 - Flags: review?(gl) → review+
Comment on attachment 8927187 [details]
Bug 1406285 - Part 15: Implement scrollable.

https://reviewboard.mozilla.org/r/198446/#review219174

::: devtools/client/themes/animation.css:54
(Diff revision 8)
>    position: absolute;
>  }
>  
>  /* Animation List */
>  .animation-list {
> +  height: calc(100% - 24px);

This looks like a red flag to me since we are subtracting what I think is the height of the header, which means we aren't setting up our layout correctly. 

It's really hard to test this on top of 15 patches tho.
Attachment #8927187 - Flags: review?(gl) → review+
Comment on attachment 8927188 [details]
Bug 1406285 - Part 16: Do async rendering.

https://reviewboard.mozilla.org/r/198448/#review219176

::: devtools/client/inspector/animation/components/graph/SummaryGraphPath.js:35
(Diff revision 8)
>  
>    constructor(props) {
>      super(props);
>  
>      this.state = {
>        durationPerPixel: 0,

Add keyframesList to the initial state. 

Also, add a description for each property in the state.
Attachment #8927188 - Flags: review?(gl) → review+
Comment on attachment 8927189 [details]
Bug 1406285 - Part 17: Add tests.

https://reviewboard.mozilla.org/r/198450/#review219190

::: devtools/client/inspector/animation/test/browser_animation_SummaryGraph_NegativeDelayPath.js:39
(Diff revision 8)
> +    } = testCase;
> +
> +    const animationItemEl =
> +      findAnimationItemElementsByTargetClassName(panel, targetClassName);
> +
> +    info(`Checking negative delay path existance for ${ targetClassName }`);

s/existance/existence/g

This is actually a spelling mistake, sorry.

::: devtools/client/inspector/animation/test/head.js:132
(Diff revision 8)
> + * Wait for rendering.
> + *
> + * @param {AnimationInspector} animationInspector
> + */
> +const waitForRendering = async function (animationInspector) {
> +  await Promise.all([waitForAllAnimationTargets(animationInspector),

Format it to be 
Promise.all([
 wait,
 wait,
])

::: devtools/client/inspector/animation/test/head.js:163
(Diff revision 8)
> +
> +/**
> + * SummaryGraph is constructed by <path> element.
> + * This function checks the vertex of path segments.
> + *
> + * @param {Element} pathEl - <path> element.

Put @param descriptions on a new line
Attachment #8927189 - Flags: review?(gl) → review+
Comment on attachment 8927190 [details]
Bug 1406285 - Part 18: Rename tests for consistency.

https://reviewboard.mozilla.org/r/198452/#review219192
Attachment #8927190 - Flags: review?(gl) → review+
Comment on attachment 8927176 [details]
Bug 1406285 - Part 4: Implement getting tracks(keyframes) from server.

https://reviewboard.mozilla.org/r/198424/#review219118

> Is this return object correct in the example?

Yes, this is correct.
Attachment #8927190 - Attachment is obsolete: true
I fixed all that are pointed in review.
Will land if the try has no problem.
https://treeherder.mozilla.org/#/jobs?repo=try&revision=75a11b7593fb73898181d94b3ec1c1095d8c9792

Thanks!
Comment on attachment 8943499 [details]
Bug 1406285 - Part 18: Rename tests for consistency.

https://reviewboard.mozilla.org/r/213832/#review219598

So, I actually think we should rename all the unit tests so it is follows our naming convention. See http://docs.firefox-dev.tools/tests/writing-tests.html. 
A good naming convention is browser_<panel>_<short-description>[_N].js

We don't typically camel case in <short-description> either so I would probably remove that and separate the words with "-".
Attachment #8943499 - Flags: review?(gl) → review+
Comment on attachment 8943499 [details]
Bug 1406285 - Part 18: Rename tests for consistency.

https://reviewboard.mozilla.org/r/213832/#review219598

Thanks, Gabriel.
I will do in this patch.
Pushed by dakatsuka@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/284d394ce46b
Part 1: Implement animation target node. r=gl
https://hg.mozilla.org/integration/autoland/rev/67a986bb3a23
Part 2: Add test for animation target node. r=gl
https://hg.mozilla.org/integration/autoland/rev/ad4a9cbf32d4
Part 3: Implement summary graph base. r=gl
https://hg.mozilla.org/integration/autoland/rev/bbe12d4a23d9
Part 4: Implement getting tracks(keyframes) from server. r=gl
https://hg.mozilla.org/integration/autoland/rev/fcac2692bb2e
Part 5: Implement computed timing graph of summary graph. r=gl
https://hg.mozilla.org/integration/autoland/rev/9226b8d3ed05
Part 6: Apply computed timing graph color by animation type. r=gl
https://hg.mozilla.org/integration/autoland/rev/92156bb3ceb6
Part 7: Implement effect timing graph. r=gl
https://hg.mozilla.org/integration/autoland/rev/2e4538c1d08e
Part 8: Implement delay component. r=gl
https://hg.mozilla.org/integration/autoland/rev/d948ac6aff2c
Part 9: Implement endDelay component. r=gl
https://hg.mozilla.org/integration/autoland/rev/101599271efd
Part 10: Implement negative delay path. r=gl
https://hg.mozilla.org/integration/autoland/rev/479b3feae1c5
Part 11: Implement negative endDelay path. r=gl
https://hg.mozilla.org/integration/autoland/rev/2f2996b6664e
Part 12: Implement animation name label. r=gl
https://hg.mozilla.org/integration/autoland/rev/217f71d0cb61
Part 13: Implement tooltip. r=gl
https://hg.mozilla.org/integration/autoland/rev/0f89a44387fd
Part 14: Implement compositor sign. r=gl
https://hg.mozilla.org/integration/autoland/rev/914edf2a0415
Part 15: Implement scrollable. r=gl
https://hg.mozilla.org/integration/autoland/rev/26ebf9ff6aff
Part 16: Do async rendering. r=gl
https://hg.mozilla.org/integration/autoland/rev/0d98859cc0a6
Part 17: Add tests. r=gl
https://hg.mozilla.org/integration/autoland/rev/7971d134b3a3
Part 18: Rename tests for consistency. r=gl
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: