Bug 1543570 - Make CustomElementConstructor exceptions more consistent; r=smaug

See https://github.com/whatwg/html/pull/4525.

Differential Revision: https://phabricator.services.mozilla.com/D61247

--HG--
extra : moz-landing-system : lando

dom/base/CustomElementRegistry.cpp

@@ -1117,7 +1117,7 @@ static void DoUpgrade(Element* aElement, CustomElementDefinition* aDefinition,
   // always forms the return value from a JSObject.
   if (NS_FAILED(UNWRAP_OBJECT(Element, &constructResult, element)) ||
       element != aElement) {
-    aRv.Throw(NS_ERROR_DOM_INVALID_STATE_ERR);
+    aRv.ThrowTypeError(u"Custom element constructor returned a wrong element");
     return;
   }
 }

dom/bindings/BindingUtils.cpp

@@ -3612,13 +3612,18 @@ bool HTMLConstructor(JSContext* aCx, unsigned aArgc, JS::Value* aVp,
   // Now we start the [HTMLConstructor] algorithm steps from
   // https://html.spec.whatwg.org/multipage/dom.html#htmlconstructor
 
+  ErrorResult rv;
+  auto scopeExit =
+      MakeScopeExit([&]() { Unused << rv.MaybeSetPendingException(aCx); });
+
   // Step 1.
   nsCOMPtr<nsPIDOMWindowInner> window =
       do_QueryInterface(global.GetAsSupports());
   if (!window) {
     // This means we ended up with an HTML Element interface object defined in
     // a non-Window scope.  That's ... pretty unexpected.
-    return Throw(aCx, NS_ERROR_UNEXPECTED);
+    rv.Throw(NS_ERROR_UNEXPECTED);
+    return false;
   }
   RefPtr<mozilla::dom::CustomElementRegistry> registry(
       window->CustomElements());
@@ -3628,7 +3633,8 @@ bool HTMLConstructor(JSContext* aCx, unsigned aArgc, JS::Value* aVp,
   // going to need a document to create an element.
   Document* doc = window->GetExtantDoc();
   if (!doc) {
-    return Throw(aCx, NS_ERROR_UNEXPECTED);
+    rv.Throw(NS_ERROR_UNEXPECTED);
+    return false;
   }
 
   // Step 2.
@@ -3643,7 +3649,8 @@ bool HTMLConstructor(JSContext* aCx, unsigned aArgc, JS::Value* aVp,
   JS::Rooted<JSObject*> newTarget(
       aCx, js::CheckedUnwrapStatic(&args.newTarget().toObject()));
   if (!newTarget) {
-    return ThrowErrorMessage(aCx, MSG_ILLEGAL_CONSTRUCTOR);
+    rv.ThrowTypeError<MSG_ILLEGAL_CONSTRUCTOR>();
+    return false;
   }
 
   // Enter the compartment of our underlying newTarget object, so we end
@@ -3661,7 +3668,8 @@ bool HTMLConstructor(JSContext* aCx, unsigned aArgc, JS::Value* aVp,
       return false;
     }
     if (newTarget == constructor) {
-      return ThrowErrorMessage(aCx, MSG_ILLEGAL_CONSTRUCTOR);
+      rv.ThrowTypeError<MSG_ILLEGAL_CONSTRUCTOR>();
+      return false;
     }
   }
 
@@ -3669,7 +3677,8 @@ bool HTMLConstructor(JSContext* aCx, unsigned aArgc, JS::Value* aVp,
   CustomElementDefinition* definition =
       registry->LookupCustomElementDefinition(aCx, newTarget);
   if (!definition) {
-    return ThrowErrorMessage(aCx, MSG_ILLEGAL_CONSTRUCTOR);
+    rv.ThrowTypeError<MSG_ILLEGAL_CONSTRUCTOR>();
+    return false;
   }
 
   // Steps 4, 5, 6 do some sanity checks on our callee.  We add to those a
@@ -3719,7 +3728,8 @@ bool HTMLConstructor(JSContext* aCx, unsigned aArgc, JS::Value* aVp,
     // CheckedUnwrapStatic is OK here, since our callee is callable, hence not a
     // cross-origin object.
     if (constructor != js::CheckedUnwrapStatic(callee)) {
-      return ThrowErrorMessage(aCx, MSG_ILLEGAL_CONSTRUCTOR);
+      rv.ThrowTypeError<MSG_ILLEGAL_CONSTRUCTOR>();
+      return false;
     }
   } else {
     if (ns == kNameSpaceID_XHTML) {
@@ -3729,7 +3739,8 @@ bool HTMLConstructor(JSContext* aCx, unsigned aArgc, JS::Value* aVp,
       // interface.
       tag = nsHTMLTags::CaseSensitiveAtomTagToId(definition->mLocalName);
       if (tag == eHTMLTag_userdefined) {
-        return ThrowErrorMessage(aCx, MSG_ILLEGAL_CONSTRUCTOR);
+        rv.ThrowTypeError<MSG_ILLEGAL_CONSTRUCTOR>();
+        return false;
       }
 
       MOZ_ASSERT(tag <= NS_HTML_TAG_MAX, "tag is out of bounds");
@@ -3740,7 +3751,8 @@ bool HTMLConstructor(JSContext* aCx, unsigned aArgc, JS::Value* aVp,
     }
 
     if (!cb) {
-      return ThrowErrorMessage(aCx, MSG_ILLEGAL_CONSTRUCTOR);
+      rv.ThrowTypeError<MSG_ILLEGAL_CONSTRUCTOR>();
+      return false;
     }
 
     // We want to get the constructor from our global's realm, not the
@@ -3754,7 +3766,8 @@ bool HTMLConstructor(JSContext* aCx, unsigned aArgc, JS::Value* aVp,
     // CheckedUnwrapStatic is OK here, since our callee is callable, hence not a
     // cross-origin object.
     if (constructor != js::CheckedUnwrapStatic(callee)) {
-      return ThrowErrorMessage(aCx, MSG_ILLEGAL_CONSTRUCTOR);
+      rv.ThrowTypeError<MSG_ILLEGAL_CONSTRUCTOR>();
+      return false;
     }
   }
 
@@ -3807,7 +3820,10 @@ bool HTMLConstructor(JSContext* aCx, unsigned aArgc, JS::Value* aVp,
 
     // Step 10.
     if (element == ALREADY_CONSTRUCTED_MARKER) {
-      return Throw(aCx, NS_ERROR_DOM_INVALID_STATE_ERR);
+      rv.ThrowTypeError(
+          u"Cannot instantiate a custom element inside its own constructor "
+          u"during upgrades");
+      return false;
     }
 
     // Step 11.

dom/tests/mochitest/webcomponents/upgrade_tests.js

@@ -77,8 +77,8 @@ test_upgrade(function(testWindow, testElement, msg) {
     InstantiatesItselfAfterSuper
   );
 
-  SimpleTest.is(uncaughtError.name, "InvalidStateError", msg);
-}, "Upgrading must report an InvalidStateError when the top of the " +
+  SimpleTest.is(uncaughtError.name, "TypeError", msg);
+}, "Upgrading must report an TypeError when the top of the " +
   "construction stack is marked AlreadyConstructed");
 
 test_upgrade(function(testWindow, testElement, msg) {
@@ -101,8 +101,8 @@ test_upgrade(function(testWindow, testElement, msg) {
     InstantiatesItselfBeforeSuper
   );
 
-  SimpleTest.is(uncaughtError.name, "InvalidStateError", msg);
-}, "Upgrading must report an InvalidStateError when the top of the " +
+  SimpleTest.is(uncaughtError.name, "TypeError", msg);
+}, "Upgrading must report an TypeError when the top of the " +
   "construction stack is marked AlreadyConstructed due to a custom element " +
   "constructor constructing itself before super() call");
 
@@ -123,6 +123,6 @@ test_upgrade(function(testWindow, testElement, msg) {
   };
   testWindow.customElements.define("unresolved-element", MyOtherElement);
 
-  SimpleTest.is(uncaughtError.name, "InvalidStateError", msg);
-}, "Upgrading must report an InvalidStateError when the returned element is " +
+  SimpleTest.is(uncaughtError.name, "TypeError", msg);
+}, "Upgrading must report an TypeError when the returned element is " +
   "not SameValue as the upgraded element");

testing/web-platform/meta/custom-elements/upgrading/Node-cloneNode.html.ini

@@ -1,10 +0,0 @@
-[Node-cloneNode.html]
-  [HTMLElement constructor must throw an TypeError 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 TypeError when the custom element's constructor returns another element]
-    expected: FAIL
-
-  [HTMLElement constructor must throw an TypeError when the top of the construction stack is marked AlreadyConstructed due to a custom element constructor constructing itself after super() call]
-    expected: FAIL
-

testing/web-platform/meta/custom-elements/upgrading/upgrading-parser-created-element.html.ini

@@ -1,16 +0,0 @@
-[upgrading-parser-created-element.html]
-  [Upgrading a custom element whose constructor returns a Text node must throw]
-    expected: FAIL
-
-  [HTMLElement constructor must throw an TypeError when the top of the construction stack is marked AlreadyConstructed due to a custom element constructor constructing itself after super() call]
-    expected: FAIL
-
-  [Upgrading a custom element whose constructor returns an Element must throw]
-    expected: FAIL
-
-  [Upgrading a custom element must throw an TypeError when the returned element is not SameValue as the upgraded element]
-    expected: FAIL
-
-  [HTMLElement constructor must throw an TypeError when the top of the construction stack is marked AlreadyConstructed due to a custom element constructor constructing itself before super() call]
-    expected: FAIL
-