Bug 1420547: Notify the pres shell before everything else on removals. r=bz
authorEmilio Cobos Álvarez <emilio@crisal.io>
Mon, 11 Dec 2017 06:42:38 +0100
changeset 397069 04a9c149d299878293af6551518e0eb42df11f18
parent 397068 e482d3fc80e40910ee6944beb5b4f2e889d0d93c
child 397070 24e90541f05637a7399f705bbce71bb12bfb5813
push id33123
push userncsoregi@mozilla.com
push dateThu, 21 Dec 2017 10:00:47 +0000
treeherdermozilla-central@06a19fbe2581 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbz
bugs1420547, 1420533, 382376, 1389743
milestone59.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 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();