Bug 1352067 - Part 1: Make sure AnimationValue::{mGecko|mServo} are mutually exclusive. draft
authorBoris Chiou <boris.chiou@gmail.com>
Fri, 31 Mar 2017 17:46:37 +0800
changeset 554281 124c34295cc61c0817485957f7abeec4235a414e
parent 554280 24877a79e3b3467b9b04bee817fc731e7609691c
child 554282 e9382da646667f275cc5de65b2f70fdfaf88173e
push id51895
push userbmo:boris.chiou@gmail.com
push dateFri, 31 Mar 2017 09:54:51 +0000
bugs1352067
milestone55.0a1
Bug 1352067 - Part 1: Make sure AnimationValue::{mGecko|mServo} are mutually exclusive. We will obsolete StyleAnimationValue in the future, and can treat AnimationValue as a wrapper of RawServoAnimationValue to hide the FFIs at that moment. For now, we still need both types, so it's better to make sure they are mutually exclusive in AnimationValue. Therefore, let's add some assertions. Besides, I think those FFIs might do many things and it seems those methods are not critical, so let's move them into the cpp file, so we can remove some dependencies to avoid re-compiling so many files if someone needs revise ServoBindings.h. MozReview-Commit-ID: FJ1uTvEQ7NT
dom/animation/KeyframeEffectReadOnly.h
layout/base/nsLayoutUtils.cpp
layout/painting/nsDisplayList.cpp
layout/style/StyleAnimationValue.cpp
layout/style/StyleAnimationValue.h
layout/style/StyleAnimationValueInlines.h
layout/style/moz.build
--- a/dom/animation/KeyframeEffectReadOnly.h
+++ b/dom/animation/KeyframeEffectReadOnly.h
@@ -18,17 +18,17 @@
 #include "mozilla/AnimationTarget.h"
 #include "mozilla/Attributes.h"
 #include "mozilla/ComputedTimingFunction.h"
 #include "mozilla/EffectCompositor.h"
 #include "mozilla/Keyframe.h"
 #include "mozilla/KeyframeEffectParams.h"
 // RawServoDeclarationBlock and associated RefPtrTraits
 #include "mozilla/ServoBindingTypes.h"
-#include "mozilla/StyleAnimationValueInlines.h"
+#include "mozilla/StyleAnimationValue.h"
 #include "mozilla/dom/AnimationEffectReadOnly.h"
 #include "mozilla/dom/BindingDeclarations.h"
 #include "mozilla/dom/Element.h"
 
 struct JSContext;
 class JSObject;
 class nsIContent;
 class nsIDocument;
--- a/layout/base/nsLayoutUtils.cpp
+++ b/layout/base/nsLayoutUtils.cpp
@@ -112,17 +112,17 @@
 #include "nsFrameSelection.h"
 #include "FrameLayerBuilder.h"
 #include "mozilla/layers/APZCTreeManager.h"
 #include "mozilla/layers/CompositorBridgeChild.h"
 #include "mozilla/Telemetry.h"
 #include "mozilla/EventDispatcher.h"
 #include "mozilla/EventStateManager.h"
 #include "mozilla/RuleNodeCacheConditions.h"
-#include "mozilla/StyleAnimationValueInlines.h"
+#include "mozilla/StyleAnimationValue.h"
 #include "mozilla/StyleSetHandle.h"
 #include "mozilla/StyleSetHandleInlines.h"
 #include "RegionBuilder.h"
 #include "SVGSVGElement.h"
 
 #ifdef MOZ_XUL
 #include "nsXULPopupManager.h"
 #endif
--- a/layout/painting/nsDisplayList.cpp
+++ b/layout/painting/nsDisplayList.cpp
@@ -56,17 +56,18 @@
 #include "mozilla/AnimationUtils.h"
 #include "mozilla/EffectCompositor.h"
 #include "mozilla/EffectSet.h"
 #include "mozilla/EventStates.h"
 #include "mozilla/LookAndFeel.h"
 #include "mozilla/OperatorNewExtensions.h"
 #include "mozilla/PendingAnimationTracker.h"
 #include "mozilla/Preferences.h"
