Bug 1389215 - Don't layerize in response to margin changes. r?mattwoodrow draft
authorMarkus Stange <mstange@themasta.com>
Sat, 12 Aug 2017 18:42:04 -0400
changeset 645464 2295210dcb650424a8ea9a29fb3a3420eb69fb4e
parent 644284 9847d8cfbe442ebca58a224dcfaaf6280874b8f3
child 646019 47d7789d5e07843b865203d9dda66a372e292aff
push id73755
push userbmo:mstange@themasta.com
push dateSat, 12 Aug 2017 22:42:32 +0000
reviewersmattwoodrow
bugs1389215
milestone57.0a1
Bug 1389215 - Don't layerize in response to margin changes. r?mattwoodrow MozReview-Commit-ID: Ahr3g1NAoQ7
layout/painting/ActiveLayerTracker.cpp
layout/painting/ActiveLayerTracker.h
layout/painting/nsDisplayList.cpp
layout/style/nsDOMCSSAttrDeclaration.cpp
--- a/layout/painting/ActiveLayerTracker.cpp
+++ b/layout/painting/ActiveLayerTracker.cpp
@@ -40,20 +40,16 @@ class LayerActivity {
 public:
   enum ActivityIndex {
     ACTIVITY_OPACITY,
     ACTIVITY_TRANSFORM,
     ACTIVITY_LEFT,
     ACTIVITY_TOP,
     ACTIVITY_RIGHT,
     ACTIVITY_BOTTOM,
-    ACTIVITY_MARGIN_LEFT,
-    ACTIVITY_MARGIN_TOP,
-    ACTIVITY_MARGIN_RIGHT,
-    ACTIVITY_MARGIN_BOTTOM,
     ACTIVITY_BACKGROUND_POSITION,
 
     ACTIVITY_SCALE,
 
     // keep as last item
     ACTIVITY_COUNT
   };
 
@@ -75,20 +71,16 @@ public:
   {
     switch (aProperty) {
     case eCSSProperty_opacity: return ACTIVITY_OPACITY;
     case eCSSProperty_transform: return ACTIVITY_TRANSFORM;
     case eCSSProperty_left: return ACTIVITY_LEFT;
     case eCSSProperty_top: return ACTIVITY_TOP;
     case eCSSProperty_right: return ACTIVITY_RIGHT;
     case eCSSProperty_bottom: return ACTIVITY_BOTTOM;
-    case eCSSProperty_margin_left: return ACTIVITY_MARGIN_LEFT;
-    case eCSSProperty_margin_top: return ACTIVITY_MARGIN_TOP;
-    case eCSSProperty_margin_right: return ACTIVITY_MARGIN_RIGHT;
-    case eCSSProperty_margin_bottom: return ACTIVITY_MARGIN_BOTTOM;
     case eCSSProperty_background_position: return ACTIVITY_BACKGROUND_POSITION;
     case eCSSProperty_background_position_x: return ACTIVITY_BACKGROUND_POSITION;
     case eCSSProperty_background_position_y: return ACTIVITY_BACKGROUND_POSITION;
     default: MOZ_ASSERT(false); return ACTIVITY_OPACITY;
     }
   }
 
   // While tracked, exactly one of mFrame or mContent is non-null, depending
@@ -444,28 +436,24 @@ ActiveLayerTracker::IsStyleAnimated(nsDi
   }
   if (aProperty == eCSSProperty_transform && aFrame->Combines3DTransformWithAncestors()) {
     return IsStyleAnimated(aBuilder, aFrame->GetParent(), aProperty);
   }
   return nsLayoutUtils::HasEffectiveAnimation(aFrame, aProperty);
 }
 
 /* static */ bool
