Prevent recursion in critical part of CustomElementRegistry::define.

third_party/WebKit/LayoutTests/custom-elements/spec/define-element.html

Index: third_party/WebKit/LayoutTests/custom-elements/spec/define-element.html
diff --git a/third_party/WebKit/LayoutTests/custom-elements/spec/define-element.html b/third_party/WebKit/LayoutTests/custom-elements/spec/define-element.html
index 3dd01c67c9c69339b7b90b0527f8a275c5f4d46a..43f0cca5cf21135b2c1e13079860ffcc3b65d7c4 100644
--- a/third_party/WebKit/LayoutTests/custom-elements/spec/define-element.html
+++ b/third_party/WebKit/LayoutTests/custom-elements/spec/define-element.html
@@ -88,6 +88,19 @@ test_with_window((w) => {
      'registry should throw a NotSupportedError');
 }, 'Reused constructor');
 
+promise_test((t) => {
+  return Promise.all([create_window_in_test(t), create_window_in_test(t)])
+  .then(([w1, w2]) => {
+    class X extends w2.HTMLElement { };
+    w1.customElements.define('first-name', X);
+    w2.customElements.define('second-name', X);

[domenic, 2016/09/03 22:50:35]

This test is very interesting. Maybe also add a test that before doing
w2.customElements.define, new X() throws (since it will go to look up the name
in w2 and not find it)?

+    assert_equals(
+      new X().localName, 'second-name',
+      'the current global object should determine which definition is ' +
+      'operative; because X extends w2.HTMLElement, w2 is operative');
+  });
+}, 'Reused constructor in a separate registry');
+
 test_with_window((w) => {
   let X = (function () {}).bind({});
   Object.defineProperty(X, 'prototype', {
@@ -104,6 +117,57 @@ test_with_window((w) => {
 }, 'Reused constructor recursively');
 
 test_with_window((w) => {
+  let X = (function () {}).bind({});
+  Object.defineProperty(X, 'prototype', {
+    get() {
+      assert_throws_dom_exception(w, 'NotSupportedError', () => {
+        w.customElements.define('second-name', class extends HTMLElement { });
+      }, 'defining an element while element definition is running should ' +
+         'throw a NotSupportedError');
+      return new Object();

[domenic, 2016/09/03 22:50:35]

Nit: `new Object()` is better written as `{}`

+    }
+  });
+  w.customElements.define('first-name', X);
+  assert_equals(w.customElements.get('first-name'), X,
+                'the first definition should have worked');
+}, 'Define while element definition is running');
+
+promise_test((t) => {
+  return Promise.all([create_window_in_test(t), create_window_in_test(t)])
+  .then(([w1, w2]) => {
+    let X = (function () {}).bind({});
+    class Y extends w2.HTMLElement { };
+    Object.defineProperty(X, 'prototype', {
+      get() {
+        w2.customElements.define('second-name', Y);
+        return new Object();
+      }
+    });
+    w1.customElements.define('first-name', X);
+    assert_equals(w1.customElements.get('first-name'), X,
+                  'the first definition should have worked');
+    assert_equals(w2.customElements.get('second-name'), Y,
+                  'the first definition should have worked, too');

[domenic, 2016/09/03 22:50:35]

typo: second

+  });
+}, 'Define while element definition is running in a separate registry');
+
+test_with_window((w) => {
+  class Y extends w.HTMLElement { };
+  class X extends w.HTMLElement {
+    constructor() {
+      super();
+      w.customElements.define('second-name', Y);
+    }
+  };
+  // the element definition flag while first-name is processed should
+  // be reset before doing upgrades
+  w.customElements.define('first-name', X);
+  assert_equals(w.customElements.get('second-name'), Y,
+                'the second definition should have worked');
+}, 'Element definition flag reset',

[domenic, 2016/09/03 22:50:35]

A more descriptive test name would be "Element definition flag resets before
upgrades happen" or similar.

+   '<first-name></first-name>');
+
+test_with_window((w) => {
   assert_throws(TypeError.prototype, () => {
     let not_a_constructor = () => {};
     let invalid_name = 'annotation-xml';
@@ -135,7 +199,6 @@ test_with_window((w) => {
   class C extends w.HTMLElement {
     constructor() {
       super();
-      console.log(this.getAttribute('id'));
       invocations.push(this);
     }
   }
@@ -185,8 +248,8 @@ test_with_window((w) => {
 }, 'Upgrade: shadow tree');
 
 // Final step in Step 14
-// 14. Finally, if the first set of steps threw an exception, then rethrow that exception, 
-// and terminate this algorithm. 
+// 14. Finally, if the first set of steps threw an exception, then rethrow that exception,
+// and terminate this algorithm.
 test_with_window((w) => {
   class Y extends w.HTMLElement {}
   let X = (function () {}).bind({});
@@ -235,14 +298,14 @@ callbacks_in_reverse.forEach((callback) => {
     });
     assert_throws({ name: callback }, () => {
       w.customElements.define('a-a', F_for_callbacks_in_reverse);
-    }, 'Exception from Get(prototype, callback) should be rethrown');  
+    }, 'Exception from Get(prototype, callback) should be rethrown');
   }, 'Rethrow any exceptions thrown while retrieving ' + callback);
 });
 
-// 14.4 If connectedCallback is not undefined, and IsCallable(connectedCallback) is false, 
+// 14.4 If connectedCallback is not undefined, and IsCallable(connectedCallback) is false,
 //      then throw a TypeError exception.
 // 14.6 If disconnectedCallback is not undefined, and IsCallable(disconnectedCallback) is false,
-//      then throw a TypeError exception.  
+//      then throw a TypeError exception.
 // 14.9. If attributeChangedCallback is not undefined, then
 //       1. If IsCallable(attributeChangedCallback) is false, then throw a TypeError exception.
 callbacks_in_reverse.forEach((callback) => {
@@ -254,7 +317,7 @@ callbacks_in_reverse.forEach((callback) => {
       assert_throws(TypeError.prototype, () => {
         w.customElements.define('a-a', F);
       }, 'defining an element with a constructor with a callback that is ' +
-       'not undefined and not callable should throw a TypeError');  
+       'not undefined and not callable should throw a TypeError');
   }, 'If retrieved callback '+ callback + ' is not undefined and not callable, throw TypeError');
 });
 
@@ -271,8 +334,8 @@ test_with_window((w) => {
   }, 'Exception from Get(constructor, observedAttributes) should be rethrown');
 }, 'Rethrow any exceptions thrown while getting observedAttributes');
 
-// 14.9.3 If observedAttributesIterable is not undefined, then set observedAttributes 
-//        to the result of converting observedAttributesIterable to a sequence<DOMString>. 
+// 14.9.3 If observedAttributesIterable is not undefined, then set observedAttributes
+//        to the result of converting observedAttributesIterable to a sequence<DOMString>.
 //        Rethrow any exceptions.
 test_with_window((w) => {
   class X extends w.HTMLElement{
@@ -283,8 +346,8 @@ test_with_window((w) => {
   assert_throws(TypeError.prototype, () => {
     w.customElements.define('a-a', X);
   }, 'converting RegExp to sequence<DOMString> should throw TypeError');
-}, 'exception thrown while converting observedAttributes to sequence<DOMString> ' +
-    'should be rethrown');
+}, 'exception thrown while converting observedAttributes to ' +
+   'sequence<DOMString> should be rethrown');
 
 // 14.9.2 test Get(constructor, observedAttributes) does not throw if
 // attributeChangedCallback is undefined.
@@ -295,22 +358,25 @@ test_with_window((w) => {
     get() { observedAttributes_invoked = true; }
   });
   assert_false( observedAttributes_invoked, 'Get(constructor, observedAttributes) should not be invoked');
-}, 'Get(constructor, observedAttributes)' + 
-    'should not execute if attributeChangedCallback is undefined');
+}, 'Get(constructor, observedAttributes) should not execute if ' +
+   'attributeChangedCallback is undefined');
+
+// TODO(dominicc): I think the order and timing of checks the tests
+// below exercise has changed. Update these tests. crbug.com/643052
 
 // step 2
