Bug 1514098 - Don't call into UA Widget distructor if the element is being CC'd r=peterv,bgrins

Differential Revision: https://phabricator.services.mozilla.com/D18130

--HG--
extra : moz-landing-system : lando

dom/base/Element.cpp

@@ -1248,7 +1248,7 @@ void Element::NotifyUAWidgetSetupOrChange() {
   MOZ_ASSERT(IsInComposedDoc());
   // Schedule a runnable, ensure the event dispatches before
   // returning to content script.
-  // This event cause UA Widget to construct or cause onattributechange callback
+  // This event cause UA Widget to construct or cause onchange callback
   // of existing UA Widget to run; dispatching this event twice should not cause
   // UA Widget to re-init.
   nsContentUtils::AddScriptRunner(NS_NewRunnableFunction(
@@ -1278,6 +1278,14 @@ void Element::NotifyUAWidgetTeardown(UnattachShadowRoot aUnattachShadowRoot) {
         }
         MOZ_ASSERT(self->GetShadowRoot()->IsUAWidget());
 
+        // Bail out if the element is being collected by CC
+        bool hasHadScriptObject = true;
+        nsIScriptGlobalObject* scriptObject =
+            ownerDoc->GetScriptHandlingObject(hasHadScriptObject);
+        if (!scriptObject && hasHadScriptObject) {
+          return;
+        }
+
         nsresult rv = nsContentUtils::DispatchChromeEvent(
             ownerDoc, self, NS_LITERAL_STRING("UAWidgetTeardown"),
             CanBubble::eYes, Cancelable::eNo);

dom/base/Element.h

@@ -1218,7 +1218,7 @@ class Element : public FragmentOrElement {
   void AttachAndSetUAShadowRoot();
 
   // Dispatch an event to UAWidgetsChild, triggering construction
-  // or onattributechange callback on the existing widget.
+  // or onchange callback on the existing widget.
   void NotifyUAWidgetSetupOrChange();
 
   enum class UnattachShadowRoot {

toolkit/actors/UAWidgetsChild.jsm

@@ -37,9 +37,9 @@ class UAWidgetsChild extends ActorChild {
       this.setupWidget(aElement);
       return;
     }
-    if (typeof widget.wrappedJSObject.onchange == "function") {
+    if (typeof widget.onchange == "function") {
       try {
-        widget.wrappedJSObject.onchange();
+        widget.onchange();
       } catch (ex) {
         Cu.reportError(ex);
       }
@@ -90,13 +90,12 @@ class UAWidgetsChild extends ActorChild {
     }
 
     let widget = new sandbox[widgetName](shadowRoot);
+    if (!isSystemPrincipal) {
+      widget = widget.wrappedJSObject;
+    }
     this.widgets.set(aElement, widget);
     try {
-      if (!isSystemPrincipal) {
-        widget.wrappedJSObject.onsetup();
-      } else {
       widget.onsetup();
-      }
     } catch (ex) {
       Cu.reportError(ex);
     }
@@ -107,9 +106,9 @@ class UAWidgetsChild extends ActorChild {
     if (!widget) {
       return;
     }
-    if (typeof widget.wrappedJSObject.destructor == "function") {
+    if (typeof widget.destructor == "function") {
       try {
-        widget.wrappedJSObject.destructor();
+        widget.destructor();
       } catch (ex) {
         Cu.reportError(ex);
       }

toolkit/content/widgets/docs/ua_widget.rst

@@ -19,6 +19,8 @@ The ``onsetup`` method is called right after the instance is constructed. The ca
 
 When the element is removed from the tree, ``UAWidgetTeardown`` is dispatched so UAWidgetsChild can destroy the widget, if it exists. If so, the UAWidgetsChild calls the ``destructor()`` method on the widget, causing the widget to destruct itself.
 
+Counter-intuitively, elements are not considered "removed from the tree" when the document is unloaded. This is considered safe as anything the widget touches should be reset or cleaned up when the document unloads. Please do not violate the assumption by having any browser state toggled by the destructor.
+
 When a UA Widget initializes, it should create its own DOM inside the passed UA Widget Shadow Root, including the ``<link>`` element necessary to load the stylesheet, add event listeners, etc. When destroyed (i.e. the destructor method is called), it should do the opposite.
 
 **Specialization**: for video controls, we do not want to do the work if the control is not needed (i.e. when the ``<video>`` or ``<audio>`` element has no "controls" attribute set), so we forgo dispatching the event from HTMLMediaElement in the BindToTree method. Instead, another ``UAWidgetSetupOrChange`` event will cause the sandbox and the widget instance to construct when the attribute is set to true. The same event is also responsible for triggering the ``onchange()`` method on UA Widgets if the widget is already initialized.