Bug 1257874 - Use getProperties instead of getFrames in animation-detail. r=pbro draft
authorNicolas Chevobbe <chevobbe.nicolas@gmail.com>
Fri, 18 Mar 2016 20:26:01 +0100
changeset 344644 3d553e5ee5aa0ca01edb9c37949e6c71aa1749dc
parent 344570 aadf5e15faf8ba88b48d16bb396f115816b8f673
child 517015 e0396033d716377bdd95208b9b454837420b896c
push id13892
push userchevobbe.nicolas@gmail.com
push dateFri, 25 Mar 2016 07:03:32 +0000
reviewerspbro
bugs1257874
milestone48.0a1
Bug 1257874 - Use getProperties instead of getFrames in animation-detail. r=pbro Add a getProperties function to the animation actor to map KeyframeEffectReadOnly.getProperties Call this new function in animation-detail and adapt the code as the structure of the returned object structure is different from what getFrames returns. Adapt a couple tests to the new object structure client-side, and a test server-side to check getProperties returns what it should. MozReview-Commit-ID: 2zKPEknukEK
devtools/client/animationinspector/animation-controller.js
devtools/client/animationinspector/animation-panel.js
devtools/client/animationinspector/components/animation-details.js
devtools/client/animationinspector/components/animation-timeline.js
devtools/client/animationinspector/test/browser_animation_animated_properties_displayed.js
devtools/client/animationinspector/test/browser_animation_keyframe_click_to_set_time.js
devtools/client/animationinspector/test/browser_animation_keyframe_markers.js
devtools/server/actors/animation.js
devtools/server/tests/browser/browser.ini
devtools/server/tests/browser/browser_animation_getProperties.js
--- a/devtools/client/animationinspector/animation-controller.js
+++ b/devtools/client/animationinspector/animation-controller.js
@@ -94,16 +94,18 @@ var getServerTraits = Task.async(functio
     { name: "hasSetPlaybackRates", actor: "animations",
       method: "setPlaybackRates" },
     { name: "hasTargetNode", actor: "domwalker",
       method: "getNodeFromActor" },
     { name: "hasSetCurrentTimes", actor: "animations",
       method: "setCurrentTimes" },
     { name: "hasGetFrames", actor: "animationplayer",
       method: "getFrames" },
+    { name: "hasGetProperties", actor: "animationplayer",
+      method: "getProperties" },
     { name: "hasSetWalkerActor", actor: "animations",
       method: "setWalkerActor" },
   ];
 
   let traits = {};
   for (let {name, actor, method} of config) {
     traits[name] = yield target.actorHasMethod(actor, method);
   }
--- a/devtools/client/animationinspector/animation-panel.js
+++ b/devtools/client/animationinspector/animation-panel.js
@@ -57,17 +57,18 @@ var AnimationsPanel = {
       "onPickerStopped", "refreshAnimationsUI", "onToggleAllClicked",
       "onTabNavigated", "onTimelineDataChanged", "onTimelinePlayClicked",
       "onTimelineRewindClicked", "onRateChanged"]) {
       this[functionName] = this[functionName].bind(this);
     }
     let hUtils = gToolbox.highlighterUtils;
     this.togglePicker = hUtils.togglePicker.bind(hUtils);
 
-    this.animationsTimelineComponent = new AnimationsTimeline(gInspector);
+    this.animationsTimelineComponent = new AnimationsTimeline(gInspector,
+      AnimationsController.traits);
     this.animationsTimelineComponent.init(this.playersEl);
 
     if (AnimationsController.traits.hasSetPlaybackRate) {
       this.rateSelectorComponent = new RateSelector();
       this.rateSelectorComponent.init(this.rateSelectorEl);
     }
 
     this.startListeners();
