Bug 1292279 - Explicitly style new children in Content{Appended/Inserted} rather than using restyle hints. r=heycam
authorBobby Holley <bobbyholley@gmail.com>
Thu, 11 Aug 2016 11:48:27 -0700
changeset 311288 543b6fffb61b1a84bb5aa3480c9899c3b1eaf0e6
parent 311287 bf5c0fb68628289ccab786054925f7eb5fab2802
child 311289 721abe87ead229cd812d108e5e14fe2454b4eaf7
push id81091
push userbholley@mozilla.com
push dateFri, 26 Aug 2016 04:37:35 +0000
treeherdermozilla-inbound@45d015eed45f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersheycam
bugs1292279
milestone51.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 1292279 - Explicitly style new children in Content{Appended/Inserted} rather than using restyle hints. r=heycam This gives us more control over what gets restyled when.
layout/base/RestyleManager.cpp
layout/base/RestyleManager.h
layout/base/RestyleManagerHandle.h
layout/base/RestyleManagerHandleInlines.h
layout/base/ServoRestyleManager.cpp
layout/base/ServoRestyleManager.h
layout/base/nsPresShell.cpp
--- a/layout/base/RestyleManager.cpp
+++ b/layout/base/RestyleManager.cpp
@@ -563,19 +563,18 @@ RestyleManager::RestyleForInsertOrChange
         }
         break;
       }
     }
   }
 }
 
 void
-RestyleManager::RestyleForRemove(Element* aContainer,
-                                 nsIContent* aOldChild,
-                                 nsIContent* aFollowingSibling)
+RestyleManager::ContentRemoved(Element* aContainer, nsIContent* aOldChild,
+                               nsIContent* aFollowingSibling)
 {
   if (aOldChild->IsRootOfAnonymousSubtree()) {
     // This should be an assert, but this is called incorrectly in
     // HTMLEditor::DeleteRefToAnonymousNode and the assertions were clogging
     // up the logs.  Make it an assert again when that's fixed.
     MOZ_ASSERT(aOldChild->GetProperty(nsGkAtoms::restylableAnonymousNode),
                "anonymous nodes should not be in child lists (bug 439258)");
   }
--- a/layout/base/RestyleManager.h
+++ b/layout/base/RestyleManager.h
@@ -276,31 +276,45 @@ public:
   AnimationsWithDestroyedFrame* GetAnimationsWithDestroyedFrame() {
     return mAnimationsWithDestroyedFrame;
   }
 
 private:
   void RestyleForEmptyChange(Element* aContainer);
 
 public:
+  // Handle ContentInserted notifications.
+  void ContentInserted(Element* aContainer, nsIContent* aChild)
+  {
+    RestyleForInsertOrChange(aContainer, aChild);
+  }
+
+  // Handle ContentAppended notifications.
+  void ContentAppended(Element* aContainer, nsIContent* aFirstNewContent)
+  {
+    RestyleForAppend(aContainer, aFirstNewContent);
+  }
+
+  // Handle ContentRemoved notifications.
+  //
+  // This would be have the same logic as RestyleForInsertOrChange if we got the
+  // notification before the removal.  However, we get it after, so we need the
+  // following sibling in addition to the old child.  |aContainer| must be
+  // non-null; when the container is null, no work is needed.  aFollowingSibling
+  // is the sibling that used to come after aOldChild before the removal.
+  void ContentRemoved(Element* aContainer, nsIContent* aOldChild,
+                      nsIContent* aFollowingSibling);
+
   // Restyling for a ContentInserted (notification after insertion) or
   // for a CharacterDataChanged.  |aContainer| must be non-null; when
   // the container is null, no work is needed.
   void RestyleForInsertOrChange(Element* aContainer, nsIContent* aChild);
 
-  // This would be the same as RestyleForInsertOrChange if we got the
-  // notification before the removal.  However, we get it after, so we need the
-  // following sibling in addition to the old child.  |aContainer| must be
-  // non-null; when the container is null, no work is needed.  aFollowingSibling
-  // is the sibling that used to come after aOldChild before the removal.
-  void RestyleForRemove(Element* aContainer,
-                        nsIContent* aOldChild,
-                        nsIContent* aFollowingSibling);
-
-  // Same for a ContentAppended.  |aContainer| must be non-null; when
+  // Restyling for a ContentAppended (notification after insertion) or
+  // for a CharacterDataChanged.  |aContainer| must be non-null; when
   // the container is null, no work is needed.
   void RestyleForAppend(Element* aContainer, nsIContent* aFirstNewContent);
 
   // Process any pending restyles. This should be called after
   // CreateNeededFrames.
   // Note: It's the caller's responsibility to make sure to wrap a
   // ProcessPendingRestyles call in a view update batch and a script blocker.
   // This function does not call ProcessAttachedQueue() on the binding manager.
--- a/layout/base/RestyleManagerHandle.h
+++ b/layout/base/RestyleManagerHandle.h
@@ -109,23 +109,27 @@ public:
                                  nsRestyleHint aRestyleHint,
                                  nsChangeHint aMinChangeHint);
     inline void PostRestyleEventForLazyConstruction();
     inline void RebuildAllStyleData(nsChangeHint aExtraHint,
                                     nsRestyleHint aRestyleHint);
     inline void PostRebuildAllStyleDataEvent(nsChangeHint aExtraHint,
                                              nsRestyleHint aRestyleHint);
     inline void ProcessPendingRestyles();
