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

@@ -568,9 +568,8 @@ RestyleManager::RestyleForInsertOrChange(Element* aContainer,
 }
 
 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

layout/base/RestyleManager.h

@@ -281,21 +281,35 @@ 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);
 

layout/base/RestyleManagerHandle.h

@@ -114,13 +114,17 @@ public:
     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,

layout/base/RestyleManagerHandleInlines.h

@@ -86,6 +86,28 @@ 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)
@@ -100,13 +122,6 @@ RestyleManagerHandle::Ptr::RestyleForAppend(dom::Element* aContainer,
   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,

layout/base/ServoRestyleManager.cpp

@@ -341,24 +341,74 @@ 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

layout/base/ServoRestyleManager.h

@@ -51,13 +51,19 @@ public:
   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,

layout/base/nsPresShell.cpp

@@ -4327,7 +4327,7 @@ PresShell::ContentAppended(nsIDocument *aDocument,
     // 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);
@@ -4363,7 +4363,7 @@ PresShell::ContentInserted(nsIDocument* aDocument,
     // 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);
@@ -4411,7 +4411,7 @@ PresShell::ContentRemoved(nsIDocument *aDocument,
 
   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