Bug 1428164: Update first-letter styles of non-block anon boxes. r?bz draft
authorEmilio Cobos Álvarez <emilio@crisal.io>
Thu, 04 Jan 2018 23:55:48 +0100
changeset 716024 21d7521c2f36d9d263016386b42cb8fae702da09
parent 716023 8678a03a5ad1326a0f1b533b06071d7c97370f6e
child 716025 6d852a33adb06041b17d619bafc1b4248d1b5b03
push id94304
push userbmo:emilio@crisal.io
push dateThu, 04 Jan 2018 23:58:01 +0000
reviewersbz
bugs1428164
milestone59.0a1
Bug 1428164: Update first-letter styles of non-block anon boxes. r?bz Also unify the pseudo-element restyling code a bit more. MozReview-Commit-ID: 5ZwASpV9u1R
layout/base/ServoRestyleManager.cpp
layout/generic/nsFrame.cpp
layout/generic/nsIFrame.h
--- a/layout/base/ServoRestyleManager.cpp
+++ b/layout/base/ServoRestyleManager.cpp
@@ -552,80 +552,16 @@ private:
   ServoRestyleState& mParentRestyleState;
   RefPtr<nsStyleContext> mStyle;
   bool mShouldPostHints;
   bool mShouldComputeHints;
   nsChangeHint mComputedHint;
 };
 
 static void