+    inline void ContentInserted(dom::Element* aContainer,
+                                nsIContent* aChild);
+    inline void ContentAppended(dom::Element* aContainer,
+                                nsIContent* aFirstNewContent);
+    inline void ContentRemoved(dom::Element* aContainer,
+                               nsIContent* aOldChild,
+                               nsIContent* aFollowingSibling);
     inline void RestyleForInsertOrChange(dom::Element* aContainer,
                                          nsIContent* aChild);
     inline void RestyleForAppend(dom::Element* aContainer,
                                  nsIContent* aFirstNewContent);
-    inline void RestyleForRemove(dom::Element* aContainer,
-                                 nsIContent* aOldChild,
-                                 nsIContent* aFollowingSibling);
     inline nsresult ContentStateChanged(nsIContent* aContent,
                                         EventStates aStateMask);
     inline void AttributeWillChange(dom::Element* aElement,
                                     int32_t aNameSpaceID,
                                     nsIAtom* aAttribute,
                                     int32_t aModType,
                                     const nsAttrValue* aNewValue);
     inline void AttributeChanged(dom::Element* aElement,
--- a/layout/base/RestyleManagerHandleInlines.h
+++ b/layout/base/RestyleManagerHandleInlines.h
@@ -82,36 +82,51 @@ RestyleManagerHandle::Ptr::ProcessRestyl
 
 void
 RestyleManagerHandle::Ptr::FlushOverflowChangedTracker()
 {
   FORWARD(FlushOverflowChangedTracker, ());
 }
 
 void
+RestyleManagerHandle::Ptr::ContentInserted(dom::Element* aContainer,
+                                           nsIContent* aChild)
+{
+  FORWARD(ContentInserted, (aContainer, aChild));
+}
+
+void
+RestyleManagerHandle::Ptr::ContentAppended(dom::Element* aContainer,
+                                           nsIContent* aFirstNewContent)
+{
+  FORWARD(ContentAppended, (aContainer, aFirstNewContent));
+}
+
+void
+RestyleManagerHandle::Ptr::ContentRemoved(dom::Element* aContainer,
+                                            nsIContent* aOldChild,
+                                            nsIContent* aFollowingSibling)
+{
+  FORWARD(ContentRemoved, (aContainer, aOldChild, aFollowingSibling));
+}
+
+void
 RestyleManagerHandle::Ptr::RestyleForInsertOrChange(dom::Element* aContainer,
                                                     nsIContent* aChild)
 {
   FORWARD(RestyleForInsertOrChange, (aContainer, aChild));
 }
 
 void
 RestyleManagerHandle::Ptr::RestyleForAppend(dom::Element* aContainer,
                                             nsIContent* aFirstNewContent)
 {
   FORWARD(RestyleForAppend, (aContainer, aFirstNewContent));
 }
 
-void
-RestyleManagerHandle::Ptr::RestyleForRemove(dom::Element* aContainer,
-                                            nsIContent* aOldChild,
-                                            nsIContent* aFollowingSibling)
-{
-  FORWARD(RestyleForRemove, (aContainer, aOldChild, aFollowingSibling));
-}
 
 nsresult
 RestyleManagerHandle::Ptr::ContentStateChanged(nsIContent* aContent,
                                           EventStates aStateMask)
 {
   FORWARD(ContentStateChanged, (aContent, aStateMask));
 }
 
--- a/layout/base/ServoRestyleManager.cpp
+++ b/layout/base/ServoRestyleManager.cpp
@@ -336,34 +336,84 @@ ServoRestyleManager::ProcessPendingResty
 
   IncrementRestyleGeneration();
 }
 
 void
 ServoRestyleManager::RestyleForInsertOrChange(Element* aContainer,
                                               nsIContent* aChild)
 {
-  // XXX Emilio we can do way better.
-  PostRestyleEvent(aContainer, eRestyle_Subtree, nsChangeHint(0));
+  //
+  // XXXbholley: We need the Gecko logic here to correctly restyle for things
+  // like :empty and positional selectors (though we may not need to post
+  // restyle events as agressively as the Gecko path does).
+  //
+  // Bug 1297899 tracks this work.
+  //
+}
+
+void
+ServoRestyleManager::ContentInserted(Element* aContainer, nsIContent* aChild)
+{
+  if (!aContainer->ServoData().get()) {
+    // This can happen with display:none. Bug 1297249 tracks more investigation
+    // and assertions here.
+    return;
+  }
+
+  // Style the new subtree because we will most likely need it during subsequent
+  // frame construction. Bug 1298281 tracks deferring this work in the lazy
+  // frame construction case.
+  StyleSet()->StyleNewSubtree(aChild);
+
+  RestyleForInsertOrChange(aContainer, aChild);
 }
 
 void
 ServoRestyleManager::RestyleForAppend(Element* aContainer,
                                       nsIContent* aFirstNewContent)
 {
-  // XXX Emilio we can do way better.
-  PostRestyleEvent(aContainer, eRestyle_Subtree, nsChangeHint(0));
+  //
+  // XXXbholley: We need the Gecko logic here to correctly restyle for things
+  // like :empty and positional selectors (though we may not need to post
+  // restyle events as agressively as the Gecko path does).
+  //
+  // Bug 1297899 tracks this work.
+  //
 }
 
 void
