Bug 1420547: Notify the pres shell before everything else on removals. r?bz draft
authorEmilio Cobos Álvarez <emilio@crisal.io>
Mon, 11 Dec 2017 06:42:38 +0100
changeset 713800 d33d7bb985f4c1c86a996c8d354e7fdb1e5bfd8f
parent 713799 d1922e351eea2868ab80e1fbaa7d207723673191
child 713801 0efbef6fe63d6435aa0cb4d6ee4e24e1ccc000e8
push id93751
push userbmo:emilio@crisal.io
push dateWed, 20 Dec 2017 23:52:04 +0000
reviewersbz
bugs1420547, 1420533, 382376, 1389743
milestone59.0a1
Bug 1420547: Notify the pres shell before everything else on removals. r?bz This allows the flattened tree to have a reasonable state before content removals, allowing us to fix bugs like bug 1420533. This used to be also the old setup, but was changed in bug 382376, since the frame constructor used to reconstruct stuff synchronously. This used to break all sorts of stylo invariants, and I fixed this in bug 1389743, so now the order can be sane again. MozReview-Commit-ID: K2pTweGKSq0
dom/base/nsNodeUtils.cpp
--- a/dom/base/nsNodeUtils.cpp
+++ b/dom/base/nsNodeUtils.cpp
@@ -37,50 +37,61 @@
 #include "mozilla/dom/BindingUtils.h"
 #include "mozilla/dom/HTMLTemplateElement.h"
 #include "mozilla/dom/ShadowRoot.h"
 
 using namespace mozilla;
 using namespace mozilla::dom;
 using mozilla::AutoJSContext;
 
+enum class IsRemoveNotification
+{
+  Yes,
+  No,
+};
+
 // This macro expects the ownerDocument of content_ to be in scope as
 // |nsIDocument* doc|
-#define IMPL_MUTATION_NOTIFICATION(func_, content_, params_)      \
-  PR_BEGIN_MACRO                                                  \
-  bool needsEnterLeave = doc->MayHaveDOMMutationObservers();      \
-  if (needsEnterLeave) {                                          \
-    nsDOMMutationObserver::EnterMutationHandling();               \
-  }                                                               \
-  nsINode* node = content_;                                       \
-  NS_ASSERTION(node->OwnerDoc() == doc, "Bogus document");        \
-  doc->BindingManager()->func_ params_;                           \
-  nsINode* last;                                                  \
-  do {                                                            \
-    nsINode::nsSlots* slots = node->GetExistingSlots();           \
-    if (slots && !slots->mMutationObservers.IsEmpty()) {          \
-      NS_OBSERVER_AUTO_ARRAY_NOTIFY_OBSERVERS(                    \
-        slots->mMutationObservers, nsIMutationObserver, 1,        \
-        func_, params_);                                          \
-    }                                                             \
-    last = node;                                                  \
-    if (ShadowRoot* shadow = ShadowRoot::FromNode(node)) {        \
-      node = shadow->GetHost();                                   \
-    } else {                                                      \
-      node = node->GetParentNode();                               \
-    }                                                             \
-  } while (node);                                                 \
-  if (last == doc) {                                              \
-    if (nsIPresShell* shell = doc->GetObservingShell()) {         \
-      shell->func_ params_;                                       \
-    }                                                             \
-  }                                                               \
-  if (needsEnterLeave) {                                          \
-    nsDOMMutationObserver::LeaveMutationHandling();               \
-  }                                                               \
+#define IMPL_MUTATION_NOTIFICATION(func_, content_, params_, remove_)       \
+  PR_BEGIN_MACRO                                                            \
+  bool needsEnterLeave = doc->MayHaveDOMMutationObservers();                \
+  if (needsEnterLeave) {                                                    \
+    nsDOMMutationObserver::EnterMutationHandling();                         \
+  }                                                                         \
+  nsINode* node = content_;                                                 \
+  NS_ASSERTION(node->OwnerDoc() == doc, "Bogus document");                  \
+  if (remove_ == IsRemoveNotification::Yes && node->GetComposedDoc()) {     \
+    if (nsIPresShell* shell = doc->GetObservingShell()) {                   \
+      shell->func_ params_;                                                 \
+    }                                                                       \
+  }                                                                         \
+  doc->BindingManager()->func_ params_;                                     \
+  nsINode* last;                                                            \
+  do {                                                                      \
+    nsINode::nsSlots* slots = node->GetExistingSlots();                     \
+    if (slots && !slots->mMutationObservers.IsEmpty()) {                    \
+      NS_OBSERVER_AUTO_ARRAY_NOTIFY_OBSERVERS(                              \
+        slots->mMutationObservers, nsIMutationObserver, 1,                  \
+        func_, params_);                                                    \
+    }                                                                       \
+    last = node;                                                            \
+    if (ShadowRoot* shadow = ShadowRoot::FromNode(node)) {                  \
+      node = shadow->GetHost();                                             \
+    } else {                                                                \
+      node = node->GetParentNode();                                         \
+    }                                                                       \
+  } while (node);                                                           \
+  if (remove_ == IsRemoveNotification::No && last == doc) {                 \
+    if (nsIPresShell* shell = doc->GetObservingShell()) {                   \
+      shell->func_ params_;                                                 \
+    }                                                                       \
+  }                                                                         \
+  if (needsEnterLeave) {                                                    \
+    nsDOMMutationObserver::LeaveMutationHandling();                         \
+  }                                                                         \
   PR_END_MACRO
 
 #define IMPL_ANIMATION_NOTIFICATION(func_, content_, params_)     \
   PR_BEGIN_MACRO                                                  \
   bool needsEnterLeave = doc->MayHaveDOMMutationObservers();      \
   if (needsEnterLeave) {                                          \
     nsDOMMutationObserver::EnterMutationHandling();               \
   }                                                               \