-UpdateBackdropIfNeeded(nsIFrame* aFrame,
-                       ServoStyleSet& aStyleSet,
-                       nsStyleChangeList& aChangeList)
-{
-  const nsStyleDisplay* display = aFrame->StyleContext()->StyleDisplay();
-  if (display->mTopLayer != NS_STYLE_TOP_LAYER_TOP) {
-    return;
-  }
-
-  // Elements in the top layer are guaranteed to have absolute or fixed
-  // position per https://fullscreen.spec.whatwg.org/#new-stacking-layer.
-  MOZ_ASSERT(display->IsAbsolutelyPositionedStyle());
-
-  nsIFrame* backdropPlaceholder =
-    aFrame->GetChildList(nsIFrame::kBackdropList).FirstChild();
-  if (!backdropPlaceholder) {
-    return;
-  }
-
-  MOZ_ASSERT(backdropPlaceholder->IsPlaceholderFrame());
-  nsIFrame* backdropFrame =
-    nsPlaceholderFrame::GetRealFrameForPlaceholder(backdropPlaceholder);
-  MOZ_ASSERT(backdropFrame->IsBackdropFrame());
-  MOZ_ASSERT(backdropFrame->StyleContext()->GetPseudoType() ==
-             CSSPseudoElementType::backdrop);
-
-  RefPtr<nsStyleContext> newContext =
-    aStyleSet.ResolvePseudoElementStyle(aFrame->GetContent()->AsElement(),
-                                        CSSPseudoElementType::backdrop,
-                                        aFrame->StyleContext()->AsServo(),
-                                        /* aPseudoElement = */ nullptr);
-
-  // NOTE(emilio): We can't use the changes handled for the owner of the
-  // backdrop frame, since it's out of flow, and parented to the viewport or
-  // canvas frame (depending on the `position` value).
-  MOZ_ASSERT(backdropFrame->GetParent()->IsViewportFrame() ||
-             backdropFrame->GetParent()->IsCanvasFrame());
-  nsTArray<nsIFrame*> wrappersToRestyle;
-  ServoRestyleState state(aStyleSet, aChangeList, wrappersToRestyle);
-  aFrame->UpdateStyleOfOwnedChildFrame(backdropFrame, newContext, state);
-}
-
-static void
-UpdateFirstLetterIfNeeded(nsIFrame* aFrame, ServoRestyleState& aRestyleState)
-{
-  MOZ_ASSERT(!aFrame->IsFrameOfType(nsIFrame::eBlockFrame),
-             "You're probably duplicating work with UpdatePseudoElementStyles!");
-  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->GetParent();
-  while (!block->IsFrameOfType(nsIFrame::eBlockFrame)) {
-    block = block->GetParent();
-  }
-
-  static_cast<nsBlockFrame*>(block->FirstContinuation())->
-    UpdateFirstLetterStyle(aRestyleState);
-}
-
-static void
 UpdateOneAdditionalStyleContext(nsIFrame* aFrame,
                                 uint32_t aIndex,
                                 ServoStyleContext& aOldContext,
                                 ServoRestyleState& aRestyleState)
 {
   auto pseudoType = aOldContext.GetPseudoType();
   MOZ_ASSERT(pseudoType != CSSPseudoElementType::NotPseudo);
   MOZ_ASSERT(
@@ -667,30 +603,16 @@ UpdateAdditionalStyleContexts(nsIFrame* 
   // virtual call?
   uint32_t index = 0;
   while (auto* oldContext = aFrame->GetAdditionalStyleContext(index)) {
     UpdateOneAdditionalStyleContext(
         aFrame, index++, *oldContext->AsServo(), aRestyleState);
   }
 }
 
-static void
-UpdateFramePseudoElementStyles(nsIFrame* aFrame,
-                               ServoRestyleState& aRestyleState)
-{
-  if (aFrame->IsFrameOfType(nsIFrame::eBlockFrame)) {
-    static_cast<nsBlockFrame*>(aFrame)->UpdatePseudoElementStyles(aRestyleState);
-  } else {
-    UpdateFirstLetterIfNeeded(aFrame, aRestyleState);
-  }
-
-  UpdateBackdropIfNeeded(
-    aFrame, aRestyleState.StyleSet(), aRestyleState.ChangeList());
-}
-
 enum class ServoPostTraversalFlags : uint32_t
 {
   Empty = 0,
   // Whether parent was restyled.
   ParentWasRestyled = 1 << 0,
   // Skip sending accessibility notifications for all descendants.
   SkipA11yNotifications = 1 << 1,
   // Always send accessibility notifications if the element is shown.
@@ -968,17 +890,17 @@ ServoRestyleManager::ProcessPostTraversa
   // kids, because some of those updates (::first-line/::first-letter) need to
   // modify the styles of the kids, and the child traversal above would just
   // clobber those modifications.
   if (styleFrame) {
     // Process anon box wrapper frames before ::first-line bits.
     childrenRestyleState.ProcessWrapperRestyles(styleFrame);
 
     if (wasRestyled) {
-      UpdateFramePseudoElementStyles(styleFrame, childrenRestyleState);
+      styleFrame->UpdatePseudoElementStylesIfNeeded(childrenRestyleState);
     } else if (traverseElementChildren &&
                styleFrame->IsFrameOfType(nsIFrame::eBlockFrame)) {
       // Even if we were not restyled, if we're a block with a first-line and
       // one of our descendant elements which is on the first line was restyled,
       // we need to update the styles of things on the first line, because
       // they're wrong now.
       //
       // FIXME(bz) Could we do better here?  For example, could we keep track of
--- a/layout/generic/nsFrame.cpp
+++ b/layout/generic/nsFrame.cpp
@@ -10629,22 +10629,100 @@ nsFrame::BoxReflow(nsBoxLayoutState&    
 nsBoxLayoutMetrics*
 nsFrame::BoxMetrics() const
 {
   nsBoxLayoutMetrics* metrics = GetProperty(BoxMetricsProperty());
   NS_ASSERTION(metrics, "A box layout method was called but InitBoxMetrics was never called");
   return metrics;
 }
 
+static void
+UpdateBackdropIfNeeded(nsIFrame* aFrame,
+                       ServoStyleSet& aStyleSet,
+                       nsStyleChangeList& aChangeList)
+{
+  const nsStyleDisplay* display = aFrame->StyleDisplay();
+  if (display->mTopLayer != NS_STYLE_TOP_LAYER_TOP) {
+    return;
+  }
+
+  // Elements in the top layer are guaranteed to have absolute or fixed
+  // position per https://fullscreen.spec.whatwg.org/#new-stacking-layer.
+  MOZ_ASSERT(display->IsAbsolutelyPositionedStyle());
+
+  nsIFrame* backdropPlaceholder =
+    aFrame->GetChildList(nsIFrame::kBackdropList).FirstChild();
+  if (!backdropPlaceholder) {
+    return;
+  }
+
+  MOZ_ASSERT(backdropPlaceholder->IsPlaceholderFrame());
+  nsIFrame* backdropFrame =
+    nsPlaceholderFrame::GetRealFrameForPlaceholder(backdropPlaceholder);
+  MOZ_ASSERT(backdropFrame->IsBackdropFrame());
+  MOZ_ASSERT(backdropFrame->StyleContext()->GetPseudoType() ==
+             CSSPseudoElementType::backdrop);
+
+  RefPtr<nsStyleContext> newContext =
+    aStyleSet.ResolvePseudoElementStyle(aFrame->GetContent()->AsElement(),
+                                        CSSPseudoElementType::backdrop,
+                                        aFrame->StyleContext()->AsServo(),
+                                        /* aPseudoElement = */ nullptr);
+
+  // NOTE(emilio): We can't use the changes handled for the owner of the
+  // backdrop frame, since it's out of flow, and parented to the viewport or
+  // canvas frame (depending on the `position` value).
+  MOZ_ASSERT(backdropFrame->GetParent()->IsViewportFrame() ||
+             backdropFrame->GetParent()->IsCanvasFrame());
+  nsTArray<nsIFrame*> wrappersToRestyle;
+  ServoRestyleState state(aStyleSet, aChangeList, wrappersToRestyle);
+  aFrame->UpdateStyleOfOwnedChildFrame(backdropFrame, newContext, state);
+}
+
+static void
+UpdateFirstLetterIfNeeded(nsIFrame* aFrame, ServoRestyleState& aRestyleState)
+{
+  MOZ_ASSERT(!aFrame->IsFrameOfType(nsIFrame::eBlockFrame),
+             "You're probably duplicating work with "
+             "nsBlockFrame::UpdatePseudoElementStyles!");
+  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->GetParent();
+  while (!block->IsFrameOfType(nsIFrame::eBlockFrame)) {
+    block = block->GetParent();
+  }
+
+  static_cast<nsBlockFrame*>(block->FirstContinuation())->
+    UpdateFirstLetterStyle(aRestyleState);
+}
+
+void
+nsIFrame::UpdatePseudoElementStylesIfNeeded(ServoRestyleState& aRestyleState)
+{
+  if (IsFrameOfType(nsIFrame::eBlockFrame)) {
+    static_cast<nsBlockFrame*>(this)->UpdatePseudoElementStyles(aRestyleState);
+  } else {
+    UpdateFirstLetterIfNeeded(this, aRestyleState);
+  }
+
+  UpdateBackdropIfNeeded(
+    this, aRestyleState.StyleSet(), aRestyleState.ChangeList());
+}
+
 void
 nsIFrame::UpdateStyleOfChildAnonBox(nsIFrame* aChildFrame,
                                     ServoRestyleState& aRestyleState)
 {
 #ifdef DEBUG
-  nsIFrame* parent = aChildFrame->GetParent();;
+  nsIFrame* parent = aChildFrame->GetParent();
   if (aChildFrame->IsTableFrame()) {
     parent = parent->GetParent();
   }
   if (parent->IsLineFrame()) {
     parent = parent->GetParent();
   }
   MOZ_ASSERT(nsLayoutUtils::FirstContinuationOrIBSplitSibling(parent) == this,
              "This should only be used for children!");
@@ -10666,31 +10744,21 @@ nsIFrame::UpdateStyleOfChildAnonBox(nsIF
   RefPtr<ServoStyleContext> newContext =
     aRestyleState.StyleSet().ResolveInheritingAnonymousBoxStyle(pseudo,
                                                                 StyleContext()->AsServo());
 
   nsChangeHint childHint =
     UpdateStyleOfOwnedChildFrame(aChildFrame, newContext, aRestyleState);
 
   // Now that we've updated the style on aChildFrame, check whether it itself
-  // has anon boxes to deal with.
+  // has anon boxes or pseudo frames to deal with.
   ServoRestyleState childrenState(
       *aChildFrame, aRestyleState, childHint, ServoRestyleState::Type::InFlow);
   aChildFrame->UpdateStyleOfOwnedAnonBoxes(childrenState);
-
-  // Assuming anon boxes don't have ::backdrop associated with them... if that
-  // ever changes, we'd need to handle that here, like we do in
-  // ServoRestyleManager::ProcessPostTraversal
-
-  // We do need to handle block pseudo-elements here, though.  Especially list
-  // bullets.
-  if (aChildFrame->IsFrameOfType(nsIFrame::eBlockFrame)) {
-    auto block = static_cast<nsBlockFrame*>(aChildFrame);
-    block->UpdatePseudoElementStyles(childrenState);
-  }
+  aChildFrame->UpdatePseudoElementStylesIfNeeded(childrenState);
 }
 
 /* static */ nsChangeHint
 nsIFrame::UpdateStyleOfOwnedChildFrame(
   nsIFrame* aChildFrame,
   nsStyleContext* aNewStyleContext,
   ServoRestyleState& aRestyleState,
   const Maybe<nsStyleContext*>& aContinuationStyleContext)
--- a/layout/generic/nsIFrame.h
+++ b/layout/generic/nsIFrame.h
@@ -3374,16 +3374,21 @@ public:
    */
   void UpdateStyleOfOwnedAnonBoxes(mozilla::ServoRestyleState& aRestyleState)
   {
     if (GetStateBits() & NS_FRAME_OWNS_ANON_BOXES) {
       DoUpdateStyleOfOwnedAnonBoxes(aRestyleState);
     }
   }
 
+  /**
+   * Updates pseudo-element styles for this frame.
+   */
+  void UpdatePseudoElementStylesIfNeeded(mozilla::ServoRestyleState&);
+
 protected:
   // This does the actual work of UpdateStyleOfOwnedAnonBoxes.  It calls
   // AppendDirectlyOwnedAnonBoxes to find all of the anonymous boxes
   // owned by this frame, and then updates styles on each of them.
   void DoUpdateStyleOfOwnedAnonBoxes(mozilla::ServoRestyleState& aRestyleState);
 
   // A helper for DoUpdateStyleOfOwnedAnonBoxes for the specific case
   // of the owned anon box being a child of this frame.