Bug 1295401: Support underlying value for discretely animation type. r?hiro,dholbert draft
authorDaisuke Akatsuka <dakatsuka@mozilla.com>
Tue, 24 Jan 2017 14:11:49 +0900
changeset 465477 1a3c5a966cbb0ef9baaa3586a004fdd7d59a3da0
parent 464862 d5e37c0a776f1f2c21ddac4612529f819e13733b
child 543153 ce10ec85154ca58618c174e66862aeb9e8263f84
push id42605
push userbmo:dakatsuka@mozilla.com
push dateTue, 24 Jan 2017 05:12:24 +0000
reviewershiro, dholbert
bugs1295401
milestone53.0a1
Bug 1295401: Support underlying value for discretely animation type. r?hiro,dholbert MozReview-Commit-ID: Rf3oLceghL
dom/animation/test/mozilla/file_underlying-discrete-value.html
layout/style/StyleAnimationValue.cpp
layout/style/nsComputedDOMStyle.cpp
layout/style/nsComputedDOMStyle.h
--- a/dom/animation/test/mozilla/file_underlying-discrete-value.html
+++ b/dom/animation/test/mozilla/file_underlying-discrete-value.html
@@ -18,34 +18,28 @@ const discreteTests = [
       { computedOffset: 1, alignContent: "flex-end" }
     ],
     explanation: "Test for fully-specified keyframes"
   },
   {
     stylesheet: {
       "@keyframes keyframes": "from { align-content: flex-start; }"
     },
-    // The value of 100% should be 'stretch',
-    // but we are not supporting underlying value.
-    // https://bugzilla.mozilla.org/show_bug.cgi?id=1295401
     expectedKeyframes: [
       { computedOffset: 0, alignContent: "flex-start" },
-      { computedOffset: 1, alignContent: "unset" }
+      { computedOffset: 1, alignContent: "normal" }
     ],
     explanation: "Test for 0% keyframe only"
   },
   {
     stylesheet: {
       "@keyframes keyframes": "to { align-content: flex-end; }"
     },
-    // The value of 0% should be 'stretch',
-    // but we are not supporting underlying value.
-    // https://bugzilla.mozilla.org/show_bug.cgi?id=1295401
     expectedKeyframes: [
-      { computedOffset: 0, alignContent: "unset" },
+      { computedOffset: 0, alignContent: "normal" },
       { computedOffset: 1, alignContent: "flex-end" }
     ],
     explanation: "Test for 100% keyframe only"
   },
   {
     stylesheet: {
       "@keyframes keyframes": "50% { align-content: center; }",
       "#target": "align-content: space-between;"
@@ -70,29 +64,27 @@ const discreteTests = [
       { computedOffset: 0.5, alignContent: "center" },
       { computedOffset: 1, alignContent: "space-between" }
     ],
     explanation: "Test for no 0%/100% keyframes " +
                  "and specified style on target element using style attribute"
   },
   {
     stylesheet: {
-      "@keyframes keyframes": "50% { align-content: center; }",
-      "#target": "align-content: inherit;"
+      "@keyframes keyframes": "0% { align-content: inherit; } " +
+                              "50% { align-content: center; }",
+      "body": "align-content: flex-start;"
     },
-    // The value of 0%/100% should be 'stretch',
-    // but we are not supporting underlying value.
-    // https://bugzilla.mozilla.org/show_bug.cgi?id=1295401
     expectedKeyframes: [
-      { computedOffset: 0, alignContent: "inherit" },
+      { computedOffset: 0, alignContent: "flex-start" },
       { computedOffset: 0.5, alignContent: "center" },
-      { computedOffset: 1, alignContent: "inherit" }
+      { computedOffset: 1, alignContent: "normal" }
     ],
-    explanation: "Test for no 0%/100% keyframes " +
-                 "and 'inherit' specified on target element"
+    explanation: "Test for no 100% keyframe and 'inherit' keyword on 0%. " +
+                 "0% should inherit, but 100% should not inherit"
   },
   {
     stylesheet: {
       "@keyframes keyframes": "50% { align-content: center; }",
       ".target": "align-content: space-between;"
     },
     attributes: {
       class: "target"
@@ -151,42 +143,55 @@ const discreteTests = [
       { computedOffset: 0, alignContent: "space-between" },
       { computedOffset: 0.5, alignContent: "center" },
       { computedOffset: 1, alignContent: "space-between" }
     ],
     explanation: "Test for no 0%/100% keyframes " +
                  "and specified style on target element " +
                  "using important type selector that overrides other rules"
   },
+  {
+    stylesheet: {
+      "@keyframes keyframes": "0% { text-align: inherit; } " +
+                              "50% { text-align: center; }",
+      "body": "text-align: right;"
+    },
+    expectedKeyframes: [
+      { computedOffset: 0, textAlign: "right" },
+      { computedOffset: 0.5, textAlign: "center" },
+      { computedOffset: 1, textAlign: "right" }
+    ],
+    explanation: "Test for no 100% keyframe and 'inherit' keyword in 0%, " +
+                 "also this textAlign property should inherit"
+  },
 ];
 
 discreteTests.forEach(testcase => {
   test(t => {
     addStyle(t, testcase.stylesheet);
 
     const div = addDiv(t, { "id": "target" });
     if (testcase.attributes) {
       for (let attributeName in testcase.attributes) {
         div.setAttribute(attributeName, testcase.attributes[attributeName]);
       }
     }
     div.style.animation = "keyframes 100s";
 
+    const expectedKeyframes = testcase.expectedKeyframes;
     const keyframes = div.getAnimations()[0].effect.getKeyframes();
-    const expectedKeyframes = testcase.expectedKeyframes;
     assert_equals(keyframes.length, expectedKeyframes.length,
                   `keyframes.length should be ${ expectedKeyframes.length }`);
 
     keyframes.forEach((keyframe, index) => {
       const expectedKeyframe = expectedKeyframes[index];
-      assert_equals(keyframe.computedOffset, expectedKeyframe.computedOffset,
-                    `computedOffset of keyframes[${ index }] should be ` +
-                    `${ expectedKeyframe.computedOffset }`);
-      assert_equals(keyframe.alignContent, expectedKeyframe.alignContent,
-                    `alignContent of keyframes[${ index }] should be ` +
-                    `${ expectedKeyframe.alignContent }`);
+      for (let name in expectedKeyframe) {
+        assert_equals(keyframe[name], expectedKeyframe[name],
+                      `${ name } of keyframes[${ index }] should be ` +
+                      `${ expectedKeyframe[name] }`);
+      }
     });
   }, testcase.explanation);
 });
 
 done();
 </script>
 </body>
--- a/layout/style/StyleAnimationValue.cpp
+++ b/layout/style/StyleAnimationValue.cpp
@@ -4800,16 +4800,24 @@ StyleAnimationValue::ExtractComputedValu
       }
       if (aStyleContext->StyleSource().IsServoComputedValues()) {
         NS_ERROR("stylo: extracting discretely animated values not supported");
         return false;
       }
       auto cssValue = MakeUnique<nsCSSValue>(eCSSUnit_Unset);
       aStyleContext->RuleNode()->GetDiscretelyAnimatedCSSValue(aProperty,
                                                                cssValue.get());
