Bug 1245075 patch 2 - Fix EffectSet::GetEffectSet(nsIFrame*) and EffectCompositor::GetAnimationElementAndPseudoForFrame to only return effects when the frame is the style frame for its content. r?birtles draft
authorL. David Baron <dbaron@dbaron.org>
Sat, 06 Feb 2016 19:31:49 -0800
changeset 329442 daa16e62d97d9d1e7ed75de87284a81aaf0371c2
parent 329441 f661512e3c1a336c3f3332a876737145e34f3ab5
child 513959 edb60b2a1333c061eaa17419679f3f1033a35d55
push id10526
push userdbaron@mozilla.com
push dateSun, 07 Feb 2016 03:32:00 +0000
reviewersbirtles
bugs1245075
milestone47.0a1
Bug 1245075 patch 2 - Fix EffectSet::GetEffectSet(nsIFrame*) and EffectCompositor::GetAnimationElementAndPseudoForFrame to only return effects when the frame is the style frame for its content. r?birtles This means that we won't associate animations with additional frames. In this case, this fixes associating off-main-thread animations with a table outer frame, when they should have been associated only with the table frame. Locally, the test fails without the patch (with opacity in the test being 0.36 instead of the expected 0.6), and passes with the patch. (Opacity 0.36 gives a color of rgb(163,163,255), whereas 0.6 gives rgb(102,102,255).)
dom/animation/EffectCompositor.cpp
dom/animation/EffectSet.cpp
layout/reftests/css-animations/animate-display-table-opacity-ref.html
layout/reftests/css-animations/animate-display-table-opacity.html
layout/reftests/css-animations/reftest.list
--- a/dom/animation/EffectCompositor.cpp
+++ b/dom/animation/EffectCompositor.cpp
@@ -511,16 +511,21 @@ EffectCompositor::GetAnimationElementAnd
       pseudoType = nsCSSPseudoElements::ePseudo_after;
     } else {
       return result;
     }
     content = content->GetParent();
     if (!content) {
       return result;
     }
+  } else {
+    if (nsLayoutUtils::GetStyleFrame(content) != aFrame) {
+      // The effects associated with an element are for its primary frame.
+      return result;
+    }
   }
 
   if (!content->IsElement()) {
     return result;
   }
 
   result = Some(MakePair(content->AsElement(), pseudoType));
 
--- a/dom/animation/EffectSet.cpp
+++ b/dom/animation/EffectSet.cpp
@@ -4,16 +4,17 @@
  * 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/. */
 
 #include "EffectSet.h"
 #include "mozilla/dom/Element.h" // For Element
 #include "RestyleManager.h"
 #include "nsCycleCollectionNoteChild.h" // For CycleCollectionNoteChild
 #include "nsPresContext.h"
+#include "nsLayoutUtils.h"
 
 namespace mozilla {
 
 /* static */ void
 EffectSet::PropertyDtor(void* aObject, nsIAtom* aPropertyName,
                         void* aPropertyValue, void* aData)
 {
   EffectSet* effectSet = static_cast<EffectSet*>(aPropertyValue);
@@ -65,16 +66,20 @@ EffectSet::GetEffectSet(const nsIFrame* 
     } else {
       return nullptr;
     }
     content = content->GetParent();
     if (!content) {
       return nullptr;
     }
   } else {
+    if (nsLayoutUtils::GetStyleFrame(content) != aFrame) {
+      // The effects associated with an element are for its primary frame.
+      return nullptr;
+    }
     propName = nsGkAtoms::animationEffectsProperty;
   }
 
   if (!content->MayHaveAnimations()) {
     return nullptr;
   }
 
   return static_cast<EffectSet*>(content->GetProperty(propName));
new file mode 100644
--- /dev/null
+++ b/layout/reftests/css-animations/animate-display-table-opacity-ref.html
@@ -0,0 +1,9 @@
+<!DOCTYPE html>
+<title>Testcase for bug 1245075</title>
+<style>
+#test {
+  width: 100px; height: 100px;
+  background: rgb(102, 102, 255);
+}
+</style>
+<div id="test"></div>
new file mode 100644
--- /dev/null
+++ b/layout/reftests/css-animations/animate-display-table-opacity.html
@@ -0,0 +1,14 @@
+<!DOCTYPE html>
+<title>Testcase for bug 1245075</title>
+<style>
+@keyframes HoldOpacity {
+  from, to { opacity: 0.6 }
+}
+#test {
+  width: 100px; height: 100px;
+  background: blue;
+  display: table;
+  animation: HoldOpacity 5s linear;
+}
+</style>
+<div id="test"></div>
--- a/layout/reftests/css-animations/reftest.list
+++ b/layout/reftests/css-animations/reftest.list
@@ -1,6 +1,7 @@
 == screen-animations.html screen-animations-ref.html
 != screen-animations.html screen-animations-notref.html
 fails == print-no-animations.html print-no-animations-ref.html # reftest harness doesn't actually make pres context non-dynamic for reftest-print tests
 fails != print-no-animations.html print-no-animations-notref.html # reftest harness doesn't actually make pres context non-dynamic for reftest-print tests
 == animate-opacity.html animate-opacity-ref.html
 == animate-preserves3d.html animate-preserves3d-ref.html
+== animate-display-table-opacity.html animate-display-table-opacity-ref.html