Bug 670467. Correctly reresolve style on non-first continuations and non-first parts of {ib} splits even when we're restyling both an ancestor of the element and the element itself. r=dbaron
authorBoris Zbarsky <bzbarsky@mit.edu>
Thu, 11 Aug 2011 23:52:21 -0400
changeset 74284 77a0d9f09820956bce5c4e1818ed540762fd6c23
parent 74283 1f84f5eb5079bffc321c115f167db8402841de8e
child 74285 c7c13a66ad615d19d2dd91484fa4a3288868a772
push id1163
push userbzbarsky@mozilla.com
push dateFri, 12 Aug 2011 03:53:35 +0000
treeherdermozilla-inbound@77a0d9f09820 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdbaron
bugs670467
milestone8.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 670467. Correctly reresolve style on non-first continuations and non-first parts of {ib} splits even when we're restyling both an ancestor of the element and the element itself. r=dbaron
layout/base/nsFrameManager.cpp
layout/reftests/bugs/670467-1-ref.html
layout/reftests/bugs/670467-1.html
layout/reftests/bugs/670467-2-ref.html
layout/reftests/bugs/670467-2.html
layout/reftests/bugs/reftest.list
--- a/layout/base/nsFrameManager.cpp
+++ b/layout/base/nsFrameManager.cpp
@@ -761,16 +761,44 @@ ElementForStyleContext(nsIContent* aPare
     nsBlockFrame* block = nsBlockFrame::GetNearestAncestorBlock(aFrame);
     return block->GetContent()->AsElement();
   }
 
   nsIContent* content = aParentContent ? aParentContent : aFrame->GetContent();
   return content->AsElement();
 }
 
+static nsIFrame*
+GetPrevContinuationWithPossiblySameStyle(nsIFrame* aFrame)
+{
+  // Account for {ib} splits when looking for "prevContinuation".  In
+  // particular, for the first-continuation of a part of an {ib} split we
+  // want to use the special prevsibling of the special prevsibling of
+  // aFrame, which should have the same style context as aFrame itself.
+  // In particular, if aFrame is the first continuation of an inline part
+  // of an {ib} split then its special prevsibling is a block, and the
+  // special prevsibling of _that_ is an inline, just like aFrame.
+  // Similarly, if aFrame is the first continuation of a block part of an
+  // {ib} split (an {ib} wrapper block), then its special prevsibling is
+  // an inline and the special prevsibling of that is either another {ib}
+  // wrapper block block or null.
+  nsIFrame *prevContinuation = aFrame->GetPrevContinuation();
+  if (!prevContinuation && (aFrame->GetStateBits() & NS_FRAME_IS_SPECIAL)) {
+    // We're the first continuation, so we can just get the frame
+    // property directly
+    prevContinuation = static_cast<nsIFrame*>(
+      aFrame->Properties().Get(nsIFrame::IBSplitSpecialPrevSibling()));
+    if (prevContinuation) {
+      prevContinuation = static_cast<nsIFrame*>(
+        prevContinuation->Properties().Get(nsIFrame::IBSplitSpecialPrevSibling()));
+    }
+  }
+  return prevContinuation;
+}
+
 nsresult
 nsFrameManager::ReparentStyleContext(nsIFrame* aFrame)
 {
   if (nsGkAtoms::placeholderFrame == aFrame->GetType()) {
     // Also reparent the out-of-flow and all its continuations.
     nsIFrame* outOfFlow =
       nsPlaceholderFrame::GetRealFrameForPlaceholder(aFrame);
     NS_ASSERTION(outOfFlow, "no out-of-flow frame");
@@ -826,17 +854,18 @@ nsFrameManager::ReparentStyleContext(nsI
                        nextContinuationContext->GetPseudo() ||
                      oldContext->GetParent() !=
                        nextContinuationContext->GetParent(),
                      "continuations should have the same style context");
       }
     }
 #endif
 
-    nsIFrame *prevContinuation = aFrame->GetPrevContinuation();
+    nsIFrame *prevContinuation =
+      GetPrevContinuationWithPossiblySameStyle(aFrame);
     nsStyleContext *prevContinuationContext;
     PRBool copyFromContinuation =
       prevContinuation &&
       (prevContinuationContext = prevContinuation->GetStyleContext())
         ->GetPseudo() == oldContext->GetPseudo() &&
        prevContinuationContext->GetParent() == newParentContext;
     if (copyFromContinuation) {
       // Just use the style context from the frame's previous
@@ -1142,22 +1171,45 @@ nsFrameManager::ReResolveStyleContext(ns
           nextContinuation->GetStyleContext();
         NS_ASSERTION(oldContext == nextContinuationContext ||
                      oldContext->GetPseudo() !=
                        nextContinuationContext->GetPseudo() ||
                      oldContext->GetParent() !=
                        nextContinuationContext->GetParent(),
                      "continuations should have the same style context");
       }
+      // And assert the same thing for {ib} splits.  See the comments in
+      // GetPrevContinuationWithPossiblySameStyle for an explanation of
+      // why we step two forward in the special sibling chain.
+      if ((aFrame->GetStateBits() & NS_FRAME_IS_SPECIAL) &&
+          !aFrame->GetPrevContinuation()) {
+        nsIFrame *nextIBSibling = static_cast<nsIFrame*>(
+          aFrame->Properties().Get(nsIFrame::IBSplitSpecialSibling()));
+        if (nextIBSibling) {
+          nextIBSibling = static_cast<nsIFrame*>(
+            nextIBSibling->Properties().Get(nsIFrame::IBSplitSpecialSibling()));
+        }
+        if (nextIBSibling) {
+          nsStyleContext *nextIBSiblingContext =
+            nextIBSibling->GetStyleContext();
+          NS_ASSERTION(oldContext == nextIBSiblingContext ||
+                       oldContext->GetPseudo() !=
+                         nextIBSiblingContext->GetPseudo() ||
+                       oldContext->GetParent() !=
+                         nextIBSiblingContext->GetParent(),
+                       "continuations should have the same style context");
+        }
+      }
     }
 #endif
 
     // do primary context
     nsRefPtr<nsStyleContext> newContext;
