Auto merge of #25575 - pshaughn:celoop, r=jdm

bors-servo · web-flow · commit 3762395e07f4 · 2020-01-25T06:37:04.000-05:00
Prevent infinite recursion when upgrading custom elements  The spec and tests were out of sync when I implemented #25410 and I mentioned I'd have an extra detail to attend to if whatwg/html#5126 landed; it did, and I did, and a couple test cases pass. ---  - [X] `./mach build -d` does not report any errors - [X] `./mach test-tidy` does not report any errors - [X] These changes fix two WPT test cases, see test metadata in commit  - [X] There are tests for these changes
diff --git a/components/script/dom/customelementregistry.rs b/components/script/dom/customelementregistry.rs
@@ -586,13 +586,17 @@ impl CustomElementDefinition {
 /// <https://html.spec.whatwg.org/multipage/#concept-upgrade-an-element>
 #[allow(unsafe_code)]
 pub fn upgrade_element(definition: Rc<CustomElementDefinition>, element: &Element) {
-    // Steps 1-2
+    // Step 1
     let state = element.get_custom_element_state();
-    if state == CustomElementState::Custom || state == CustomElementState::Failed {
+    if state != CustomElementState::Undefined && state != CustomElementState::Uncustomized {
         return;
     }
 
-    // Step 3 happens later to save having to undo it in an exception
+    // Step 2
+    element.set_custom_element_definition(Rc::clone(&definition));
+
+    // Step 3
+    element.set_custom_element_state(CustomElementState::Failed);
 
     // Step 4
     for attr in element.attrs().iter() {
@@ -630,14 +634,12 @@ pub fn upgrade_element(definition: Rc<CustomElementDefinition>, element: &Elemen
     // Step 8 exception handling
     if let Err(error) = result {
         // Step 8.exception.1
-        element.set_custom_element_state(CustomElementState::Failed);
+        element.clear_custom_element_definition();
 
-        // Step 8.exception.2 isn't needed since step 3 hasn't happened yet
-
-        // Step 8.exception.3
+        // Step 8.exception.2
         element.clear_reaction_queue();
 
-        // Step 8.exception.4
+        // Step 8.exception.3
         let global = GlobalScope::current().expect("No current global");
         let cx = global.get_cx();
         unsafe {
@@ -651,11 +653,7 @@ pub fn upgrade_element(definition: Rc<CustomElementDefinition>, element: &Elemen
     // TODO Step 9: "If element is a form-associated custom element..."
 
     // Step 10
-
     element.set_custom_element_state(CustomElementState::Custom);
-
-    // Step 3
-    element.set_custom_element_definition(definition);
 }
 
 /// <https://html.spec.whatwg.org/multipage/#concept-upgrade-an-element>
diff --git a/components/script/dom/element.rs b/components/script/dom/element.rs
@@ -342,6 +342,10 @@ impl Element {
         self.rare_data().as_ref()?.custom_element_definition.clone()
     }
 
+    pub fn clear_custom_element_definition(&self) {
+        self.ensure_rare_data().custom_element_definition = None;
+    }
+
     pub fn push_callback_reaction(&self, function: Rc<Function>, args: Box<[Heap<JSVal>]>) {
         self.ensure_rare_data()
             .custom_element_reaction_queue
diff --git a/tests/wpt/metadata/custom-elements/upgrading.html.ini b/tests/wpt/metadata/custom-elements/upgrading.html.ini
@@ -4,10 +4,3 @@
 
   [If definition's disable shadow is true and element's shadow root is non-null, then throw a "NotSupportedError" DOMException.]
     expected: FAIL
-
-  [Infinite constructor recursion with upgrade(this) should not be possible]
-    expected: FAIL
-
-  [Infinite constructor recursion with appendChild should not be possible]
-    expected: FAIL
-
