Bug 1324619 part 3. Implement ReparentStyleContext in ServoRestyleManager, for ::first-line use. r?emilio draft
authorBoris Zbarsky <bzbarsky@mit.edu>
Fri, 28 Jul 2017 21:11:18 -0400
changeset 617968 4f82a38e9a851fafa38b6f214236aad1fe4fbb3c
parent 617967 2d179abdd7d94101cec02652f710106a142a6563
child 617969 4a988cbca13b3500b7aa05899fab9b671cdf785c
push id71168
push userbzbarsky@mozilla.com
push dateSat, 29 Jul 2017 01:21:16 +0000
reviewersemilio
bugs1324619
milestone56.0a1
Bug 1324619 part 3. Implement ReparentStyleContext in ServoRestyleManager, for ::first-line use. r?emilio This doesn't actually implement style context reparenting in the style set yet; that part is next. There is one behavior difference being introduced here compared to Gecko: we don't reparent the first block piece of an {ib} (block-inside-inline) split whose first inline piece is being reparented. This is actually a correctness fix. In this testcase: <style> #target { color: green; } #target::first-line { color: red; } </style> <div id="target"> <span> <div>This should be green</div> </span> </div> Gecko makes the text red, while every other browser makes it green. We're preserving Gecko's behavior for out-of-flows in first-line so far, but arguably it's wrong per spec and doesn't match other browsers either. We can look into changing it later. MozReview-Commit-ID: 5eC6G449Mlh
layout/base/ServoRestyleManager.cpp
layout/base/ServoRestyleManager.h
layout/reftests/first-line/first-line-in-columnset-1-ref.html
layout/reftests/first-line/first-line-in-columnset-1.html
layout/reftests/first-line/ib-split-1-ref.html
layout/reftests/first-line/ib-split-1.html
layout/reftests/first-line/reftest.list
layout/style/ServoStyleSet.cpp
layout/style/ServoStyleSet.h
--- a/layout/base/ServoRestyleManager.cpp
+++ b/layout/base/ServoRestyleManager.cpp
@@ -84,17 +84,20 @@ ExpectedOwnerForChild(const nsIFrame& aF
   return parent;
 }
 
 void
 ServoRestyleState::AssertOwner(const ServoRestyleState& aParent) const
 {
   MOZ_ASSERT(mOwner);
   MOZ_ASSERT(!mOwner->HasAnyStateBits(NS_FRAME_OUT_OF_FLOW));
-  MOZ_ASSERT(ExpectedOwnerForChild(*mOwner) == aParent.mOwner);
+  // We allow aParent.mOwner to be null, for cases when we're not starting at
+  // the root of the tree.
+  MOZ_ASSERT_IF(aParent.mOwner,
+                ExpectedOwnerForChild(*mOwner) == aParent.mOwner);
 }
 
 nsChangeHint
 ServoRestyleState::ChangesHandledFor(const nsIFrame& aFrame) const
 {
   if (!mOwner) {
     MOZ_ASSERT(!mChangesHandled);
     return mChangesHandled;
@@ -1177,13 +1180,202 @@ ServoRestyleManager::AttributeChanged(El
     // undisplayed elements, since we don't know if it is needed.
     IncrementUndisplayedRestyleGeneration();
   }
 }
 
 nsresult
 ServoRestyleManager::ReparentStyleContext(nsIFrame* aFrame)
 {
-  NS_WARNING("stylo: ServoRestyleManager::ReparentStyleContext not implemented");
+  // This is only called when moving frames in or out of the first-line
+  // pseudo-element (or one of its inline descendants).  So aFrame's ancestors
+  // must all be inline frames up until we find a first-line frame.  Note that
+  // the first-line frame may not actually be the one that corresponds to
+  // ::first-line; when we're moving _out_ of the first-line it will be one of
+  // the continuations instead.
+#ifdef DEBUG
+  {
+    nsIFrame* f = aFrame->GetParent();
+    while (f && !f->IsLineFrame()) {
+      MOZ_ASSERT(f->IsInlineFrame(),
+                 "Must only have inline frames between us and the first-line "
+                 "frame");
+      f = f->GetParent();
+    }
+    MOZ_ASSERT(f, "Must have found a first-line frame");
+  }
+#endif
+
+  DoReparentStyleContext(aFrame, *StyleSet());
+
   return NS_OK;
 }
 
+void
+ServoRestyleManager::DoReparentStyleContext(nsIFrame* aFrame,
+                                            ServoStyleSet& aStyleSet)
+{
+  if (aFrame->IsBackdropFrame()) {
+    // Style context of backdrop frame has no parent style context, and
+    // thus we do not need to reparent it.
+    return;
+  }
+
+  if (aFrame->IsPlaceholderFrame()) {
+    // Also reparent the out-of-flow and all its continuations.  We're doing
+    // this to match Gecko for now, but it's not clear that this behavior is
+    // correct per spec.  It's certainly pretty odd for out-of-flows whose
+    // containing block is not within the first line.
+    //
+    // Right now we're somewhat inconsistent in this testcase:
+    //
+    //  <style>
+    //    div { color: orange; clear: left; }
+    //    div::first-line { color: blue; }
+    //  </style>
+    //  <div>
+    //    <span style="float: left">What color is this text?</span>
+    //  </div>
+    //  <div>
+    //    <span><span style="float: left">What color is this text?</span></span>
+    //  </div>
+    //
+    // We make the first float orange and the second float blue.  On the other
+    // hand, if the float were within an inline-block that was on the first
+    // line, arguably it _should_ inherit from the ::first-line...
+    nsIFrame* outOfFlow =
+      nsPlaceholderFrame::GetRealFrameForPlaceholder(aFrame);
+    MOZ_ASSERT(outOfFlow, "no out-of-flow frame");
+    for (; outOfFlow; outOfFlow = outOfFlow->GetNextContinuation()) {
+      DoReparentStyleContext(outOfFlow, aStyleSet);
+    }
+  }
+
+  nsIFrame* providerFrame;
+  nsStyleContext* newParentContext =
+    aFrame->GetParentStyleContext(&providerFrame);
+  if (!newParentContext) {
+    // No need to do anything here.
+#ifdef DEBUG
+    // Make sure we have no children, so we really know there is nothing to do.
+    nsIFrame::ChildListIterator lists(aFrame);
+    for (; !lists.IsDone(); lists.Next()) {
+      MOZ_ASSERT(lists.CurrentList().IsEmpty(),
+                 "Failing to reparent style context for child of "
+                 "non-inheriting anon box");
+    }
+#endif // DEBUG
+    return;
+  }
+
+  // If our provider is our child, we want to reparent it first, because we
+  // inherit style from it.
+  bool isChild = providerFrame && providerFrame->GetParent() == aFrame;
+  nsIFrame* providerChild = nullptr;
+  if (isChild) {
+    DoReparentStyleContext(providerFrame, aStyleSet);
+    // Get the style context again after ReparentStyleContext() which might have
+    // changed it.
+    newParentContext = providerFrame->StyleContext();
+    providerChild = providerFrame;
+    MOZ_ASSERT(!providerFrame->HasAnyStateBits(NS_FRAME_OUT_OF_FLOW),
+               "Out of flow provider?");
+  }
+
+  bool isElement = aFrame->GetContent()->IsElement();
+
+  // We probably don't want to initiate transitions from
+  // ReparentStyleContext, since we call it during frame
+  // construction rather than in response to dynamic changes.
+  // Also see the comment at the start of
+  // nsTransitionManager::ConsiderInitiatingTransition.
+  //
+  // We don't try to do the fancy copying from previous continuations that
+  // GeckoRestyleManager does here, because that relies on knowing the parents
+  // of style contexts, and we don't know those.
+  ServoStyleContext* oldContext = aFrame->StyleContext()->AsServo();
+  Element* ourElement =
+    oldContext->GetPseudoType() == CSSPseudoElementType::NotPseudo &&
+    isElement ?
+      aFrame->GetContent()->AsElement() :
+      nullptr;
+  ServoStyleContext* newParent = newParentContext->AsServo();
+
+  ServoStyleContext* newParentIgnoringFirstLine;
+  if (newParent->GetPseudoType() == CSSPseudoElementType::firstLine) {
+    MOZ_ASSERT(providerFrame && providerFrame->GetParent()->
+               IsFrameOfType(nsIFrame::eBlockFrame),
+               "How could we get a ::first-line parent style without having "
+               "a ::first-line provider frame?");
+    // If newParent is a ::first-line style, get the parent blockframe, and then
+    // correct it for our pseudo as needed (e.g. stepping out of anon boxes).
+    // Use the resulting style for the "parent style ignoring ::first-line".
+    nsIFrame* blockFrame = providerFrame->GetParent();
+    nsIFrame* correctedFrame =
+      nsFrame::CorrectStyleParentFrame(blockFrame, oldContext->GetPseudo());
+    newParentIgnoringFirstLine = correctedFrame->StyleContext()->AsServo();
+  } else {
+    newParentIgnoringFirstLine = newParent;
+  }
+
+  if (!providerFrame) {
+    // No providerFrame means we inherited from a display:contents thing.  Our
+    // layout parent style is the style of our nearest ancestor frame.
+    providerFrame = nsFrame::CorrectStyleParentFrame(aFrame->GetParent(),
+                                                     oldContext->GetPseudo());
+  }
+  ServoStyleContext* layoutParent = providerFrame->StyleContext()->AsServo();
+
+  RefPtr<ServoStyleContext> newContext =
+    aStyleSet.ReparentStyleContext(oldContext,
+                                   newParent,
+                                   newParentIgnoringFirstLine,
+                                   layoutParent,
+                                   ourElement);
+  aFrame->SetStyleContext(newContext);
+
+  // This logic somewhat mirrors the logic in
+  // ServoRestyleManager::ProcessPostTraversal.
+  if (isElement) {
+    // We can't use UpdateAdditionalStyleContexts as-is because it needs a
+    // ServoRestyleState and maintaining one of those during a _frametree_
+    // traversal is basically impossible.
+    uint32_t index = 0;
+    while (nsStyleContext* oldAdditionalContext =
+             aFrame->GetAdditionalStyleContext(index)) {
+      RefPtr<ServoStyleContext> newAdditionalContext =
+        aStyleSet.ReparentStyleContext(oldAdditionalContext->AsServo(),
+                                       newContext,
+                                       newContext,
+                                       newContext,
+                                       nullptr);
+      aFrame->SetAdditionalStyleContext(index, newAdditionalContext);
+      ++index;
+    }
+  }
+
+  // Generally, owned anon boxes are our descendants.  The only exceptions are
+  // tables (for the table wrapper) and inline frames (for the block part of the
+  // block-in-inline split).  We're going to update our descendants when looping
+  // over kids, and we don't want to update the block part of a block-in-inline
+  // split if the inline is on the first line but the block is not (and if the
+  // block is, it's the child of something else on the first line and will get
+  // updated as a child).  And given how this method ends up getting called, if
+  // we reach here for a table frame, we are already in the middle of
+  // reparenting the table wrapper frame.  So no need to
+  // UpdateStyleOfOwnedAnonBoxes() here.
+
+  nsIFrame::ChildListIterator lists(aFrame);
+  for (; !lists.IsDone(); lists.Next()) {
+    for (nsIFrame* child : lists.CurrentList()) {
+      // only do frames that are in flow
+      if (!(child->GetStateBits() & NS_FRAME_OUT_OF_FLOW) &&
+          child != providerChild) {
+        DoReparentStyleContext(child, aStyleSet);
+      }
+    }
+  }
+
+  // We do not need to do the equivalent of UpdateFramePseudoElementStyles,
+  // because those are hadled by our descendant walk.
+}
+
 } // namespace mozilla