+      if (cssValue->GetUnit() == eCSSUnit_Unset ||
+          cssValue->GetUnit() == eCSSUnit_Inherit ||
+          cssValue->GetUnit() == eCSSUnit_Initial) {
+        // Use computed style of aStyleContext as underlying value.
+        nsComputedDOMStyle::GetDiscretelyAnimatedCSSValue(aStyleContext,
+                                                          aProperty,
+                                                          cssValue.get());
+      }
       aComputedValue.SetAndAdoptCSSValueValue(cssValue.release(),
                                               eUnit_DiscreteCSSValue);
       return true;
     }
     case eStyleAnimType_None:
       NS_NOTREACHED("shouldn't use on non-animatable properties");
   }
   return false;
--- a/layout/style/nsComputedDOMStyle.cpp
+++ b/layout/style/nsComputedDOMStyle.cpp
@@ -236,16 +236,21 @@ nsComputedStyleMap::Update()
   for (uint32_t i = 0; i < eComputedStyleProperty_COUNT; i++) {
     if (kEntries[i].IsEnabled()) {
       mIndexMap[index++] = i;
     }
   }
   mExposedPropertyCount = index;
 }
 
+nsComputedDOMStyle::nsComputedDOMStyle(nsStyleContext* aContext)
+{
+  SetFrameStyleContext(aContext);
+}
+
 nsComputedDOMStyle::nsComputedDOMStyle(dom::Element* aElement,
                                        const nsAString& aPseudoElt,
                                        nsIPresShell* aPresShell,
                                        StyleType aStyleType)
   : mDocumentWeak(nullptr)
   , mOuterFrame(nullptr)
   , mInnerFrame(nullptr)
   , mPresShell(nullptr)
@@ -337,16 +342,56 @@ nsComputedDOMStyle::GetPropertyValue(con
   // This is mostly to avoid code duplication with GetPropertyCSSValue(); if
   // perf ever becomes an issue here (doubtful), we can look into changing
   // this.
   return GetPropertyValue(
     NS_ConvertASCIItoUTF16(nsCSSProps::GetStringValue(aPropID)),
     aValue);
 }
 
