Bug 1324619 part 3. Implement ReparentStyleContext in ServoRestyleManager, for ::first-line use. r=emilio
authorBoris Zbarsky <bzbarsky@mit.edu>
Fri, 28 Jul 2017 21:11:18 -0400
changeset 420509 d5d6e006b251e3a32688a9f41c9439782ccd9cc3
parent 420508 b1aa311b6b42be5eb1ec516c637bc2fd0e177778
child 420510 e1b802894f33d9cfa36dfeb4dd7aa1b769e76f89
push id7566
push usermtabara@mozilla.com
push dateWed, 02 Aug 2017 08:25:16 +0000
treeherdermozilla-beta@86913f512c3c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersemilio
bugs1324619
milestone56.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 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)