--- a/layout/base/ServoRestyleManager.h
+++ b/layout/base/ServoRestyleManager.h
@@ -145,16 +145,19 @@ public:
                            int32_t aModType,
                            const nsAttrValue* aNewValue);
   void ClassAttributeWillBeChangedBySMIL(dom::Element* aElement);
 
   void AttributeChanged(dom::Element* aElement, int32_t aNameSpaceID,
                         nsIAtom* aAttribute, int32_t aModType,
                         const nsAttrValue* aOldValue);
 
+  // This is only used to reparent things when moving them in/out of the
+  // ::first-line.  Once we get rid of the Gecko style system, we should rename
+  // this method accordingly (e.g. to ReparentStyleContextForFirstLine).
   nsresult ReparentStyleContext(nsIFrame* aFrame);
 
   /**
    * Gets the appropriate frame given a content and a pseudo-element tag.
    *
    * Right now only supports a null tag, before or after. If the pseudo-element
    * is not null, the content needs to be an element.
    */
@@ -222,16 +225,21 @@ private:
   void ClearSnapshots();
   ServoElementSnapshot& SnapshotFor(mozilla::dom::Element* aElement);
   void TakeSnapshotForAttributeChange(mozilla::dom::Element* aElement,
                                       int32_t aNameSpaceID,
                                       nsIAtom* aAttribute);
 
   void DoProcessPendingRestyles(ServoTraversalFlags aFlags);
 
