Bug 1397983: Synchronously bind/unbind XBL anonymous content from the bound content's Bind/UnbindFromTree. r=smaug

Doing it off a runnable makes the flattened tree inconsistent until that
runnable runs.

Also add an assert in frame construction that would've caught the first
only-unbind patch.

MozReview-Commit-ID: Hnua3aWSMHi

--HG--
extra : rebase_source : 2781e3b0a3f28b6b6a620902e7414dfe682fba51

dom/base/ChildIterator.cpp

@@ -168,12 +168,10 @@ FlattenedChildIterator::Init(bool aIgnoreXBL)
     mParent->OwnerDoc()->BindingManager()->GetBindingWithContent(mParent);
 
   if (binding) {
-    nsIContent* anon = binding->GetAnonymousContent();
+    MOZ_ASSERT(binding->GetAnonymousContent());
-    if (anon) {
+    mParent = binding->GetAnonymousContent();
-      mParent = anon;
     mXBLInvolved = true;
   }
-  }
 
   // We set mXBLInvolved to true if either:
   // - The node we're iterating has a binding with content attached to it.

dom/base/Element.cpp

@@ -1668,31 +1668,26 @@ Element::BindToTree(nsIDocument* aDocument, nsIContent* aParent,
 
   uint32_t editableDescendantCount = 0;
 
-  // If NODE_FORCE_XBL_BINDINGS was set we might have anonymous children
-  // that also need to be told that they are moving.
-  nsresult rv;
-  if (hadForceXBL) {
-    nsBindingManager* bmgr = OwnerDoc()->BindingManager();
-
-    nsXBLBinding* contBinding = bmgr->GetBindingWithContent(this);
-    // First check if we have a binding...
-    if (contBinding) {
-      nsCOMPtr<nsIContent> anonRoot = contBinding->GetAnonymousContent();
-      bool allowScripts = contBinding->AllowScripts();
-      for (nsCOMPtr<nsIContent> child = anonRoot->GetFirstChild();
-           child;
-           child = child->GetNextSibling()) {
-        rv = child->BindToTree(aDocument, this, this, allowScripts);
-        NS_ENSURE_SUCCESS(rv, rv);
-
-        editableDescendantCount += EditableInclusiveDescendantCount(child);
-      }
-    }
-  }
-
   UpdateEditableState(false);
 
+  // If NODE_FORCE_XBL_BINDINGS was set, or we had a pre-existing XBL binding,
+  // we might have anonymous children that also need to be told that they are
+  // moving.
+  if (hadForceXBL ||
+      (HasFlag(NODE_MAY_BE_IN_BINDING_MNGR) && !GetShadowRoot())) {
+    nsXBLBinding* binding =
+      OwnerDoc()->BindingManager()->GetBindingWithContent(this);
+
+    if (binding) {
+      binding->BindAnonymousContent(
+        binding->GetAnonymousContent(),
+        this,
+        binding->PrototypeBinding()->ChromeOnlyContent());
+    }
+  }
+
   // Now recurse into our kids
+  nsresult rv;
   for (nsIContent* child = GetFirstChild(); child;
        child = child->GetNextSibling()) {
     rv = child->BindToTree(aDocument, this, aBindingParent,
@@ -1927,6 +1922,26 @@ Element::UnbindFromTree(bool aDeep, bool aNullParent)
     SetSubtreeRootPointer(aNullParent ? this : mParent->SubtreeRoot());
   }
 
+  // Unset this since that's what the old code effectively did.
+  UnsetFlags(NODE_FORCE_XBL_BINDINGS);
+  bool clearBindingParent = true;
+
+#ifdef MOZ_XUL
+  if (nsXULElement* xulElem = nsXULElement::FromContent(this)) {;
+    xulElem->SetXULBindingParent(nullptr);
+    clearBindingParent = false;
+  }
+#endif
+
+  if (nsExtendedDOMSlots* slots = GetExistingExtendedDOMSlots()) {
+    if (clearBindingParent) {
+      slots->mBindingParent = nullptr;
+    }
+    if (aNullParent || !mParent->IsInShadowTree()) {
+      slots->mContainingShadow = nullptr;
+    }
+  }
+
   if (document) {
     // Notify XBL- & nsIAnonymousContentCreator-generated
     // anonymous content that the document is changing.
@@ -1934,8 +1949,16 @@ Element::UnbindFromTree(bool aDeep, bool aNullParent)
     // do not get uninstalled.
     if (HasFlag(NODE_MAY_BE_IN_BINDING_MNGR) && !GetShadowRoot()) {
       nsContentUtils::AddScriptRunner(
-        new RemoveFromBindingManagerRunnable(document->BindingManager(), this,
+        new RemoveFromBindingManagerRunnable(
-                                             document));
+          document->BindingManager(), this, document));
+      nsXBLBinding* binding =
+        document->BindingManager()->GetBindingWithContent(this);
+      if (binding) {
+        nsXBLBinding::UnbindAnonymousContent(
+          document,
+          binding->GetAnonymousContent(),
+          /* aNullParent */ false);
+      }
     }
 
     document->ClearBoxObjectFor(this);
@@ -1949,28 +1972,6 @@ Element::UnbindFromTree(bool aDeep, bool aNullParent)
     }
   }
 
