Bug 1359859 - Use the right entry global for XBL constructor/destructor/field execution. r=bholley a=gchang
authorBoris Zbarsky <bzbarsky@mit.edu>
Wed, 26 Apr 2017 13:57:55 -0400
changeset 396315 779f28c6c7d5e7b4e9a49f902b4ae60294bc532d
parent 396314 029eee8618cf6abc8ba9506f91fece92d070e5ea
child 396316 0884adb687d1b4cf95820feae19939d931053ae7
push id1468
push userasasaki@mozilla.com
push dateMon, 05 Jun 2017 19:31:07 +0000
treeherdermozilla-release@0641fc6ee9d1 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbholley, gchang
bugs1359859
milestone54.0
Bug 1359859 - Use the right entry global for XBL constructor/destructor/field execution. r=bholley a=gchang The test doesn't verify that the exceptions are reported to the browser console, but I manually checked that they are. Adding a test for that part could be pretty annoying.
dom/xbl/nsXBLProtoImplField.cpp
dom/xbl/nsXBLProtoImplMethod.cpp
dom/xbl/test/mochitest.ini
dom/xbl/test/test_bug1098628_throw_from_construct.xhtml
dom/xbl/test/test_bug1359859.xhtml
--- a/dom/xbl/nsXBLProtoImplField.cpp
+++ b/dom/xbl/nsXBLProtoImplField.cpp
@@ -398,35 +398,42 @@ nsXBLProtoImplField::InstallField(JS::Ha
 
   nsIGlobalObject* globalObject = xpc::WindowGlobalOrNull(aBoundNode);
   if (!globalObject) {
     return NS_OK;
   }
 
   // We are going to run script via EvaluateString, so we need a script entry
   // point, but as this is XBL related it does not appear in the HTML spec.
-  AutoEntryScript aes(globalObject, "XBL <field> initialization", true);
-  JSContext* cx = aes.cx();
-
-  NS_ASSERTION(!::JS_IsExceptionPending(cx),
-               "Shouldn't get here when an exception is pending!");
+  // We need an actual JSContext to do GetScopeForXBLExecution, and it needs to
+  // be in the compartment of globalObject.  But we want our XBL execution scope
+  // to be our entry global.
+  AutoJSAPI jsapi;
+  if (!jsapi.Init(globalObject)) {
+    return NS_ERROR_UNEXPECTED;
+  }
+  MOZ_ASSERT(!::JS_IsExceptionPending(jsapi.cx()),
+             "Shouldn't get here when an exception is pending!");
 
   JSAddonId* addonId = MapURIToAddonID(aBindingDocURI);
 
   Element* boundElement = nullptr;
   rv = UNWRAP_OBJECT(Element, aBoundNode, boundElement);
   if (NS_WARN_IF(NS_FAILED(rv))) {
     return rv;
   }
 
   // First, enter the xbl scope, build the element's scope chain, and use
   // that as the scope chain for the evaluation.
-  JS::Rooted<JSObject*> scopeObject(cx, xpc::GetScopeForXBLExecution(cx, aBoundNode, addonId));
+  JS::Rooted<JSObject*> scopeObject(jsapi.cx(),
+    xpc::GetScopeForXBLExecution(jsapi.cx(), aBoundNode, addonId));
   NS_ENSURE_TRUE(scopeObject, NS_ERROR_OUT_OF_MEMORY);
-  JSAutoCompartment ac(cx, scopeObject);
+
+  AutoEntryScript aes(scopeObject, "XBL <field> initialization", true);
+  JSContext* cx = aes.cx();
 
   JS::Rooted<JS::Value> result(cx);
   JS::CompileOptions options(cx);
   options.setFileAndLine(uriSpec.get(), mLineNumber)
          .setVersion(JSVERSION_LATEST);
   nsJSUtils::EvaluateOptions evalOptions(cx);
   if (!nsJSUtils::GetScopeChainForElement(cx, boundElement,
                                           evalOptions.scopeChain)) {
@@ -436,17 +443,18 @@ nsXBLProtoImplField::InstallField(JS::Ha
                                                        mFieldTextLength),
                                  scopeObject, options, evalOptions, &result);
   if (NS_FAILED(rv)) {
     return rv;
   }
 
   if (rv == NS_SUCCESS_DOM_SCRIPT_EVALUATION_THREW) {
     // Report the exception now, before we try using the JSContext for
-    // the JS_DefineUCProperty call.
+    // the JS_DefineUCProperty call.  Note that this reports in our current
+    // compartment, which is the XBL scope.
     aes.ReportException();
   }
 
   // Now, enter the node's compartment, wrap the eval result, and define it on
   // the bound node.
   JSAutoCompartment ac2(cx, aBoundNode);
   nsDependentString name(mName);
   if (!JS_WrapValue(cx, &result) ||
--- a/dom/xbl/nsXBLProtoImplMethod.cpp
+++ b/dom/xbl/nsXBLProtoImplMethod.cpp
@@ -280,25 +280,34 @@ nsXBLProtoImplAnonymousMethod::Execute(n
   if (!global) {
     return NS_OK;
   }
 
   nsAutoMicroTask mt;
 
   // We are going to run script via JS::Call, so we need a script entry point,
   // but as this is XBL related it does not appear in the HTML spec.
-  dom::AutoEntryScript aes(global, "XBL <constructor>/<destructor> invocation");
-  JSContext* cx = aes.cx();
+  // We need an actual JSContext to do GetScopeForXBLExecution, and it needs to
+  // be in the compartment of globalObject.  But we want our XBL execution scope
+  // to be our entry global.
+  AutoJSAPI jsapi;
+  if (!jsapi.Init(global)) {
+    return NS_ERROR_UNEXPECTED;
+  }
 
-  JS::Rooted<JSObject*> globalObject(cx, global->GetGlobalJSObject());
+  JS::Rooted<JSObject*> globalObject(jsapi.cx(), global->GetGlobalJSObject());
 
-  JS::Rooted<JSObject*> scopeObject(cx, xpc::GetScopeForXBLExecution(cx, globalObject, aAddonId));
+  JS::Rooted<JSObject*> scopeObject(jsapi.cx(),
+    xpc::GetScopeForXBLExecution(jsapi.cx(), globalObject, aAddonId));
   NS_ENSURE_TRUE(scopeObject, NS_ERROR_OUT_OF_MEMORY);
 
-  JSAutoCompartment ac(cx, scopeObject);
+  dom::AutoEntryScript aes(scopeObject,
+                           "XBL <constructor>/<destructor> invocation",
+                           true);
+  JSContext* cx = aes.cx();
   JS::AutoObjectVector scopeChain(cx);
   if (!nsJSUtils::GetScopeChainForElement(cx, aBoundElement->AsElement(),
                                           scopeChain)) {
     return NS_ERROR_OUT_OF_MEMORY;
   }
   MOZ_ASSERT(scopeChain.length() != 0);
 
   // Clone the function object, using our scope chain (for backwards
--- a/dom/xbl/test/mochitest.ini
+++ b/dom/xbl/test/mochitest.ini
@@ -34,8 +34,9 @@ support-files =
 [test_bug591198.html]
 [test_bug639338.xhtml]
 [test_bug790265.xhtml]
 [test_bug821850.html]
 [test_bug844783.html]
 [test_bug872273.xhtml]
 [test_bug1086996.xhtml]
 [test_bug1098628_throw_from_construct.xhtml]
+[test_bug1359859.xhtml]
\ No newline at end of file
--- a/dom/xbl/test/test_bug1098628_throw_from_construct.xhtml
+++ b/dom/xbl/test/test_bug1098628_throw_from_construct.xhtml
@@ -6,17 +6,16 @@ https://bugzilla.mozilla.org/show_bug.cg
   <title>Test for Bug 1098628</title>
   <script type="text/javascript" src="/tests/SimpleTest/SimpleTest.js"></script>
   <link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css" />
   <script class="testbody" type="text/javascript">
   <![CDATA[
 
   /** Test for Bug 1098628 **/
   SimpleTest.waitForExplicitFinish();
-  SimpleTest.expectUncaughtException();
   SimpleTest.monitorConsole(SimpleTest.finish, [{errorMessage: new RegExp('flimfniffle')}]);
   addLoadEvent(function() {
     SimpleTest.executeSoon(SimpleTest.endMonitorConsole);
   });
   ]]>
   </script>
   <bindings xmlns="http://www.mozilla.org/xbl">
     <binding id="test">
new file mode 100644
--- /dev/null
+++ b/dom/xbl/test/test_bug1359859.xhtml
@@ -0,0 +1,41 @@
+<html xmlns="http://www.w3.org/1999/xhtml">
+  <!--
+  https://bugzilla.mozilla.org/show_bug.cgi?id=1359859
+  -->
+  <head>
+    <script type="application/javascript" src="/tests/SimpleTest/SimpleTest.js"></script>
+    <link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css"/>
+    <bindings xmlns="http://www.mozilla.org/xbl">
+      <binding id="testBinding">
+        <implementation>
+          <constructor>
+            XPCNativeWrapper.unwrap(window).running();
+            this.constructed = true;
+            throw new Error("Constructor threw");
+          </constructor>
+          <field name="throwingField">throw new Error("field threw")</field>
+          <field name="normalField">"hello"</field>
+        </implementation>
+      </binding>
+    </bindings>
+    <script>
+      // We need to wait for the binding to load.
+      SimpleTest.waitForExplicitFinish();
+      function running() {
+        // Wait for the rest of the constructor to run
+        SimpleTest.executeSoon(function() {
+          is(document.getElementById("bound").throwingField, undefined,
+             "Should not have a value for a throwing field");
+          is(document.getElementById("bound").normalField, "hello",
+             "Binding should be installed");
+          // The real test is that we haven't gotten any error events so far.
+          SimpleTest.finish();
+        });
+      }
+    </script>
+  </head>
+  <body>
+    <div id="bound" style="-moz-binding: url(#testBinding)"/>
+  </body>
+</html>
+