Bug 795657. Don't reframe for adding a transform when absolute descendants are present, when the frame is already positioned. r=bz
authorRobert O'Callahan <robert@ocallahan.org>
Wed, 10 Oct 2012 23:25:57 +1300
changeset 110384 fa3a84d645fc4a44ee064e6397f334ef54aee1e1
parent 110383 9edabc0ddc99d86d18378cdccf9c7fc0bb3b34e4
child 110385 0c8ac346e41bac80534be22964967550785a0607
push id93
push usernmatsakis@mozilla.com
push dateWed, 31 Oct 2012 21:26:57 +0000
reviewersbz
bugs795657
milestone19.0a1
Bug 795657. Don't reframe for adding a transform when absolute descendants are present, when the frame is already positioned. r=bz
layout/base/nsCSSFrameConstructor.cpp
--- a/layout/base/nsCSSFrameConstructor.cpp
+++ b/layout/base/nsCSSFrameConstructor.cpp
@@ -7990,40 +7990,76 @@ nsCSSFrameConstructor::CharacterDataChan
   }
 
   return rv;
 }
 
 NS_DECLARE_FRAME_PROPERTY(ChangeListProperty, nullptr)
 
 /**
- * Return true if aFrame's subtree has placeholders for abs-pos or fixed-pos
- * content.
+ * Return true if aFrame's subtree has placeholders for out-of-flow content
+ * whose 'position' style's bit in aPositionMask is set.
  */
 static bool
-FrameHasAbsPosPlaceholderDescendants(nsIFrame* aFrame)
+FrameHasPositionedPlaceholderDescendants(nsIFrame* aFrame, uint32_t aPositionMask)
 {
   const nsIFrame::ChildListIDs skip(nsIFrame::kAbsoluteList |
                                     nsIFrame::kFixedList);
   for (nsIFrame::ChildListIterator lists(aFrame); !lists.IsDone(); lists.Next()) {
     if (!skip.Contains(lists.CurrentID())) {
       for (nsFrameList::Enumerator childFrames(lists.CurrentList());
            !childFrames.AtEnd(); childFrames.Next()) {
         nsIFrame* f = childFrames.get();
-        if ((f->GetType() == nsGkAtoms::placeholderFrame &&
-             nsPlaceholderFrame::GetRealFrameForPlaceholder(f)->IsAbsolutelyPositioned()) ||
-            FrameHasAbsPosPlaceholderDescendants(f)) {
+        if (f->GetType() == nsGkAtoms::placeholderFrame) {
+          nsIFrame* outOfFlow = nsPlaceholderFrame::GetRealFrameForPlaceholder(f);
+          // If SVG text frames could appear here, they could confuse us since
+          // they ignore their position style ... but they can't.
+          NS_ASSERTION(!outOfFlow->IsSVGText(),
+                       "SVG text frames can't be out of flow");
+          if (aPositionMask & (1 << outOfFlow->GetStyleDisplay()->mPosition)) {
+            return true;
+          }
+        }
+        if (FrameHasPositionedPlaceholderDescendants(f, aPositionMask)) {
           return true;
         }
       }
     }
   }
   return false;
 }
 
+static bool
+NeedToReframeForAddingOrRemovingTransform(nsIFrame* aFrame)
+{
+  MOZ_STATIC_ASSERT(0 <= NS_STYLE_POSITION_ABSOLUTE &&
+                    NS_STYLE_POSITION_ABSOLUTE < 32, "Style constant out of range");
+  MOZ_STATIC_ASSERT(0 <= NS_STYLE_POSITION_FIXED &&
+                    NS_STYLE_POSITION_FIXED < 32, "Style constant out of range");
+
+  const nsStyleDisplay* style = aFrame->GetStyleDisplay();
+  uint32_t positionMask;
+  // Don't call aFrame->IsPositioned here, since that returns true if
+  // the frame already has a transform, and we want to ignore that here
+  if (aFrame->IsAbsolutelyPositioned() ||
+      aFrame->IsRelativelyPositioned()) {
+    // This frame is a container for abs-pos descendants whether or not it
+    // has a transform.
+    // So abs-pos descendants are no problem; we only need to reframe if
+    // we have fixed-pos descendants.
+    positionMask = 1 << NS_STYLE_POSITION_FIXED;
+  } else {
+    // This frame may not be a container for abs-pos descendants already.
+    // So reframe if we have abs-pos or fixed-pos descendants.
+    positionMask = (1 << NS_STYLE_POSITION_FIXED) |
+        (1 << NS_STYLE_POSITION_ABSOLUTE);
+  }
+  return FrameHasPositionedPlaceholderDescendants(aFrame, positionMask);
+}
+
 nsresult
 nsCSSFrameConstructor::ProcessRestyledFrames(nsStyleChangeList& aChangeList)
 {
   NS_ASSERTION(!nsContentUtils::IsSafeToRunScript(),
                "Someone forgot a script blocker");
   int32_t count = aChangeList.Count();
   if (!count)
     return NS_OK;
@@ -8076,17 +8112,17 @@ nsCSSFrameConstructor::ProcessRestyledFr
     // skip any frame that has been destroyed due to a ripple effect
     if (frame) {
       if (!propTable->Get(frame, ChangeListProperty()))
         continue;
     }
 
     if ((hint & nsChangeHint_AddOrRemoveTransform) && frame &&
         !(hint & nsChangeHint_ReconstructFrame)) {
-      if (FrameHasAbsPosPlaceholderDescendants(frame)) {
+      if (NeedToReframeForAddingOrRemovingTransform(frame)) {
         NS_UpdateHint(hint, nsChangeHint_ReconstructFrame);
       } else {
         // We can just add this state bit unconditionally, since it's
         // conservative. Normally frame construction would set this if needed,
         // but we're not going to reconstruct the frame so we need to set it.
         // It's because we need to set this bit on each affected frame
         // that we can't coalesce nsChangeHint_AddOrRemoveTransform hints up
         // to ancestors (i.e. it can't be an inherited change hint).