-ActiveLayerTracker::IsOffsetOrMarginStyleAnimated(nsIFrame* aFrame)
+ActiveLayerTracker::IsOffsetStyleAnimated(nsIFrame* aFrame)
 {
   LayerActivity* layerActivity = GetLayerActivity(aFrame);
   if (layerActivity) {
     if (layerActivity->mRestyleCounts[LayerActivity::ACTIVITY_LEFT] >= 2 ||
         layerActivity->mRestyleCounts[LayerActivity::ACTIVITY_TOP] >= 2 ||
         layerActivity->mRestyleCounts[LayerActivity::ACTIVITY_RIGHT] >= 2 ||
-        layerActivity->mRestyleCounts[LayerActivity::ACTIVITY_BOTTOM] >= 2 ||
-        layerActivity->mRestyleCounts[LayerActivity::ACTIVITY_MARGIN_LEFT] >= 2 ||
-        layerActivity->mRestyleCounts[LayerActivity::ACTIVITY_MARGIN_TOP] >= 2 ||
-        layerActivity->mRestyleCounts[LayerActivity::ACTIVITY_MARGIN_RIGHT] >= 2 ||
-        layerActivity->mRestyleCounts[LayerActivity::ACTIVITY_MARGIN_BOTTOM] >= 2) {
+        layerActivity->mRestyleCounts[LayerActivity::ACTIVITY_BOTTOM] >= 2) {
       return true;
     }
   }
   // We should also check for running CSS animations of these properties once
   // bug 1009693 is fixed. Until that happens, layerization isn't useful for
   // animations of these properties because we'll invalidate the layer contents
   // on every change anyway.
   // See bug 1151346 for a patch that adds a check for CSS animations.
--- a/layout/painting/ActiveLayerTracker.h
+++ b/layout/painting/ActiveLayerTracker.h
@@ -81,17 +81,17 @@ public:
    * for constructing active layers.
    */
   static bool IsStyleAnimated(nsDisplayListBuilder* aBuilder, nsIFrame* aFrame,
                               nsCSSPropertyID aProperty);
   /**
    * Return true if any of aFrame's offset property styles should be considered
    * as being animated for constructing active layers.
    */
-  static bool IsOffsetOrMarginStyleAnimated(nsIFrame* aFrame);
+  static bool IsOffsetStyleAnimated(nsIFrame* aFrame);
   /**
    * Return true if aFrame's background-position-x or background-position-y
    * property is animated.
    */
   static bool IsBackgroundPositionAnimated(nsDisplayListBuilder* aBuilder,
                                            nsIFrame* aFrame);
   /**
    * Return true if aFrame either has an animated scale now, or is likely to
--- a/layout/painting/nsDisplayList.cpp
+++ b/layout/painting/nsDisplayList.cpp
@@ -1491,17 +1491,17 @@ nsDisplayListBuilder::IsAnimatedGeometry
     if (aParent) {
       *aParent = nsLayoutUtils::GetCrossDocParentFrame(aFrame);
     }
     return AGR_NO;
   }
 
   if (nsLayoutUtils::IsPopup(aFrame))
     return AGR_YES;
-  if (ActiveLayerTracker::IsOffsetOrMarginStyleAnimated(aFrame)) {
+  if (ActiveLayerTracker::IsOffsetStyleAnimated(aFrame)) {
     const bool inBudget = AddToAGRBudget(aFrame);
     if (inBudget) {
       return AGR_YES;
     }
   }
   if (!aFrame->GetParent() &&
       nsLayoutUtils::ViewportHasDisplayPort(aFrame->PresContext())) {
     // Viewport frames in a display port need to be animated geometry roots
--- a/layout/style/nsDOMCSSAttrDeclaration.cpp
+++ b/layout/style/nsDOMCSSAttrDeclaration.cpp
@@ -206,18 +206,16 @@ nsDOMCSSAttributeDeclaration::SetPropert
   // Scripted modifications to style.opacity or style.transform
   // could immediately force us into the animated state if heuristics suggest
   // this is scripted animation.
   // FIXME: This is missing the margin shorthand and the logical versions of
   // the margin properties, see bug 1266287.
   if (aPropID == eCSSProperty_opacity || aPropID == eCSSProperty_transform ||
       aPropID == eCSSProperty_left || aPropID == eCSSProperty_top ||
       aPropID == eCSSProperty_right || aPropID == eCSSProperty_bottom ||
-      aPropID == eCSSProperty_margin_left || aPropID == eCSSProperty_margin_top ||
-      aPropID == eCSSProperty_margin_right || aPropID == eCSSProperty_margin_bottom ||
       aPropID == eCSSProperty_background_position_x ||
       aPropID == eCSSProperty_background_position_y ||
       aPropID == eCSSProperty_background_position) {
     nsIFrame* frame = mElement->GetPrimaryFrame();
     if (frame) {
       ActiveLayerTracker::NotifyInlineStyleRuleModified(frame, aPropID, aValue, this);
     }
   }