Handle exceptions during upgrades

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());

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<CustomElementDefinition>, 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<Function>, 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<Rc<CustomElementDefinition>>) {
         // 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,
         };

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);

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);

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<Rc<CustomElementDefinition>>) {
         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);
             }
         })
     }

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
-

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
 

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
-

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
 

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