Bug 1282150 followup: placate the rooting analysis in SimpleGlobalObject::Create so we can reopen the CLOSED TREE
authorBoris Zbarsky <bzbarsky@mit.edu>
Thu, 07 Jul 2016 22:24:37 -0400
changeset 304168 9acdf39431fc0034073c9d9889b959ae3878daa4
parent 304167 8f5af40eb3654406bd13abc5e310386075c515d0
child 304169 db366c498f6350f13d3354c0c3f23b922449dfc1
push id30414
push usercbook@mozilla.com
push dateFri, 08 Jul 2016 09:59:01 +0000
treeherdermozilla-central@45682df2d2d4 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
bugs1282150
milestone50.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1282150 followup: placate the rooting analysis in SimpleGlobalObject::Create so we can reopen the CLOSED TREE
dom/bindings/SimpleGlobalObject.cpp
--- a/dom/bindings/SimpleGlobalObject.cpp
+++ b/dom/bindings/SimpleGlobalObject.cpp
@@ -7,16 +7,17 @@
 #include "mozilla/dom/SimpleGlobalObject.h"
 
 #include "jsapi.h"
 #include "js/Class.h"
 
 #include "nsJSPrincipals.h"
 #include "nsNullPrincipal.h"
 #include "nsThreadUtils.h"
+#include "nsContentUtils.h"
 
 #include "xpcprivate.h"
 
 #include "mozilla/dom/ScriptSettings.h"
 
 namespace mozilla {
 namespace dom {
 
@@ -86,71 +87,77 @@ const js::Class SimpleGlobalClass = {
     &SimpleGlobalClassExtension,
     JS_NULL_OBJECT_OPS
 };
 
 // static
 JSObject*
 SimpleGlobalObject::Create(GlobalType globalType, JS::Handle<JS::Value> proto)
 {
-  AutoJSAPI jsapi;
-  jsapi.Init();
-  JSContext* cx = jsapi.cx();
-
-  JS::CompartmentOptions options;
-  options.creationOptions().setInvisibleToDebugger(true);
-
-  JS::Rooted<JSObject*> global(cx);
+  // We can't root our return value with our AutoJSAPI because the rooting
+  // analysis thinks ~AutoJSAPI can GC.  So we need to root in a scope outside
+  // the lifetime of the AutoJSAPI.
+  JS::Rooted<JSObject*> global(nsContentUtils::RootingCx());
 
-  if (NS_IsMainThread()) {
-    nsCOMPtr<nsIPrincipal> principal = nsNullPrincipal::Create();
-    options.creationOptions().setTrace(xpc::TraceXPCGlobal);
-    global = xpc::CreateGlobalObject(cx, js::Jsvalify(&SimpleGlobalClass),
-                                     nsJSPrincipals::get(principal),
-                                     options);
-  } else {
-    global = JS_NewGlobalObject(cx, js::Jsvalify(&SimpleGlobalClass),
-                                nullptr,
-                                JS::DontFireOnNewGlobalHook, options);
-  }
+  { // Scope to ensure the AutoJSAPI destructor runs before we end up returning
+    AutoJSAPI jsapi;
+    jsapi.Init();
+    JSContext* cx = jsapi.cx();
+
+    JS::CompartmentOptions options;
+    options.creationOptions().setInvisibleToDebugger(true);
 
-  if (!global) {
-    jsapi.ClearException();
-    return nullptr;
-  }
-
-  JSAutoCompartment ac(cx, global);
+    if (NS_IsMainThread()) {
+      nsCOMPtr<nsIPrincipal> principal = nsNullPrincipal::Create();
+      options.creationOptions().setTrace(xpc::TraceXPCGlobal);
+      global = xpc::CreateGlobalObject(cx, js::Jsvalify(&SimpleGlobalClass),
+                                       nsJSPrincipals::get(principal),
+                                       options);
+    } else {
+      global = JS_NewGlobalObject(cx, js::Jsvalify(&SimpleGlobalClass),
+                                  nullptr,
+                                  JS::DontFireOnNewGlobalHook, options);
+    }
 
-  // It's important to create the nsIGlobalObject for our new global before we
-  // start trying to wrap things like the prototype into its compartment,
-  // because the wrap operation relies on the global having its nsIGlobalObject
-  // already.
-  RefPtr<SimpleGlobalObject> globalObject =
-    new SimpleGlobalObject(global, globalType);
-
-  // Pass on ownership of globalObject to |global|.
-  JS_SetPrivate(global, globalObject.forget().take());
-
-  if (proto.isObjectOrNull()) {
-    JS::Rooted<JSObject*> protoObj(cx, proto.toObjectOrNull());
-    if (!JS_WrapObject(cx, &protoObj)) {
+    if (!global) {
       jsapi.ClearException();
       return nullptr;
     }
 
-    if (!JS_SplicePrototype(cx, global, protoObj)) {
-      jsapi.ClearException();
+    JSAutoCompartment ac(cx, global);
+
+    // It's important to create the nsIGlobalObject for our new global before we
+    // start trying to wrap things like the prototype into its compartment,
+    // because the wrap operation relies on the global having its
+    // nsIGlobalObject already.
+    RefPtr<SimpleGlobalObject> globalObject =
+      new SimpleGlobalObject(global, globalType);
+
+    // Pass on ownership of globalObject to |global|.
+    JS_SetPrivate(global, globalObject.forget().take());
+
+    if (proto.isObjectOrNull()) {
+      JS::Rooted<JSObject*> protoObj(cx, proto.toObjectOrNull());
+      if (!JS_WrapObject(cx, &protoObj)) {
+        jsapi.ClearException();
+        return nullptr;
+      }
+
+      if (!JS_SplicePrototype(cx, global, protoObj)) {
+        jsapi.ClearException();
+        return nullptr;
+      }
+    } else if (!proto.isUndefined()) {
+      // Bogus proto.
       return nullptr;
     }
-  } else if (!proto.isUndefined()) {
-    // Bogus proto.
-    return nullptr;
+
+    JS_FireOnNewGlobalObject(cx, global);
   }
 
-  JS_FireOnNewGlobalObject(cx, global);
   return global;
 }
 
 // static
 SimpleGlobalObject::GlobalType
 SimpleGlobalObject::SimpleGlobalType(JSObject* obj)
 {
   if (js::GetObjectClass(obj) != &SimpleGlobalClass) {