Bug 1334044: Replace detached callback (v0) with disconnected callback (v1).

Tag UXP Issue #1344

9 files changed, 28 insertions, 84 deletions

diff --git a/dom/base/CustomElementRegistry.cpp b/dom/base/CustomElementRegistry.cpp
index 67498a59b..069611f65 100644
--- a/dom/base/CustomElementRegistry.cpp
+++ b/dom/base/CustomElementRegistry.cpp
@@ -57,8 +57,8 @@ CustomElementCallback::Call()
     case nsIDocument::eConnected:
       static_cast<LifecycleConnectedCallback *>(mCallback.get())->Call(mThisObject, rv);
       break;
-    case nsIDocument::eDetached:
-      static_cast<LifecycleDetachedCallback *>(mCallback.get())->Call(mThisObject, rv);
+    case nsIDocument::eDisconnected:
+      static_cast<LifecycleDisconnectedCallback *>(mCallback.get())->Call(mThisObject, rv);
       break;
     case nsIDocument::eAttributeChanged:
       static_cast<LifecycleAttributeChangedCallback *>(mCallback.get())->Call(mThisObject,
@@ -354,9 +354,9 @@ CustomElementRegistry::CreateCustomElementCallback(
       }
       break;
 
-    case nsIDocument::eDetached:
-      if (aDefinition->mCallbacks->mDetachedCallback.WasPassed()) {
-        func = aDefinition->mCallbacks->mDetachedCallback.Value();
+    case nsIDocument::eDisconnected:
+      if (aDefinition->mCallbacks->mDisconnectedCallback.WasPassed()) {
+        func = aDefinition->mCallbacks->mDisconnectedCallback.Value();
       }
       break;
 
@@ -1025,14 +1025,14 @@ CustomElementReactionsStack::Enqueue(Element* aElement,
 
   // Add element to the current element queue.
   if (!mReactionsStack.IsEmpty()) {
-    mReactionsStack.LastElement()->AppendElement(do_GetWeakReference(aElement));
+    mReactionsStack.LastElement()->AppendElement(aElement);
     elementData->mReactionQueue.AppendElement(aReaction);
     return;
   }
 
   // If the custom element reactions stack is empty, then:
   // Add element to the backup element queue.
-  mBackupQueue.AppendElement(do_GetWeakReference(aElement));
+  mBackupQueue.AppendElement(aElement);
   elementData->mReactionQueue.AppendElement(aReaction);
 
   if (mIsBackupQueueProcessing) {
@@ -1070,14 +1070,18 @@ CustomElementReactionsStack::InvokeReactions(ElementQueue* aElementQueue,
 
   // Note: It's possible to re-enter this method.
   for (uint32_t i = 0; i < aElementQueue->Length(); ++i) {
-    nsCOMPtr<Element> element = do_QueryReferent(aElementQueue->ElementAt(i));
+    Element* element = aElementQueue->ElementAt(i);
 
     if (!element) {
       continue;
     }
 
     RefPtr<CustomElementData> elementData = element->GetCustomElementData();
-    MOZ_ASSERT(elementData, "CustomElementData should exist");
+    if (!elementData) {
+      // This happens when the document is destroyed and the element is already
+      // unlinked, no need to fire the callbacks in this case.
+      return;
+    }
 
     auto& reactions = elementData->mReactionQueue;
     for (uint32_t j = 0; j < reactions.Length(); ++j) {
@@ -1132,9 +1136,9 @@ NS_IMPL_CYCLE_COLLECTION_TRAVERSE_BEGIN(CustomElementDefinition)
     cb.NoteXPCOMChild(callbacks->mConnectedCallback.Value());
   }
 
-  if (callbacks->mDetachedCallback.WasPassed()) {
-    NS_CYCLE_COLLECTION_NOTE_EDGE_NAME(cb, "mCallbacks->mDetachedCallback");
-    cb.NoteXPCOMChild(callbacks->mDetachedCallback.Value());
+  if (callbacks->mDisconnectedCallback.WasPassed()) {
+    NS_CYCLE_COLLECTION_NOTE_EDGE_NAME(cb, "mCallbacks->mDisconnectedCallback");
+    cb.NoteXPCOMChild(callbacks->mDisconnectedCallback.Value());
   }
 
   NS_CYCLE_COLLECTION_NOTE_EDGE_NAME(cb, "mConstructor");
diff --git a/dom/base/CustomElementRegistry.h b/dom/base/CustomElementRegistry.h
index f73a8707e..30e2c00ab 100644
--- a/dom/base/CustomElementRegistry.h
+++ b/dom/base/CustomElementRegistry.h
@@ -256,11 +256,12 @@ public:
   {
   }
 
-  // nsWeakPtr is a weak pointer of Element
+  // Hold a strong reference of Element so that it does not get cycle collected
+  // before the reactions in its reaction queue are invoked.
   // The element reaction queues are stored in CustomElementData.
   // We need to lookup ElementReactionQueueMap again to get relevant reaction queue.
   // The choice of 1 for the auto size here is based on gut feeling.
-  typedef AutoTArray<nsWeakPtr, 1> ElementQueue;
+  typedef AutoTArray<RefPtr<Element>, 1> ElementQueue;
 
   /**
    * Enqueue a custom element upgrade reaction
diff --git a/dom/base/Element.cpp b/dom/base/Element.cpp
index bfa0869c3..bc1e27ba7 100644
--- a/dom/base/Element.cpp
+++ b/dom/base/Element.cpp
@@ -1983,11 +1983,12 @@ Element::UnbindFromTree(bool aDeep, bool aNullParent)
 
     document->ClearBoxObjectFor(this);
 
-    // Detached must be enqueued whenever custom element is removed from
-    // the document and this document has a browsing context.
-    if (GetCustomElementData() && document->GetDocShell()) {
-      // Enqueue a detached callback for the custom element.
-      nsContentUtils::EnqueueLifecycleCallback(nsIDocument::eDetached, this);
+     // Disconnected must be enqueued whenever a connected custom element becomes
+     // disconnected.
+    if (CustomElementRegistry::IsCustomElementEnabled() &&
+        GetCustomElementData()) {
+      nsContentUtils::EnqueueLifecycleCallback(nsIDocument::eDisconnected,
+                                               this);
     }
   }
 
diff --git a/dom/base/nsIDocument.h b/dom/base/nsIDocument.h
index 0f05bbfcb..c620d1d09 100644
--- a/dom/base/nsIDocument.h
+++ b/dom/base/nsIDocument.h
@@ -2581,7 +2581,7 @@ public:
   enum ElementCallbackType {
     eCreated,
     eConnected,
-    eDetached,
+    eDisconnected,
     eAttributeChanged
   };
 
diff --git a/dom/webidl/WebComponents.webidl b/dom/webidl/WebComponents.webidl
index 8623bcbae..2c2d9fb53 100644
--- a/dom/webidl/WebComponents.webidl
+++ b/dom/webidl/WebComponents.webidl
@@ -12,7 +12,7 @@
 
 callback LifecycleCreatedCallback = void();
 callback LifecycleConnectedCallback = void();
-callback LifecycleDetachedCallback = void();
+callback LifecycleDisconnectedCallback = void();
 callback LifecycleAttributeChangedCallback = void(DOMString attrName,
                                                   DOMString? oldValue,
                                                   DOMString? newValue,
@@ -21,7 +21,7 @@ callback LifecycleAttributeChangedCallback = void(DOMString attrName,
 dictionary LifecycleCallbacks {
   LifecycleCreatedCallback? createdCallback;
   LifecycleConnectedCallback? connectedCallback;
-  LifecycleDetachedCallback? detachedCallback;
+  LifecycleDisconnectedCallback? disconnectedCallback;
   LifecycleAttributeChangedCallback? attributeChangedCallback;
 };
 
diff --git a/testing/web-platform/meta/custom-elements/disconnected-callbacks.html.ini b/testing/web-platform/meta/custom-elements/disconnected-callbacks.html.ini
index 631dc8b5d..03c6f9937 100644
--- a/testing/web-platform/meta/custom-elements/disconnected-callbacks.html.ini
+++ b/testing/web-platform/meta/custom-elements/disconnected-callbacks.html.ini
@@ -1,11 +1,5 @@
 [disconnected-callbacks.html]
   type: testharness
-  [Removing a custom element from the document must enqueue and invoke disconnectedCallback]
-    expected: FAIL
-
-  [Removing an ancestor of custom element from the document must enqueue and invoke disconnectedCallback]
-    expected: FAIL
-
   [Removing a custom element from a shadow tree in the document must enqueue and invoke disconnectedCallback]
     expected: FAIL
 
@@ -15,12 +9,6 @@
   [Removing a custom element from a detached shadow tree that belongs to the document must not enqueue and invoke disconnectedCallback]
     expected: FAIL
 
-  [Removing a custom element from the document of the template elements must enqueue and invoke disconnectedCallback]
-    expected: FAIL
-
-  [Removing an ancestor of custom element from the document of the template elements must enqueue and invoke disconnectedCallback]
-    expected: FAIL
-
   [Removing a custom element from a shadow tree in the document of the template elements must enqueue and invoke disconnectedCallback]
     expected: FAIL
 
@@ -30,12 +18,6 @@
   [Removing a custom element from a detached shadow tree that belongs to the document of the template elements must not enqueue and invoke disconnectedCallback]
     expected: FAIL
 
-  [Removing a custom element from a new document must enqueue and invoke disconnectedCallback]
-    expected: FAIL
-
-  [Removing an ancestor of custom element from a new document must enqueue and invoke disconnectedCallback]
-    expected: FAIL
-
   [Removing a custom element from a shadow tree in a new document must enqueue and invoke disconnectedCallback]
     expected: FAIL
 
@@ -45,12 +27,6 @@
   [Removing a custom element from a detached shadow tree that belongs to a new document must not enqueue and invoke disconnectedCallback]
     expected: FAIL
 
-  [Removing a custom element from a cloned document must enqueue and invoke disconnectedCallback]
-    expected: FAIL
-
-  [Removing an ancestor of custom element from a cloned document must enqueue and invoke disconnectedCallback]
-    expected: FAIL
-
   [Removing a custom element from a shadow tree in a cloned document must enqueue and invoke disconnectedCallback]
     expected: FAIL
 
@@ -60,12 +36,6 @@
   [Removing a custom element from a detached shadow tree that belongs to a cloned document must not enqueue and invoke disconnectedCallback]
     expected: FAIL
 
-  [Removing a custom element from a document created by createHTMLDocument must enqueue and invoke disconnectedCallback]
-    expected: FAIL
-
-  [Removing an ancestor of custom element from a document created by createHTMLDocument must enqueue and invoke disconnectedCallback]
-    expected: FAIL
-
   [Removing a custom element from a shadow tree in a document created by createHTMLDocument must enqueue and invoke disconnectedCallback]
     expected: FAIL
 
@@ -75,12 +45,6 @@
   [Removing a custom element from a detached shadow tree that belongs to a document created by createHTMLDocument must not enqueue and invoke disconnectedCallback]
     expected: FAIL
 
-  [Removing a custom element from an HTML document created by createDocument must enqueue and invoke disconnectedCallback]
-    expected: FAIL
-
-  [Removing an ancestor of custom element from an HTML document created by createDocument must enqueue and invoke disconnectedCallback]
-    expected: FAIL
-
   [Removing a custom element from a shadow tree in an HTML document created by createDocument must enqueue and invoke disconnectedCallback]
     expected: FAIL
 
@@ -90,12 +54,6 @@
   [Removing a custom element from a detached shadow tree that belongs to an HTML document created by createDocument must not enqueue and invoke disconnectedCallback]
     expected: FAIL
 
-  [Removing a custom element from the document of an iframe must enqueue and invoke disconnectedCallback]
-    expected: FAIL
-
-  [Removing an ancestor of custom element from the document of an iframe must enqueue and invoke disconnectedCallback]
-    expected: FAIL
-
   [Removing a custom element from a shadow tree in the document of an iframe must enqueue and invoke disconnectedCallback]
     expected: FAIL
 
@@ -105,12 +63,6 @@
   [Removing a custom element from a detached shadow tree that belongs to the document of an iframe must not enqueue and invoke disconnectedCallback]
     expected: FAIL
 
-  [Removing a custom element from an HTML document fetched by XHR must enqueue and invoke disconnectedCallback]
-    expected: FAIL
-
-  [Removing an ancestor of custom element from an HTML document fetched by XHR must enqueue and invoke disconnectedCallback]
-    expected: FAIL
-
   [Removing a custom element from a shadow tree in an HTML document fetched by XHR must enqueue and invoke disconnectedCallback]
     expected: FAIL
 
diff --git a/testing/web-platform/meta/custom-elements/reactions/ChildNode.html.ini b/testing/web-platform/meta/custom-elements/reactions/ChildNode.html.ini
index 637a74dbc..edef1ed9f 100644
--- a/testing/web-platform/meta/custom-elements/reactions/ChildNode.html.ini
+++ b/testing/web-platform/meta/custom-elements/reactions/ChildNode.html.ini
@@ -8,7 +8,3 @@
 
   [replaceWith on ChildNode must enqueue a disconnected reaction, an adopted reaction, and a connected reaction when the custom element was in another document]
     expected: FAIL
-
-  [replaceWith on ChildNode must enqueue a disconnected reaction]
-    expected: FAIL
-
diff --git a/testing/web-platform/meta/custom-elements/reactions/Node.html.ini b/testing/web-platform/meta/custom-elements/reactions/Node.html.ini
index e6bacabbd..b65e80813 100644
--- a/testing/web-platform/meta/custom-elements/reactions/Node.html.ini
+++ b/testing/web-platform/meta/custom-elements/reactions/Node.html.ini
@@ -29,7 +29,3 @@
 
   [replaceChild on ChildNode must enqueue a disconnected reaction, an adopted reaction, and a connected reaction when the custom element was in another document]
     expected: FAIL
-
-  [removeChild on ChildNode must enqueue a disconnected reaction]
-    expected: FAIL
-
diff --git a/testing/web-platform/meta/custom-elements/reactions/Range.html.ini b/testing/web-platform/meta/custom-elements/reactions/Range.html.ini
index 21de92aa3..e44d6d389 100644
--- a/testing/web-platform/meta/custom-elements/reactions/Range.html.ini
+++ b/testing/web-platform/meta/custom-elements/reactions/Range.html.ini
@@ -1,11 +1,5 @@
 [Range.html]
   type: testharness
-  [deleteContents on Range must enqueue a disconnected reaction]
-    expected: FAIL
-
-  [extractContents on Range must enqueue a disconnected reaction]
-    expected: FAIL
-
   [cloneContents on Range must enqueue an attributeChanged reaction when cloning an element with an observed attribute]
     expected: FAIL