+  // Function to do the actual (recursive) work of ReparentStyleContext, once we
+  // have asserted the invariants that only hold on the initial call.
+  void DoReparentStyleContext(nsIFrame* aFrame,
+                              ServoStyleSet& aStyleSet);
+
   // We use a separate data structure from nsStyleChangeList because we need a
   // frame to create nsStyleChangeList entries, and the primary frame may not be
   // attached yet.
   struct ReentrantChange {
     nsCOMPtr<nsIContent> mContent;
     nsChangeHint mHint;
   };
   typedef AutoTArray<ReentrantChange, 10> ReentrantChangeList;
new file mode 100644
--- /dev/null
+++ b/layout/reftests/first-line/first-line-in-columnset-1-ref.html
@@ -0,0 +1,8 @@
+<!DOCTYPE html>
+<style>
+  div { color: red; border: 10px solid green; column-count: 2 }
+  div::first-line { color: green; border: 10px solid red; }
+</style>
+<div>
+  <span style="border: 10px solid green">Some text</span>
+</div>
new file mode 100644
--- /dev/null
+++ b/layout/reftests/first-line/first-line-in-columnset-1.html
@@ -0,0 +1,8 @@
+<!DOCTYPE html>
+<style>
+  div { color: red; border: 10px solid green; column-count: 2 }
+  div::first-line { color: green; border: 10px solid red; }
+</style>
+<div>
+  <span style="border: inherit">Some text</span>
+</div>
new file mode 100644
--- /dev/null
+++ b/layout/reftests/first-line/ib-split-1-ref.html
@@ -0,0 +1,9 @@
+<!DOCTYPE html>
+<style>
+  #target { color: green; }
+</style>
+<div id="target">
+  <span>
+    <div>This should be green</div>
+  </span>
+</div>
new file mode 100644
--- /dev/null
+++ b/layout/reftests/first-line/ib-split-1.html
@@ -0,0 +1,10 @@
+<!DOCTYPE html>
+<style>
+  #target { color: green; }
+  #target::first-line { color: red; }
+</style>
+<div id="target">
+  <span>
+    <div>This should be green</div>
+  </span>
+</div>
--- a/layout/reftests/first-line/reftest.list
+++ b/layout/reftests/first-line/reftest.list
@@ -30,8 +30,12 @@ fails-if(styloVsGecko||stylo) == 287088-
 fails-if(styloVsGecko||stylo) == 287088-2.html 287088-2-ref.html
 fails-if(styloVsGecko) == 403177-1.html 403177-1-ref.html
 == 469227-2.html 469227-2-ref.html
 == 469227-3.html 469227-3-ref.html
 
 fails-if(styloVsGecko||stylo) == restyle-inside-first-line.html restyle-inside-first-line-ref.html
 fails-if(styloVsGecko||stylo) == font-styles.html font-styles-ref.html
 fuzzy-if(OSX==1010,1,2) fails-if(styloVsGecko||stylo) == font-styles-nooverflow.html font-styles-ref.html