-ServoRestyleManager::RestyleForRemove(Element* aContainer,
-                                      nsIContent* aOldChild,
-                                      nsIContent* aFollowingSibling)
+ServoRestyleManager::ContentAppended(Element* aContainer,
+                                     nsIContent* aFirstNewContent)
 {
-  NS_WARNING("stylo: ServoRestyleManager::RestyleForRemove not implemented");
+  if (!aContainer->ServoData().get()) {
+    // This can happen with display:none. Bug 1297249 tracks more investigation
+    // and assertions here.
+    return;
+  }
+
+  // Style the new subtree because we will most likely need it during subsequent
+  // frame construction. Bug 1298281 tracks deferring this work in the lazy
+  // frame construction case.
+  if (aFirstNewContent->GetNextSibling()) {
+    aContainer->SetHasDirtyDescendantsForServo();
+    StyleSet()->StyleNewChildren(aContainer);
+  } else {
+    StyleSet()->StyleNewSubtree(aFirstNewContent);
+  }
+
+  RestyleForAppend(aContainer, aFirstNewContent);
+}
+
+void
+ServoRestyleManager::ContentRemoved(Element* aContainer,
+                                    nsIContent* aOldChild,
+                                    nsIContent* aFollowingSibling)
+{
+  NS_WARNING("stylo: ServoRestyleManager::ContentRemoved not implemented");
 }
 
 nsresult
 ServoRestyleManager::ContentStateChanged(nsIContent* aContent,
                                          EventStates aChangedBits)
 {
   if (!aContent->IsElement()) {
     return NS_OK;
--- a/layout/base/ServoRestyleManager.h
+++ b/layout/base/ServoRestyleManager.h
@@ -46,23 +46,29 @@ public:
                         nsRestyleHint aRestyleHint,
                         nsChangeHint aMinChangeHint);
   void PostRestyleEventForLazyConstruction();
   void RebuildAllStyleData(nsChangeHint aExtraHint,
                            nsRestyleHint aRestyleHint);
   void PostRebuildAllStyleDataEvent(nsChangeHint aExtraHint,
                                     nsRestyleHint aRestyleHint);
   void ProcessPendingRestyles();
+
+  void ContentInserted(dom::Element* aContainer, nsIContent* aChild);
+  void ContentAppended(dom::Element* aContainer,
+                       nsIContent* aFirstNewContent);
+  void ContentRemoved(dom::Element* aContainer,
+                      nsIContent* aOldChild,
+                      nsIContent* aFollowingSibling);
+
   void RestyleForInsertOrChange(dom::Element* aContainer,
                                 nsIContent* aChild);
   void RestyleForAppend(dom::Element* aContainer,
                         nsIContent* aFirstNewContent);
-  void RestyleForRemove(dom::Element* aContainer,
-                        nsIContent* aOldChild,
-                        nsIContent* aFollowingSibling);
+
   nsresult ContentStateChanged(nsIContent* aContent,
                                EventStates aStateMask);
   void AttributeWillChange(dom::Element* aElement,
                            int32_t aNameSpaceID,
                            nsIAtom* aAttribute,
                            int32_t aModType,
                            const nsAttrValue* aNewValue);
 
--- a/layout/base/nsPresShell.cpp
+++ b/layout/base/nsPresShell.cpp
@@ -4322,17 +4322,17 @@ PresShell::ContentAppended(nsIDocument *
   // Call this here so it only happens for real content mutations and
   // not cases when the frame constructor calls its own methods to force
   // frame reconstruction.
   if (aContainer->IsElement()) {
     // Ensure the container is an element before trying to restyle
     // because it can be the case that the container is a ShadowRoot
     // which is a document fragment.
     mPresContext->RestyleManager()->
-      RestyleForAppend(aContainer->AsElement(), aFirstNewContent);
+      ContentAppended(aContainer->AsElement(), aFirstNewContent);
   }
 
   mFrameConstructor->ContentAppended(aContainer, aFirstNewContent, true);
 
   if (static_cast<nsINode*>(aContainer) == static_cast<nsINode*>(aDocument) &&
       aFirstNewContent->NodeType() == nsIDOMNode::DOCUMENT_TYPE_NODE) {
     NotifyFontSizeInflationEnabledIsDirty();
   }
@@ -4358,17 +4358,17 @@ PresShell::ContentInserted(nsIDocument* 
   // Call this here so it only happens for real content mutations and
   // not cases when the frame constructor calls its own methods to force
   // frame reconstruction.
   if (aContainer && aContainer->IsElement()) {
     // Ensure the container is an element before trying to restyle
     // because it can be the case that the container is a ShadowRoot
     // which is a document fragment.
     mPresContext->RestyleManager()->
-      RestyleForInsertOrChange(aContainer->AsElement(), aChild);
+      ContentInserted(aContainer->AsElement(), aChild);
   }
 
   mFrameConstructor->ContentInserted(aContainer, aChild, nullptr, true);
 
   if (((!aContainer && aDocument) ||
       (static_cast<nsINode*>(aContainer) == static_cast<nsINode*>(aDocument))) &&
       aChild->NodeType() == nsIDOMNode::DOCUMENT_TYPE_NODE) {
     NotifyFontSizeInflationEnabledIsDirty();
@@ -4406,17 +4406,17 @@ PresShell::ContentRemoved(nsIDocument *a
   if (aContainer) {
     oldNextSibling = aContainer->GetChildAt(aIndexInContainer);
   } else {
     oldNextSibling = nullptr;
   }
 
   if (aContainer && aContainer->IsElement()) {
     mPresContext->RestyleManager()->
-      RestyleForRemove(aContainer->AsElement(), aChild, oldNextSibling);
+      ContentRemoved(aContainer->AsElement(), aChild, oldNextSibling);
   }
 
   // After removing aChild from tree we should save information about live ancestor
   if (mPointerEventTarget) {
     if (nsContentUtils::ContentIsDescendantOf(mPointerEventTarget, aChild)) {
       mPointerEventTarget = aContainer;
     }
   }