-#include "mozilla/StyleAnimationValueInlines.h"
+#include "mozilla/StyleAnimationValue.h"
+#include "mozilla/ServoBindings.h"
 #include "mozilla/Telemetry.h"
 #include "mozilla/UniquePtr.h"
 #include "mozilla/Unused.h"
 #include "mozilla/ViewportFrame.h"
 #include "mozilla/gfx/gfxVars.h"
 #include "ActiveLayerTracker.h"
 #include "nsContentUtils.h"
 #include "nsPrintfCString.h"
--- a/layout/style/StyleAnimationValue.cpp
+++ b/layout/style/StyleAnimationValue.cpp
@@ -6,16 +6,17 @@
 
 /* Utilities for animation of computed style values */
 
 #include "mozilla/StyleAnimationValue.h"
 
 #include "mozilla/ArrayUtils.h"
 #include "mozilla/MathAlgorithms.h"
 #include "mozilla/RuleNodeCacheConditions.h"
+#include "mozilla/ServoBindings.h"
 #include "mozilla/StyleSetHandle.h"
 #include "mozilla/StyleSetHandleInlines.h"
 #include "mozilla/Tuple.h"
 #include "mozilla/UniquePtr.h"
 #include "nsAutoPtr.h"
 #include "nsCOMArray.h"
 #include "nsIStyleRule.h"
 #include "mozilla/css/StyleRule.h"
@@ -5187,8 +5188,57 @@ StyleAnimationValue::operator==(const St
                         aOther.GetStringBufferValue()) == 0);
     case eUnit_ComplexColor:
       return *mValue.mComplexColor == *aOther.mValue.mComplexColor;
   }
 
   NS_NOTREACHED("incomplete case");
   return false;
 }
+
+// AnimationValue Implementation
+
+bool
+AnimationValue::operator==(const AnimationValue& aOther) const
+{
+  // It is possible to compare an empty AnimationValue with others, so both
+  // mServo and mGecko could be null while comparing.
+  MOZ_ASSERT(!mServo || mGecko.IsNull());
+  if (mServo) {
+    return Servo_AnimationValue_DeepEqual(mServo, aOther.mServo);
+  }
+  return mGecko == aOther.mGecko;
+}
+
+float
+AnimationValue::GetOpacity() const
+{
+  MOZ_ASSERT(!mServo != mGecko.IsNull());
+  return mServo ? Servo_AnimationValue_GetOpacity(mServo)
+                : mGecko.GetFloatValue();
+}
+
+gfxSize
+AnimationValue::GetScaleValue(const nsIFrame* aFrame) const
+{
+  MOZ_ASSERT(!mServo != mGecko.IsNull());
+  if (mServo) {
+    RefPtr<nsCSSValueSharedList> list;
+    Servo_AnimationValue_GetTransform(mServo, &list);
+    return nsStyleTransformMatrix::GetScaleValue(list, aFrame);
+  }
+  return mGecko.GetScaleValue(aFrame);
+}
+
+void
+AnimationValue::SerializeSpecifiedValue(nsCSSPropertyID aProperty,
+                                        nsAString& aString) const
+{
+  MOZ_ASSERT(!mServo != mGecko.IsNull());
+  if (mServo) {
+    Servo_AnimationValue_Serialize(mServo, aProperty, &aString);
+    return;
+  }
+
+  DebugOnly<bool> uncomputeResult =
+    StyleAnimationValue::UncomputeValue(aProperty, mGecko, aString);
+  MOZ_ASSERT(uncomputeResult, "failed to uncompute StyleAnimationValue");
+}
--- a/layout/style/StyleAnimationValue.h
+++ b/layout/style/StyleAnimationValue.h
@@ -565,32 +565,37 @@ private:
   }
   static bool IsStringUnit(Unit aUnit) {
     return aUnit == eUnit_UnparsedString;
   }
 };
 
 struct AnimationValue
 {
+  // mGecko and mServo are mutually exclusive: only one or the other should
+  // ever be set.
+  // FIXME: After obsoleting StyleAnimationValue, we should remove mGecko, and
+  // make AnimationValue a wrapper of RawServoAnimationValue to hide these
+  // FFIs.
   StyleAnimationValue mGecko;
   RefPtr<RawServoAnimationValue> mServo;
 
-  inline bool operator==(const AnimationValue& aOther) const;
+  bool operator==(const AnimationValue& aOther) const;
 
   bool IsNull() const { return mGecko.IsNull() && !mServo; }
 
-  inline float GetOpacity() const;
+  float GetOpacity() const;
 
   // Returns the scale for mGecko or mServo, which are calculated with
   // reference to aFrame.
-  inline gfxSize GetScaleValue(const nsIFrame* aFrame) const;
+  gfxSize GetScaleValue(const nsIFrame* aFrame) const;
 
   // Uncompute this AnimationValue and then serialize it.
-  inline void SerializeSpecifiedValue(nsCSSPropertyID aProperty,
-                                      nsAString& aString) const;
+  void SerializeSpecifiedValue(nsCSSPropertyID aProperty,
+                               nsAString& aString) const;
 };
 
 struct PropertyStyleAnimationValuePair
 {
   nsCSSPropertyID mProperty;
   AnimationValue mValue;
 };
 } // namespace mozilla