+/* static */ void
+nsComputedDOMStyle::GetDiscretelyAnimatedCSSValue(nsStyleContext* aContext,
+                                                  const nsCSSPropertyID aPropID,
+                                                  nsCSSValue* aReturn)
+{
+  nsComputedDOMStyle computedStyle(aContext);
+  computedStyle.GetDiscretelyAnimatedCSSValue(aPropID, aReturn);
+}
+
+void
+nsComputedDOMStyle::GetDiscretelyAnimatedCSSValue(const nsCSSPropertyID aPropID,
+                                                  nsCSSValue* aReturn)
+{
+  MOZ_ASSERT(!nsCSSProps::IsShorthand(aPropID),
+             "nsCSSPropertyID should be longhand property");
+  MOZ_ASSERT(nsCSSProps::kAnimTypeTable[aPropID] == eStyleAnimType_Discrete,
+             "Animation type should be eStyleAnimType_Discrete");
+
+  MOZ_ASSERT(mStyleContext, "NULL StyleContext");
+
+  const nsComputedStyleMap::Entry* propEntry =
+    GetComputedStyleMap()->FindEntryForProperty(aPropID);
+  RefPtr<CSSValue> cssValue = (this->*propEntry->mGetter)();
+  MOZ_ASSERT(cssValue, "CSSValue is not found");
+
+  nsString cssText;
+  ErrorResult error;
+  cssValue->GetCssText(cssText, error);
+  MOZ_ASSERT(error.StealNSResult() == NS_OK, "Error in CSSValue::GetCssText");
+
+  nsIDocument* doc = mStyleContext->Arena()->GetDocument();
+  nsCSSParser parser;
+  parser.ParseLonghandProperty(aPropID,
+                               cssText,
+                               doc->GetDocumentURI(),
+                               doc->GetDocumentURI(),
+                               doc->NodePrincipal(),
+                               *aReturn);
+}
+
 NS_IMETHODIMP
 nsComputedDOMStyle::SetPropertyValue(const nsCSSPropertyID aPropID,
                                      const nsAString& aValue)
 {
   return NS_ERROR_DOM_NO_MODIFICATION_ALLOWED_ERR;
 }
 
 NS_IMETHODIMP
@@ -767,17 +812,19 @@ nsComputedDOMStyle::GetCSSParsingEnviron
   aCSSParseEnv.mPrincipal = nullptr;
 }
 
 void
 nsComputedDOMStyle::ClearStyleContext()
 {
   if (mResolvedStyleContext) {
     mResolvedStyleContext = false;
-    mContent->RemoveMutationObserver(this);
+    if (mContent) {
+      mContent->RemoveMutationObserver(this);
+    }
   }
   mStyleContext = nullptr;
 }
 
 void
 nsComputedDOMStyle::SetResolvedStyleContext(RefPtr<nsStyleContext>&& aContext)
 {
   if (!mResolvedStyleContext) {
--- a/layout/style/nsComputedDOMStyle.h
+++ b/layout/style/nsComputedDOMStyle.h
@@ -126,17 +126,24 @@ public:
     MatrixToCSSValue(const mozilla::gfx::Matrix4x4& aMatrix);
 
   static void RegisterPrefChangeCallbacks();
   static void UnregisterPrefChangeCallbacks();
 
   // nsIMutationObserver
   NS_DECL_NSIMUTATIONOBSERVER_PARENTCHAINCHANGED
 
+  static void
+  GetDiscretelyAnimatedCSSValue(nsStyleContext* aContext,
+                                const nsCSSPropertyID aPropID,
+                                nsCSSValue* aValue);
+
 private:
+  explicit nsComputedDOMStyle(nsStyleContext* aContext);
+
   virtual ~nsComputedDOMStyle();
 
   void AssertFlushedPendingReflows() {
     NS_ASSERTION(mFlushedPendingReflows,
                  "property getter should have been marked layout-dependent");
   }
 
   nsMargin GetAdjustedValuesForBoxSizing();
@@ -156,16 +163,20 @@ private:
 
   static already_AddRefed<nsStyleContext>
   DoGetStyleContextForElementNoFlush(mozilla::dom::Element* aElement,
                                      nsIAtom* aPseudo,
                                      nsIPresShell* aPresShell,
                                      StyleType aStyleType,
                                      AnimationFlag aAnimationFlag);
 
+
+  void GetDiscretelyAnimatedCSSValue(const nsCSSPropertyID aPropID,
+                                     nsCSSValue* aValue);
+
 #define STYLE_STRUCT(name_, checkdata_cb_)                              \
   const nsStyle##name_ * Style##name_() {                               \
     return mStyleContext->Style##name_();                               \
   }
 #include "nsStyleStructList.h"
 #undef STYLE_STRUCT
 
   already_AddRefed<CSSValue> GetEllipseRadii(const nsStyleCorners& aRadius,