Bug 1511130 - Part II, Allow UAWidgetsChild to destruct widgets even if it throws during construction r=bgrins

This patch move the actual widget construction to a onsetup method, allow UAWidgetsChild to hold the reference of the widget instance even if the actual setup (happens in the onsetup call) throws. With the reference of the widget kept, UAWidgetsChild will finally able to call its destructor later on.

Depends on D13607

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

--HG--
extra : moz-landing-system : lando
This commit is contained in:
Timothy Guan-tin Chien 2018-12-13 20:59:00 +00:00
Родитель 9d9da75ab4
Коммит 0bdd6f623a
9 изменённых файлов: 94 добавлений и 20 удалений

Просмотреть файл

@ -2,7 +2,7 @@
<script>
window.onload=function() {
b.src=document.getElementById('c').innerHTML;
b.setAttribute('src', 'data:audio/mpeg,');
b.setAttribute('src', 'video-crash.webm');
document.documentElement.style.display='none';
window.top.open('');
var o = window.frames[0].document.body.childNodes[0];

Просмотреть файл

@ -0,0 +1,12 @@
<script>
document.addEventListener("DOMContentLoaded", function(){
var o = window.frames[0].document.body.childNodes[0];
a.appendChild(o.parentNode.removeChild(o));
o = document.body;
o.parentNode.appendChild(o);
o.parentNode.appendChild(o);
})
</script>
<body text=''>
<hgroup id='a'>
<iframe src="video-crash.webm"></iframe>

Просмотреть файл

@ -25,3 +25,4 @@ pref(browser.link.open_newwindow,2) load 1429507_2.html # window.open() in tab d
load 1453030.html
skip-if(Android) load 1490700.html # No screenshare on Android
load 1505957.html
load 1511130.html

Просмотреть файл

@ -38,8 +38,12 @@ class UAWidgetsChild extends ActorChild {
this.setupWidget(aElement);
return;
}
if (typeof widget.wrappedJSObject.onattributechange == "function") {
widget.wrappedJSObject.onattributechange();
if (typeof widget.wrappedJSObject.onchange == "function") {
try {
widget.wrappedJSObject.onchange();
} catch (ex) {
Cu.reportError(ex);
}
}
}
@ -50,7 +54,7 @@ class UAWidgetsChild extends ActorChild {
case "video":
case "audio":
uri = "chrome://global/content/elements/videocontrols.js";
widgetName = "VideoControlsPageWidget";
widgetName = "VideoControlsWidget";
break;
case "input":
uri = "chrome://global/content/elements/datetimebox.js";
@ -72,7 +76,13 @@ class UAWidgetsChild extends ActorChild {
}
let shadowRoot = aElement.openOrClosedShadowRoot;
let sandbox = aElement.nodePrincipal.isSystemPrincipal ?
if (!shadowRoot) {
Cu.reportError("Getting a UAWidgetBindToTree/UAWidgetAttributeChanged event without the Shadow Root.");
return;
}
let isSystemPrincipal = aElement.nodePrincipal.isSystemPrincipal;
let sandbox = isSystemPrincipal ?
Object.create(null) : Cu.getUAWidgetScope(aElement.nodePrincipal);
if (!sandbox[widgetName]) {
@ -81,6 +91,15 @@ class UAWidgetsChild extends ActorChild {
let widget = new sandbox[widgetName](shadowRoot);
this.widgets.set(aElement, widget);
try {
if (!isSystemPrincipal) {
widget.wrappedJSObject.onsetup();
} else {
widget.onsetup();
}
} catch (ex) {
Cu.reportError(ex);
}
}
teardownWidget(aElement) {
@ -89,7 +108,11 @@ class UAWidgetsChild extends ActorChild {
return;
}
if (typeof widget.wrappedJSObject.destructor == "function") {
widget.wrappedJSObject.destructor();
try {
widget.wrappedJSObject.destructor();
} catch (ex) {
Cu.reportError(ex);
}
}
this.widgets.delete(aElement);
}

Просмотреть файл

@ -17,14 +17,19 @@ this.DateTimeBoxWidget = class {
this.element = shadowRoot.host;
this.document = this.element.ownerDocument;
this.window = this.document.defaultView;
}
/*
* Callback called by UAWidgets right after constructor.
*/
onsetup() {
this.switchImpl();
}
/*
* Callback called by UAWidgets when the "type" property changes.
*/
onattributechange() {
onchange() {
this.switchImpl();
}
@ -52,6 +57,7 @@ this.DateTimeBoxWidget = class {
}
if (newImpl) {
this.impl = new newImpl(this.shadowRoot);
this.impl.onsetup();
} else {
this.impl = undefined;
}
@ -73,12 +79,14 @@ this.DateTimeInputBaseImplWidget = class {
this.element = shadowRoot.host;
this.document = this.element.ownerDocument;
this.window = this.document.defaultView;
}
onsetup() {
this.generateContent();
this.DEBUG = false;
this.mDateTimeBoxElement = shadowRoot.firstChild;
this.mDateTimeBoxElement = this.shadowRoot.firstChild;
this.mInputElement = this.element;
this.mLocales = this.window.getRegionalPrefsLocales();
@ -628,6 +636,10 @@ this.DateTimeInputBaseImplWidget = class {
this.DateInputImplWidget = class extends DateTimeInputBaseImplWidget {
constructor(shadowRoot) {
super(shadowRoot);
}
onsetup() {
super.onsetup();
this.mMinMonth = 1;
this.mMaxMonth = 12;
@ -966,6 +978,10 @@ this.DateInputImplWidget = class extends DateTimeInputBaseImplWidget {
this.TimeInputImplWidget = class extends DateTimeInputBaseImplWidget {
constructor(shadowRoot) {
super(shadowRoot);
}
onsetup() {
super.onsetup();
const kDefaultAMString = "AM";
const kDefaultPMString = "PM";

Просмотреть файл

@ -15,11 +15,13 @@ When the element is appended to the tree, a chrome-only ``UAWidgetBindToTree`` e
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, ``UAWidgetUnbindFromTree`` 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.
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, a ``UAWidgetAttributeChanged`` 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 ``onattributechange()`` method on UA Widgets if the widget is already initialized.
**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 ``UAWidgetAttributeChanged`` 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`.
@ -47,6 +49,6 @@ Other things to watch out for
As part of the implementation of the Web Platform, it is important to make sure the web-observable characteristics of the widget correctly reflect what the script on the web expects.
* Do not dispatch non-spec compliant events on the UA Widget Shadow Root host element, as event listeners in web content scripts can access them.
* The layout and the dimensions of the widget should be ready by the time the constructor returns, since they can be detectable as soon as the content script gets the reference of the host element (i.e. when ``appendChild()`` returns). In order to make this easier we load ``<link>`` elements load chrome stylesheets synchronously when inside a UA Widget Shadow DOM.
* The layout and the dimensions of the widget should be ready by the time the constructor and ``onsetup`` returns, since they can be detectable as soon as the content script gets the reference of the host element (i.e. when ``appendChild()`` returns). In order to make this easier we load ``<link>`` elements load chrome stylesheets synchronously when inside a UA Widget Shadow DOM.
* There shouldn't be any white-spaces nodes in the Shadow DOM, because UA Widget could be placed inside ``white-space: pre``. See bug 1502205.
* CSP will block inline styles in the Shadow DOM. ``<link>`` is the only safe way to load styles.

Просмотреть файл

@ -12,7 +12,12 @@ this.MarqueeWidget = class {
constructor(shadowRoot) {
this.shadowRoot = shadowRoot;
this.element = shadowRoot.host;
}
/*
* Callback called by UAWidgets right after constructor.
*/
onsetup() {
this.switchImpl();
}
@ -20,7 +25,7 @@ this.MarqueeWidget = class {
* Callback called by UAWidgetsChild wheen the direction property
* changes.
*/
onattributechange() {
onchange() {
this.switchImpl();
}
@ -45,6 +50,7 @@ this.MarqueeWidget = class {
this.destructor();
if (newImpl) {
this.impl = new newImpl(this.shadowRoot);
this.impl.onsetup();
}
}
@ -64,7 +70,9 @@ this.MarqueeBaseImplWidget = class {
this.element = shadowRoot.host;
this.document = this.element.ownerDocument;
this.window = this.document.defaultView;
}
onsetup() {
this.generateContent();
// Set up state.

Просмотреть файл

@ -13,7 +13,9 @@ this.PluginProblemWidget = class {
this.element = shadowRoot.host;
// ownerGlobal is chrome-only, not accessible to UA Widget script here.
this.window = this.element.ownerDocument.defaultView; // eslint-disable-line mozilla/use-ownerGlobal
}
onsetup() {
const parser = new this.window.DOMParser();
let parserDoc = parser.parseFromString(`
<!DOCTYPE bindings [

Просмотреть файл

@ -4,14 +4,14 @@
"use strict";
// This is a page widget. It runs in per-origin UA widget scope,
// This is a UA widget. It runs in per-origin UA widget scope,
// to be loaded by UAWidgetsChild.jsm.
/*
* This is the class of entry. It will construct the actual implementation
* according to the value of the "controls" property.
*/
this.VideoControlsPageWidget = class {
this.VideoControlsWidget = class {
constructor(shadowRoot) {
this.shadowRoot = shadowRoot;
this.element = shadowRoot.host;
@ -19,29 +19,34 @@ this.VideoControlsPageWidget = class {
this.window = this.document.defaultView;
this.isMobile = this.window.navigator.appVersion.includes("Android");
}
/*
* Callback called by UAWidgets right after constructor.
*/
onsetup() {
this.switchImpl();
}
/*
* Callback called by UAWidgets when the "controls" property changes.
*/
onattributechange() {
onchange() {
this.switchImpl();
}
/*
* Actually switch the implementation.
* - With "controls" set, the VideoControlsImplPageWidget controls should load.
* - Without it, on mobile, the NoControlsImplPageWidget should load, so
* - With "controls" set, the VideoControlsImplWidget controls should load.
* - Without it, on mobile, the NoControlsImplWidget should load, so
* the user could see the click-to-play button when the video/audio is blocked.
*/
switchImpl() {
let newImpl;
if (this.element.controls) {
newImpl = VideoControlsImplPageWidget;
newImpl = VideoControlsImplWidget;
} else if (this.isMobile) {
newImpl = NoControlsImplPageWidget;
newImpl = NoControlsImplWidget;
}
// Skip if we are asked to load the same implementation.
// This can happen if the property is set again w/o value change.
@ -54,6 +59,7 @@ this.VideoControlsPageWidget = class {
}
if (newImpl) {
this.impl = new newImpl(this.shadowRoot);
this.impl.onsetup();
} else {
this.impl = undefined;
}
@ -69,13 +75,15 @@ this.VideoControlsPageWidget = class {
}
};
this.VideoControlsImplPageWidget = class {
this.VideoControlsImplWidget = class {
constructor(shadowRoot) {
this.shadowRoot = shadowRoot;
this.element = shadowRoot.host;
this.document = this.element.ownerDocument;
this.window = this.document.defaultView;
}
onsetup() {
this.generateContent();
this.Utils = {
@ -2289,13 +2297,15 @@ this.VideoControlsImplPageWidget = class {
}
};
this.NoControlsImplPageWidget = class {
this.NoControlsImplWidget = class {
constructor(shadowRoot) {
this.shadowRoot = shadowRoot;
this.element = shadowRoot.host;
this.document = this.element.ownerDocument;
this.window = this.document.defaultView;
}
onsetup() {
this.generateContent();
this.Utils = {