Bug 1334044: Replace detached callback (v0) with disconnected callback (v1). r=smaug
authorJessica Jong <jjong@mozilla.com>
Tue, 26 Sep 2017 13:56:11 +0800
changeset 670370 3dd36c1c48e3b5c83808efd5dd871da731da1758
parent 670369 b900023d886cf52415f4eda41efa03da7429c084
child 670371 9e7267c06e4e7fb037e539809b409519ea3d63c0
push id81612
push userbmo:dharvey@mozilla.com
push dateTue, 26 Sep 2017 10:16:26 +0000
reviewerssmaug
bugs1334044
milestone58.0a1
Bug 1334044: Replace detached callback (v0) with disconnected callback (v1). r=smaug MozReview-Commit-ID: 8jxFK1fze15
dom/base/CustomElementRegistry.cpp
dom/base/CustomElementRegistry.h
dom/base/Element.cpp
dom/base/nsIDocument.h
dom/webidl/WebComponents.webidl
testing/web-platform/meta/custom-elements/disconnected-callbacks.html.ini
testing/web-platform/meta/custom-elements/reactions/ChildNode.html.ini
testing/web-platform/meta/custom-elements/reactions/Document.html.ini
testing/web-platform/meta/custom-elements/reactions/Element.html.ini
testing/web-platform/meta/custom-elements/reactions/HTMLElement.html.ini
testing/web-platform/meta/custom-elements/reactions/HTMLOptionsCollection.html.ini
testing/web-platform/meta/custom-elements/reactions/HTMLSelectElement.html.ini
testing/web-platform/meta/custom-elements/reactions/HTMLTitleElement.html.ini
testing/web-platform/meta/custom-elements/reactions/Node.html.ini
testing/web-platform/meta/custom-elements/reactions/Range.html.ini
testing/web-platform/meta/custom-elements/reactions/Selection.html.ini
--- a/dom/base/CustomElementRegistry.cpp
+++ b/dom/base/CustomElementRegistry.cpp
@@ -53,18 +53,18 @@ CustomElementCallback::Call()
 
       static_cast<LifecycleCreatedCallback *>(mCallback.get())->Call(mThisObject, rv);
       mOwnerData->mElementIsBeingCreated = false;
       break;
     }
     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,
         mArgs.name, mArgs.oldValue, mArgs.newValue, mArgs.namespaceURI, rv);
       break;
   }
 }
 
@@ -340,19 +340,19 @@ CustomElementRegistry::CreateCustomEleme
       break;
 
     case nsIDocument::eConnected:
       if (aDefinition->mCallbacks->mConnectedCallback.WasPassed()) {
         func = aDefinition->mCallbacks->mConnectedCallback.Value();
       }
       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;
 
     case nsIDocument::eAttributeChanged:
       if (aDefinition->mCallbacks->mAttributeChangedCallback.WasPassed()) {
         func = aDefinition->mCallbacks->mAttributeChangedCallback.Value();
       }
       break;
@@ -1029,24 +1029,24 @@ void
 CustomElementReactionsStack::Enqueue(Element* aElement,
                                      CustomElementReaction* aReaction)
 {
   RefPtr<CustomElementData> elementData = aElement->GetCustomElementData();
   MOZ_ASSERT(elementData, "CustomElementData should exist");
 
   // 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) {
     return;
   }
 
   CycleCollectedJSContext* context = CycleCollectedJSContext::Get();
   RefPtr<ProcessBackupQueueRunnable> processBackupQueueRunnable =
