Bug 1483404 - Move conversion of CompositeOperation enum types to KeyframeUtils::GetAnimationPropertiesFromKeyframes; r=smaug
authorBrian Birtles <birtles@gmail.com>
Thu, 16 Aug 2018 16:24:52 +0000
changeset 432031 6f8ddade347deb25d3d3dfc9df97dad6e933d0bf
parent 432030 e7869a533fac3a2adef73f4b81c29a92dcf453ec
child 432032 85496f02e642767418641817493097099071e158
push id34457
push userebalazs@mozilla.com
push dateFri, 17 Aug 2018 09:45:26 +0000
treeherdermozilla-central@d2ba1d6c76f2 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssmaug
bugs1483404
milestone63.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1483404 - Move conversion of CompositeOperation enum types to KeyframeUtils::GetAnimationPropertiesFromKeyframes; r=smaug Differential Revision: https://phabricator.services.mozilla.com/D3457
dom/animation/Keyframe.h
dom/animation/KeyframeEffect.cpp
dom/animation/KeyframeUtils.cpp
dom/animation/KeyframeUtils.h
dom/webidl/BaseKeyframeTypes.webidl
--- a/dom/animation/Keyframe.h
+++ b/dom/animation/Keyframe.h
@@ -5,25 +5,23 @@
  * You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #ifndef mozilla_dom_Keyframe_h
 #define mozilla_dom_Keyframe_h
 
 #include "nsCSSPropertyID.h"
 #include "nsCSSValue.h"
 #include "nsTArray.h"
+#include "mozilla/dom/BaseKeyframeTypesBinding.h" // CompositeOperationOrAuto
 #include "mozilla/ComputedTimingFunction.h"
 #include "mozilla/Maybe.h"
 #include "mozilla/RefPtr.h"
 
 struct RawServoDeclarationBlock;
 namespace mozilla {
-namespace dom {
-enum class CompositeOperation : uint8_t;
-}
 
 /**
  * A property-value pair specified on a keyframe.
  */
 struct PropertyValuePair
 {
   explicit PropertyValuePair(nsCSSPropertyID aProperty)
     : mProperty(aProperty) { }
@@ -84,15 +82,16 @@ struct Keyframe
     return *this;
   }
 
   Maybe<double>                 mOffset;
   static constexpr double kComputedOffsetNotSet = -1.0;
   double                        mComputedOffset = kComputedOffsetNotSet;
   Maybe<ComputedTimingFunction> mTimingFunction; // Nothing() here means
                                                  // "linear"
-  Maybe<dom::CompositeOperation> mComposite;
+  dom::CompositeOperationOrAuto mComposite =
+                                  dom::CompositeOperationOrAuto::Auto;
   nsTArray<PropertyValuePair>   mPropertyValues;
 };
 
 }
 
 #endif // mozilla_dom_Keyframe_h
--- a/dom/animation/KeyframeEffect.cpp
+++ b/dom/animation/KeyframeEffect.cpp
@@ -1094,18 +1094,17 @@ KeyframeEffect::GetKeyframes(JSContext*&
     MOZ_ASSERT(keyframe.mComputedOffset != Keyframe::kComputedOffsetNotSet,
                "Invalid computed offset");
     keyframeDict.mComputedOffset.Construct(keyframe.mComputedOffset);
     if (keyframe.mTimingFunction) {
       keyframeDict.mEasing.Truncate();
       keyframe.mTimingFunction.ref().AppendToString(keyframeDict.mEasing);
     } // else if null, leave easing as its default "linear".
 
-    keyframeDict.mComposite =
-      KeyframeUtils::ToCompositeOperationOrAuto(keyframe.mComposite);
+    keyframeDict.mComposite = keyframe.mComposite;
 
     JS::Rooted<JS::Value> keyframeJSValue(aCx);
     if (!ToJSValue(aCx, keyframeDict, &keyframeJSValue)) {
       aRv.Throw(NS_ERROR_FAILURE);
       return;
     }
 
     RefPtr<RawServoDeclarationBlock> customProperties;
--- a/dom/animation/KeyframeUtils.cpp
+++ b/dom/animation/KeyframeUtils.cpp
@@ -329,18 +329,22 @@ KeyframeUtils::GetAnimationPropertiesFro
     for (auto& value : computedValues[i]) {
       MOZ_ASSERT(frame.mComputedOffset != Keyframe::kComputedOffsetNotSet,
                  "Invalid computed offset");
       KeyframeValueEntry* entry = entries.AppendElement();
       entry->mOffset = frame.mComputedOffset;
       entry->mProperty = value.mProperty;
       entry->mValue = value.mValue;
       entry->mTimingFunction = frame.mTimingFunction;
+      // The following assumes that CompositeOperation is a strict subset of
+      // CompositeOperationOrAuto.
       entry->mComposite =
-        frame.mComposite ? frame.mComposite.value() : aEffectComposite;
+        frame.mComposite == dom::CompositeOperationOrAuto::Auto
+        ? aEffectComposite
+        : static_cast<dom::CompositeOperation>(frame.mComposite);
     }
   }
 
   BuildSegmentsFromValueEntries(entries, result);
   return result;
 }
 
 /* static */ bool