-  // Unset this since that's what the old code effectively did.
-  UnsetFlags(NODE_FORCE_XBL_BINDINGS);
-  bool clearBindingParent = true;
-
-#ifdef MOZ_XUL
-  nsXULElement* xulElem = nsXULElement::FromContent(this);
-  if (xulElem) {
-    xulElem->SetXULBindingParent(nullptr);
-    clearBindingParent = false;
-  }
-#endif
-
-  nsExtendedDOMSlots* slots = GetExistingExtendedDOMSlots();
-  if (slots) {
-    if (clearBindingParent) {
-      slots->mBindingParent = nullptr;
-    }
-    if (aNullParent || !mParent->IsInShadowTree()) {
-      slots->mContainingShadow = nullptr;
-    }
-  }
-
   // This has to be here, rather than in nsGenericHTMLElement::UnbindFromTree,
   //  because it has to happen after unsetting the parent pointer, but before
   //  recursively unbinding the kids.
@@ -1995,8 +1996,7 @@ Element::UnbindFromTree(bool aDeep, bool aNullParent)
   nsNodeUtils::ParentChainChanged(this);
 
   // Unbind children of shadow root.
-  ShadowRoot* shadowRoot = GetShadowRoot();
+  if (ShadowRoot* shadowRoot = GetShadowRoot()) {
-  if (shadowRoot) {
     for (nsIContent* child = shadowRoot->GetFirstChild(); child;
          child = child->GetNextSibling()) {
       child->UnbindFromTree(true, false);

dom/xbl/nsXBLBinding.cpp

@@ -239,7 +239,8 @@ nsXBLBinding::BindAnonymousContent(nsIContent* aAnonParent,
 
 void
 nsXBLBinding::UnbindAnonymousContent(nsIDocument* aDocument,
-                                     nsIContent* aAnonParent)
+                                     nsIContent* aAnonParent,
+                                     bool aNullParent)
 {
   nsAutoScriptBlocker scriptBlocker;
   // Hold a strong ref while doing this, just in case.
@@ -251,7 +252,7 @@ nsXBLBinding::UnbindAnonymousContent(nsIDocument* aDocument,
   for (nsIContent* child = aAnonParent->GetFirstChild();
        child;
        child = child->GetNextSibling()) {
-    child->UnbindFromTree();
+    child->UnbindFromTree(true, aNullParent);
 #ifdef MOZ_XUL
     if (xuldoc) {
       xuldoc->RemoveSubtreeFromDocument(child);

dom/xbl/nsXBLBinding.h

@@ -108,7 +108,8 @@ public:
   void BindAnonymousContent(nsIContent* aAnonParent, nsIContent* aElement,
                             bool aNativeAnon);
   static void UnbindAnonymousContent(nsIDocument* aDocument,
-                                     nsIContent* aAnonParent);
+                                     nsIContent* aAnonParent,
+                                     bool aNullParent = true);
   void InstallEventHandlers();
   nsresult InstallImplementation();
 

layout/base/nsCSSFrameConstructor.cpp

@@ -5882,6 +5882,7 @@ nsCSSFrameConstructor::AddFrameConstructionItemsInternal(nsFrameConstructorState
   NS_PRECONDITION(aContent->IsNodeOfType(nsINode::eTEXT) ||
                   aContent->IsElement(),
                   "Shouldn't get anything else here!");
+  MOZ_ASSERT(aContent->IsInComposedDoc());
   MOZ_ASSERT(!aContent->GetPrimaryFrame() || aState.mCreatingExtraFrames ||
              aContent->NodeInfo()->NameAtom() == nsGkAtoms::area);