-    nsIFrame *prevContinuation = aFrame->GetPrevContinuation();
+    nsIFrame *prevContinuation =
+      GetPrevContinuationWithPossiblySameStyle(aFrame);
     nsStyleContext *prevContinuationContext;
     PRBool copyFromContinuation =
       prevContinuation &&
       (prevContinuationContext = prevContinuation->GetStyleContext())
         ->GetPseudo() == oldContext->GetPseudo() &&
        prevContinuationContext->GetParent() == parentContext;
     if (copyFromContinuation) {
       // Just use the style context from the frame's previous
@@ -1166,17 +1218,25 @@ nsFrameManager::ReResolveStyleContext(ns
       // continuation).
       newContext = prevContinuationContext;
     }
     else if (pseudoTag == nsCSSAnonBoxes::mozNonElement) {
       NS_ASSERTION(localContent,
                    "non pseudo-element frame without content node");
       newContext = styleSet->ResolveStyleForNonElement(parentContext);
     }
-    else if (!aRestyleHint) {
+    else if (!aRestyleHint && !prevContinuation) {
+      // Unfortunately, if prevContinuation is non-null then we may have
+      // already stolen the restyle tracker entry for this element while
+      // processing prevContinuation.  So we don't know whether aRestyleHint
+      // should really be 0 here or whether it should be eRestyle_Self.  Be
+      // pessimistic and force an actual reresolve in that situation.  The good
+      // news is that in the common case when prevContinuation is non-null we
+      // just used prevContinuationContext anyway and aren't reaching this code
+      // to start with.
       newContext =
         styleSet->ReparentStyleContext(oldContext, parentContext,
                                        ElementForStyleContext(aParentContent,
                                                               aFrame,
                                                               pseudoType));
     } else if (pseudoType == nsCSSPseudoElements::ePseudo_AnonBox) {
       newContext = styleSet->ResolveAnonymousBoxStyle(pseudoTag,
                                                       parentContext);
new file mode 100644
--- /dev/null
+++ b/layout/reftests/bugs/670467-1-ref.html
@@ -0,0 +1,10 @@
+<!DOCTYPE html>
+<body style="color: green">
+  <div style="display: inline;">
+    Line 1
+    <div>Line 2</div>
+    Line 3
+    <div>Line 4</div>
+    Line 5
+  </div>
+</body>
new file mode 100644
--- /dev/null
+++ b/layout/reftests/bugs/670467-1.html
@@ -0,0 +1,14 @@
+<!DOCTYPE html>
+<body style="color: red">
+  <div id="x" style="display: inline; visibility: hidden">
+    Line 1
+    <div>Line 2</div>
+    Line 3
+    <div>Line 4</div>
+    Line 5
+  </div><script type="text/javascript">
+    document.body.offsetWidth;
+    document.getElementById("x").style.visibility = "visible";
+    document.body.style.color = "green";
+  </script>
+</body>
new file mode 100644
--- /dev/null
+++ b/layout/reftests/bugs/670467-2-ref.html
@@ -0,0 +1,6 @@
+<!DOCTYPE html>
+<style>
+  body::first-line { color: purple; }
+</style>
+<body style="width: 0; color: green"><span>This is some text</span>
+</body>
new file mode 100644
--- /dev/null
+++ b/layout/reftests/bugs/670467-2.html
@@ -0,0 +1,11 @@
+<!DOCTYPE html>
+<style>
+  body::first-line { color: purple; }
+</style>
+<body style="width: 0; color: red"><span id="x" style="visibility: hidden">This is some text</span>
+  <script type="text/javascript">
+    document.body.offsetWidth;
+    document.getElementById("x").style.visibility = "visible";
+    document.body.style.color = "green";
+  </script>
+</body>
--- a/layout/reftests/bugs/reftest.list
+++ b/layout/reftests/bugs/reftest.list
@@ -1649,8 +1649,10 @@ fails-if(layersGPUAccelerated&&cocoaWidg
 == 652775-1.html 652775-1-ref.html
 == 655549-1.html 655549-1-ref.html
 == 658952.html 658952-ref.html
 == 664127-1.xul 664127-1-ref.xul
 == 660682-1.html 660682-1-ref.html
 != 669015-1.xul 669015-1-notref.xul
 == 668319-1.xul about:blank
 == 670442-1.html 670442-1-ref.html
+== 670467-1.html 670467-1-ref.html
+== 670467-2.html 670467-2-ref.html