Bug 1415761 - Catch the exception and rethrow it after invoking custo…

…m elements reactions; r=bz The spec was unclear on how CEReactions interact with thrown exceptions; see whatwg/html#3217. The spec is now being clarified in whatwg/html#3235. MozReview-Commit-ID: 521HprTRS7k
JerryShih · Nov 17, 2017 · 1aedd16 · 1aedd16
1 parent c17704c
commit 1aedd16
Show file tree

Hide file tree

Showing 6 changed files with 57 additions and 5 deletions.
diff --git a/dom/base/CustomElementRegistry.h b/dom/base/CustomElementRegistry.h
@@ -480,15 +480,24 @@ class CustomElementRegistry final : public nsISupports,
 
 class MOZ_RAII AutoCEReaction final {
   public:
-    explicit AutoCEReaction(CustomElementReactionsStack* aReactionsStack)
-      : mReactionsStack(aReactionsStack) {
+    // JSContext is allowed to be a nullptr if we are guaranteeing that we're
+    // not doing something that might throw but not finish reporting a JS
+    // exception during the lifetime of the AutoCEReaction.
+    AutoCEReaction(CustomElementReactionsStack* aReactionsStack, JSContext* aCx)
+      : mReactionsStack(aReactionsStack)
+      , mCx(aCx) {
       mReactionsStack->CreateAndPushElementQueue();
     }
     ~AutoCEReaction() {
+      Maybe<JS::AutoSaveExceptionState> ases;
+      if (mCx) {
+        ases.emplace(mCx);
+      }
       mReactionsStack->PopAndInvokeElementQueue();
     }
   private:
     RefPtr<CustomElementReactionsStack> mReactionsStack;
+    JSContext* mCx;
 };
 
 } // namespace dom

diff --git a/dom/base/nsDocument.cpp b/dom/base/nsDocument.cpp
@@ -6510,7 +6510,8 @@ nsDocument::RegisterElement(JSContext* aCx, const nsAString& aType,
     return;
   }
 
-  AutoCEReaction ceReaction(this->GetDocGroup()->CustomElementReactionsStack());
+  AutoCEReaction ceReaction(this->GetDocGroup()->CustomElementReactionsStack(),
+                            aCx);
   // Unconditionally convert TYPE to lowercase.
   nsAutoString lcType;
   nsContentUtils::ASCIIToLower(aType, lcType);

diff --git a/dom/bindings/Codegen.py b/dom/bindings/Codegen.py
@@ -7865,7 +7865,7 @@ def __init__(self, returnType, arguments, nativeMethodName, static,
                 if (CustomElementRegistry::IsCustomElementEnabled()) {
                   CustomElementReactionsStack* reactionsStack = GetCustomElementReactionsStack(${obj});
                   if (reactionsStack) {
-                    ceReaction.emplace(reactionsStack);
+                    ceReaction.emplace(reactionsStack, cx);
                   }
                 }
                 """, obj=objectName)))

diff --git a/parser/html/nsHtml5TreeOperation.cpp b/parser/html/nsHtml5TreeOperation.cpp
@@ -428,7 +428,8 @@ nsHtml5TreeOperation::CreateHTMLElement(
       nsAutoMicroTask mt;
     }
     dom::AutoCEReaction
-      autoCEReaction(document->GetDocGroup()->CustomElementReactionsStack());
+      autoCEReaction(document->GetDocGroup()->CustomElementReactionsStack(),
+                     nullptr);
 
     nsCOMPtr<dom::Element> newElement;
     NS_NewHTMLElement(getter_AddRefs(newElement), nodeInfo.forget(),

diff --git a/testing/web-platform/meta/MANIFEST.json b/testing/web-platform/meta/MANIFEST.json
@@ -312868,6 +312868,12 @@
      {}
     ]
    ],
+   "custom-elements/reactions/with-exceptions.html": [
+    [
+     "/custom-elements/reactions/with-exceptions.html",
+     {}
+    ]
+   ],
    "custom-elements/upgrading.html": [
     [
      "/custom-elements/upgrading.html",
@@ -533823,6 +533829,10 @@
    "e966b6c608ee9b4183e040b8be7adb2b73722c7b",
    "support"
   ],
+  "custom-elements/reactions/with-exceptions.html": [
+   "b72ee82a3d98c61683e62c4834f54269898e12d5",
+   "testharness"
+  ],
   "custom-elements/resources/custom-elements-helpers.js": [
    "accaee861d319e0caf00356521479dc5951ac162",
    "support"

diff --git a/testing/web-platform/tests/custom-elements/reactions/with-exceptions.html b/testing/web-platform/tests/custom-elements/reactions/with-exceptions.html
@@ -0,0 +1,31 @@
+<!DOCTYPE html>
+<meta charset="utf-8">
+<title>Custom Elements: CEReactions interaction with exceptions</title>
+<link rel="author" title="Domenic Denicola" href="mailto:[email protected]">
+<meta name="help" content="https://html.spec.whatwg.org/multipage/#cereactions">
+<meta name="help" content="https://github.com/whatwg/html/pull/3235">
+
+<script src="/resources/testharness.js"></script>
+<script src="/resources/testharnessreport.js"></script>
+<script src="../resources/custom-elements-helpers.js"></script>
+
+<div id="log"></div>
+
+<script>
+"use strict";
+// Basically from https://github.com/whatwg/html/issues/3217#issuecomment-343633273
+test_with_window((contentWindow, contentDocument) => {
+  let reactionRan = false;
+  contentWindow.customElements.define("custom-element", class extends contentWindow.HTMLElement {
+    disconnectedCallback() {
+      reactionRan = true;
+    }
+  });
+  const text = contentDocument.createTextNode("");
+  contentDocument.documentElement.appendChild(text);
+  const element = contentDocument.createElement("custom-element");
+  contentDocument.documentElement.appendChild(element);
+  assert_throws("HierarchyRequestError", () => text.before("", contentDocument.documentElement));
+  assert_true(reactionRan);
+}, "Reaction must run even after the exception is thrown");
+</script>