deleted file mode 100644
--- a/layout/style/StyleAnimationValueInlines.h
+++ /dev/null
@@ -1,58 +0,0 @@
-/* -*- Mode: C++; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
-/* This Source Code Form is subject to the terms of the Mozilla Public
- * License, v. 2.0. If a copy of the MPL was not distributed with this
- * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
-
-#ifndef mozilla_StyleAnimationValueInlines_h_
-#define mozilla_StyleAnimationValueInlines_h_
-
-#include "mozilla/StyleAnimationValue.h"
-#include "mozilla/ServoBindings.h"
-
-namespace mozilla {
-
-bool
-AnimationValue::operator==(const AnimationValue& aOther) const
-{
-    // mGecko and mServo are mutual exclusive, one of them must be null
-    if (mServo) {
-      return Servo_AnimationValue_DeepEqual(mServo, aOther.mServo);
-    }
-    return mGecko == aOther.mGecko;
-}
-
-float
-AnimationValue::GetOpacity() const
-{
-  return mServo ? Servo_AnimationValue_GetOpacity(mServo)
-                : mGecko.GetFloatValue();
-}
-
-gfxSize
-AnimationValue::GetScaleValue(const nsIFrame* aFrame) const
-{
-  if (mServo) {
-    RefPtr<nsCSSValueSharedList> list;
-    Servo_AnimationValue_GetTransform(mServo, &list);
-    return nsStyleTransformMatrix::GetScaleValue(list, aFrame);
-  }
-  return mGecko.GetScaleValue(aFrame);
-}
-
-void
-AnimationValue::SerializeSpecifiedValue(nsCSSPropertyID aProperty,
-                                        nsAString& aString) const
-{
-  if (mServo) {
-    Servo_AnimationValue_Serialize(mServo, aProperty, &aString);
-    return;
-  }
-
-  DebugOnly<bool> uncomputeResult =
-    StyleAnimationValue::UncomputeValue(aProperty, mGecko, aString);
-  MOZ_ASSERT(uncomputeResult, "failed to uncompute StyleAnimationValue");
-}
-
-} // namespace mozilla
-
-#endif // mozilla_StyleAnimationValueInlines_h_
--- a/layout/style/moz.build
+++ b/layout/style/moz.build
@@ -110,17 +110,16 @@ EXPORTS.mozilla += [
     'ServoSpecifiedValues.h',
     'ServoStyleRule.h',
     'ServoStyleSet.h',
     'ServoStyleSheet.h',
     'ServoTypes.h',
     'ServoUtils.h',
     'SheetType.h',
     'StyleAnimationValue.h',
-    'StyleAnimationValueInlines.h',
     'StyleBackendType.h',
     'StyleComplexColor.h',
     'StyleContextSource.h',
     'StyleSetHandle.h',
     'StyleSetHandleInlines.h',
     'StyleSheet.h',
     'StyleSheetInfo.h',
     'StyleSheetInlines.h',