Bug 1324618 part 7. Set our new style context on all our continuations in ServoRestyleManager. r=emilio

MozReview-Commit-ID: 4gVXPDCPZnq

--HG--
extra : rebase_source : c6f6102afee259daa92c41a083acf70cc52b93f3

layout/base/ServoRestyleManager.cpp

@@ -341,9 +341,8 @@ ServoRestyleManager::ProcessPostTraversal(Element* aElement,
   // ServoComputedValues case, which uses atomic refcounting.
   //
   // Hold the old style context alive, because it could become a dangling
-  // pointer during the replacement. In practice it's not a huge deal (on
-  // GetNextContinuationWithSameStyle the pointer is not dereferenced, only
-  // compared), but better not playing with dangling pointers if not needed.
+  // pointer during the replacement. In practice it's not a huge deal, but
+  // better not playing with dangling pointers if not needed.
   RefPtr<nsStyleContext> oldStyleContext =
     styleFrame ? styleFrame->StyleContext() : nullptr;
 
@@ -389,14 +388,17 @@ ServoRestyleManager::ProcessPostTraversal(Element* aElement,
 
     newContext->EnsureSameStructsCached(oldStyleContext);
 
-    // XXX This could not always work as expected: there are kinds of content
-    // with the first split and the last sharing style, but others not. We
-    // should handle those properly.
-    // XXXbz I think the UpdateStyleOfOwnedAnonBoxes call below handles _that_
-    // right, but not other cases where we happen to have different styles on
-    // different continuations... (e.g. first-line).
-    for (nsIFrame* f = styleFrame; f;
-         f = GetNextContinuationWithSameStyle(f, oldStyleContext)) {
+    // We want to walk all the continuations here, even the ones with different
+    // styles.  In practice, the only reason we get continuations with different
+    // styles here is ::first-line (::first-letter never affects element
+    // styles).  But in that case, newContext is the right context for the
+    // _later_ continuations anyway (the ones not affected by ::first-line), not
+    // the earlier ones, so there is no point stopping right at the point when
+    // we'd actually be setting the right style context.
+    //
+    // This does mean that we may be setting the wrong style context on our
+    // initial continuations; ::first-line fixes that up after the fact.
+    for (nsIFrame* f = styleFrame; f; f = f->GetNextContinuation()) {
       f->SetStyleContext(newContext);
     }
 
@@ -493,8 +495,18 @@ ServoRestyleManager::ProcessPostTraversalForText(
   nsStyleContext& newContext = aPostTraversalState.ComputeStyle(aTextNode);
   aPostTraversalState.ComputeHintIfNeeded(aTextNode, primaryFrame, newContext);
 
-  for (nsIFrame* f = primaryFrame; f;
-       f = GetNextContinuationWithSameStyle(f, oldStyleContext)) {
+  // We want to walk all the continuations here, even the ones with different
+  // styles.  In practice, the only reasons we get continuations with different
+  // styles are ::first-line and ::first-letter.  But in those cases,
+  // newContext is the right context for the _later_ continuations anyway (the
+  // ones not affected by ::first-line/::first-letter), not the earlier ones,
+  // so there is no point stopping right at the point when we'd actually be
+  // setting the right style context.
+  //
+  // This does mean that we may be setting the wrong style context on our
+  // initial continuations; ::first-line/::first-letter fix that up after the
+  // fact.
+  for (nsIFrame* f = primaryFrame; f; f = f->GetNextContinuation()) {
     f->SetStyleContext(&newContext);
   }
 

layout/generic/nsInlineFrame.cpp

@@ -1032,11 +1032,14 @@ nsInlineFrame::UpdateStyleOfOwnedAnonBoxesForIBSplit(
   nsIFrame* blockFrame = GetProperty(nsIFrame::IBSplitSibling());
   MOZ_ASSERT(blockFrame, "Why did we have an IB split?");
 
+  // The later inlines need to get our style.
+  nsStyleContext* ourStyle = StyleContext();
+
   // The anonymous block's style inherits from ours, and we already have our new
   // style context.
   RefPtr<nsStyleContext> newContext =
     aRestyleState.StyleSet().ResolveInheritingAnonymousBoxStyle(
-      nsCSSAnonBoxes::mozBlockInsideInlineWrapper, StyleContext());
+      nsCSSAnonBoxes::mozBlockInsideInlineWrapper, ourStyle);
 
   // We're guaranteed that newContext only differs from the old style context on
   // the block in things they might inherit from us.  And changehint processing
@@ -1052,16 +1055,19 @@ nsInlineFrame::UpdateStyleOfOwnedAnonBoxesForIBSplit(
                nsCSSAnonBoxes::mozBlockInsideInlineWrapper,
                "Unexpected kind of style context");
 
-    // We _could_ just walk along using GetNextContinuationWithSameStyle here,
-    // but it would involve going back to the first continuation every so often,
-    // which is a bit silly when we can just keep track of our first
-    // continuations.
+    // We don't want to just walk through using GetNextContinuationWithSameStyle
+    // here, because we want to set updated style contexts on both our
+    // ib-sibling blocks and inlines.
     for (nsIFrame* cont = blockFrame; cont; cont = cont->GetNextContinuation()) {
       cont->SetStyleContext(newContext);
     }
 
     nsIFrame* nextInline = blockFrame->GetProperty(nsIFrame::IBSplitSibling());
     MOZ_ASSERT(nextInline, "There is always a trailing inline in an IB split");
+
+    for (nsIFrame* cont = nextInline; cont; cont = cont->GetNextContinuation()) {
+      cont->SetStyleContext(ourStyle);
+    }
     blockFrame = nextInline->GetProperty(nsIFrame::IBSplitSibling());
   }
 }

layout/reftests/ib-split/insert-into-split-inline-17-ref.html

@@ -0,0 +1,9 @@
+<!DOCTYPE html>
+<style>
+  span { color: green; }
+</style>
+<span>
+  <div></div>
+  First Second Third
+  <div></div>
+</span>

layout/reftests/ib-split/insert-into-split-inline-17.html

@@ -0,0 +1,20 @@
+<!DOCTYPE html>
+<style>
+  #target { color: red; }
+  #target.green { color: green; }
+</style>
+<span id="target">
+  <div></div>
+  First <span id="next"> Third</span>
+  <div></div>
+</span>
+<script>
+  onload = function() {
+    var s = document.querySelector("#target");
+    window.color = getComputedStyle(s).color;
+    s.className = "green";
+    window.newColor = getComputedStyle(s).color;
+    s.insertBefore(document.createTextNode("Second"),
+                   s.querySelector("#next"));
+  }
+</script>

layout/reftests/ib-split/reftest.list

@@ -64,6 +64,7 @@
 == insert-into-split-inline-16a.html insert-into-split-inline-16-ref.html
 == insert-into-split-inline-16b.html insert-into-split-inline-16-ref.html
 == insert-into-split-inline-16-ref.html insert-into-split-inline-16-noib-ref.html
+== insert-into-split-inline-17.html insert-into-split-inline-17-ref.html
 == float-inside-inline-between-blocks-1.html float-inside-inline-between-blocks-1-ref.html
 == table-pseudo-in-part3-1.html table-pseudo-in-part3-1-ref.html
 == emptyspan-1.html emptyspan-1-ref.html