Bug 1514098 - Don't call into UA Widget distructor if the element is being CC'd r=peterv,bgrins
authorTimothy Guan-tin Chien <timdream@gmail.com>
Mon, 04 Feb 2019 13:56:22 +0000
changeset 457245 4d21e1285e2a334414eb1d45446fdea9c4922072
parent 457244 1c3a7347d22489324a49ad670e2c7228fe1ee7f0
child 457246 65be959384732ebcb4d2329dc35823988722aa52
push id111711
push usercsabou@mozilla.com
push dateTue, 05 Feb 2019 23:12:20 +0000
treeherdermozilla-inbound@48b467365ea8 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerspeterv, bgrins
bugs1514098
milestone67.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 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
dom/base/Element.cpp
dom/base/Element.h
toolkit/actors/UAWidgetsChild.jsm
toolkit/content/widgets/docs/ua_widget.rst
--- a/dom/base/Element.cpp
+++ b/dom/base/Element.cpp
@@ -1243,17 +1243,17 @@ void Element::AttachAndSetUAShadowRoot()
         shadowRoot->SetIsUAWidget();
       }));
 }
 
 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(
       "Element::NotifyUAWidgetSetupOrChange::UAWidgetSetupOrChange",
       [self = RefPtr<Element>(this),
        ownerDoc = RefPtr<Document>(OwnerDoc())]() {
         MOZ_ASSERT(self->GetShadowRoot() &&
                    self->GetShadowRoot()->IsUAWidget());
@@ -1273,16 +1273,24 @@ void Element::NotifyUAWidgetTeardown(Una
       [aUnattachShadowRoot, self = RefPtr<Element>(this),
        ownerDoc = RefPtr<Document>(OwnerDoc())]() {
         if (!self->GetShadowRoot()) {
           // No UA Widget Shadow Root was ever attached.
           return;
         }
         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);
         if (NS_WARN_IF(NS_FAILED(rv))) {
           return;
         }
 
         if (aUnattachShadowRoot == UnattachShadowRoot::Yes) {
--- a/dom/base/Element.h
+++ b/dom/base/Element.h
@@ -1213,17 +1213,17 @@ class Element : public FragmentOrElement
 
   already_AddRefed<ShadowRoot> AttachShadowWithoutNameChecks(
       ShadowRootMode aMode);
 
   // Attach UA Shadow Root if it is not attached.
   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 {
     No,
     Yes,
   };
 
   // Dispatch an event to UAWidgetsChild, triggering UA Widget destruction.
--- a/toolkit/actors/UAWidgetsChild.jsm
+++ b/toolkit/actors/UAWidgetsChild.jsm
@@ -32,19 +32,19 @@ class UAWidgetsChild extends ActorChild 
   }
 
   setupOrNotifyWidget(aElement) {
     let widget = this.widgets.get(aElement);
     if (!widget) {
       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);
       }
     }
   }
 
   setupWidget(aElement) {
     let uri;
@@ -85,35 +85,34 @@ class UAWidgetsChild extends ActorChild 
     let sandbox = isSystemPrincipal ?
       Object.create(null) : Cu.getUAWidgetScope(aElement.nodePrincipal);
 
     if (!sandbox[widgetName]) {
       Services.scriptloader.loadSubScript(uri, sandbox);
     }
 
     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();
-      }
+      widget.onsetup();
     } catch (ex) {
       Cu.reportError(ex);
     }
   }
 
   teardownWidget(aElement) {
     let widget = this.widgets.get(aElement);
     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);
       }
     }
     this.widgets.delete(aElement);
   }
 }
--- a/toolkit/content/widgets/docs/ua_widget.rst
+++ b/toolkit/content/widgets/docs/ua_widget.rst
@@ -14,16 +14,18 @@ UA Widgets are generally constructed whe
 When the element is appended to the tree, a chrome-only ``UAWidgetSetupOrChange`` event is dispatched and is caught by a frame script, namely UAWidgetsChild.
 
 UAWidgetsChild then grabs the sandbox for that origin (lazily creating it as needed), loads the script as needed, and initializes an instance by calling the JS constructor with a reference to the UA Widget Shadow Root created by the DOM. We will discuss the sandbox in the latter section.
 
 The ``onsetup`` method is called right after the instance is constructed. The call to constructor must not throw, or UAWidgetsChild will be confused since an instance of the widget will not be returned, but the widget is already half-initalized. If the ``onsetup`` method call throws, UAWidgetsChild will still be able to hold the reference of the widget and call the destructor later on.
 
 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.
 
 Likewise, the datetime box widget is only loaded when the ``type`` attribute of an ``<input>`` is either `date` or `time`.
 
 The specialization does not apply to the lifecycle of the UA Widget Shadow Root. It is always constructed in order to suppress children of the DOM element from the web content from receiving a layout frame.