+
+fails-if(!stylo) == ib-split-1.html ib-split-1-ref.html
+
+== first-line-in-columnset-1.html first-line-in-columnset-1-ref.html
--- a/layout/style/ServoStyleSet.cpp
+++ b/layout/style/ServoStyleSet.cpp
@@ -1285,9 +1285,22 @@ ServoStyleSet::MightHaveAttributeDepende
 bool
 ServoStyleSet::HasStateDependency(const Element& aElement,
                                   EventStates aState) const
 {
   return Servo_StyleSet_HasStateDependency(
       mRawSet.get(), &aElement, aState.ServoValue());
 }
 
+already_AddRefed<ServoStyleContext>
+ServoStyleSet::ReparentStyleContext(ServoStyleContext* aStyleContext,
+                                    ServoStyleContext* aNewParent,
+                                    ServoStyleContext* aNewParentIgnoringFirstLine,
+                                    ServoStyleContext* aNewLayoutParent,
+                                    Element* aElement)
+{
+  // For now just return aStyleContext; actually doing something here is coming
+  // up next.
+  RefPtr<ServoStyleContext> ctx = aStyleContext;
+  return ctx.forget();
+}
+
 ServoStyleSet* ServoStyleSet::sInServoTraversal = nullptr;
--- a/layout/style/ServoStyleSet.h
+++ b/layout/style/ServoStyleSet.h
@@ -449,16 +449,31 @@ public:
    *
    * This function allows us to skip taking a state snapshot when
    * the changed state isn't depended upon by any pseudo-class selectors
    * in a style sheet.
    */
   bool HasStateDependency(const dom::Element& aElement,
                           EventStates aState) const;
 
+  /**
+   * Get a new style context that uses the same rules as the given style context
+   * but has a different parent.
+   *
+   * aElement is non-null if this is a style context for a frame whose mContent
+   * is an element and which has no pseudo on its style context (so it's the
+   * actual style for the element being passed).
+   */
+  already_AddRefed<ServoStyleContext>
+  ReparentStyleContext(ServoStyleContext* aStyleContext,
+                       ServoStyleContext* aNewParent,
+                       ServoStyleContext* aNewParentIgnoringFirstLine,
+                       ServoStyleContext* aNewLayoutParent,
+                       Element* aElement);
+
 private:
   // On construction, sets sInServoTraversal to the given ServoStyleSet.
   // On destruction, clears sInServoTraversal and calls RunPostTraversalTasks.
   class MOZ_STACK_CLASS AutoSetInServoTraversal
   {
   public:
     explicit AutoSetInServoTraversal(ServoStyleSet* aSet)
       : mSet(aSet)