Upgrading and constructing element should always report exception instead of rethrowing
authorrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 7 Oct 2016 00:46:31 +0000 (00:46 +0000)
committerrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 7 Oct 2016 00:46:31 +0000 (00:46 +0000)
https://bugs.webkit.org/show_bug.cgi?id=162996

Reviewed by Darin Adler.

Source/WebCore:

The latest HTML specification specifies that we must report exceptions thrown during element upgrades:
https://html.spec.whatwg.org/#upgrades

In addition, F2F during 2016 TPAC had a consensus that we should do the same for document.createElement:
https://github.com/w3c/webcomponents/issues/569

Since the HTML parser already reports the exception thrown during custom element construction as it does
not have any JS stack, these changes make exceptions thrown during upgrades and constructions.

In our implementation, this only reduces the code complexity as now we can push the logic to fallback
to HTMLUnknownElement into JSCustomElementInterface's constructElement, which has been renamed
to constructElementWithFallback, and eliminate ShouldClearException enum class entirely. Moreover,
constructElementWithFallback can now return Ref instead of RefPtr.

No new tests. Existing tests have been updated.

* bindings/js/JSCustomElementInterface.cpp:
(WebCore::JSCustomElementInterface::constructElementWithFallback): Create a HTMLUnknownElement if
an attempt to construct a custom element had failed in lieu of returning nullptr.
(WebCore::JSCustomElementInterface::tryToConstructCustomElement): Renamed from constructElement.
Always report exceptions (the same behavior as ShouldClearException::Clear).
(WebCore::JSCustomElementInterface::upgradeElement): Report exceptions instead of rethrowing.
* bindings/js/JSCustomElementInterface.h:
* dom/Document.cpp:
(WebCore::createHTMLElementWithNameValidation):
* html/parser/HTMLDocumentParser.cpp:
(WebCore::HTMLDocumentParser::runScriptsForPausedTreeBuilder):

LayoutTests:

Updated the tests to expect exceptions thrown during custom element constructions are always reported.

* fast/custom-elements/Document-createElement-expected.txt:
* fast/custom-elements/Document-createElement.html:
* fast/custom-elements/defined-pseudo-class-expected.txt:
* fast/custom-elements/defined-pseudo-class.html:
* fast/custom-elements/upgrading/Node-cloneNode.html:
* fast/custom-elements/upgrading/upgrading-parser-created-element.html:

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@206890 268f45cc-cd09-0410-ab3c-d52691b4dbfc

12 files changed:
LayoutTests/ChangeLog
LayoutTests/fast/custom-elements/Document-createElement-expected.txt
LayoutTests/fast/custom-elements/Document-createElement.html
LayoutTests/fast/custom-elements/defined-pseudo-class-expected.txt
LayoutTests/fast/custom-elements/defined-pseudo-class.html
LayoutTests/fast/custom-elements/upgrading/Node-cloneNode.html
LayoutTests/fast/custom-elements/upgrading/upgrading-parser-created-element.html
Source/WebCore/ChangeLog
Source/WebCore/bindings/js/JSCustomElementInterface.cpp
Source/WebCore/bindings/js/JSCustomElementInterface.h
Source/WebCore/dom/Document.cpp
Source/WebCore/html/parser/HTMLDocumentParser.cpp

index 5ff2fc4..daa08fc 100644 (file)
@@ -1,3 +1,19 @@
+2016-10-06  Ryosuke Niwa  <rniwa@webkit.org>
+
+        Upgrading and constructing element should always report exception instead of rethrowing
+        https://bugs.webkit.org/show_bug.cgi?id=162996
+
+        Reviewed by Darin Adler.
+
+        Updated the tests to expect exceptions thrown during custom element constructions are always reported.
+
+        * fast/custom-elements/Document-createElement-expected.txt:
+        * fast/custom-elements/Document-createElement.html:
+        * fast/custom-elements/defined-pseudo-class-expected.txt:
+        * fast/custom-elements/defined-pseudo-class.html:
+        * fast/custom-elements/upgrading/Node-cloneNode.html:
+        * fast/custom-elements/upgrading/upgrading-parser-created-element.html:
+
 2016-10-06  Jiewen Tan  <jiewen_tan@apple.com>
 
         Add a dummy SubtleCrypto interface
index 1dee518..f417eda 100644 (file)
@@ -1,37 +1,37 @@
 
 PASS document.createElement must create an instance of custom elements 
