Bug 996796 patch 18 - Fix RestyleTracker to handle restyle hints exactly rather than pessimistically when restyling continuations with varying styles (e.g., spans inside ::first-line or ::first-letter). r=heycam
authorL. David Baron <dbaron@dbaron.org>
Sun, 03 Aug 2014 13:11:55 -0700
changeset 197620 2919311231ecd1bd53e18eee26bc7106b2ab806c
parent 197619 a4a8b3b58191206f53748d823cf255fba4042253
child 197621 69c78fb96b8ab6e1fc71ccff46f9dc41354bcd95
push id27249
push userryanvm@gmail.com
push dateMon, 04 Aug 2014 20:14:35 +0000
treeherdermozilla-central@7f81be7db528 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersheycam
bugs996796
milestone34.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 996796 patch 18 - Fix RestyleTracker to handle restyle hints exactly rather than pessimistically when restyling continuations with varying styles (e.g., spans inside ::first-line or ::first-letter). r=heycam This will be necessary when we use the restyle tracker for the animation-only style flush, because animation-only style flushes need to update *only* the animation style data and no other style data. Thus using the RestyleTracker for animation-only style flushes requires that we do this.
layout/base/RestyleManager.cpp
layout/base/RestyleTracker.h
--- a/layout/base/RestyleManager.cpp
+++ b/layout/base/RestyleManager.cpp
@@ -1938,17 +1938,18 @@ GetPrevContinuationWithSameStyle(nsIFram
  * Since this is used when deciding to copy the new style context, it
  * takes as an argument the old style context to check if the style is
  * the same.  When it is used in other contexts (i.e., where the next
  * continuation would already have the new style context), the current
  * style context should be passed.
  */
 static nsIFrame*
 GetNextContinuationWithSameStyle(nsIFrame* aFrame,
-                                 nsStyleContext* aOldStyleContext)
+                                 nsStyleContext* aOldStyleContext,
+                                 bool* aHaveMoreContinuations = nullptr)
 {
   // See GetPrevContinuationWithSameStyle about {ib} splits.
 
   nsIFrame *nextContinuation = aFrame->GetNextContinuation();
   if (!nextContinuation &&
       (aFrame->GetStateBits() & NS_FRAME_PART_OF_IBSPLIT)) {
     // We're the last continuation, so we have to hop back to the first
     // before getting the frame property
@@ -1968,16 +1969,19 @@ GetNextContinuationWithSameStyle(nsIFram
                "unexpected content mismatch");
 
   nsStyleContext* nextStyle = nextContinuation->StyleContext();
   if (nextStyle != aOldStyleContext) {
     NS_ASSERTION(aOldStyleContext->GetPseudo() != nextStyle->GetPseudo() ||
                  aOldStyleContext->GetParent() != nextStyle->GetParent(),
                  "continuations should have the same style context");
     nextContinuation = nullptr;
+    if (aHaveMoreContinuations) {
+      *aHaveMoreContinuations = true;
+    }
   }
   return nextContinuation;
 }
 
 nsresult
 RestyleManager::ReparentStyleContext(nsIFrame* aFrame)
 {
   if (nsGkAtoms::placeholderFrame == aFrame->GetType()) {
@@ -2330,39 +2334,51 @@ ElementRestyler::Restyle(nsRestyleHint a
                "frame must have content (unless at the top of the tree)");
 
   NS_ASSERTION(!GetPrevContinuationWithSameStyle(mFrame),
                "should not be trying to restyle this frame separately");
 
   MOZ_ASSERT(!(aRestyleHint & eRestyle_LaterSiblings),
              "eRestyle_LaterSiblings must not be part of aRestyleHint");
 
+  nsRestyleHint hintToRestore = nsRestyleHint(0);
   if (mContent && mContent->IsElement()) {
     mContent->OwnerDoc()->FlushPendingLinkUpdates();
     RestyleTracker::RestyleData restyleData;
     if (mRestyleTracker.GetRestyleData(mContent->AsElement(), &restyleData)) {
       if (NS_UpdateHint(mHintsHandled, restyleData.mChangeHint)) {
         mChangeList->AppendChange(mFrame, mContent, restyleData.mChangeHint);
       }
+      hintToRestore = restyleData.mRestyleHint;
       aRestyleHint = nsRestyleHint(aRestyleHint | restyleData.mRestyleHint);
     }
   }
 
   nsRestyleHint childRestyleHint = nsRestyleHint(aRestyleHint & eRestyle_Subtree);
 
   {
     nsRefPtr<nsStyleContext> oldContext = mFrame->StyleContext();
 
     // TEMPORARY (until bug 918064):  Call RestyleSelf for each
     // continuation or block-in-inline sibling.
 
+    bool haveMoreContinuations = false;
     for (nsIFrame* f = mFrame; f;
-         f = GetNextContinuationWithSameStyle(f, oldContext)) {
+         f = GetNextContinuationWithSameStyle(f, oldContext,
+                                              &haveMoreContinuations)) {
       RestyleSelf(f, aRestyleHint);
     }
+
+    if (haveMoreContinuations && hintToRestore) {
+      // If we have more continuations with different style (e.g., because
+      // we're inside a ::first-letter or ::first-line), put the restyle
+      // hint back.
+      mRestyleTracker.AddPendingRestyleToTable(mContent->AsElement(),
+                                               hintToRestore, nsChangeHint(0));
+    }
   }
 
   RestyleChildren(childRestyleHint);
 }
 
 void
 ElementRestyler::RestyleSelf(nsIFrame* aSelf, nsRestyleHint aRestyleHint)
 {
@@ -2450,27 +2466,17 @@ ElementRestyler::RestyleSelf(nsIFrame* a
     // continuation.
     newContext = prevContinuationContext;
   }
   else if (pseudoTag == nsCSSAnonBoxes::mozNonElement) {
     NS_ASSERTION(aSelf->GetContent(),
                  "non pseudo-element frame without content node");
     newContext = styleSet->ResolveStyleForNonElement(parentContext);
   }
-  else if (!(aRestyleHint & (eRestyle_Self | eRestyle_Subtree)) &&
-           !prevContinuation) {
-    // Unfortunately, if prevContinuation is non-null then we may have
-    // already stolen the restyle tracker entry for this element while
-    // processing prevContinuation.  So we don't know whether aRestyleHint
-    // should really be 0 here or whether it should be eRestyle_Self.  Be
-    // pessimistic and force an actual reresolve in that situation.  The good
-    // news is that in the common case when prevContinuation is non-null we
-    // just used prevContinuationContext anyway and aren't reaching this code
-    // to start with.
-
+  else if (!(aRestyleHint & (eRestyle_Self | eRestyle_Subtree))) {
     Element* element = ElementForStyleContext(mParentContent, aSelf, pseudoType);
     if (aRestyleHint == nsRestyleHint(0)) {
       newContext =
         styleSet->ReparentStyleContext(oldContext, parentContext, element);
     } else {
       newContext =
         styleSet->ResolveStyleWithReplacement(element, parentContext, oldContext,
                                               aRestyleHint);
--- a/layout/base/RestyleTracker.h
+++ b/layout/base/RestyleTracker.h
@@ -14,16 +14,17 @@
 #include "mozilla/dom/Element.h"
 #include "nsDataHashtable.h"
 #include "nsContainerFrame.h"
 #include "mozilla/SplayTree.h"
 
 namespace mozilla {
 
 class RestyleManager;
+class ElementRestyler;
 
 /** 
  * Helper class that collects a list of frames that need
  * UpdateOverflow() called on them, and coalesces them
  * to avoid walking up the same ancestor tree multiple times.
  */
 class OverflowChangedTracker
 {
@@ -225,16 +226,18 @@ private:
   /* Don't update overflow of this frame or its ancestors. */
   const nsIFrame* mSubtreeRoot;
 };
 
 class RestyleTracker {
 public:
   typedef mozilla::dom::Element Element;
 
+  friend class ElementRestyler; // for AddPendingRestyleToTable
+
   RestyleTracker(Element::FlagsType aRestyleBits) :
     mRestyleBits(aRestyleBits),
     mHaveLaterSiblingRestyles(false)
   {
     NS_PRECONDITION((mRestyleBits & ~ELEMENT_ALL_RESTYLE_FLAGS) == 0,
                     "Why do we have these bits set?");
     NS_PRECONDITION((mRestyleBits & ELEMENT_PENDING_RESTYLE_FLAGS) != 0,
                     "Must have a restyle flag");
@@ -256,17 +259,17 @@ public:
     return mPendingRestyles.Count();
   }
 
   /**
    * Add a restyle for the given element to the tracker.  Returns true
    * if the element already had eRestyle_LaterSiblings set on it.
    */
   bool AddPendingRestyle(Element* aElement, nsRestyleHint aRestyleHint,
-                           nsChangeHint aMinChangeHint);
+                         nsChangeHint aMinChangeHint);
 
   /**
    * Process the restyles we've been tracking.
    */
   void ProcessRestyles() {
     // Fast-path the common case (esp. for the animation restyle
     // tracker) of not having anything to do.
     if (mPendingRestyles.Count()) {
@@ -307,16 +310,19 @@ public:
    */
   inline nsIDocument* Document() const;
 
   struct RestyleEnumerateData : public RestyleData {
     nsRefPtr<Element> mElement;
   };
 
 private:
+  bool AddPendingRestyleToTable(Element* aElement, nsRestyleHint aRestyleHint,
+                                nsChangeHint aMinChangeHint);
+
   /**
    * Handle a single mPendingRestyles entry.  aRestyleHint must not
    * include eRestyle_LaterSiblings; that needs to be dealt with
    * before calling this function.
    */
   inline void ProcessOneRestyle(Element* aElement,
                                 nsRestyleHint aRestyleHint,
                                 nsChangeHint aChangeHint);
@@ -347,19 +353,20 @@ private:
   // array.
   RestyleRootArray mRestyleRoots;
   // True if we have some entries with the eRestyle_LaterSiblings
   // flag.  We need this to avoid enumerating the hashtable looking
   // for such entries when we can't possibly have any.
   bool mHaveLaterSiblingRestyles;
 };
 
-inline bool RestyleTracker::AddPendingRestyle(Element* aElement,
-                                                nsRestyleHint aRestyleHint,
-                                                nsChangeHint aMinChangeHint)
+inline bool
+RestyleTracker::AddPendingRestyleToTable(Element* aElement,
+                                         nsRestyleHint aRestyleHint,
+                                         nsChangeHint aMinChangeHint)
 {
   RestyleData existingData;
   existingData.mRestyleHint = nsRestyleHint(0);
   existingData.mChangeHint = NS_STYLE_HINT_NONE;
 
   // Check the RestyleBit() flag before doing the hashtable Get, since
   // it's possible that the data in the hashtable isn't actually
   // relevant anymore (if the flag is not set).
@@ -372,16 +379,27 @@ inline bool RestyleTracker::AddPendingRe
   bool hadRestyleLaterSiblings =
     (existingData.mRestyleHint & eRestyle_LaterSiblings) != 0;
   existingData.mRestyleHint =
     nsRestyleHint(existingData.mRestyleHint | aRestyleHint);
   NS_UpdateHint(existingData.mChangeHint, aMinChangeHint);
 
   mPendingRestyles.Put(aElement, existingData);
 
+  return hadRestyleLaterSiblings;
+}
+
+inline bool
+RestyleTracker::AddPendingRestyle(Element* aElement,
+                                  nsRestyleHint aRestyleHint,
+                                  nsChangeHint aMinChangeHint)
+{
+  bool hadRestyleLaterSiblings =
+    AddPendingRestyleToTable(aElement, aRestyleHint, aMinChangeHint);
+
   // We can only treat this element as a restyle root if we would
   // actually restyle its descendants (so either call
   // ReResolveStyleContext on it or just reframe it).
   if ((aRestyleHint & ~eRestyle_LaterSiblings) ||
       (aMinChangeHint & nsChangeHint_ReconstructFrame)) {
     for (const Element* cur = aElement; !cur->HasFlag(RootBit()); ) {
       nsIContent* parent = cur->GetFlattenedTreeParent();
       // Stop if we have no parent or the parent is not an element or