-// 2. If constructor is an interface object whose corresponding interface either is 
-//    HTMLElement or has HTMLElement in its set of inherited interfaces, throw 
+// 2. If constructor is an interface object whose corresponding interface either is
+//    HTMLElement or has HTMLElement in its set of inherited interfaces, throw
 //    a TypeError and abort these steps.
-// 3. If name is not a valid custom element name, then throw a "SyntaxError" DOMException 
+// 3. If name is not a valid custom element name, then throw a "SyntaxError" DOMException
 //    and abort these steps.
 test_with_window((w) => {
   let invalid_name = 'annotation-xml';
   // TODO(davaajav): change this to TypeError, when we add a failure expectation to this file
   assert_throws_dom_exception(w, 'SYNTAX_ERR', () => {
     w.customElements.define(invalid_name, HTMLElement);
-  }, 'defining a constructor that is an interface object whose interface is HTMLElement' + 
-       'should throw TypeError not SyntaxError');
+  }, 'defining a constructor that is an interface object whose interface is HTMLElement' +
+     'should throw TypeError not SyntaxError');
 }, 'Invalid constructor');
 
 // step 2
@@ -318,8 +384,8 @@ test_with_window((w) => {
   let invalid_name = 'annotation-xml';
   assert_throws_dom_exception(w, 'SYNTAX_ERR', () => {
     w.customElements.define(invalid_name, HTMLButtonElement);
-  }, 'defining a constructor that is an interface object who has HTMLElement' + 
-       'in its set of inhertied interfaces should throw TypeError not SyntaxError');
+  }, 'defining a constructor that is an interface object who has HTMLElement' +
+     'in its set of inhertied interfaces should throw TypeError not SyntaxError');
 }, 'Invalid constructor');
 
 // step 2
@@ -327,8 +393,8 @@ test_with_window((w) => {
   let invalid_name = 'annotation-xml';
   assert_throws_dom_exception(w, 'SYNTAX_ERR', () => {
     w.customElements.define(invalid_name, class extends HTMLElement {});
-  }, 'defining author-defined custom element constructor' + 
-       'should pass this step without throwing TypeError');
+  }, 'defining author-defined custom element constructor should pass this ' +
+     'step without throwing TypeError');
 }, 'Invalid constructor');
 </script>
-</body>
+</body>