Bug 1254419 - Rename getPropertyState() to getProperties(); r=heycam, r=bz draft
authorBrian Birtles <birtles@gmail.com>
Sun, 13 Mar 2016 19:10:10 +0800
changeset 341455 3d0ec847647ba471773a8f7329b2361d34a66866
parent 341454 91d5f177e1230f6ce6d59177bb9f32148d40503f
child 341456 5c606be7c89e66892b25e9fc2d21c73128de9c04
child 341461 6d35edc0acca015cf7b7b725ec6a996754e71f88
push id13214
push userbbirtles@mozilla.com
push dateThu, 17 Mar 2016 02:15:12 +0000
reviewersheycam, bz
bugs1254419
milestone48.0a1
Bug 1254419 - Rename getPropertyState() to getProperties(); r=heycam, r=bz We are now extending this API to include more than just metadata about each animated property but also the property values themselves. Note that we can't use the name AnimationProperty for the dictionary since we already use that name internally and [BinaryName] doesn't seem to apply to dictionaries. MozReview-Commit-ID: AcXeN4fsgTz
dom/animation/KeyframeEffect.cpp
dom/animation/KeyframeEffect.h
dom/animation/test/chrome/test_animation_property_state.html
dom/webidl/KeyframeEffect.webidl
--- a/dom/animation/KeyframeEffect.cpp
+++ b/dom/animation/KeyframeEffect.cpp
@@ -1795,37 +1795,38 @@ KeyframeEffectReadOnly::GetTarget(
 
     default:
       NS_NOTREACHED("Animation of unsupported pseudo-type");
       aRv.SetNull();
   }
 }
 
 void
