Bug 565819. Switch RestyleForRemove/Append/InsertOrChange to using Element and the new DOM traversal APIs. r=dbaron

layout/base/nsCSSFrameConstructor.cpp

@@ -11321,19 +11321,14 @@ nsCSSFrameConstructor::ReframeContainingBlock(nsIFrame* aFrame)
 }
 
 void
-nsCSSFrameConstructor::RestyleForAppend(nsIContent* aContainer,
-                                        PRInt32 aNewIndexInContainer)
+nsCSSFrameConstructor::RestyleForAppend(Element* aContainer,
+                                        nsIContent* aFirstNewContent)
 {
   NS_ASSERTION(aContainer, "must have container for append");
 #ifdef DEBUG
   {
-    for (PRInt32 index = aNewIndexInContainer;; ++index) {
-      nsIContent *content = aContainer->GetChildAt(index);
-      if (!content) {
-        NS_ASSERTION(index != aNewIndexInContainer, "yikes, nothing appended");
-        break;
-      }
-      NS_ASSERTION(!content->IsRootOfAnonymousSubtree(),
+    for (nsIContent* cur = aFirstNewContent; cur; cur = cur->GetNextSibling()) {
+      NS_ASSERTION(!cur->IsRootOfAnonymousSubtree(),
                    "anonymous nodes should not be in child lists");
     }
   }
@@ -11353,13 +11348,14 @@ nsCSSFrameConstructor::RestyleForAppend(nsIContent* aContainer,
   if (selectorFlags & NODE_HAS_EMPTY_SELECTOR) {
     // see whether we need to restyle the container
     PRBool wasEmpty = PR_TRUE; // :empty or :-moz-only-whitespace
-    for (PRInt32 index = 0; index < aNewIndexInContainer; ++index) {
+    for (nsIContent* cur = aContainer->GetFirstChild();
+         cur != aFirstNewContent;
+         cur = cur->GetNextSibling()) {
       // We don't know whether we're testing :empty or :-moz-only-whitespace,
       // so be conservative and assume :-moz-only-whitespace (i.e., make
       // IsSignificantChild less likely to be true, and thus make us more
       // likely to restyle).
-      if (nsStyleUtil::IsSignificantChild(aContainer->GetChildAt(index),
-                                          PR_TRUE, PR_FALSE)) {
+      if (nsStyleUtil::IsSignificantChild(cur, PR_TRUE, PR_FALSE)) {
         wasEmpty = PR_FALSE;
         break;
       }
@@ -11372,10 +11368,11 @@ nsCSSFrameConstructor::RestyleForAppend(nsIContent* aContainer,
   }
   if (selectorFlags & NODE_HAS_EDGE_CHILD_SELECTOR) {
     // restyle the last element child before this node
-    for (PRInt32 index = aNewIndexInContainer - 1; index >= 0; --index) {
-      nsIContent *content = aContainer->GetChildAt(index);
-      if (content->IsElement()) {
-        PostRestyleEvent(content, eRestyle_Self, NS_STYLE_HINT_NONE);
+    for (nsIContent* cur = aFirstNewContent->GetPreviousSibling();
+         cur;
+         cur = cur->GetPreviousSibling()) {
+      if (cur->IsElement()) {
+        PostRestyleEvent(cur->AsElement(), eRestyle_Self, NS_STYLE_HINT_NONE);
         break;
       }
     }
@@ -11389,7 +11386,7 @@ nsCSSFrameConstructor::RestyleForAppend(nsIContent* aContainer,
 // The comments are written and variables are named in terms of it being
 // a ContentInserted notification.
 void
-nsCSSFrameConstructor::RestyleForInsertOrChange(nsIContent* aContainer,
+nsCSSFrameConstructor::RestyleForInsertOrChange(Element* aContainer,
                                                 nsIContent* aChild)
 {
   NS_ASSERTION(!aChild->IsRootOfAnonymousSubtree(),
@@ -11409,10 +11406,9 @@ nsCSSFrameConstructor::RestyleForInsertOrChange(nsIContent* aContainer,
   if (selectorFlags & NODE_HAS_EMPTY_SELECTOR) {
     // see whether we need to restyle the container
     PRBool wasEmpty = PR_TRUE; // :empty or :-moz-only-whitespace
-    for (PRInt32 index = 0; ; ++index) {
-      nsIContent *child = aContainer->GetChildAt(index);
-      if (!child) // last child
-        break;
+    for (nsIContent* child = aContainer->GetFirstChild();
+         child;
+         child = child->GetNextSibling()) {
       if (child == aChild)
         continue;
       // We don't know whether we're testing :empty or :-moz-only-whitespace,
@@ -11434,33 +11430,34 @@ nsCSSFrameConstructor::RestyleForInsertOrChange(nsIContent* aContainer,
   if (selectorFlags & NODE_HAS_EDGE_CHILD_SELECTOR) {
     // restyle the previously-first element child if it is after this node
     PRBool passedChild = PR_FALSE;
-    for (PRInt32 index = 0; ; ++index) {
-      nsIContent *content = aContainer->GetChildAt(index);
-      if (!content)
-        break; // went through all children
+    for (nsIContent* content = aContainer->GetFirstChild();
+         content;
+         content = content->GetNextSibling()) {
       if (content == aChild) {
         passedChild = PR_TRUE;
         continue;
       }
       if (content->IsElement()) {
         if (passedChild) {
-          PostRestyleEvent(content, eRestyle_Self, NS_STYLE_HINT_NONE);
+          PostRestyleEvent(content->AsElement(), eRestyle_Self,
+                           NS_STYLE_HINT_NONE);
         }
         break;
       }
     }
     // restyle the previously-last element child if it is before this node
     passedChild = PR_FALSE;
-    for (PRInt32 index = aContainer->GetChildCount() - 1;
-         index >= 0; --index) {
-      nsIContent *content = aContainer->GetChildAt(index);
+    for (nsIContent* content = aContainer->GetLastChild();
+         content;
+         content = content->GetPreviousSibling()) {
       if (content == aChild) {
         passedChild = PR_TRUE;
         continue;
       }
       if (content->IsElement()) {
         if (passedChild) {
-          PostRestyleEvent(content, eRestyle_Self, NS_STYLE_HINT_NONE);
+          PostRestyleEvent(content->AsElement(), eRestyle_Self,
+                           NS_STYLE_HINT_NONE);
         }
         break;
       }
@@ -11469,9 +11466,9 @@ nsCSSFrameConstructor::RestyleForInsertOrChange(nsIContent* aContainer,
 }
 
 void
-nsCSSFrameConstructor::RestyleForRemove(nsIContent* aContainer,
+nsCSSFrameConstructor::RestyleForRemove(Element* aContainer,
                                         nsIContent* aOldChild,
-                                        PRInt32 aIndexInContainer)
+                                        nsIContent* aFollowingSibling)
 {
   NS_ASSERTION(!aOldChild->IsRootOfAnonymousSubtree(),
                "anonymous nodes should not be in child lists");
@@ -11490,10 +11487,9 @@ nsCSSFrameConstructor::RestyleForRemove(nsIContent* aContainer,
   if (selectorFlags & NODE_HAS_EMPTY_SELECTOR) {
     // see whether we need to restyle the container
     PRBool isEmpty = PR_TRUE; // :empty or :-moz-only-whitespace
-    for (PRInt32 index = 0; ; ++index) {
-      nsIContent *child = aContainer->GetChildAt(index);
-      if (!child) // last child
-        break;
+    for (nsIContent* child = aContainer->GetFirstChild();
+         child;
+         child = child->GetNextSibling()) {
       // We don't know whether we're testing :empty or :-moz-only-whitespace,
       // so be conservative and assume :-moz-only-whitespace (i.e., make
       // IsSignificantChild less likely to be true, and thus make us more
@@ -11511,28 +11507,37 @@ nsCSSFrameConstructor::RestyleForRemove(nsIContent* aContainer,
   }
 
   if (selectorFlags & NODE_HAS_EDGE_CHILD_SELECTOR) {
-    // restyle the previously-first element child if it is after aOldChild
-    for (PRInt32 index = 0; ; ++index) {
-      nsIContent *content = aContainer->GetChildAt(index);
-      if (!content)
-        break; // went through all children
+    // restyle the now-first element child if it was after aOldChild
+    PRBool reachedFollowingSibling = PR_FALSE;
+    for (nsIContent* content = aContainer->GetFirstChild();
+         content;
+         content = content->GetNextSibling()) {
+      if (content == aFollowingSibling) {
+        reachedFollowingSibling = PR_TRUE;
+        // do NOT continue here; we might want to restyle this node
+      }
       if (content->IsElement()) {
-        if (index >= aIndexInContainer) {
-          PostRestyleEvent(content, eRestyle_Self, NS_STYLE_HINT_NONE);
+        if (reachedFollowingSibling) {
+          PostRestyleEvent(content->AsElement(), eRestyle_Self,
+                           NS_STYLE_HINT_NONE);
         }
         break;
       }
     }
-    // restyle the previously-last element child if it is before aOldChild
-    for (PRInt32 index = aContainer->GetChildCount() - 1;
-         index >= 0; --index) {
-      nsIContent *content = aContainer->GetChildAt(index);
+    // restyle the now-last element child if it was before aOldChild
+    reachedFollowingSibling = (aFollowingSibling == nsnull);
+    for (nsIContent* content = aContainer->GetLastChild();
+         content;
+         content = content->GetPreviousSibling()) {
       if (content->IsElement()) {
-        if (index < aIndexInContainer) {
-          PostRestyleEvent(content, eRestyle_Self, NS_STYLE_HINT_NONE);
+        if (reachedFollowingSibling) {
+          PostRestyleEvent(content->AsElement(), eRestyle_Self, NS_STYLE_HINT_NONE);
         }
         break;
       }
+      if (content == aFollowingSibling) {
+        reachedFollowingSibling = PR_TRUE;
+      }
     }
   }
 }

layout/base/nsCSSFrameConstructor.h

@@ -315,18 +315,20 @@ public:
   // 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(nsIContent* aContainer,
+  void RestyleForInsertOrChange(mozilla::dom::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
   // have to use the index.  |aContainer| must be non-null; when the
-  // container is null, no work is needed.
-  void RestyleForRemove(nsIContent* aContainer, nsIContent* aOldChild,
-                        PRInt32 aIndexInContainer);
+  // container is null, no work is needed.  aFollowingSibling is the
+  // sibling that used to come after aOldChild before the removal.
+  void RestyleForRemove(mozilla::dom::Element* aContainer,
+                        nsIContent* aOldChild,
+                        nsIContent* aFollowingSibling);
   // Same for a ContentAppended.  |aContainer| must be non-null; when
   // the container is null, no work is needed.
-  void RestyleForAppend(nsIContent* aContainer,
-                        PRInt32 aNewIndexInContainer);
+  void RestyleForAppend(mozilla::dom::Element* aContainer,
+                        nsIContent* aFirstNewContent);
 
   // Process any pending restyles. This should be called after
   // CreateNeededFrames.

layout/base/nsPresShell.cpp

@@ -4689,13 +4689,11 @@ PresShell::CharacterDataChanged(nsIDocument *aDocument,
   PRUint32 selectorFlags =
     container ? (container->GetFlags() & NODE_ALL_SELECTOR_FLAGS) : 0;
   if (selectorFlags != 0 && !aContent->IsRootOfAnonymousSubtree()) {
-    PRUint32 index;
-    if (aInfo->mAppend &&
-        container->GetChildAt((index = container->GetChildCount() - 1)) ==
-          aContent)
-      mFrameConstructor->RestyleForAppend(container, index);
+    Element* element = container->AsElement();
+    if (aInfo->mAppend && !aContent->GetNextSibling())
+      mFrameConstructor->RestyleForAppend(element, aContent);
     else
-      mFrameConstructor->RestyleForInsertOrChange(container, aContent);
+      mFrameConstructor->RestyleForInsertOrChange(element, aContent);
   }
 
   mFrameConstructor->CharacterDataChanged(aContent, aInfo);
@@ -4796,7 +4794,7 @@ PresShell::ContentAppended(nsIDocument *aDocument,
   // 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.
-  mFrameConstructor->RestyleForAppend(aContainer, aNewIndexInContainer);
+  mFrameConstructor->RestyleForAppend(aContainer->AsElement(), aFirstNewContent);
 
   mFrameConstructor->ContentAppended(aContainer, aFirstNewContent,
                                      aNewIndexInContainer, PR_TRUE);
@@ -4822,7 +4820,7 @@ PresShell::ContentInserted(nsIDocument* aDocument,
   // not cases when the frame constructor calls its own methods to force
   // frame reconstruction.
   if (aContainer)
-    mFrameConstructor->RestyleForInsertOrChange(aContainer, aChild);
+    mFrameConstructor->RestyleForInsertOrChange(aContainer->AsElement(), aChild);
 
   mFrameConstructor->ContentInserted(aContainer, aChild,
                                      aIndexInContainer, nsnull, PR_TRUE);
@@ -4853,7 +4851,8 @@ PresShell::ContentRemoved(nsIDocument *aDocument,
   // not cases when the frame constructor calls its own methods to force
   // frame reconstruction.
   if (aContainer)
-    mFrameConstructor->RestyleForRemove(aContainer, aChild, aIndexInContainer);
+    mFrameConstructor->RestyleForRemove(aContainer->AsElement(), aChild,
+                                        aContainer->GetChildAt(aIndexInContainer));
 
   PRBool didReconstruct;
   mFrameConstructor->ContentRemoved(aContainer, aChild, aIndexInContainer,

layout/reftests/bugs/565819-1.html

@@ -0,0 +1,17 @@
+<!DOCTYPE html>
+<html>
+  <head>
+    <style>
+      div { color: green; }
+      div:empty { color: red; }
+    </style>
+    <script>
+      window.onload = function () {
+        document.getElementById("x").appendChild(document.createTextNode("This should be green"));
+      }
+    </script>
+  </head>
+  <body>
+    <div id="x"></div>
+  </body>
+</html>

layout/reftests/bugs/565819-2.html

@@ -0,0 +1,19 @@
+<!DOCTYPE html>
+<html>
+  <head>
+    <style>
+      div { color: red; }
+      div:last-child { color: green; }
+    </style>
+    <script>
+      window.onload = function() {
+      dump('aaa');
+        var r = document.getElementById("r");
+        r.parentNode.removeChild(r);
+      }
+    </script>
+  </head>
+  <body>
+    <!-- Need extra wrapper div, because whitespace inside <body> is all weird -->
+    <div><div>This should be green</div><div id="r"></div></div></body>
+</html>

layout/reftests/bugs/565819-ref.html

@@ -0,0 +1,6 @@
+<!DOCTYPE html>
+<html>
+  <body style="color: green">
+    This should be green
+  </body>
+</html>

layout/reftests/bugs/reftest.list

@@ -1431,3 +1431,5 @@ random-if(!haveTestPlugin) == 546071-1.html 546071-1-ref.html
 == 562835-1.html 562835-ref.html
 == 562835-2.html 562835-ref.html
 == 564054-1.html 564054-1-ref.html
+== 565819-1.html 565819-ref.html
+== 565819-2.html 565819-ref.html