@@ -447,18 +451,17 @@ ConvertKeyframeSequence(JSContext* aCx,
     if (!keyframe) {
       return false;
     }
     if (!keyframeDict.mOffset.IsNull()) {
       keyframe->mOffset.emplace(keyframeDict.mOffset.Value());
     }
 
     if (StaticPrefs::dom_animations_api_compositing_enabled()) {
-      keyframe->mComposite =
-        KeyframeUtils::ToCompositeOperation(keyframeDict.mComposite);
+      keyframe->mComposite = keyframeDict.mComposite;
     }
 
     // Look for additional property-values pairs on the object.
     nsTArray<PropertyValuesPair> propertyValuePairs;
     if (value.isObject()) {
       JS::Rooted<JSObject*> object(aCx, &value.toObject());
       if (!GetPropertyValuesPairs(aCx, object,
                                   ListAllowance::eDisallow,
@@ -1200,18 +1203,17 @@ GetKeyframeListFromPropertyIndexedKeyfra
     } else if (composite.IsCompositeOperationOrAutoSequence()) {
       compositeOps = &composite.GetAsCompositeOperationOrAutoSequence();
     }
 
     // Fill in and repeat as needed.
     if (compositeOps && !compositeOps->IsEmpty()) {
       size_t length = compositeOps->Length();
       for (size_t i = 0; i < aResult.Length(); i++) {
-        dom::CompositeOperationOrAuto op = compositeOps->ElementAt(i % length);
-        aResult[i].mComposite = KeyframeUtils::ToCompositeOperation(op);
+        aResult[i].mComposite = compositeOps->ElementAt(i % length);
       }
     }
   }
 }
 
 /**
  * Returns true if the supplied set of keyframes has keyframe values for
  * any property for which it does not also supply a value for the 0% and 100%
--- a/dom/animation/KeyframeUtils.h
+++ b/dom/animation/KeyframeUtils.h
@@ -99,42 +99,13 @@ public:
    * its subproperties, is animatable.
    *
    * @param aProperty The property to check.
    * @param aBackend  The style backend, Servo or Gecko, that should determine
    *                  if the property is animatable or not.
    * @return true if |aProperty| is animatable.
    */
   static bool IsAnimatableProperty(nsCSSPropertyID aProperty);
-
-  /*
-   * The spec defines two enums: CompositeOperation and
-   * CompositeOperationOrAuto.
-   *
-   * Internally, however, it's more convenient to always deal with
-   * CompositeOperation, represent the 'auto' case as a Nothing() value, and
-   * convert to and from CompositeOperationOrAuto at the API boundary.
-   *
-   * The following methods convert between these two representations and allow
-   * us to encapsulate the assumption that CompositeOperation is a strict subset
-   * of CompositeOperationOrAuto, in one location.
-   */
-
-  static dom::CompositeOperationOrAuto
-  ToCompositeOperationOrAuto(const Maybe<dom::CompositeOperation>& aComposite)
-  {
-    return aComposite
-           ? static_cast<dom::CompositeOperationOrAuto>(aComposite.value())
-           : dom::CompositeOperationOrAuto::Auto;
-  }
-
-  static Maybe<dom::CompositeOperation>
-  ToCompositeOperation(dom::CompositeOperationOrAuto aComposite)
-  {
-    return aComposite == dom::CompositeOperationOrAuto::Auto
-           ? Nothing()
-           : Some(static_cast<dom::CompositeOperation>(aComposite));
-  }
 };
 
 } // namespace mozilla
 
 #endif // mozilla_KeyframeUtils_h
--- a/dom/webidl/BaseKeyframeTypes.webidl
+++ b/dom/webidl/BaseKeyframeTypes.webidl
@@ -16,17 +16,17 @@
 enum CompositeOperation { "replace", "add", "accumulate" };
 
 // NOTE: The order of the values in this enum are important.
 //
 // We assume that CompositeOperation is a subset of CompositeOperationOrAuto so
 // that we can cast between the two types (provided the value is not "auto").
 //
 // If that assumption ceases to hold we will need to update the conversion
-// routines in KeyframeUtils.
+// in KeyframeUtils::GetAnimationPropertiesFromKeyframes.
 enum CompositeOperationOrAuto { "replace", "add", "accumulate", "auto" };
 
 // The following dictionary types are not referred to by other .webidl files,
 // but we use it for manual JS->IDL and IDL->JS conversions in KeyframeEffect's
 // implementation.
 
 dictionary BasePropertyIndexedKeyframe {
   (double? or sequence<double?>) offset = [];