-KeyframeEffectReadOnly::GetPropertyState(
-    nsTArray<AnimationPropertyState>& aStates) const
+KeyframeEffectReadOnly::GetProperties(
+    nsTArray<AnimationPropertyDetails>& aProperties) const
 {
   for (const AnimationProperty& property : mProperties) {
     // Bug 1252730: We should also expose this winsInCascade as well.
     if (!property.mWinsInCascade) {
       continue;
     }
 
-    AnimationPropertyState state;
-    state.mProperty.Construct(
+    AnimationPropertyDetails propertyDetails;
+    propertyDetails.mProperty.Construct(
       NS_ConvertASCIItoUTF16(nsCSSProps::GetStringValue(property.mProperty)));
-    state.mRunningOnCompositor.Construct(property.mIsRunningOnCompositor);
+    propertyDetails.mRunningOnCompositor.Construct(
+      property.mIsRunningOnCompositor);
 
     nsXPIDLString localizedString;
     if (property.mPerformanceWarning &&
         property.mPerformanceWarning->ToLocalizedString(localizedString)) {
-      state.mWarning.Construct(localizedString);
+      propertyDetails.mWarning.Construct(localizedString);
     }
 
-    aStates.AppendElement(state);
+    aProperties.AppendElement(propertyDetails);
   }
 }
 
 void
 KeyframeEffectReadOnly::GetFrames(JSContext*& aCx,
                                   nsTArray<JSObject*>& aResult,
                                   ErrorResult& aRv)
 {
--- a/dom/animation/KeyframeEffect.h
+++ b/dom/animation/KeyframeEffect.h
@@ -41,17 +41,17 @@ enum class CSSPseudoElementType : uint8_
 
 namespace dom {
 class ElementOrCSSPseudoElement;
 class OwningElementOrCSSPseudoElement;
 class UnrestrictedDoubleOrKeyframeAnimationOptions;
 class UnrestrictedDoubleOrKeyframeEffectOptions;
 enum class IterationCompositeOperation : uint32_t;
 enum class CompositeOperation : uint32_t;
-struct AnimationPropertyState;
+struct AnimationPropertyDetails;
 }
 
 /**
  * Stores the results of calculating the timing properties of an animation
  * at a given sample time.
  */
 struct ComputedTiming
 {
@@ -204,17 +204,17 @@ public:
               JS::Handle<JSObject*> aFrames,
               const UnrestrictedDoubleOrKeyframeEffectOptions& aOptions,
               ErrorResult& aRv);
 
   void GetTarget(Nullable<OwningElementOrCSSPseudoElement>& aRv) const;
   void GetFrames(JSContext*& aCx,
                  nsTArray<JSObject*>& aResult,
                  ErrorResult& aRv);
-  void GetPropertyState(nsTArray<AnimationPropertyState>& aStates) const;
+  void GetProperties(nsTArray<AnimationPropertyDetails>& aProperties) const;
 
   // Temporary workaround to return both the target element and pseudo-type
   // until we implement PseudoElement (bug 1174575).
   void GetTarget(Element*& aTarget,
                  CSSPseudoElementType& aPseudoType) const {
     aTarget = mTarget;
     aPseudoType = mPseudoType;
   }
--- a/dom/animation/test/chrome/test_animation_property_state.html
+++ b/dom/animation/test/chrome/test_animation_property_state.html
@@ -1,13 +1,13 @@
 <!doctype html>
 <head>
 <meta charset=utf-8>
-<title>Bug 1196114 - Animation property which indicates
-       running on the compositor or not</title>
+<title>Bug 1196114 - Test metadata related to which animation properties
+       are running on the compositor</title>
 <script type="application/javascript" src="../testharness.js"></script>
 <script type="application/javascript" src="../testharnessreport.js"></script>
 <script type="application/javascript" src="../testcommon.js"></script>
 <style>
 .compositable {
   /* Element needs geometry to be eligible for layerization */
   width: 100px;
   height: 100px;
@@ -210,17 +210,17 @@ var gAnimationsTests = [
 ];
 
 gAnimationsTests.forEach(function(subtest) {
   promise_test(function(t) {
     var div = addDiv(t, { class: 'compositable' });
     var animation = div.animate(subtest.frames, 100000);
     return animation.ready.then(t.step_func(function() {
       assert_animation_property_state_equals(
-        animation.effect.getPropertyState(),
+        animation.effect.getProperties(),
         subtest.expected);
     }));
   }, subtest.desc);
 });
 
 var gPerformanceWarningTests = [
   {
     desc: 'preserve-3d transform',
@@ -496,46 +496,46 @@ function start() {
     .createBundle("chrome://global/locale/layout_errors.properties");
 
   gAnimationsTests.forEach(function(subtest) {
     promise_test(function(t) {
       var div = addDiv(t, { class: 'compositable' });
       var animation = div.animate(subtest.frames, 100000);
       return animation.ready.then(t.step_func(function() {
         assert_animation_property_state_equals(
-          animation.effect.getPropertyState(),
+          animation.effect.getProperties(),
           subtest.expected);
       }));
     }, subtest.desc);
   });
 
   gPerformanceWarningTests.forEach(function(subtest) {
     // FIXME: Bug 1255710: 'preserve-3d' frame breaks other tests,
     // we should skip all 'preserve-3d' tests here.
     if (subtest.desc.includes('preserve-3d')) {
       return;
     }
     promise_test(function(t) {
       var div = addDiv(t, { class: 'compositable' });
       var animation = div.animate(subtest.frames, 100000);
       return animation.ready.then(t.step_func(function() {
         assert_property_state_on_compositor(
-          animation.effect.getPropertyState(),
+          animation.effect.getProperties(),
           subtest.expected);
         div.style = subtest.style;
         return waitForFrame();
       })).then(t.step_func(function() {
         assert_animation_property_state_equals(
-          animation.effect.getPropertyState(),
+          animation.effect.getProperties(),
           subtest.expected);
         div.style = '';
         return waitForFrame();
       })).then(t.step_func(function() {
         assert_property_state_on_compositor(
-          animation.effect.getPropertyState(),
+          animation.effect.getProperties(),
           subtest.expected);
       }));
     }, subtest.desc);
   });
 
   gMultipleAsyncAnimationsTests.forEach(function(subtest) {
     // FIXME: Bug 1255710: 'preserve-3d' frame breaks other tests,
     // we should skip all 'preserve-3d' tests here.
@@ -549,33 +549,33 @@ function start() {
 
         // Bind expected values to animation object.
         animation.expected = anim.expected;
         return animation;
       });
       return waitForAllAnimations(animations).then(t.step_func(function() {
         animations.forEach(t.step_func(function(anim) {
           assert_property_state_on_compositor(
-            anim.effect.getPropertyState(),
+            anim.effect.getProperties(),
             anim.expected);
         }));
         div.style = subtest.style;
         return waitForFrame();
       })).then(t.step_func(function() {
         animations.forEach(t.step_func(function(anim) {
           assert_animation_property_state_equals(
-            anim.effect.getPropertyState(),
+            anim.effect.getProperties(),
             anim.expected);
         }));
         div.style = '';
         return waitForFrame();
       })).then(t.step_func(function() {
         animations.forEach(t.step_func(function(anim) {
           assert_property_state_on_compositor(
-            anim.effect.getPropertyState(),
+            anim.effect.getProperties(),
             anim.expected);
         }));
       }));
     }, 'Multiple animations: ' + subtest.desc);
   });
 
   gMultipleAsyncAnimationsWithGeometricKeyframeTests.forEach(function(subtest) {
     promise_test(function(t) {
@@ -585,17 +585,17 @@ function start() {
 
         // Bind expected values to animation object.
         animation.expected = anim.expected;
         return animation;
       });
       return waitForAllAnimations(animations).then(t.step_func(function() {
         animations.forEach(t.step_func(function(anim) {
           assert_animation_property_state_equals(
-            anim.effect.getPropertyState(),
+            anim.effect.getProperties(),
             anim.expected);
         }));
       }));
     }, 'Multiple animations with geometric property: ' + subtest.desc);
   });
 
   gMultipleAsyncAnimationsWithGeometricAnimationTests.forEach(function(subtest) {
     promise_test(function(t) {
@@ -608,74 +608,74 @@ function start() {
         return animation;
       });
 
       var widthAnimation;
 
       return waitForAllAnimations(animations).then(t.step_func(function() {
         animations.forEach(t.step_func(function(anim) {
           assert_property_state_on_compositor(
-            anim.effect.getPropertyState(),
+            anim.effect.getProperties(),
             anim.expected);
         }));
       })).then(t.step_func(function() {
         // Append 'width' animation on the same element.
         widthAnimation = div.animate({ width: ['100px', '200px'] }, 100000);
         return waitForFrame();
       })).then(t.step_func(function() {
         // Now transform animations are not running on compositor because of
         // the 'width' animation.
         animations.forEach(t.step_func(function(anim) {
           assert_animation_property_state_equals(
-            anim.effect.getPropertyState(),
+            anim.effect.getProperties(),
             anim.expected);
         }));
         // Remove the 'width' animation.
         widthAnimation.cancel();
         return waitForFrame();
       })).then(t.step_func(function() {
         // Now all animations are running on compositor.
         animations.forEach(t.step_func(function(anim) {
           assert_property_state_on_compositor(
-            anim.effect.getPropertyState(),
+            anim.effect.getProperties(),
             anim.expected);
         }));
       }));
     }, 'Multiple async animations and geometric animation: ' + subtest.desc);
   });
 
   promise_test(function(t) {
     var div = addDiv(t, { class: 'compositable' });
     var animation = div.animate(
       { transform: ['translate(0px)', 'translate(100px)'] }, 100000);
     return animation.ready.then(t.step_func(function() {
       assert_animation_property_state_equals(
-        animation.effect.getPropertyState(),
+        animation.effect.getProperties(),
         [ { property: 'transform', runningOnCompositor: true } ]);
       div.style = 'width: 10000px; height: 10000px';
       return waitForFrame();
     })).then(t.step_func(function() {
       // viewport depends on test environment.
       var expectedWarning = new RegExp(
         "Async animation disabled because frame size \\(10000, 10000\\) is " +
         "bigger than the viewport \\(\\d+, \\d+\\) or the visual rectangle " +
         "\\(10000, 10000\\) is larger than the max allowed value \\(\\d+\\)");
       assert_animation_property_state_equals(
-        animation.effect.getPropertyState(),
+        animation.effect.getProperties(),
         [ {
           property: 'transform',
           runningOnCompositor: false,
           warning: expectedWarning
         } ]);
       div.style = 'width: 100px; height: 100px';
       return waitForFrame();
     })).then(t.step_func(function() {
       // FIXME: Bug 1253164: the animation should get back on compositor.
       assert_animation_property_state_equals(
-        animation.effect.getPropertyState(),
+        animation.effect.getProperties(),
         [ { property: 'transform', runningOnCompositor: false } ]);
     }));
   }, 'transform on too big element');
 
   promise_test(function(t) {
     var svg  = document.createElementNS('http://www.w3.org/2000/svg', 'svg');
     svg.setAttribute('width', '100');
     svg.setAttribute('height', '100');
@@ -688,33 +688,33 @@ function start() {
     t.add_cleanup(function() {
       svg.remove();
     });
 
     var animation = svg.animate(
       { transform: ['translate(0px)', 'translate(100px)'] }, 100000);
     return animation.ready.then(t.step_func(function() {
       assert_animation_property_state_equals(
-        animation.effect.getPropertyState(),
+        animation.effect.getProperties(),
         [ { property: 'transform', runningOnCompositor: true } ]);
       svg.setAttribute('transform', 'translate(10, 20)');
       return waitForFrame();
     })).then(t.step_func(function() {
       assert_animation_property_state_equals(
-        animation.effect.getPropertyState(),
+        animation.effect.getProperties(),
         [ {
           property: 'transform',
           runningOnCompositor: false,
           warning: 'AnimationWarningTransformSVG'
         } ]);
       svg.removeAttribute('transform');
       return waitForFrame();
     })).then(t.step_func(function() {
       assert_animation_property_state_equals(
-        animation.effect.getPropertyState(),
+        animation.effect.getProperties(),
         [ { property: 'transform', runningOnCompositor: true } ]);
     }));
   }, 'transform of nsIFrame with SVG transform');
 }
 
 </script>
 
 </body>
--- a/dom/webidl/KeyframeEffect.webidl
+++ b/dom/webidl/KeyframeEffect.webidl
@@ -41,24 +41,24 @@ interface KeyframeEffectReadOnly : Anima
   // KeyframeEffect             clone();
 
   // We use object instead of ComputedKeyframe so that we can put the
   // property-value pairs on the object.
   [Throws] sequence<object> getFrames();
 };
 
 // Non-standard extensions
-dictionary AnimationPropertyState {
-  DOMString property;
-  boolean runningOnCompositor;
+dictionary AnimationPropertyDetails {
+  DOMString  property;
+  boolean    runningOnCompositor;
   DOMString? warning;
 };
 
 partial interface KeyframeEffectReadOnly {
-  [ChromeOnly] sequence<AnimationPropertyState> getPropertyState();
+  [ChromeOnly] sequence<AnimationPropertyDetails> getProperties();
 };
 
 [Func="nsDocument::IsWebAnimationsEnabled",
  Constructor ((Element or CSSPseudoElement)? target,
               object? frames,
               optional (unrestricted double or KeyframeEffectOptions) options)]
 interface KeyframeEffect : KeyframeEffectReadOnly {
   // Bug 1067769 - Allow setting KeyframeEffect.target