-PASS document.createElement must throw a TypeError when the result of Construct is not a DOM node 
-PASS document.createElement must throw a TypeError when the result of Construct is a TextNode 
-PASS document.createElement must throw a NotSupportedError when attribute is added by setAttribute during construction 
-PASS document.createElement must throw a NotSupportedError when attribute is added by attributes.setNamedItem during construction 
-PASS document.createElement must not throw a NotSupportedError when attribute is added and removed during construction 
-PASS document.createElement must throw a NotSupportedError when a Text child is added during construction 
-PASS document.createElement must throw a NotSupportedError when a Comment child is added during construction 
-PASS document.createElement must throw a NotSupportedError when an element child is added during construction 
-PASS document.createElement must not throw a NotSupportedError when an element child is added and removed during construction 
-PASS document.createElement must throw a NotSupportedError when the element gets inserted into another element during construction 
-PASS document.createElement must not throw a NotSupportedError when the element is inserted and removed from another element during construction 
-PASS document.createElement must throw a NotSupportedError when the element is adopted into a document of a template element during construction 
-PASS document.createElement must throw a NotSupportedError when the element is inserted into a document of a template element during construction 
-PASS document.createElement must not throw a NotSupportedError when the element is adopted back from a document of a template element during construction 
-PASS document.createElement must throw a NotSupportedError when the element is adopted into a new document during construction 
-PASS document.createElement must throw a NotSupportedError when the element is inserted into a new document during construction 
-PASS document.createElement must not throw a NotSupportedError when the element is adopted back from a new document during construction 
-PASS document.createElement must throw a NotSupportedError when the element is adopted into a cloned document during construction 
-PASS document.createElement must throw a NotSupportedError when the element is inserted into a cloned document during construction 
-PASS document.createElement must not throw a NotSupportedError when the element is adopted back from a cloned document during construction 
-PASS document.createElement must throw a NotSupportedError when the element is adopted into a document created by createHTMLDocument during construction 
-PASS document.createElement must throw a NotSupportedError when the element is inserted into a document created by createHTMLDocument during construction 
-PASS document.createElement must not throw a NotSupportedError when the element is adopted back from a document created by createHTMLDocument during construction 
-PASS document.createElement must throw a NotSupportedError when the element is adopted into a HTML document created by createDocument during construction 
-PASS document.createElement must throw a NotSupportedError when the element is inserted into a HTML document created by createDocument during construction 
-PASS document.createElement must not throw a NotSupportedError when the element is adopted back from a HTML document created by createDocument during construction 
-PASS document.createElement must throw a NotSupportedError when the element is adopted into a document in an iframe during construction 
-PASS document.createElement must throw a NotSupportedError when the element is inserted into a document in an iframe during construction 
-PASS document.createElement must not throw a NotSupportedError when the element is adopted back from a document in an iframe during construction 
-PASS document.createElement must throw a NotSupportedError when the element is adopted into a HTML document fetched by XHR during construction 
-PASS document.createElement must throw a NotSupportedError when the element is inserted into a HTML document fetched by XHR during construction 
-PASS document.createElement must not throw a NotSupportedError when the element is adopted back from a HTML document fetched by XHR during construction 
-PASS document.createElement must throw a NotSupportedError when the local name of the element does not match that of the custom element 
-PASS document.createElement must re-throw an exception thrown by a custom element constructor 
+PASS document.createElement must report a TypeError when the result of Construct is not a DOM node 
+PASS document.createElement must report a TypeError when the result of Construct is a TextNode 
+PASS document.createElement must report a NotSupportedError when attribute is added by setAttribute during construction 
+PASS document.createElement must report a NotSupportedError when attribute is added by attributes.setNamedItem during construction 
+PASS document.createElement must not report a NotSupportedError when attribute is added and removed during construction 
+PASS document.createElement must report a NotSupportedError when a Text child is added during construction 
+PASS document.createElement must report a NotSupportedError when a Comment child is added during construction 
+PASS document.createElement must report a NotSupportedError when an element child is added during construction 
+PASS document.createElement must not report a NotSupportedError when an element child is added and removed during construction 
+PASS document.createElement must report a NotSupportedError when the element gets inserted into another element during construction 
+PASS document.createElement must not report a NotSupportedError when the element is inserted and removed from another element during construction 
+PASS document.createElement must report a NotSupportedError when the element is adopted into a document of a template element during construction 
+PASS document.createElement must report a NotSupportedError when the element is inserted into a document of a template element during construction 
+PASS document.createElement must not report a NotSupportedError when the element is adopted back from a document of a template element during construction 
+PASS document.createElement must report a NotSupportedError when the element is adopted into a new document during construction 
+PASS document.createElement must report a NotSupportedError when the element is inserted into a new document during construction 
+PASS document.createElement must not report a NotSupportedError when the element is adopted back from a new document during construction 
+PASS document.createElement must report a NotSupportedError when the element is adopted into a cloned document during construction 
+PASS document.createElement must report a NotSupportedError when the element is inserted into a cloned document during construction 
+PASS document.createElement must not report a NotSupportedError when the element is adopted back from a cloned document during construction 
+PASS document.createElement must report a NotSupportedError when the element is adopted into a document created by createHTMLDocument during construction 
+PASS document.createElement must report a NotSupportedError when the element is inserted into a document created by createHTMLDocument during construction 
+PASS document.createElement must not report a NotSupportedError when the element is adopted back from a document created by createHTMLDocument during construction 
+PASS document.createElement must report a NotSupportedError when the element is adopted into a HTML document created by createDocument during construction 
+PASS document.createElement must report a NotSupportedError when the element is inserted into a HTML document created by createDocument during construction 
+PASS document.createElement must not report a NotSupportedError when the element is adopted back from a HTML document created by createDocument during construction 
+PASS document.createElement must report a NotSupportedError when the element is adopted into a document in an iframe during construction 
+PASS document.createElement must report a NotSupportedError when the element is inserted into a document in an iframe during construction 
+PASS document.createElement must not report a NotSupportedError when the element is adopted back from a document in an iframe during construction 
+PASS document.createElement must report a NotSupportedError when the element is adopted into a HTML document fetched by XHR during construction 
+PASS document.createElement must report a NotSupportedError when the element is inserted into a HTML document fetched by XHR during construction 
+PASS document.createElement must not report a NotSupportedError when the element is adopted back from a HTML document fetched by XHR during construction 
+PASS document.createElement must report a NotSupportedError when the local name of the element does not match that of the custom element 
+PASS document.createElement must report an exception thrown by a custom element constructor 
 