@@ -105,81 +116,84 @@ using mozilla::AutoJSContext;
   PR_END_MACRO
 
 void
 nsNodeUtils::CharacterDataWillChange(nsIContent* aContent,
                                      CharacterDataChangeInfo* aInfo)
 {
   nsIDocument* doc = aContent->OwnerDoc();
   IMPL_MUTATION_NOTIFICATION(CharacterDataWillChange, aContent,
-                             (doc, aContent, aInfo));
+                             (doc, aContent, aInfo), IsRemoveNotification::No);
 }
 
 void
 nsNodeUtils::CharacterDataChanged(nsIContent* aContent,
                                   CharacterDataChangeInfo* aInfo)
 {
   nsIDocument* doc = aContent->OwnerDoc();
   IMPL_MUTATION_NOTIFICATION(CharacterDataChanged, aContent,
-                             (doc, aContent, aInfo));
+                             (doc, aContent, aInfo), IsRemoveNotification::No);
 }
 
 void
 nsNodeUtils::AttributeWillChange(Element* aElement,
                                  int32_t aNameSpaceID,
                                  nsAtom* aAttribute,
                                  int32_t aModType,
                                  const nsAttrValue* aNewValue)
 {
   nsIDocument* doc = aElement->OwnerDoc();
   IMPL_MUTATION_NOTIFICATION(AttributeWillChange, aElement,
                              (doc, aElement, aNameSpaceID, aAttribute,
-                              aModType, aNewValue));
+                              aModType, aNewValue), IsRemoveNotification::No);
 }
 
 void
 nsNodeUtils::AttributeChanged(Element* aElement,
                               int32_t aNameSpaceID,
                               nsAtom* aAttribute,
                               int32_t aModType,
                               const nsAttrValue* aOldValue)
 {
   nsIDocument* doc = aElement->OwnerDoc();
   IMPL_MUTATION_NOTIFICATION(AttributeChanged, aElement,
                              (doc, aElement, aNameSpaceID, aAttribute,
-                              aModType, aOldValue));
+                              aModType, aOldValue), IsRemoveNotification::No);
 }
 
 void
 nsNodeUtils::AttributeSetToCurrentValue(Element* aElement,
                                         int32_t aNameSpaceID,
                                         nsAtom* aAttribute)
 {
   nsIDocument* doc = aElement->OwnerDoc();
   IMPL_MUTATION_NOTIFICATION(AttributeSetToCurrentValue, aElement,
-                             (doc, aElement, aNameSpaceID, aAttribute));
+                             (doc, aElement, aNameSpaceID, aAttribute),
+                             IsRemoveNotification::No);
 }
 
 void
 nsNodeUtils::ContentAppended(nsIContent* aContainer,
                              nsIContent* aFirstNewContent)
 {
   nsIDocument* doc = aContainer->OwnerDoc();
 
   IMPL_MUTATION_NOTIFICATION(ContentAppended, aContainer,
-                             (doc, aContainer, aFirstNewContent));
+                             (doc, aContainer, aFirstNewContent),
+                             IsRemoveNotification::No);
 }
 
 void
 nsNodeUtils::NativeAnonymousChildListChange(nsIContent* aContent,
                                             bool aIsRemove)
 {
   nsIDocument* doc = aContent->OwnerDoc();
   IMPL_MUTATION_NOTIFICATION(NativeAnonymousChildListChange, aContent,
-                            (doc, aContent, aIsRemove));
+                            (doc, aContent, aIsRemove),
+                            IsRemoveNotification::No);
 }
 
 void
 nsNodeUtils::ContentInserted(nsINode* aContainer,
                              nsIContent* aChild)
 {
   NS_PRECONDITION(aContainer->IsContent() ||
                   aContainer->IsNodeOfType(nsINode::eDOCUMENT),
@@ -191,17 +205,18 @@ nsNodeUtils::ContentInserted(nsINode* aC
     container = aContainer->AsContent();
     document = doc;
   } else {
     container = nullptr;
     document = static_cast<nsIDocument*>(aContainer);
   }
 
   IMPL_MUTATION_NOTIFICATION(ContentInserted, aContainer,
-                             (document, container, aChild));
+                             (document, container, aChild),
+                             IsRemoveNotification::No);
 }
 
 void
 nsNodeUtils::ContentRemoved(nsINode* aContainer,
                             nsIContent* aChild,
                             nsIContent* aPreviousSibling)
 {
   NS_PRECONDITION(aContainer->IsContent() ||
@@ -214,17 +229,18 @@ nsNodeUtils::ContentRemoved(nsINode* aCo
     container = static_cast<nsIContent*>(aContainer);
     document = doc;
   } else {
     container = nullptr;
     document = static_cast<nsIDocument*>(aContainer);
   }
 
   IMPL_MUTATION_NOTIFICATION(ContentRemoved, aContainer,
-                             (document, container, aChild, aPreviousSibling));
+                             (document, container, aChild, aPreviousSibling),
+                             IsRemoveNotification::Yes);
 }
 
 Maybe<NonOwningAnimationTarget>
 nsNodeUtils::GetTargetForAnimation(const Animation* aAnimation)
 {
   AnimationEffectReadOnly* effect = aAnimation->GetEffect();
   if (!effect || !effect->AsKeyframeEffect()) {
     return Nothing();