@@ -1074,24 +1074,28 @@ CustomElementReactionsStack::InvokeReact
   // This is used for error reporting.
   Maybe<AutoEntryScript> aes;
   if (aGlobal) {
     aes.emplace(aGlobal, "custom elements reaction invocation");
   }
 
   // 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) {
       // Transfer the ownership of the entry due to reentrant invocation of
       // this funciton. The entry will be removed when bug 1379573 is landed.
       auto reaction(Move(reactions.ElementAt(j)));
       if (reaction) {
         ErrorResult rv;
@@ -1136,19 +1140,19 @@ NS_IMPL_CYCLE_COLLECTION_TRAVERSE_BEGIN(
     cb.NoteXPCOMChild(callbacks->mCreatedCallback.Value());
   }
 
   if (callbacks->mConnectedCallback.WasPassed()) {
     NS_CYCLE_COLLECTION_NOTE_EDGE_NAME(cb, "mCallbacks->mConnectedCallback");
     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");
   cb.NoteXPCOMChild(tmp->mConstructor);
 NS_IMPL_CYCLE_COLLECTION_TRAVERSE_END
 
 NS_IMPL_CYCLE_COLLECTION_TRACE_BEGIN(CustomElementDefinition)
   NS_IMPL_CYCLE_COLLECTION_TRACE_JS_MEMBER_CALLBACK(mPrototype)
--- a/dom/base/CustomElementRegistry.h
+++ b/dom/base/CustomElementRegistry.h
@@ -253,21 +253,22 @@ class CustomElementReactionsStack
 public:
   NS_INLINE_DECL_REFCOUNTING(CustomElementReactionsStack)
 
   CustomElementReactionsStack()
     : mIsBackupQueueProcessing(false)
   {
   }
 
-  // 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
    * https://html.spec.whatwg.org/multipage/scripting.html#enqueue-a-custom-element-upgrade-reaction
    */
   void EnqueueUpgradeReaction(Element* aElement,
                               CustomElementDefinition* aDefinition);
 
--- a/dom/base/Element.cpp
+++ b/dom/base/Element.cpp
@@ -1943,21 +1943,22 @@ Element::UnbindFromTree(bool aDeep, bool
           document,
           binding->GetAnonymousContent(),
           /* aNullParent */ false);
       }
     }
 
     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);
     }
   }
 
   // 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.
   if (IsHTMLElement()) {
     ResetDir(this);
--- a/dom/base/nsIDocument.h
+++ b/dom/base/nsIDocument.h
@@ -2738,17 +2738,17 @@ public:
   Element* GetDocumentElement() const
   {
     return GetRootElement();
   }
 
   enum ElementCallbackType {
     eCreated,
     eConnected,
-    eDetached,
+    eDisconnected,
     eAttributeChanged
   };
 
   nsIDocument* GetTopLevelContentDocument();
 
   virtual void
     RegisterElement(JSContext* aCx, const nsAString& aName,
                     const mozilla::dom::ElementRegistrationOptions& aOptions,
--- a/dom/webidl/WebComponents.webidl
+++ b/dom/webidl/WebComponents.webidl
@@ -7,25 +7,25 @@
  * http://dvcs.w3.org/hg/webcomponents/raw-file/tip/spec/custom/index.html
  *
  * Copyright © 2012 W3C® (MIT, ERCIM, Keio), All Rights Reserved. W3C
  * liability, trademark and document use rules apply.
  */
 
 callback LifecycleCreatedCallback = void();
 callback LifecycleConnectedCallback = void();
-callback LifecycleDetachedCallback = void();
+callback LifecycleDisconnectedCallback = void();
 callback LifecycleAttributeChangedCallback = void(DOMString attrName,
                                                   DOMString? oldValue,
                                                   DOMString? newValue,
                                                   DOMString? namespaceURI);
 
 dictionary LifecycleCallbacks {
   LifecycleCreatedCallback? createdCallback;
   LifecycleConnectedCallback? connectedCallback;
-  LifecycleDetachedCallback? detachedCallback;
+  LifecycleDisconnectedCallback? disconnectedCallback;
   LifecycleAttributeChangedCallback? attributeChangedCallback;
 };
 
 dictionary ElementRegistrationOptions {
   object? prototype = null;
   DOMString? extends = null;
 };
--- a/testing/web-platform/meta/custom-elements/disconnected-callbacks.html.ini
+++ b/testing/web-platform/meta/custom-elements/disconnected-callbacks.html.ini
@@ -1,121 +1,73 @@
 [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
 
   [Removing the shadow host of a custom element from athe document must enqueue and invoke disconnectedCallback]
     expected: FAIL
 
   [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
 
   [Removing the shadow host of a custom element from athe document of the template elements must enqueue and invoke disconnectedCallback]
     expected: FAIL
 
   [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
 
   [Removing the shadow host of a custom element from aa new document must enqueue and invoke disconnectedCallback]
     expected: FAIL
 
   [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
 
   [Removing the shadow host of a custom element from aa cloned document must enqueue and invoke disconnectedCallback]
     expected: FAIL
 
   [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
 
   [Removing the shadow host of a custom element from aa document created by createHTMLDocument must enqueue and invoke disconnectedCallback]
     expected: FAIL
 
   [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
 
   [Removing the shadow host of a custom element from aan HTML document created by createDocument must enqueue and invoke disconnectedCallback]
     expected: FAIL
 
   [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
 
   [Removing the shadow host of a custom element from athe document of an iframe must enqueue and invoke disconnectedCallback]
     expected: FAIL
 
   [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
 
   [Removing the shadow host of a custom element from aan HTML document fetched by XHR must enqueue and invoke disconnectedCallback]
     expected: FAIL
 
   [Removing a custom element from a detached shadow tree that belongs to an HTML document fetched by XHR must not enqueue and invoke disconnectedCallback]
     expected: FAIL
--- a/testing/web-platform/meta/custom-elements/reactions/ChildNode.html.ini
+++ b/testing/web-platform/meta/custom-elements/reactions/ChildNode.html.ini
@@ -3,15 +3,8 @@
   [before on ChildNode must enqueue a disconnected reaction, an adopted reaction, and a connected reaction when the custom element was in another document]
     expected: FAIL
 
   [after 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, 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
-
-  [remove on ChildNode must enqueue a disconnected reaction]
-    expected: FAIL
-
--- a/testing/web-platform/meta/custom-elements/reactions/Document.html.ini
+++ b/testing/web-platform/meta/custom-elements/reactions/Document.html.ini
@@ -7,19 +7,16 @@
     expected: FAIL
 
   [importNode on Document must construct a new custom element when importing a custom element from a template]
     expected: FAIL
 
   [execCommand on Document must enqueue a disconnected reaction when deleting a custom element from a contenteditable element]
     expected: FAIL
 
-  [title on Document must enqueue disconnectedCallback when removing a custom element]
-    expected: FAIL
-
   [body on Document must enqueue disconnectedCallback when removing a custom element]
     expected: FAIL
 
   [open on Document must enqueue disconnectedCallback when removing a custom element]
     expected: FAIL
 
   [write on Document must enqueue disconnectedCallback when removing a custom element]
     expected: FAIL
--- a/testing/web-platform/meta/custom-elements/reactions/Element.html.ini
+++ b/testing/web-platform/meta/custom-elements/reactions/Element.html.ini
@@ -25,26 +25,20 @@
     expected: FAIL
 
   [innerHTML on Element must enqueue a connected reaction for a newly constructed custom element]
     expected: FAIL
 
   [innerHTML on Element must enqueue a attributeChanged reaction for a newly constructed custom element]
     expected: FAIL
 
-  [innerHTML on Element must enqueue a disconnected reaction]
-    expected: FAIL
-
   [outerHTML on Element must enqueue a connected reaction for a newly constructed custom element]
     expected: FAIL
 
   [outerHTML on Element must enqueue a attributeChanged reaction for a newly constructed custom element]
     expected: FAIL
 
-  [outerHTML on Element must enqueue a disconnected reaction]
-    expected: FAIL
-
   [insertAdjacentHTML on Element must enqueue a connected reaction for a newly constructed custom element]
     expected: FAIL
 
   [insertAdjacentHTML on Element must enqueue a attributeChanged reaction for a newly constructed custom element]
     expected: FAIL
 
--- a/testing/web-platform/meta/custom-elements/reactions/HTMLElement.html.ini
+++ b/testing/web-platform/meta/custom-elements/reactions/HTMLElement.html.ini
@@ -13,11 +13,8 @@
     expected: FAIL
 
   [contextMenu on HTMLElement must enqueue an attributeChanged reaction when adding contextmenu content attribute]
     expected: FAIL
 
   [contextMenu on HTMLElement must enqueue an attributeChanged reaction when replacing an existing attribute]
     expected: FAIL
 
-  [innerText on HTMLElement must enqueue a disconnected reaction]
-    expected: FAIL
-
deleted file mode 100644
--- a/testing/web-platform/meta/custom-elements/reactions/HTMLOptionsCollection.html.ini
+++ /dev/null
@@ -1,11 +0,0 @@
-[HTMLOptionsCollection.html]
-  type: testharness
-  [length on HTMLOptionsCollection must enqueue disconnectedCallback when removing a custom element]
-    expected: FAIL
-
-  [The indexed setter on HTMLOptionsCollection must enqueue disconnectedCallback when removing a custom element]
-    expected: FAIL
-
-  [remove on HTMLOptionsCollection must enqueue disconnectedCallback when removing a custom element]
-    expected: FAIL
-
deleted file mode 100644
--- a/testing/web-platform/meta/custom-elements/reactions/HTMLSelectElement.html.ini
+++ /dev/null
@@ -1,11 +0,0 @@
-[HTMLSelectElement.html]
-  type: testharness
-  [length on HTMLSelectElement must enqueue disconnectedCallback when removing a custom element]
-    expected: FAIL
-
-  [The indexed setter on HTMLSelectElement must enqueue disconnectedCallback when removing a custom element]
-    expected: FAIL
-
-  [remove on HTMLSelectElement must enqueue disconnectedCallback when removing a custom element]
-    expected: FAIL
-
deleted file mode 100644
--- a/testing/web-platform/meta/custom-elements/reactions/HTMLTitleElement.html.ini
+++ /dev/null
@@ -1,5 +0,0 @@
-[HTMLTitleElement.html]
-  type: testharness
-  [text on HTMLTitleElement must enqueue disconnectedCallback when removing a custom element]
-    expected: FAIL
-
--- a/testing/web-platform/meta/custom-elements/reactions/Node.html.ini
+++ b/testing/web-platform/meta/custom-elements/reactions/Node.html.ini
@@ -12,12 +12,8 @@
   [insertBefore on ChildNode must enqueue a disconnected reaction, an adopted reaction, and a connected reaction when the custom element was in another document]
     expected: FAIL
 
   [appendChild on ChildNode must enqueue a disconnected reaction, an adopted reaction, and a connected reaction when the custom element was in another document]
     expected: FAIL
 
   [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
-
--- a/testing/web-platform/meta/custom-elements/reactions/Range.html.ini
+++ b/testing/web-platform/meta/custom-elements/reactions/Range.html.ini
@@ -1,16 +1,10 @@
 [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
 
   [cloneContents on Range must not enqueue an attributeChanged reaction when cloning an element with an unobserved attribute]
     expected: FAIL
 
   [cloneContents on Range must enqueue an attributeChanged reaction when cloning an element only for observed attributes]
     expected: FAIL
deleted file mode 100644
--- a/testing/web-platform/meta/custom-elements/reactions/Selection.html.ini
+++ /dev/null
@@ -1,5 +0,0 @@
-[Selection.html]
-  type: testharness
-  [deleteFromDocument on Selection must enqueue a disconnected reaction]
-    expected: FAIL
-