index d15165e..0064e5c 100644 (file)
@@ -12,6 +12,7 @@
 <body>
 <div id="log"></div>
 <script>
+setup({allow_uncaught_exception:true});
 
 test(function () {
     class MyCustomElement extends HTMLElement {};
@@ -27,6 +28,23 @@ test(function () {
 
 }, 'document.createElement must create an instance of custom elements');
 
+function assert_reports(expected, testFunction, message) {
+    var uncaughtError = null;
+    window.onerror = function (message, url, lineNumber, columnNumber, error) { uncaughtError = error; return true; }
+    testFunction();
+    if (typeof(expected) == 'string')
+        assert_equals(uncaughtError, expected, message);
+    else if (expected && 'name' in expected)
+        assert_equals(uncaughtError.name, expected.name, message);
+    else
+      assert_equals(uncaughtError, expected, message);
+    window.onerror = null;
+}
+
+function assert_not_reports(testFunction, message) {
+    assert_reports(null, testFunction, message);
+}
+
 test(function () {
     class ObjectCustomElement extends HTMLElement {
         constructor()
@@ -40,8 +58,11 @@ test(function () {
     assert_true(instance instanceof Object);
     assert_equals(instance.foo, 'bar');
 
-    assert_throws({name: 'TypeError'}, function () { document.createElement('object-custom-element'); });
-}, 'document.createElement must throw a TypeError when the result of Construct is not a DOM node');
+    var instance;
+    assert_reports({name: 'TypeError'}, function () { instance = document.createElement('object-custom-element'); });
+    assert_equals(instance.localName, 'object-custom-element');
+    assert_true(instance instanceof HTMLUnknownElement);
+}, 'document.createElement must report a TypeError when the result of Construct is not a DOM node');
 
 test(function () {
     class TextCustomElement extends HTMLElement {
@@ -52,8 +73,11 @@ test(function () {
     };
     customElements.define('text-custom-element', TextCustomElement);
     assert_true(new TextCustomElement instanceof Text);
-    assert_throws({name: 'TypeError'}, function () { document.createElement('text-custom-element'); });
-}, 'document.createElement must throw a TypeError when the result of Construct is a TextNode');
+    var instance;
+    assert_reports({name: 'TypeError'}, function () { instance = document.createElement('text-custom-element'); });
+    assert_equals(instance.localName, 'text-custom-element');
+    assert_true(instance instanceof HTMLUnknownElement);
+}, 'document.createElement must report a TypeError when the result of Construct is a TextNode');
 
 test(function () {
     class ElementWithAttribute extends HTMLElement {
@@ -65,8 +89,11 @@ test(function () {
     };
     customElements.define('element-with-attribute', ElementWithAttribute);
     assert_true(new ElementWithAttribute instanceof ElementWithAttribute);
-    assert_throws({name: 'NotSupportedError'}, function () { document.createElement('element-with-attribute'); });
-}, 'document.createElement must throw a NotSupportedError when attribute is added by setAttribute during construction');
+    var instance;
+    assert_reports({name: 'NotSupportedError'}, function () { instance = document.createElement('element-with-attribute'); });
+    assert_equals(instance.localName, 'element-with-attribute');
+    assert_true(instance instanceof HTMLUnknownElement);
+}, 'document.createElement must report a NotSupportedError when attribute is added by setAttribute during construction');
 
 test(function () {
     class ElementWithAttrNode extends HTMLElement {
@@ -78,8 +105,11 @@ test(function () {
     };
     customElements.define('element-with-attr-node', ElementWithAttrNode);
     assert_true(new ElementWithAttrNode instanceof ElementWithAttrNode);
-    assert_throws({name: 'NotSupportedError'}, function () { document.createElement('element-with-attr-node'); });
-}, 'document.createElement must throw a NotSupportedError when attribute is added by attributes.setNamedItem during construction');
+    var instance;
+    assert_reports({name: 'NotSupportedError'}, function () { instance = document.createElement('element-with-attr-node'); });
+    assert_equals(instance.localName, 'element-with-attr-node');
+    assert_true(instance instanceof HTMLUnknownElement);
+}, 'document.createElement must report a NotSupportedError when attribute is added by attributes.setNamedItem during construction');
 
 test(function () {
     class ElementWithNoAttributes extends HTMLElement {
@@ -92,8 +122,10 @@ test(function () {
     };
     customElements.define('element-with-no-attiributes', ElementWithNoAttributes);
     assert_true(new ElementWithNoAttributes instanceof ElementWithNoAttributes);
-    assert_true(document.createElement('element-with-no-attiributes') instanceof ElementWithNoAttributes);
-}, 'document.createElement must not throw a NotSupportedError when attribute is added and removed during construction');
+    var instance;
+    assert_not_reports(function () { instance = document.createElement('element-with-no-attiributes'); });
+    assert_true(instance instanceof ElementWithNoAttributes);
+}, 'document.createElement must not report a NotSupportedError when attribute is added and removed during construction');
 
 test(function () {
     class ElementWithChildText extends HTMLElement {
@@ -105,8 +137,11 @@ test(function () {
     };
     customElements.define('element-with-child-text', ElementWithChildText);
     assert_true(new ElementWithChildText instanceof ElementWithChildText);
-    assert_throws({name: 'NotSupportedError'}, function () { document.createElement('element-with-child-text'); });
-}, 'document.createElement must throw a NotSupportedError when a Text child is added during construction');
+    var instance;
+    assert_reports({name: 'NotSupportedError'}, function () { instance = document.createElement('element-with-child-text'); });
+    assert_equals(instance.localName, 'element-with-child-text');
+    assert_true(instance instanceof HTMLUnknownElement);
+}, 'document.createElement must report a NotSupportedError when a Text child is added during construction');
 
 test(function () {
     class ElementWithChildComment extends HTMLElement {
@@ -118,8 +153,11 @@ test(function () {
     };
     customElements.define('element-with-child-comment', ElementWithChildComment);
     assert_true(new ElementWithChildComment instanceof ElementWithChildComment);
-    assert_throws({name: 'NotSupportedError'}, function () { document.createElement('element-with-child-comment'); });
-}, 'document.createElement must throw a NotSupportedError when a Comment child is added during construction');
+    var instance;
+    assert_reports({name: 'NotSupportedError'}, function () { instance = document.createElement('element-with-child-comment'); });
+    assert_equals(instance.localName, 'element-with-child-comment');
+    assert_true(instance instanceof HTMLUnknownElement);
+}, 'document.createElement must report a NotSupportedError when a Comment child is added during construction');
 
 test(function () {
     class ElementWithChildElement extends HTMLElement {
@@ -131,8 +169,11 @@ test(function () {
     };
     customElements.define('element-with-child-element', ElementWithChildElement);
     assert_true(new ElementWithChildElement instanceof ElementWithChildElement);
-    assert_throws({name: 'NotSupportedError'}, function () { document.createElement('element-with-child-element'); });
-}, 'document.createElement must throw a NotSupportedError when an element child is added during construction');
+    var instance;
+    assert_reports({name: 'NotSupportedError'}, function () { instance = document.createElement('element-with-child-element'); });
+    assert_equals(instance.localName, 'element-with-child-element');
+    assert_true(instance instanceof HTMLUnknownElement);
+}, 'document.createElement must report a NotSupportedError when an element child is added during construction');
 
 test(function () {
     class ElementWithNoChildElements extends HTMLElement {
@@ -144,8 +185,10 @@ test(function () {
         }
     };
     customElements.define('element-with-no-child-elements', ElementWithNoChildElements);
-    assert_true(document.createElement('element-with-no-child-elements') instanceof ElementWithNoChildElements);
-}, 'document.createElement must not throw a NotSupportedError when an element child is added and removed during construction');
+    var instance;
+    assert_not_reports(function () { instance = document.createElement('element-with-no-child-elements'); });
+    assert_true(instance instanceof ElementWithNoChildElements);
+}, 'document.createElement must not report a NotSupportedError when an element child is added and removed during construction');
 
 test(function () {
     class ElementWithParent extends HTMLElement {
@@ -157,8 +200,11 @@ test(function () {
     };
     customElements.define('element-with-parent', ElementWithParent);
     assert_true(new ElementWithParent instanceof ElementWithParent);
-    assert_throws({name: 'NotSupportedError'}, function () { document.createElement('element-with-parent'); });
-}, 'document.createElement must throw a NotSupportedError when the element gets inserted into another element during construction');
+    var instance;
+    assert_reports({name: 'NotSupportedError'}, function () { instance = document.createElement('element-with-parent'); });
+    assert_equals(instance.localName, 'element-with-parent');
+    assert_true(instance instanceof HTMLUnknownElement);
+}, 'document.createElement must report a NotSupportedError when the element gets inserted into another element during construction');
 
 test(function () {
     class ElementWithNoParent extends HTMLElement {
@@ -170,8 +216,10 @@ test(function () {
         }
     };
     customElements.define('element-with-no-parent', ElementWithNoParent);
-    assert_true(document.createElement('element-with-no-parent') instanceof ElementWithNoParent);
-}, 'document.createElement must not throw a NotSupportedError when the element is inserted and removed from another element during construction');
+    var instance;
+    assert_not_reports(function () { instance = document.createElement('element-with-no-parent'); });
+    assert_true(instance instanceof ElementWithNoParent);
+}, 'document.createElement must not report a NotSupportedError when the element is inserted and removed from another element during construction');
 
 DocumentTypes.forEach(function (entry, testNumber) {
     if (entry.isOwner)
@@ -192,9 +240,12 @@ DocumentTypes.forEach(function (entry, testNumber) {
             var name = 'element-with-adopt-call-' + testNumber;
             customElements.define(name, ElementWithAdoptCall);
             assert_true(new ElementWithAdoptCall instanceof ElementWithAdoptCall);
-            assert_throws({name: 'NotSupportedError'}, function () { document.createElement(name); });
+            var instance;
+            assert_reports({name: 'NotSupportedError'}, function () { instance = document.createElement(name); });
+            assert_equals(instance.localName, name);
+            assert_true(instance instanceof HTMLUnknownElement);
         });
-    }, 'document.createElement must throw a NotSupportedError when the element is adopted into a ' + docuemntName + ' during construction');
+    }, 'document.createElement must report a NotSupportedError when the element is adopted into a ' + docuemntName + ' during construction');
 
     promise_test(function () {
         return getDocument().then(function (doc) {
@@ -208,9 +259,12 @@ DocumentTypes.forEach(function (entry, testNumber) {
             var name = 'element-inserted-into-another-document-' + testNumber;
             customElements.define(name, ElementInsertedIntoAnotherDocument);
             assert_true(new ElementInsertedIntoAnotherDocument instanceof ElementInsertedIntoAnotherDocument);
-            assert_throws({name: 'NotSupportedError'}, function () { document.createElement(name); });
+            var instance;
+            assert_reports({name: 'NotSupportedError'}, function () { instance = document.createElement(name); });
+            assert_equals(instance.localName, name);
+            assert_true(instance instanceof HTMLUnknownElement);
         });
-    }, 'document.createElement must throw a NotSupportedError when the element is inserted into a ' + docuemntName + ' during construction');
+    }, 'document.createElement must report a NotSupportedError when the element is inserted into a ' + docuemntName + ' during construction');
 
     promise_test(function () {
         return getDocument().then(function (doc) {
@@ -224,9 +278,11 @@ DocumentTypes.forEach(function (entry, testNumber) {
             };
             var name = 'element-that-get-adopted-back' + testNumber;
             customElements.define(name, ElementThatGetAdoptedBack);
-            assert_true(document.createElement(name) instanceof ElementThatGetAdoptedBack);
+            var instance;
+            assert_not_reports(function () { instance = document.createElement(name); });
+            assert_true(instance instanceof ElementThatGetAdoptedBack);
         });
-    }, 'document.createElement must not throw a NotSupportedError when the element is adopted back from a ' + docuemntName + ' during construction');
+    }, 'document.createElement must not report a NotSupportedError when the element is adopted back from a ' + docuemntName + ' during construction');
 });
 
 test(function () {
@@ -239,11 +295,14 @@ test(function () {
     };
     customElements.define('div-custom-element', DivCustomElement);
     assert_true(new DivCustomElement instanceof HTMLDivElement);
-    assert_throws({name: 'NotSupportedError'}, function () { document.createElement('div-custom-element'); });
-}, 'document.createElement must throw a NotSupportedError when the local name of the element does not match that of the custom element');
+    var instance;
+    assert_reports({name: 'NotSupportedError'}, function () { instance = document.createElement('div-custom-element'); });
+    assert_equals(instance.localName, 'div-custom-element');
+    assert_true(instance instanceof HTMLUnknownElement);
+}, 'document.createElement must report a NotSupportedError when the local name of the element does not match that of the custom element');
 
 test(function () {
-    var exceptionToThrow = {message: 'exception thrown by a custom constructor'};
+    var exceptionToThrow = {name: 'exception thrown by a custom constructor'};
     class ThrowCustomElement extends HTMLElement {
         constructor()
         {
@@ -254,21 +313,18 @@ test(function () {
     };
     customElements.define('throw-custom-element', ThrowCustomElement);
 
-    assert_throws(null, function () { new ThrowCustomElement; });
-
-    try {
-        document.createElement('throw-custom-element');
-        assert(false, 'document.createElement must throw when a custom element constructor throws');
-    } catch (exception) {
-        assert_equals(exception, exceptionToThrow, 'document.createElement must throw the same exception custom element constructor throws');
-    }
+    assert_throws(exceptionToThrow, function () { new ThrowCustomElement; });
+    var instance;
+    assert_reports(exceptionToThrow, function () { instance = document.createElement('throw-custom-element'); });
+    assert_equals(instance.localName, 'throw-custom-element');
+    assert_true(instance instanceof HTMLUnknownElement);
 
     exceptionToThrow = false;
     var instance = document.createElement('throw-custom-element');
     assert_true(instance instanceof ThrowCustomElement);
     assert_equals(instance.localName, 'throw-custom-element');
 
-}, 'document.createElement must re-throw an exception thrown by a custom element constructor');
+}, 'document.createElement must report an exception thrown by a custom element constructor');
 
 </script>
 </body>
index 24bbd56..18d4009 100644 (file)
@@ -1,6 +1,3 @@
-CONSOLE MESSAGE: line 79: TypeError: The result of constructing a custom element must be a HTMLElement
-
-Harness Error (FAIL), message = TypeError: The result of constructing a custom element must be a HTMLElement
 
 PASS The defined flag of a custom element must not be set if a custom element has not been upgraded yet 
 PASS The defined flag of a custom element must not be set if a custom element has not been upgraded yet even if the element has been defined 
index eca0eff..0620fcd 100644 (file)
@@ -11,6 +11,7 @@
 <body>
 <div id="log"></div>
 <script>
+setup({allow_uncaught_exception:true});
 
 var upgradeCandidate = document.createElement('my-element');
 
@@ -76,10 +77,14 @@ class ReturnsAnotherNode extends HTMLElement {
 }
 customElements.define('returns-another-node', ReturnsAnotherNode);
 
+var uncaughtError;
+window.onerror = function (message, url, lineNumber, columnNumber, error) { uncaughtError = error; return true; }
 document.write('<returns-another-node></returns-another-node>');
+window.onerror = null;
 
 test(function () {
     var instance = document.querySelector('returns-another-node');
+    assert_equals(uncaughtError.name, 'TypeError', 'The HTML parser must report a TypeError when a custom element returns a non-Element node.');
     assert_not_equals(instance, ReturnsAnotherNode.lastInstance, 'The element inserted by HTML parser must not be the one returned by super() call');
     assert_true(instance instanceof HTMLElement, 'The element inserted by HTML parser must be a HTMLElement');
     assert_false(instance instanceof ReturnsAnotherNode, 'The element inserted by HTML parser must be a custom element');
@@ -100,15 +105,16 @@ test(function () {
 
 test(function () {
     var matchInsideConstructor = false;
-    customElements.define('throws-exception', class extends HTMLElement {
+    class ThrowsException extends HTMLElement {
         constructor() {
             super();
             matchInsideConstructor = this.matches(':defined');
             throw {name: 'bad'};
         }
-    });
+    };
+    customElements.define('throws-exception', ThrowsException);
     var instance;
-    assert_throws({name: 'bad'}, function () { instance = document.createElement('throws-exception'); });
+    assert_throws({name: 'bad'}, function () { instance = new ThrowsException; });
     assert_true(matchInsideConstructor);
 }, 'The defined flag of a custom element must be set inside a constructor when constructing a custom element synchronously'
     + ' even if the constructor threw an exception later');
@@ -116,13 +122,14 @@ test(function () {
 test(function () {
     var instance = document.createElement('throws-exception-2');
     document.body.appendChild(instance);
-    assert_throws({name: 'bad'}, function () {
-        customElements.define('throws-exception-2', class extends HTMLElement {
-            constructor() {
-                throw {name: 'bad'};
-            }
-        });
+    var uncaughtError;
+    window.onerror = function (message, url, lineNumber, columnNumber, error) { uncaughtError = error; return true; }
+    customElements.define('throws-exception-2', class extends HTMLElement {
+        constructor() {
+            throw {name: 'bad'};
+        }
     });
+    assert_equals(uncaughtError.name, 'bad');
     assert_false(instance.matches(':defined'));
 }, 'The defined flag of a custom element must not be set when an upgrade of a custom element fails');
 
index b5583a3..3d3a859 100644 (file)
@@ -123,8 +123,11 @@ test(function () {
         }
         contentWindow.customElements.define('my-custom-element', MyCustomElement);
 
-        var instance = contentDocument.createElement('my-custom-element');
-        assert_throws({'name': 'InvalidStateError'}, function () { instance.cloneNode(false); });
+        var instance = new MyCustomElement(false);
+        var uncaughtError;
+        contentWindow.onerror = function (message, url, lineNumber, columnNumber, error) { uncaughtError = error; return true; }
+        instance.cloneNode(false);
+        assert_equals(uncaughtError.name, 'InvalidStateError');
     });
 }, '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');
@@ -140,27 +143,33 @@ test(function () {
         }
         contentWindow.customElements.define('my-custom-element', MyCustomElement);
 
-        var instance = contentDocument.createElement('my-custom-element');
-        assert_throws({'name': 'InvalidStateError'}, function () { instance.cloneNode(false); });
+        var instance = new MyCustomElement(false);
+        var uncaughtError;
+        contentWindow.onerror = function (message, url, lineNumber, columnNumber, error) { uncaughtError = error; return true; }
+        instance.cloneNode(false);
+        assert_equals(uncaughtError.name, 'InvalidStateError');
     });
 }, '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');
 
 test(function () {
     withNewDocumentWithABrowsingContext(function (contentWindow, contentDocument) {
-        var cloning = false;
+        var returnSpan = false;
         class MyCustomElement extends contentWindow.HTMLElement {
             constructor() {
                 super();
-                if (cloning)
+                if (returnSpan)
                     return contentDocument.createElement('span');
             }
         }
         contentWindow.customElements.define('my-custom-element', MyCustomElement);
 
-        var instance = contentDocument.createElement('my-custom-element');
-        cloning = true;
-        assert_throws({'name': 'InvalidStateError'}, function () { instance.cloneNode(false); });
+        var instance = new MyCustomElement(false);
+        returnSpan = true;
+        var uncaughtError;
+        contentWindow.onerror = function (message, url, lineNumber, columnNumber, error) { uncaughtError = error; return true; }
+        instance.cloneNode(false);
+        assert_equals(uncaughtError.name, 'InvalidStateError');
     });
 }, 'Upgrading a custom element must throw InvalidStateError when the custom element\'s constructor returns another element');
 
@@ -179,9 +188,10 @@ test(function () {
             }
         }
 
-        try {
-            contentWindow.customElements.define('my-custom-element', MyCustomElement);
-        } catch (e) { }
+        var uncaughtError;
+        contentWindow.onerror = function (message, url, lineNumber, columnNumber, error) { uncaughtError = error; return true; }
+        contentWindow.customElements.define('my-custom-element', MyCustomElement);
+        assert_equals(uncaughtError, 'bad');
 
         assert_array_equals(calls, [instance]);
         contentDocument.body.removeChild(instance);
index 4425f87..a889163 100644 (file)
@@ -18,6 +18,8 @@
 <my-other-element id="otherInstance"></my-other-element>
 <script>
 
+setup({allow_uncaught_exception:true});
+
 test(function () {
     class MyCustomElement extends HTMLElement { }
 
@@ -35,7 +37,6 @@ test(function () {
 
 }, 'Element.prototype.createElement must add an unresolved custom element to the upgrade candidates map');
 
-
 test(function () {
     class InstantiatesItselfAfterSuper extends HTMLElement {
         constructor(doNotCreateItself) {
@@ -45,9 +46,10 @@ test(function () {
         }
     }
 
-    assert_throws({'name': 'InvalidStateError'}, function () {
-        customElements.define('instantiates-itself-after-super', InstantiatesItselfAfterSuper);
-    });
+    var uncaughtError;
+    window.onerror = function (message, url, lineNumber, columnNumber, error) { uncaughtError = error; return true; }
+    customElements.define('instantiates-itself-after-super', InstantiatesItselfAfterSuper);
+    assert_equals(uncaughtError.name, 'InvalidStateError');
 }, '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');
 
@@ -60,9 +62,10 @@ test(function () {
         }
     }
 
-    assert_throws({'name': 'InvalidStateError'}, function () {
-        customElements.define('instantiates-itself-before-super', InstantiatesItselfBeforeSuper);
-    });
+    var uncaughtError;
+    window.onerror = function (message, url, lineNumber, columnNumber, error) { uncaughtError = error; return true; }
+    customElements.define('instantiates-itself-before-super', InstantiatesItselfBeforeSuper);
+    assert_equals(uncaughtError.name, 'InvalidStateError');
 }, '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');
 
@@ -80,12 +83,14 @@ test(function () {
     assert_false(instance instanceof MyOtherElement);
     assert_false(otherInstance instanceof MyOtherElement);
 
-    assert_throws({'name': 'InvalidStateError'}, function () {
-        customElements.define('my-other-element', MyOtherElement);
-    });
+    var uncaughtError;
+    window.onerror = function (message, url, lineNumber, columnNumber, error) { uncaughtError = error; return true; }
+    customElements.define('my-other-element', MyOtherElement);
+    assert_equals(uncaughtError.name, 'InvalidStateError');
 
     assert_true(document.createElement('my-other-element') instanceof MyOtherElement,
         'Upgrading of custom elements must happen after the definition was added to the registry.');
+
 }, 'Upgrading a custom element must throw an InvalidStateError when the returned element is not SameValue as the upgraded element');
 
 </script>
index ec24232..946b6c3 100644 (file)
@@ -1,3 +1,38 @@
+2016-10-06  Ryosuke Niwa  <rniwa@webkit.org>
+
+        Upgrading and constructing element should always report exception instead of rethrowing
+        https://bugs.webkit.org/show_bug.cgi?id=162996
+
+        Reviewed by Darin Adler.
+
+        The latest HTML specification specifies that we must report exceptions thrown during element upgrades:
+        https://html.spec.whatwg.org/#upgrades
+
+        In addition, F2F during 2016 TPAC had a consensus that we should do the same for document.createElement:
+        https://github.com/w3c/webcomponents/issues/569
+
+        Since the HTML parser already reports the exception thrown during custom element construction as it does
+        not have any JS stack, these changes make exceptions thrown during upgrades and constructions.
+
+        In our implementation, this only reduces the code complexity as now we can push the logic to fallback
+        to HTMLUnknownElement into JSCustomElementInterface's constructElement, which has been renamed
+        to constructElementWithFallback, and eliminate ShouldClearException enum class entirely. Moreover,
+        constructElementWithFallback can now return Ref instead of RefPtr.
+
+        No new tests. Existing tests have been updated.
+
+        * bindings/js/JSCustomElementInterface.cpp:
+        (WebCore::JSCustomElementInterface::constructElementWithFallback): Create a HTMLUnknownElement if
+        an attempt to construct a custom element had failed in lieu of returning nullptr.
+        (WebCore::JSCustomElementInterface::tryToConstructCustomElement): Renamed from constructElement.
+        Always report exceptions (the same behavior as ShouldClearException::Clear).
+        (WebCore::JSCustomElementInterface::upgradeElement): Report exceptions instead of rethrowing.
+        * bindings/js/JSCustomElementInterface.h:
+        * dom/Document.cpp:
+        (WebCore::createHTMLElementWithNameValidation):
+        * html/parser/HTMLDocumentParser.cpp:
+        (WebCore::HTMLDocumentParser::runScriptsForPausedTreeBuilder):
+
 2016-10-06  Chris Dumez  <cdumez@apple.com>
 
         Overwriting an attribute event listener can lead to wrong event listener firing order
index 5990312..3d68538 100644 (file)
@@ -31,6 +31,7 @@
 #if ENABLE(CUSTOM_ELEMENTS)
 
 #include "DOMWrapperWorld.h"
+#include "HTMLUnknownElement.h"
 #include "JSDOMBinding.h"
 #include "JSDOMGlobalObject.h"
 #include "JSElement.h"
@@ -59,7 +60,19 @@ JSCustomElementInterface::~JSCustomElementInterface()
 
 static RefPtr<Element> constructCustomElementSynchronously(Document&, VM&, ExecState&, JSObject* constructor, const AtomicString& localName);
 
-RefPtr<Element> JSCustomElementInterface::constructElement(const AtomicString& localName, ShouldClearException shouldClearException)
+Ref<Element> JSCustomElementInterface::constructElementWithFallback(Document& document, const AtomicString& localName)
+{
+    if (auto element = tryToConstructCustomElement(document, localName))
+        return element.releaseNonNull();
+
+    auto element = HTMLUnknownElement::create(QualifiedName(nullAtom, localName, HTMLNames::xhtmlNamespaceURI), document);
+    element->setIsCustomElementUpgradeCandidate();
+    element->setIsFailedCustomElement(*this);
+
+    return element.get();
+}
+
+RefPtr<Element> JSCustomElementInterface::tryToConstructCustomElement(Document& document, const AtomicString& localName)
 {
     if (!canInvokeCallback())
         return nullptr;
@@ -73,20 +86,14 @@ RefPtr<Element> JSCustomElementInterface::constructElement(const AtomicString& l
     if (!m_constructor)
         return nullptr;
 
-    ScriptExecutionContext* context = scriptExecutionContext();
-    if (!context)
-        return nullptr;
-    ASSERT(context->isDocument());
-
-    auto& state = *context->execState();
-    RefPtr<Element> element = constructCustomElementSynchronously(downcast<Document>(*context), vm, state, m_constructor.get(), localName);
+    ASSERT(&document == scriptExecutionContext());
+    auto& state = *document.execState();
+    auto element = constructCustomElementSynchronously(document, vm, state, m_constructor.get(), localName);
     ASSERT(!!scope.exception() == !element);
     if (!element) {
-        if (shouldClearException == ShouldClearException::Clear) {
-            auto* exception = scope.exception();
-            scope.clearException();
-            reportException(&state, exception);
-        }
+        auto* exception = scope.exception();
+        scope.clearException();
+        reportException(&state, exception);
         return nullptr;
     }
 
@@ -186,13 +193,14 @@ void JSCustomElementInterface::upgradeElement(Element& element)
 
     if (UNLIKELY(scope.exception())) {
         element.setIsFailedCustomElement(*this);
+        reportException(state, scope.exception());
         return;
     }
 
     Element* wrappedElement = JSElement::toWrapped(returnedElement);
     if (!wrappedElement || wrappedElement != &element) {
         element.setIsFailedCustomElement(*this);
-        throwInvalidStateError(*state, scope, "Custom element constructor failed to upgrade an element");
+        reportException(state, createDOMException(state, INVALID_STATE_ERR, "Custom element constructor failed to upgrade an element"));
         return;
     }
     element.setIsDefinedCustomElement(*this);
index e6811a4..83955ff 100644 (file)
@@ -63,8 +63,7 @@ public:
         return adoptRef(*new JSCustomElementInterface(name, callback, globalObject));
     }
 
-    enum class ShouldClearException { Clear, DoNotClear };
-    RefPtr<Element> constructElement(const AtomicString&, ShouldClearException);
+    Ref<Element> constructElementWithFallback(Document&, const AtomicString&);
 
     void upgradeElement(Element&);
 
@@ -98,6 +97,8 @@ public:
 private:
     JSCustomElementInterface(const QualifiedName&, JSC::JSObject* callback, JSDOMGlobalObject*);
 
+    RefPtr<Element> tryToConstructCustomElement(Document&, const AtomicString&);
+
     void invokeCallback(Element&, JSC::JSObject* callback, const WTF::Function<void(JSC::ExecState*, JSDOMGlobalObject*, JSC::MarkedArgumentBuffer&)>& addArguments = { });
 
     QualifiedName m_name;
index c1ccdc0..bf380e7 100644 (file)
@@ -893,7 +893,7 @@ static RefPtr<Element> createHTMLElementWithNameValidation(Document& document, c
         auto* registry = window->customElementRegistry();
         if (UNLIKELY(registry)) {
             if (auto* elementInterface = registry->findInterface(localName))
-                return elementInterface->constructElement(localName, JSCustomElementInterface::ShouldClearException::DoNotClear);
+                return elementInterface->constructElementWithFallback(document, localName);
         }
     }
 #endif
index 6a669db..5940e06 100644 (file)
@@ -197,15 +197,8 @@ void HTMLDocumentParser::runScriptsForPausedTreeBuilder()
 
         // https://html.spec.whatwg.org/#create-an-element-for-the-token
         auto& elementInterface = constructionData->elementInterface.get();
-        RefPtr<Element> newElement = elementInterface.constructElement(constructionData->name, JSCustomElementInterface::ShouldClearException::Clear);
-        if (!newElement) {
-            ASSERT(!m_treeBuilder->isParsingTemplateContents());
-            newElement = HTMLUnknownElement::create(QualifiedName(nullAtom, constructionData->name, xhtmlNamespaceURI), *document());
-            newElement->setIsCustomElementUpgradeCandidate();
-            newElement->setIsFailedCustomElement(elementInterface);
-        }
-
-        m_treeBuilder->didCreateCustomOrCallbackElement(newElement.releaseNonNull(), *constructionData);
+        auto newElement = elementInterface.constructElementWithFallback(*document(), constructionData->name);
+        m_treeBuilder->didCreateCustomOrCallbackElement(WTFMove(newElement), *constructionData);
         return;
     }
 #endif