From e700006fb218f04cdfdfab0d6c4e07d6901ab94d Mon Sep 17 00:00:00 2001 From: Connor Brewster Date: Fri, 21 Jul 2017 14:38:58 -0600 Subject: [PATCH] Handle exceptions during upgrades --- components/script/dom/attr.rs | 2 +- .../script/dom/customelementregistry.rs | 67 +++++++++++++------ components/script/dom/element.rs | 22 ++++-- components/script/dom/node.rs | 6 +- components/script/script_thread.rs | 6 +- .../custom-element-reaction-queue.html.ini | 8 --- .../reactions/Document.html.ini | 12 ---- .../reactions/Element.html.ini | 12 ---- .../reactions/HTMLAnchorElement.html.ini | 5 -- .../reactions/HTMLOptionElement.html.ini | 5 -- .../reactions/HTMLTableElement.html.ini | 21 ------ .../reactions/HTMLTableRowElement.html.ini | 5 -- .../HTMLTableSectionElement.html.ini | 8 --- .../custom-elements/reactions/Node.html.ini | 8 --- .../custom-elements/reactions/Range.html.ini | 6 -- .../upgrading/Node-cloneNode.html.ini | 9 --- .../upgrading-enqueue-reactions.html.ini | 14 ---- .../upgrading-parser-created-element.html.ini | 11 --- 18 files changed, 71 insertions(+), 156 deletions(-) delete mode 100644 tests/wpt/metadata/custom-elements/custom-element-reaction-queue.html.ini delete mode 100644 tests/wpt/metadata/custom-elements/reactions/HTMLAnchorElement.html.ini delete mode 100644 tests/wpt/metadata/custom-elements/reactions/HTMLOptionElement.html.ini delete mode 100644 tests/wpt/metadata/custom-elements/reactions/HTMLTableRowElement.html.ini delete mode 100644 tests/wpt/metadata/custom-elements/reactions/HTMLTableSectionElement.html.ini delete mode 100644 tests/wpt/metadata/custom-elements/reactions/Node.html.ini delete mode 100644 tests/wpt/metadata/custom-elements/upgrading/upgrading-enqueue-reactions.html.ini delete mode 100644 tests/wpt/metadata/custom-elements/upgrading/upgrading-parser-created-element.html.ini diff --git a/components/script/dom/attr.rs b/components/script/dom/attr.rs index bf2ecd6e9105..3ce627b46775 100644 --- a/components/script/dom/attr.rs +++ b/components/script/dom/attr.rs @@ -188,7 +188,7 @@ impl Attr { if owner.get_custom_element_definition().is_some() { let reaction = CallbackReaction::AttributeChanged(name, Some(old_value), Some(new_value), namespace); - ScriptThread::enqueue_callback_reaction(owner, reaction); + ScriptThread::enqueue_callback_reaction(owner, reaction, None); } assert!(Some(owner) == self.owner().r()); diff --git a/components/script/dom/customelementregistry.rs b/components/script/dom/customelementregistry.rs index 3f41f5da1cc2..100aae7cae33 100644 --- a/components/script/dom/customelementregistry.rs +++ b/components/script/dom/customelementregistry.rs @@ -11,7 +11,7 @@ use dom::bindings::codegen::Bindings::ElementBinding::ElementMethods; use dom::bindings::codegen::Bindings::FunctionBinding::Function; use dom::bindings::codegen::Bindings::WindowBinding::WindowBinding::WindowMethods; use dom::bindings::conversions::{ConversionResult, FromJSValConvertible, StringificationBehavior}; -use dom::bindings::error::{Error, ErrorResult, Fallible}; +use dom::bindings::error::{Error, ErrorResult, Fallible, report_pending_exception, throw_dom_exception}; use dom::bindings::inheritance::Castable; use dom::bindings::js::{JS, Root}; use dom::bindings::reflector::{DomObject, Reflector, reflect_dom_object}; @@ -486,48 +486,72 @@ pub fn upgrade_element(definition: Rc, element: &Elemen let value = DOMString::from(&**attr.value()); let namespace = attr.namespace().clone(); ScriptThread::enqueue_callback_reaction(element, - CallbackReaction::AttributeChanged(local_name, None, Some(value), namespace)); + CallbackReaction::AttributeChanged(local_name, None, Some(value), namespace), Some(definition.clone())); } // Step 4 if element.is_connected() { - ScriptThread::enqueue_callback_reaction(element, CallbackReaction::Connected); + ScriptThread::enqueue_callback_reaction(element, CallbackReaction::Connected, Some(definition.clone())); } // Step 5 definition.construction_stack.borrow_mut().push(ConstructionStackEntry::Element(Root::from_ref(element))); - // Step 6 + // Step 7 + let result = run_upgrade_constructor(&definition.constructor, element); + + definition.construction_stack.borrow_mut().pop(); + + // Step 7 exception handling + if let Err(error) = result { + // TODO: Step 7.1 + // Track custom element state + + // Step 7.2 + element.clear_reaction_queue(); + + // Step 7.3 + let global = GlobalScope::current().expect("No current global"); + let cx = global.get_cx(); + unsafe { + throw_dom_exception(cx, &global, error); + report_pending_exception(cx, true); + } + return; + } + + element.set_custom_element_definition(definition); +} + +/// https://html.spec.whatwg.org/multipage/#concept-upgrade-an-element +/// Steps 7.1-7.2 +#[allow(unsafe_code)] +fn run_upgrade_constructor(constructor: &Rc, element: &Element) -> ErrorResult { let window = window_from_node(element); let cx = window.get_cx(); - rooted!(in(cx) let constructor = ObjectValue(definition.constructor.callback())); + rooted!(in(cx) let constructor_val = ObjectValue(constructor.callback())); rooted!(in(cx) let mut element_val = UndefinedValue()); unsafe { element.to_jsval(cx, element_val.handle_mut()); } rooted!(in(cx) let mut construct_result = ptr::null_mut()); { // Go into the constructor's compartment - let _ac = JSAutoCompartment::new(cx, definition.constructor.callback()); + let _ac = JSAutoCompartment::new(cx, constructor.callback()); let args = HandleValueArray::new(); - // Step 7 - if unsafe { !Construct1(cx, constructor.handle(), &args, construct_result.handle_mut()) } { - // TODO: Catch exceptions - return; + // Step 7.1 + if unsafe { !Construct1(cx, constructor_val.handle(), &args, construct_result.handle_mut()) } { + return Err(Error::JSFailed); } - // Step 8 + // Step 7.2 let mut same = false; rooted!(in(cx) let construct_result_val = ObjectValue(construct_result.get())); if unsafe { !JS_SameValue(cx, construct_result_val.handle(), element_val.handle(), &mut same) } { - // TODO: Catch exceptions - return; + return Err(Error::JSFailed); } if !same { - // TODO: Throw InvalidStateError + return Err(Error::InvalidState); } } - definition.construction_stack.borrow_mut().pop(); - // TODO: Handle Exceptions - - element.set_custom_element_definition(definition); + Ok(()) } /// https://html.spec.whatwg.org/multipage/#concept-try-upgrade @@ -655,9 +679,12 @@ impl CustomElementReactionStack { /// https://html.spec.whatwg.org/multipage/#enqueue-a-custom-element-callback-reaction #[allow(unsafe_code)] - pub fn enqueue_callback_reaction(&self, element: &Element, reaction: CallbackReaction) { + pub fn enqueue_callback_reaction(&self, + element: &Element, + reaction: CallbackReaction, + definition: Option>) { // Step 1 - let definition = match element.get_custom_element_definition() { + let definition = match definition.or_else(|| element.get_custom_element_definition()) { Some(definition) => definition, None => return, }; diff --git a/components/script/dom/element.rs b/components/script/dom/element.rs index b44757d3aa1c..270d7d7e020c 100644 --- a/components/script/dom/element.rs +++ b/components/script/dom/element.rs @@ -101,6 +101,7 @@ use std::cell::{Cell, Ref}; use std::convert::TryFrom; use std::default::Default; use std::fmt; +use std::mem; use std::rc::Rc; use style::CaseSensitivityExt; use style::applicable_declarations::ApplicableDeclarationBlock; @@ -304,12 +305,21 @@ impl Element { self.custom_element_reaction_queue.borrow_mut().push(CustomElementReaction::Upgrade(definition)); } + pub fn clear_reaction_queue(&self) { + self.custom_element_reaction_queue.borrow_mut().clear(); + } + pub fn invoke_reactions(&self) { - let mut reaction_queue = self.custom_element_reaction_queue.borrow_mut(); - for reaction in reaction_queue.iter() { - reaction.invoke(self); + // TODO: This is not spec compliant, as this will allow some reactions to be processed + // after clear_reaction_queue has been called. + rooted_vec!(let mut reactions); + while !self.custom_element_reaction_queue.borrow().is_empty() { + mem::swap(&mut *reactions, &mut *self.custom_element_reaction_queue.borrow_mut()); + for reaction in reactions.iter() { + reaction.invoke(self); + } + reactions.clear(); } - reaction_queue.clear(); } // https://drafts.csswg.org/cssom-view/#css-layout-box @@ -1081,7 +1091,7 @@ impl Element { if self.get_custom_element_definition().is_some() { let reaction = CallbackReaction::AttributeChanged(name, None, Some(value), namespace); - ScriptThread::enqueue_callback_reaction(self, reaction); + ScriptThread::enqueue_callback_reaction(self, reaction, None); } assert!(attr.GetOwnerElement().r() == Some(self)); @@ -1224,7 +1234,7 @@ impl Element { MutationObserver::queue_a_mutation_record(&self.node, mutation); let reaction = CallbackReaction::AttributeChanged(name, Some(old_value), None, namespace); - ScriptThread::enqueue_callback_reaction(self, reaction); + ScriptThread::enqueue_callback_reaction(self, reaction, None); self.attrs.borrow_mut().remove(idx); attr.set_owner(None); diff --git a/components/script/dom/node.rs b/components/script/dom/node.rs index 3a978008e99b..7503618884b7 100644 --- a/components/script/dom/node.rs +++ b/components/script/dom/node.rs @@ -317,7 +317,7 @@ impl Node { node.style_and_layout_data.get().map(|d| node.dispose(d)); // https://dom.spec.whatwg.org/#concept-node-remove step 14 if let Some(element) = node.as_custom_element() { - ScriptThread::enqueue_callback_reaction(&*element, CallbackReaction::Disconnected); + ScriptThread::enqueue_callback_reaction(&*element, CallbackReaction::Disconnected, None); } } @@ -1425,7 +1425,7 @@ impl Node { for descendant in node.traverse_preorder().filter_map(|d| d.as_custom_element()) { // Step 3.2. ScriptThread::enqueue_callback_reaction(&*descendant, - CallbackReaction::Adopted(old_doc.clone(), Root::from_ref(document))); + CallbackReaction::Adopted(old_doc.clone(), Root::from_ref(document)), None); } for descendant in node.traverse_preorder() { // Step 3.3. @@ -1641,7 +1641,7 @@ impl Node { if descendant.is_connected() { if descendant.get_custom_element_definition().is_some() { // Step 7.7.2.1. - ScriptThread::enqueue_callback_reaction(&*descendant, CallbackReaction::Connected); + ScriptThread::enqueue_callback_reaction(&*descendant, CallbackReaction::Connected, None); } else { // Step 7.7.2.2. try_upgrade_element(&*descendant); diff --git a/components/script/script_thread.rs b/components/script/script_thread.rs index 49c09787ecc5..4a4657e28a51 100644 --- a/components/script/script_thread.rs +++ b/components/script/script_thread.rs @@ -774,11 +774,13 @@ impl ScriptThread { }) } - pub fn enqueue_callback_reaction(element: &Element, reaction: CallbackReaction) { + pub fn enqueue_callback_reaction(element: &Element, + reaction: CallbackReaction, + definition: Option>) { SCRIPT_THREAD_ROOT.with(|root| { if let Some(script_thread) = root.get() { let script_thread = unsafe { &*script_thread }; - script_thread.custom_element_reaction_stack.enqueue_callback_reaction(element, reaction); + script_thread.custom_element_reaction_stack.enqueue_callback_reaction(element, reaction, definition); } }) } diff --git a/tests/wpt/metadata/custom-elements/custom-element-reaction-queue.html.ini b/tests/wpt/metadata/custom-elements/custom-element-reaction-queue.html.ini deleted file mode 100644 index b36672713c36..000000000000 --- a/tests/wpt/metadata/custom-elements/custom-element-reaction-queue.html.ini +++ /dev/null @@ -1,8 +0,0 @@ -[custom-element-reaction-queue.html] - type: testharness - [Upgrading a custom element must invoke attributeChangedCallback and connectedCallback before start upgrading another element] - expected: FAIL - - [Mutating a undefined custom element while upgrading a custom element must not enqueue or invoke reactions on the mutated element] - expected: FAIL - diff --git a/tests/wpt/metadata/custom-elements/reactions/Document.html.ini b/tests/wpt/metadata/custom-elements/reactions/Document.html.ini index 0076f82da665..cf4ac38d747b 100644 --- a/tests/wpt/metadata/custom-elements/reactions/Document.html.ini +++ b/tests/wpt/metadata/custom-elements/reactions/Document.html.ini @@ -3,15 +3,3 @@ [execCommand on Document must enqueue a disconnected reaction when deleting a custom element from a contenteditable 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 - - [writeln on Document must enqueue disconnectedCallback when removing a custom element] - expected: FAIL - diff --git a/tests/wpt/metadata/custom-elements/reactions/Element.html.ini b/tests/wpt/metadata/custom-elements/reactions/Element.html.ini index 988eddc141ac..8661d4318748 100644 --- a/tests/wpt/metadata/custom-elements/reactions/Element.html.ini +++ b/tests/wpt/metadata/custom-elements/reactions/Element.html.ini @@ -12,18 +12,6 @@ [setAttributeNodeNS on Element must enqueue an attributeChanged reaction when replacing an existing attribute] 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 - - [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 - [insertAdjacentHTML on Element must enqueue a connected reaction for a newly constructed custom element] expected: FAIL diff --git a/tests/wpt/metadata/custom-elements/reactions/HTMLAnchorElement.html.ini b/tests/wpt/metadata/custom-elements/reactions/HTMLAnchorElement.html.ini deleted file mode 100644 index 4f0453090304..000000000000 --- a/tests/wpt/metadata/custom-elements/reactions/HTMLAnchorElement.html.ini +++ /dev/null @@ -1,5 +0,0 @@ -[HTMLAnchorElement.html] - type: testharness - [text on HTMLAnchorElement must enqueue disconnectedCallback when removing a custom element] - expected: FAIL - diff --git a/tests/wpt/metadata/custom-elements/reactions/HTMLOptionElement.html.ini b/tests/wpt/metadata/custom-elements/reactions/HTMLOptionElement.html.ini deleted file mode 100644 index fe5243ef3f36..000000000000 --- a/tests/wpt/metadata/custom-elements/reactions/HTMLOptionElement.html.ini +++ /dev/null @@ -1,5 +0,0 @@ -[HTMLOptionElement.html] - type: testharness - [text on HTMLOptionElement must enqueue disconnectedCallback when removing a custom element] - expected: FAIL - diff --git a/tests/wpt/metadata/custom-elements/reactions/HTMLTableElement.html.ini b/tests/wpt/metadata/custom-elements/reactions/HTMLTableElement.html.ini index 2349ffff761f..6a698e897813 100644 --- a/tests/wpt/metadata/custom-elements/reactions/HTMLTableElement.html.ini +++ b/tests/wpt/metadata/custom-elements/reactions/HTMLTableElement.html.ini @@ -3,30 +3,9 @@ [caption on HTMLTableElement must enqueue connectedCallback when inserting a custom element] expected: FAIL - [caption on HTMLTableElement must enqueue disconnectedCallback when removing a custom element] - expected: FAIL - - [deleteCaption() on HTMLTableElement must enqueue disconnectedCallback when removing a custom element] - expected: FAIL - [tHead on HTMLTableElement must enqueue connectedCallback when inserting a custom element] expected: FAIL - [tHead on HTMLTableElement must enqueue disconnectedCallback when removing a custom element] - expected: FAIL - - [deleteTHead() on HTMLTableElement must enqueue disconnectedCallback when removing a custom element] - expected: FAIL - [tFoot on HTMLTableElement must enqueue connectedCallback when inserting a custom element] expected: FAIL - [tFoot on HTMLTableElement must enqueue disconnectedCallback when removing a custom element] - expected: FAIL - - [deleteTFoot() on HTMLTableElement must enqueue disconnectedCallback when removing a custom element] - expected: FAIL - - [deleteRow() on HTMLTableElement must enqueue disconnectedCallback when removing a custom element] - expected: FAIL - diff --git a/tests/wpt/metadata/custom-elements/reactions/HTMLTableRowElement.html.ini b/tests/wpt/metadata/custom-elements/reactions/HTMLTableRowElement.html.ini deleted file mode 100644 index 697e2e8a17d3..000000000000 --- a/tests/wpt/metadata/custom-elements/reactions/HTMLTableRowElement.html.ini +++ /dev/null @@ -1,5 +0,0 @@ -[HTMLTableRowElement.html] - type: testharness - [deleteCell() on HTMLTableRowElement must enqueue disconnectedCallback when removing a custom element] - expected: FAIL - diff --git a/tests/wpt/metadata/custom-elements/reactions/HTMLTableSectionElement.html.ini b/tests/wpt/metadata/custom-elements/reactions/HTMLTableSectionElement.html.ini deleted file mode 100644 index 38ece7c0697d..000000000000 --- a/tests/wpt/metadata/custom-elements/reactions/HTMLTableSectionElement.html.ini +++ /dev/null @@ -1,8 +0,0 @@ -[HTMLTableSectionElement.html] - type: testharness - [deleteRow() on HTMLTableSectionElement on thead must enqueue disconnectedCallback when removing a custom element] - expected: FAIL - - [deleteRow() on HTMLTableSectionElement on tfoot must enqueue disconnectedCallback when removing a custom element] - expected: FAIL - diff --git a/tests/wpt/metadata/custom-elements/reactions/Node.html.ini b/tests/wpt/metadata/custom-elements/reactions/Node.html.ini deleted file mode 100644 index e3396bed69fd..000000000000 --- a/tests/wpt/metadata/custom-elements/reactions/Node.html.ini +++ /dev/null @@ -1,8 +0,0 @@ -[Node.html] - type: testharness - [cloneNode on Node must enqueue an attributeChanged reaction when cloning an element with an observed attribute] - expected: FAIL - - [cloneNode on Node must enqueue an attributeChanged reaction when cloning an element only for observed attributes] - expected: FAIL - diff --git a/tests/wpt/metadata/custom-elements/reactions/Range.html.ini b/tests/wpt/metadata/custom-elements/reactions/Range.html.ini index 37eaa10d28b1..a5696b0e0fed 100644 --- a/tests/wpt/metadata/custom-elements/reactions/Range.html.ini +++ b/tests/wpt/metadata/custom-elements/reactions/Range.html.ini @@ -1,11 +1,5 @@ [Range.html] type: testharness - [cloneContents on Range must enqueue an attributeChanged reaction when cloning an element with an observed attribute] - expected: FAIL - - [cloneContents on Range must enqueue an attributeChanged reaction when cloning an element only for observed attributes] - expected: FAIL - [createContextualFragment on Range must construct a custom element] expected: FAIL diff --git a/tests/wpt/metadata/custom-elements/upgrading/Node-cloneNode.html.ini b/tests/wpt/metadata/custom-elements/upgrading/Node-cloneNode.html.ini index 2e78908601d2..10ac904ac528 100644 --- a/tests/wpt/metadata/custom-elements/upgrading/Node-cloneNode.html.ini +++ b/tests/wpt/metadata/custom-elements/upgrading/Node-cloneNode.html.ini @@ -1,14 +1,5 @@ [Node-cloneNode.html] type: testharness - [HTMLElement constructor must throw an InvalidStateError when the top of the construction stack is marked AlreadyConstructed due to a custom element constructor constructing itself after super() call] - expected: FAIL - - [HTMLElement constructor must throw an InvalidStateError when the top of the construction stack is marked AlreadyConstructed due to a custom element constructor constructing itself before super() call] - expected: FAIL - - [Upgrading a custom element must throw InvalidStateError when the custom element's constructor returns another element] - expected: FAIL - [Inserting an element must not try to upgrade a custom element when it had already failed to upgrade once] expected: FAIL diff --git a/tests/wpt/metadata/custom-elements/upgrading/upgrading-enqueue-reactions.html.ini b/tests/wpt/metadata/custom-elements/upgrading/upgrading-enqueue-reactions.html.ini deleted file mode 100644 index c45292cb290a..000000000000 --- a/tests/wpt/metadata/custom-elements/upgrading/upgrading-enqueue-reactions.html.ini +++ /dev/null @@ -1,14 +0,0 @@ -[upgrading-enqueue-reactions.html] - type: testharness - [Upgrading a custom element must enqueue attributeChangedCallback on each attribute] - expected: FAIL - - [Upgrading a custom element not must enqueue attributeChangedCallback on unobserved attributes] - expected: FAIL - - [Upgrading a custom element must enqueue connectedCallback if the element in the document] - expected: FAIL - - [Upgrading a custom element must enqueue attributeChangedCallback before connectedCallback] - expected: FAIL - diff --git a/tests/wpt/metadata/custom-elements/upgrading/upgrading-parser-created-element.html.ini b/tests/wpt/metadata/custom-elements/upgrading/upgrading-parser-created-element.html.ini deleted file mode 100644 index bf6273a3516e..000000000000 --- a/tests/wpt/metadata/custom-elements/upgrading/upgrading-parser-created-element.html.ini +++ /dev/null @@ -1,11 +0,0 @@ -[upgrading-parser-created-element.html] - type: testharness - [HTMLElement constructor must throw an InvalidStateError when the top of the construction stack is marked AlreadyConstructed due to a custom element constructor constructing itself after super() call] - expected: FAIL - - [HTMLElement constructor must throw an InvalidStateError when the top of the construction stack is marked AlreadyConstructed due to a custom element constructor constructing itself before super() call] - expected: FAIL - - [Upgrading a custom element must throw an InvalidStateError when the returned element is not SameValue as the upgraded element] - expected: FAIL -