--- a/devtools/client/animationinspector/components/animation-details.js
+++ b/devtools/client/animationinspector/components/animation-details.js
@@ -11,100 +11,134 @@ const {Task} = Cu.import("resource://gre
 const {createNode, TimeScale} = require("devtools/client/animationinspector/utils");
 const {Keyframes} = require("devtools/client/animationinspector/components/keyframes");
 
 /**
  * UI component responsible for displaying detailed information for a given
  * animation.
  * This includes information about timing, easing, keyframes, animated
  * properties.
+ *
+ * @param {Object} serverTraits The list of server-side capabilities.
  */
-function AnimationDetails() {
+function AnimationDetails(serverTraits) {
   EventEmitter.decorate(this);
 
   this.onFrameSelected = this.onFrameSelected.bind(this);
 
   this.keyframeComponents = [];
+  this.serverTraits = serverTraits;
 }
 
 exports.AnimationDetails = AnimationDetails;
 
 AnimationDetails.prototype = {
   // These are part of frame objects but are not animated properties. This
   // array is used to skip them.
   NON_PROPERTIES: ["easing", "composite", "computedOffset", "offset"],
 
   init: function(containerEl) {
     this.containerEl = containerEl;
   },
 
   destroy: function() {
     this.unrender();
     this.containerEl = null;
+    this.serverTraits = null;
   },
 
   unrender: function() {
     for (let component of this.keyframeComponents) {
       component.off("frame-selected", this.onFrameSelected);
       component.destroy();
     }
     this.keyframeComponents = [];
 
     while (this.containerEl.firstChild) {
       this.containerEl.firstChild.remove();
     }
   },
 
   /**
-   * Convert a list of frames into a list of tracks, one per animated property,
-   * each with a list of frames.
+   * Get a list of the tracks of the animation actor
+   * @return {Object} A list of tracks, one per animated property, each
+   * with a list of keyframes
    */
-  getTracksFromFrames: function(frames) {
+  getTracks: Task.async(function*() {
     let tracks = {};
 
-    for (let frame of frames) {
-      for (let name in frame) {
-        if (this.NON_PROPERTIES.indexOf(name) != -1) {
-          continue;
-        }
+    /*
+     * getFrames is a AnimationPlayorActor method that returns data about the
+     * keyframes of the animation.
+     * In FF48, the data it returns change, and will hold only longhand
+     * properties ( e.g. borderLeftWidth ), which does not match what we
+     * want to display in the animation detail.
+     * A new AnimationPlayerActor function, getProperties, is introduced,
+     * that returns the animated css properties of the animation and their
+     * keyframes values.
+     * If the animation actor has the getProperties function, we use it, and if
+     * not, we fall back to getFrames, which then returns values we used to
+     * handle.
+     */
+    if (this.serverTraits.hasGetProperties) {
+      let properties = yield this.animation.getProperties();
+      for (let propertyObject of properties) {
+        let name = propertyObject.property;
 
         if (!tracks[name]) {
           tracks[name] = [];
         }
 
-        tracks[name].push({
-          value: frame[name],
-          offset: frame.computedOffset
-        });
+        for (let {value, offset} of propertyObject.values) {
+          tracks[name].push({value, offset});
+        }
+      }
+    } else {
+      let frames = yield this.animation.getFrames();
+      for (let frame of frames) {
+        for (let name in frame) {
+          if (this.NON_PROPERTIES.indexOf(name) != -1) {
+            continue;
+          }
+
+          if (!tracks[name]) {
+            tracks[name] = [];
+          }
+
+          tracks[name].push({
+            value: frame[name],
+            offset: frame.computedOffset
+          });
+        }
       }
     }
 
     return tracks;
-  },
+  }),
 
   render: Task.async(function*(animation) {
     this.unrender();
 
     if (!animation) {
       return;
     }
     this.animation = animation;
 
-    let frames = yield animation.getFrames();
-
     // We might have been destroyed in the meantime, or the component might
     // have been re-rendered.
     if (!this.containerEl || this.animation !== animation) {
       return;
     }
+
+    // Build an element for each animated property track.
+    this.tracks = yield this.getTracks(animation, this.serverTraits);
+
     // Useful for tests to know when the keyframes have been retrieved.
     this.emit("keyframes-retrieved");
 
-    // Build an element for each animated property track.
-    this.tracks = this.getTracksFromFrames(frames);
     for (let propertyName in this.tracks) {
       let line = createNode({
         parent: this.containerEl,
         attributes: {"class": "property"}
       });
 
       createNode({
         // text-overflow doesn't work in flex items, so we need a second level
--- a/devtools/client/animationinspector/components/animation-timeline.js
+++ b/devtools/client/animationinspector/components/animation-timeline.js
@@ -31,23 +31,25 @@ const TIMELINE_BACKGROUND_RESIZE_DEBOUNC
  * time play head.
  * Animations are organized by lines, with a left margin containing the preview
  * of the target DOM element the animation applies to.
  * The current time play head can be moved by clicking/dragging in the header.
  * when this happens, the component emits "current-data-changed" events with the
  * new time and state of the timeline.
  *
  * @param {InspectorPanel} inspector.
+ * @param {Object} serverTraits The list of server-side capabilities.
  */
-function AnimationsTimeline(inspector) {
+function AnimationsTimeline(inspector, serverTraits) {
   this.animations = [];
   this.targetNodes = [];
   this.timeBlocks = [];
   this.details = [];
   this.inspector = inspector;
+  this.serverTraits = serverTraits;
 
   this.onAnimationStateChanged = this.onAnimationStateChanged.bind(this);
   this.onScrubberMouseDown = this.onScrubberMouseDown.bind(this);
   this.onScrubberMouseUp = this.onScrubberMouseUp.bind(this);
   this.onScrubberMouseOut = this.onScrubberMouseOut.bind(this);
   this.onScrubberMouseMove = this.onScrubberMouseMove.bind(this);
   this.onAnimationSelected = this.onAnimationSelected.bind(this);
   this.onWindowResize = this.onWindowResize.bind(this);
@@ -127,16 +129,17 @@ AnimationsTimeline.prototype = {
 
     this.rootWrapperEl = null;
     this.timeHeaderEl = null;
     this.animationsEl = null;
     this.scrubberEl = null;
     this.scrubberHandleEl = null;
     this.win = null;
     this.inspector = null;
+    this.serverTraits = null;
   },
 
   /**
    * Destroy sub-components that have been created and stored on this instance.
    * @param {String} name An array of components will be expected in this[name]
    * @param {Array} handlers An option list of event handlers information that
    * should be used to remove these handlers.
    */
@@ -299,17 +302,17 @@ AnimationsTimeline.prototype = {
       let detailsEl = createNode({
         parent: this.animationsEl,
         nodeType: "li",
         attributes: {
           "class": "animated-properties"
         }
       });
 
-      let details = new AnimationDetails();
+      let details = new AnimationDetails(this.serverTraits);
       details.init(detailsEl);
       details.on("frame-selected", this.onFrameSelected);
       this.details.push(details);
 
       // Left sidebar for the animated node.
       let animatedNodeEl = createNode({
         parent: animationEl,
         attributes: {
--- a/devtools/client/animationinspector/test/browser_animation_animated_properties_displayed.js
+++ b/devtools/client/animationinspector/test/browser_animation_animated_properties_displayed.js
@@ -34,17 +34,17 @@ add_task(function*() {
   info("Click to select the animation");
   yield clickOnAnimation(panel, 0);
 
   ok(isNodeVisible(propertiesList),
      "The list of properties panel is shown");
   ok(propertiesList.querySelectorAll(".property").length,
      "The list of properties panel actually contains properties");
   ok(hasExpectedProperties(propertiesList),
-     "The list of proeprties panel contains the right properties");
+     "The list of properties panel contains the right properties");
 
   info("Click to unselect the animation");
   yield clickOnAnimation(panel, 0, true);
 
   ok(!isNodeVisible(propertiesList),
      "The list of properties panel is hidden again");
 });
 
--- a/devtools/client/animationinspector/test/browser_animation_keyframe_click_to_set_time.js
+++ b/devtools/client/animationinspector/test/browser_animation_keyframe_click_to_set_time.js
@@ -17,17 +17,17 @@ add_task(function*() {
   // the animations to be slightly offset with the header when it appears.
   // So for now, let's hide the scrollbar. Bug 1229340 should fix this.
   timeline.animationsEl.style.overflow = "hidden";
 
   info("Expand the animation");
   yield clickOnAnimation(panel, 0);
 
   info("Click on the first keyframe of the first animated property");
-  yield clickKeyframe(panel, 0, "backgroundColor", 0);
+  yield clickKeyframe(panel, 0, "background-color", 0);
 
   info("Make sure the scrubber stopped moving and is at the right position");
   yield assertScrubberMoving(panel, false);
   checkScrubberPos(scrubberEl, 0);
 
   info("Click on a keyframe in the middle");
   yield clickKeyframe(panel, 0, "transform", 2);
 
--- a/devtools/client/animationinspector/test/browser_animation_keyframe_markers.js
+++ b/devtools/client/animationinspector/test/browser_animation_keyframe_markers.js
@@ -49,26 +49,29 @@ add_task(function*() {
       is(markers[i].dataset.value, values[i],
          "Marker " + i + " for " + propertyName + " has the right value");
     }
   }
 });
 
 function* getExpectedKeyframesData(animation) {
   // We're testing the UI state here, so it's fine to get the list of expected
-  // keyframes from the animation actor.
-  let frames = yield animation.getFrames();
+  // properties from the animation actor.
+  let properties = yield animation.getProperties();
   let data = {};
 
-  for (let property of EXPECTED_PROPERTIES) {
-    data[property] = [];
-    for (let frame of frames) {
-      if (typeof frame[property] !== "undefined") {
-        data[property].push({
-          offset: frame.computedOffset,
-          value: frame[property]
+  for (let expectedProperty of EXPECTED_PROPERTIES) {
+    data[expectedProperty] = [];
+    for (let propertyObject of properties) {
+      if (propertyObject.property !== expectedProperty) {
+        continue;
+      }
+      for (let valueObject of propertyObject.values) {
+        data[expectedProperty].push({
+          offset: valueObject.offset,
+          value: valueObject.value
         });
       }
     }
   }
 
   return data;
 }
--- a/devtools/server/actors/animation.js
+++ b/devtools/server/actors/animation.js
@@ -385,16 +385,30 @@ var AnimationPlayerActor = ActorClass({
    */
   getFrames: method(function() {
     return this.player.effect.getFrames();
   }, {
     request: {},
     response: {
       frames: RetVal("json")
     }
+  }),
+
+  /**
+   * Get data about the animated properties of this animation player.
+   * @return {Object} Returns a list of animated properties.
+   * Each property contains a list of values and their offsets
+   */
+  getProperties: method(function() {
+    return this.player.effect.getProperties();
+  }, {
+    request: {},
+    response: {
+      frames: RetVal("json")
+    }
   })
 });
 
 exports.AnimationPlayerActor = AnimationPlayerActor;
 
 var AnimationPlayerFront = FrontClass(AnimationPlayerActor, {
   initialize: function(conn, form, detail, ctx) {
     Front.prototype.initialize.call(this, conn, form, detail, ctx);
--- a/devtools/server/tests/browser/browser.ini
+++ b/devtools/server/tests/browser/browser.ini
@@ -19,16 +19,17 @@ support-files =
   stylesheets-nested-iframes.html
   timeline-iframe-child.html
   timeline-iframe-parent.html
   director-script-target.html
   storage-helpers.js
 
 [browser_animation_emitMutations.js]
 [browser_animation_getFrames.js]
+[browser_animation_getProperties.js]
 [browser_animation_getMultipleStates.js]
 [browser_animation_getPlayers.js]
 [browser_animation_getStateAfterFinished.js]
 [browser_animation_getSubTreeAnimations.js]
 [browser_animation_keepFinished.js]
 [browser_animation_playerState.js]
 [browser_animation_playPauseIframe.js]
 [browser_animation_playPauseSeveral.js]
new file mode 100644
--- /dev/null
+++ b/devtools/server/tests/browser/browser_animation_getProperties.js
@@ -0,0 +1,37 @@
+/* vim: set ft=javascript ts=2 et sw=2 tw=80: */
+/* Any copyright is dedicated to the Public Domain.
+   http://creativecommons.org/publicdomain/zero/1.0/ */
+
+"use strict";
+
+// Check that the AnimationPlayerActor exposes a getProperties method that
+// returns the list of animated properties in the animation.
+
+const URL = MAIN_DOMAIN + "animation.html";
+
+add_task(function*() {
+  let {client, walker, animations} =
+    yield initAnimationsFrontForUrl(MAIN_DOMAIN + "animation.html");
+
+  info("Get the test node and its animation front");
+  let node = yield walker.querySelector(walker.rootNode, ".simple-animation");
+  let [player] = yield animations.getAnimationPlayersForNode(node);
+
+  ok(player.getProperties, "The front has the getProperties method");
+
+  let properties = yield player.getProperties();
+  is(properties.length, 1, "The correct number of properties was retrieved");
+
+  let propertyObject = properties[0];
+  is(propertyObject.property, "transform", "Property 0 is transform");
+
+  is(propertyObject.values.length, 2,
+    "The correct number of property values was retrieved");
+
+  // Note that we don't really test the content of the frame object here on
+  // purpose. This object comes straight out of the web animations API
+  // unmodified.
+
+  yield closeDebuggerClient(client);
+  gBrowser.removeCurrentTab();
+});