Bug 1376640. Fix dynamic updates when an inline that sits between a first-letter and its block changes style. r=heycam

MozReview-Commit-ID: 8osMUpYVRvR

layout/base/ServoRestyleManager.cpp

@@ -15,7 +15,6 @@
 #include "mozilla/dom/ElementInlines.h"
 #include "nsBlockFrame.h"
 #include "nsBulletFrame.h"
-#include "nsFirstLetterFrame.h"
 #include "nsPlaceholderFrame.h"
 #include "nsContentUtils.h"
 #include "nsCSSFrameConstructor.h"
@@ -56,13 +55,6 @@ ExpectedOwnerForChild(const nsIFrame& aFrame)
   }
 
   const nsIFrame* parent = FirstContinuationOrPartOfIBSplit(aFrame.GetParent());
-  if (nsFirstLetterFrame* fl = do_QueryFrame(const_cast<nsIFrame*>(&aFrame))) {
-    Unused << fl;
-    while (!parent->IsFrameOfType(nsIFrame::eBlockFrame)) {
-      parent = FirstContinuationOrPartOfIBSplit(parent->GetParent());
-    }
-    return parent;
-  }
 
   if (aFrame.IsTableFrame()) {
     MOZ_ASSERT(parent->IsTableWrapperFrame());
@@ -398,10 +390,32 @@ UpdateBackdropIfNeeded(nsIFrame* aFrame,
   aFrame->UpdateStyleOfOwnedChildFrame(backdropFrame, newContext, state);
 }
 
+static void
+UpdateFirstLetterIfNeeded(nsIFrame* aFrame, ServoRestyleState& aRestyleState)
+{
+  if (!aFrame->HasFirstLetterChild()) {
+    return;
+  }
+
+  // We need to find the block the first-letter is associated with so we can
+  // find the right element for the first-letter's style resolution.  Might as
+  // well just delegate the whole thing to that block.
+  nsIFrame* block = aFrame;
+  while (!block->IsFrameOfType(nsIFrame::eBlockFrame)) {
+    block = block->GetParent();
+  }
+  static_cast<nsBlockFrame*>(block->FirstContinuation())->
+    UpdateFirstLetterStyle(aRestyleState);
+}
+
 static void
 UpdateFramePseudoElementStyles(nsIFrame* aFrame,
                                ServoRestyleState& aRestyleState)
 {
+  // first-letter needs to be updated before first-line, because first-line can
+  // change the style of the first-letter.
+  UpdateFirstLetterIfNeeded(aFrame, aRestyleState);
+
   if (aFrame->IsFrameOfType(nsIFrame::eBlockFrame)) {
     static_cast<nsBlockFrame*>(aFrame)->UpdatePseudoElementStyles(aRestyleState);
   }

layout/base/nsCSSFrameConstructor.cpp

@@ -11811,6 +11811,8 @@ nsCSSFrameConstructor::CreateLetterFrame(nsContainerFrame* aBlockFrame,
     }
     MOZ_ASSERT(!aBlockFrame->GetPrevContinuation(),
                "Setting up a first-letter frame on a non-first block continuation?");
+    auto parent = static_cast<nsContainerFrame*>(aParentFrame->FirstContinuation());
+    parent->SetHasFirstLetterChild();
     aBlockFrame->SetProperty(nsContainerFrame::FirstLetterProperty(),
                              letterFrame);
     aTextContent->SetPrimaryFrame(textFrame);
@@ -11956,6 +11958,8 @@ nsCSSFrameConstructor::RemoveFloatingFirstLetterFrames(
     // Somethings really wrong
     return;
   }
+  static_cast<nsContainerFrame*>(parentFrame->FirstContinuation())->
+    ClearHasFirstLetterChild();
 
   // Create a new text frame with the right style context that maps
   // all of the content that was previously part of the letter frame
@@ -12021,6 +12025,8 @@ nsCSSFrameConstructor::RemoveFirstLetterFrames(nsIPresShell* aPresShell,
   while (kid) {
     if (kid->IsLetterFrame()) {
       // Bingo. Found it. First steal away the text frame.
+      static_cast<nsContainerFrame*>(aFrame->FirstContinuation())->
+        ClearHasFirstLetterChild();
       nsIFrame* textFrame = kid->PrincipalChildList().FirstChild();
       if (!textFrame) {
         break;

layout/generic/nsBlockFrame.cpp

@@ -5626,12 +5626,16 @@ nsBlockInFlowLineIterator::nsBlockInFlowLineIterator(nsBlockFrame* aFrame,
 }
 
 void
-nsBlockFrame::UpdateFirstLetterStyle(nsIFrame* aLetterFrame,
-                                     ServoRestyleState& aRestyleState)
+nsBlockFrame::UpdateFirstLetterStyle(ServoRestyleState& aRestyleState)
 {
+  nsIFrame* letterFrame = GetFirstLetter();
+  if (!letterFrame) {
+    return;
+  }
+
   // Figure out what the right style parent is.  This needs to match
   // nsCSSFrameConstructor::CreateLetterFrame.
-  nsIFrame* inFlowFrame = aLetterFrame;
+  nsIFrame* inFlowFrame = letterFrame;
   if (inFlowFrame->GetStateBits() & NS_FRAME_OUT_OF_FLOW) {
     inFlowFrame = inFlowFrame->GetPlaceholderFrame();
   }
@@ -5649,13 +5653,13 @@ nsBlockFrame::UpdateFirstLetterStyle(nsIFrame* aLetterFrame,
   // styles: those will be handled by the styleParent already.
   RefPtr<nsStyleContext> continuationStyle =
     aRestyleState.StyleSet().ResolveStyleForFirstLetterContinuation(parentStyle);
-  UpdateStyleOfOwnedChildFrame(aLetterFrame, firstLetterStyle, aRestyleState,
+  UpdateStyleOfOwnedChildFrame(letterFrame, firstLetterStyle, aRestyleState,
                                Some(continuationStyle.get()));
 
   // We also want to update the style on the textframe inside the first-letter.
   // We don't need to compute a changehint for this, though, since any changes
   // to it are handled by the first-letter anyway.
-  nsIFrame* textFrame = aLetterFrame->PrincipalChildList().FirstChild();
+  nsIFrame* textFrame = letterFrame->PrincipalChildList().FirstChild();
   RefPtr<nsStyleContext> firstTextStyle =
     aRestyleState.StyleSet().ResolveStyleForText(textFrame->GetContent(),
                                                  firstLetterStyle);
@@ -7564,10 +7568,6 @@ nsBlockFrame::UpdatePseudoElementStyles(ServoRestyleState& aRestyleState)
       ResolveBulletStyle(type, &aRestyleState.StyleSet());
     UpdateStyleOfOwnedChildFrame(bullet, newBulletStyle, aRestyleState);
   }
-
-  if (nsIFrame* firstLetter = GetFirstLetter()) {
-    UpdateFirstLetterStyle(firstLetter, aRestyleState);
-  }
 }
 
 already_AddRefed<nsStyleContext>

layout/generic/nsBlockFrame.h

@@ -403,11 +403,15 @@ public:
   };
 
   /**
-   * Update the styles of our various pseudo-elements (bullets, first-letter,
-   * first-line, etc).
+   * Update the styles of our various pseudo-elements (bullets, first-line,
+   * etc, but _not_ first-letter).
    */
   void UpdatePseudoElementStyles(mozilla::ServoRestyleState& aRestyleState);
 
+  // Update our first-letter styles during stylo post-traversal.  This needs to
+  // be done at a slightly different time than our other pseudo-elements.
+  void UpdateFirstLetterStyle(mozilla::ServoRestyleState& aRestyleState);
+
 protected:
   explicit nsBlockFrame(nsStyleContext* aContext, ClassID aID = kClassID)
     : nsContainerFrame(aContext, aID)
@@ -923,10 +927,6 @@ protected:
     mozilla::CSSPseudoElementType aType,
     mozilla::StyleSetHandle aStyleSet);
 
-  // Update our first-letter styles during stylo post-traversal.
-  void UpdateFirstLetterStyle(nsIFrame* aLetterFrame,
-                              mozilla::ServoRestyleState& aRestyleState);
-
 #ifdef DEBUG
   void VerifyLines(bool aFinalCheckOK);
   void VerifyOverflowSituation();

layout/generic/nsContainerFrame.h

@@ -527,6 +527,16 @@ public:
   // have arbitrary nsContainerFrames.
   NS_DECLARE_FRAME_PROPERTY_WITHOUT_DTOR(FirstLetterProperty, nsIFrame)
 
+  void SetHasFirstLetterChild()
+  {
+    mHasFirstLetterChild = true;
+  }
+
+  void ClearHasFirstLetterChild()
+  {
+    mHasFirstLetterChild = false;
+  }
+
 #ifdef DEBUG
   // Use this to suppress the CRAZY_SIZE assertions.
   NS_DECLARE_FRAME_PROPERTY_SMALL_VALUE(DebugReflowingWithInfiniteISize, bool)

layout/generic/nsFrame.cpp

@@ -10248,7 +10248,7 @@ nsIFrame::UpdateStyleOfChildAnonBox(nsIFrame* aChildFrame,
   }
 }
 
-nsChangeHint
+/* static */ nsChangeHint
 nsIFrame::UpdateStyleOfOwnedChildFrame(
   nsIFrame* aChildFrame,
   nsStyleContext* aNewStyleContext,
@@ -10270,8 +10270,13 @@ nsIFrame::UpdateStyleOfOwnedChildFrame(
     aNewStyleContext,
     &equalStructs,
     &samePointerStructs);
-  childHint = NS_RemoveSubsumedHints(
-    childHint, aRestyleState.ChangesHandledFor(*aChildFrame));
+  // If aChildFrame is out of flow, then aRestyleState's "changes handled by the
+  // parent" doesn't apply to it, because it may have some other parent in the
+  // frame tree.
+  if (!aChildFrame->HasAnyStateBits(NS_FRAME_OUT_OF_FLOW)) {
+    childHint = NS_RemoveSubsumedHints(
+      childHint, aRestyleState.ChangesHandledFor(*aChildFrame));
+  }
   if (childHint) {
     if (childHint & nsChangeHint_ReconstructFrame) {
       // If we generate a reconstruct here, remove any non-reconstruct hints we

layout/generic/nsIFrame.h

@@ -618,6 +618,7 @@ public:
     , mClass(aID)
     , mMayHaveRoundedCorners(false)
     , mHasImageRequest(false)
+    , mHasFirstLetterChild(false)
   {
     mozilla::PodZero(&mOverflow);
   }
@@ -3354,7 +3355,7 @@ public:
   // only computed based on aNewStyleContext.
   //
   // Returns the generated change hint for the frame.
-  nsChangeHint UpdateStyleOfOwnedChildFrame(
+  static nsChangeHint UpdateStyleOfOwnedChildFrame(
     nsIFrame* aChildFrame,
     nsStyleContext* aNewStyleContext,
     mozilla::ServoRestyleState& aRestyleState,
@@ -3979,6 +3980,15 @@ public:
    */
   void SetHasImageRequest(bool aHasRequest) { mHasImageRequest = aHasRequest; }
 
+  /**
+   * Whether this frame has a first-letter child.  If it does, the frame is
+   * actually an nsContainerFrame and the first-letter frame can be gotten by
+   * walking up to the nearest ancestor blockframe and getting its first
+   * continuation's nsContainerFrame::FirstLetterProperty() property.  This will
+   * only return true for the first continuation of the first-letter's parent.
+   */
+  bool HasFirstLetterChild() const { return mHasFirstLetterChild; }
+
   /**
    * If this returns true, the frame it's called on should get the
    * NS_FRAME_HAS_DIRTY_CHILDREN bit set on it by the caller; either directly
@@ -4117,7 +4127,15 @@ protected:
    */
   bool mHasImageRequest : 1;
 
-  // There is a 14-bit gap left here.
+  /**
+   * True if this frame has a continuation that has a first-letter frame, or its
+   * placeholder, as a child.  In that case this frame has a blockframe ancestor
+   * that has the first-letter frame hanging off it in the
+   * nsContainerFrame::FirstLetterProperty() property.
+   */
+  bool mHasFirstLetterChild : 1;
+
+  // There is a 13-bit gap left here.
 
   // Helpers
   /**

layout/reftests/first-letter/dynamic-4-ref.html

@@ -0,0 +1,10 @@
+<!DOCTYPE html>
+<html>
+  <style>
+    span { color: green; }
+    div::first-letter { font-size: 100px; }
+  </style>
+  <div style="color: red">
+    <span>X</span>
+  </div>
+</html>

layout/reftests/first-letter/dynamic-4.html

@@ -0,0 +1,23 @@
+<!DOCTYPE html>
+<html class="reftest-wait">
+  <style>
+    span.x { color: green; }
+    div::first-letter { font-size: 100px; }
+  </style>
+  <div style="color: red">
+    <span>X</span>
+  </div>
+  <script>
+  addEventListener("load", function() {
+    // Make sure we do our class change after we've done our next restyle, so we
+    // won't get a spurious restyle hiding the problem we're testing.
+    requestAnimationFrame(() => {
+      requestAnimationFrame(() => {
+        var s = document.querySelector("span");
+        s.className = 'x'
+        document.documentElement.className = "";
+      });
+    });
+  });
+  </script>
+</html>

layout/reftests/first-letter/reftest.list

@@ -26,6 +26,7 @@ fails-if(!stylo) fails-if(styloVsGecko) == dynamic-1.html dynamic-1-ref.html # b
 random-if(d2d) == dynamic-2.html dynamic-2-ref.html
 == dynamic-3a.html dynamic-3-ref.html
 == dynamic-3b.html dynamic-3-ref.html
+== dynamic-4.html dynamic-4-ref.html
 == 23605-1.html 23605-1-ref.html
 == 23605-2.html 23605-2-ref.html
 == 23605-3.html 23605-3-ref.html