Bug 1560667 - Collection of fixes for things uncovered by improvements to the hazard analysis. r=bzbarsky
authorSteve Fink <sfink@mozilla.com>
Wed, 02 Oct 2019 03:20:33 +0000
changeset 495914 ef6db5e180346ec17f0f039e60f4e4505572ccaa
parent 495913 d721104b2c8cf4afaac54529f31fdd3e57ec0a85
child 495915 55b80c2e2228aeddd724898e6f2511b9099c5f24
push id114140
push userdvarga@mozilla.com
push dateWed, 02 Oct 2019 18:04:51 +0000
treeherdermozilla-inbound@32eb0ea893f3 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbzbarsky
bugs1560667
milestone71.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 1560667 - Collection of fixes for things uncovered by improvements to the hazard analysis. r=bzbarsky Differential Revision: https://phabricator.services.mozilla.com/D46534
dom/base/SerializedStackHolder.cpp
dom/base/StructuredCloneHolder.cpp
dom/base/test/gtest/TestContentUtils.cpp
dom/bindings/BindingUtils.h
dom/script/ScriptSettings.cpp
dom/workers/RuntimeService.cpp
js/src/gc/Zone.cpp
js/xpconnect/loader/mozJSComponentLoader.cpp
js/xpconnect/loader/mozJSSubScriptLoader.cpp
js/xpconnect/loader/mozJSSubScriptLoader.h
js/xpconnect/src/XPCComponents.cpp
js/xpconnect/src/XPCConvert.cpp
js/xpconnect/src/XPCJSRuntime.cpp
js/xpconnect/src/XPCVariant.cpp
js/xpconnect/src/XPCWrappedNativeJSOps.cpp
js/xpconnect/src/nsXPConnect.cpp
storage/mozStorageStatementJSHelper.cpp
toolkit/components/places/History.cpp
toolkit/components/telemetry/geckoview/TelemetryGeckoViewPersistence.cpp
toolkit/components/telemetry/tests/gtest/TelemetryFixture.cpp
xpcom/base/CycleCollectedJSRuntime.cpp
--- a/dom/base/SerializedStackHolder.cpp
+++ b/dom/base/SerializedStackHolder.cpp
@@ -68,23 +68,27 @@ void SerializedStackHolder::SerializeCur
 }
 
 JSObject* SerializedStackHolder::ReadStack(JSContext* aCx) {
   MOZ_ASSERT(NS_IsMainThread());
   if (!mHolder.HasData()) {
     return nullptr;
   }
 
-  Maybe<nsJSPrincipals::AutoSetActiveWorkerPrincipal> set;
-  if (mWorkerRef) {
-    set.emplace(mWorkerRef->Private()->GetPrincipal());
+  JS::RootedValue stackValue(aCx);
+
+  {
+    Maybe<nsJSPrincipals::AutoSetActiveWorkerPrincipal> set;
+    if (mWorkerRef) {
+      set.emplace(mWorkerRef->Private()->GetPrincipal());
+    }
+
+    mHolder.Read(xpc::CurrentNativeGlobal(aCx), aCx, &stackValue, IgnoreErrors());
   }
 
-  JS::RootedValue stackValue(aCx);
-  mHolder.Read(xpc::CurrentNativeGlobal(aCx), aCx, &stackValue, IgnoreErrors());
   return stackValue.isObject() ? &stackValue.toObject() : nullptr;
 }
 
 UniquePtr<SerializedStackHolder> GetCurrentStackForNetMonitor(JSContext* aCx) {
   MOZ_ASSERT_IF(!NS_IsMainThread(),
                 GetCurrentThreadWorkerPrivate()->IsWatchedByDevtools());
 
   UniquePtr<SerializedStackHolder> stack = MakeUnique<SerializedStackHolder>();
--- a/dom/base/StructuredCloneHolder.cpp
+++ b/dom/base/StructuredCloneHolder.cpp
@@ -341,30 +341,32 @@ JSObject* StructuredCloneHolder::ReadFul
 
   if (aTag == SCTAG_DOM_NULL_PRINCIPAL || aTag == SCTAG_DOM_SYSTEM_PRINCIPAL ||
       aTag == SCTAG_DOM_CONTENT_PRINCIPAL ||
       aTag == SCTAG_DOM_EXPANDED_PRINCIPAL) {
     JSPrincipals* prin;
     if (!nsJSPrincipals::ReadKnownPrincipalType(aCx, aReader, aTag, &prin)) {
       return nullptr;
     }
-    // nsJSPrincipals::ReadKnownPrincipalType addrefs for us, but because of the
-    // casting between JSPrincipals* and nsIPrincipal* we can't use
-    // getter_AddRefs above and have to already_AddRefed here.
-    nsCOMPtr<nsIPrincipal> principal =
-        already_AddRefed<nsIPrincipal>(nsJSPrincipals::get(prin));
 
     JS::RootedValue result(aCx);
-    nsresult rv = nsContentUtils::WrapNative(
+    {
+      // nsJSPrincipals::ReadKnownPrincipalType addrefs for us, but because of
+      // the casting between JSPrincipals* and nsIPrincipal* we can't use
+      // getter_AddRefs above and have to already_AddRefed here.
+      nsCOMPtr<nsIPrincipal> principal =
+        already_AddRefed<nsIPrincipal>(nsJSPrincipals::get(prin));
+
+      nsresult rv = nsContentUtils::WrapNative(
         aCx, principal, &NS_GET_IID(nsIPrincipal), &result);
-    if (NS_FAILED(rv)) {
-      xpc::Throw(aCx, NS_ERROR_DOM_DATA_CLONE_ERR);
-      return nullptr;
+      if (NS_FAILED(rv)) {
+        xpc::Throw(aCx, NS_ERROR_DOM_DATA_CLONE_ERR);
+        return nullptr;
+      }
     }
-
     return result.toObjectOrNull();
   }
 
   // Don't know what this is. Bail.
   xpc::Throw(aCx, NS_ERROR_DOM_DATA_CLONE_ERR);
   return nullptr;
 }
 
@@ -431,27 +433,27 @@ JSObject* ReadBlob(JSContext* aCx, uint3
                    StructuredCloneHolder* aHolder) {
   MOZ_ASSERT(aHolder);
 #ifdef FUZZING
   if (aIndex >= aHolder->BlobImpls().Length()) {
     return nullptr;
   }
 #endif
   MOZ_ASSERT(aIndex < aHolder->BlobImpls().Length());
-  RefPtr<BlobImpl> blobImpl = aHolder->BlobImpls()[aIndex];
-
-  MOZ_ALWAYS_SUCCEEDS(blobImpl->SetMutable(false));
-
-  // RefPtr<File> needs to go out of scope before toObject() is
-  // called because the static analysis thinks dereferencing XPCOM objects
-  // can GC (because in some cases it can!), and a return statement with a
-  // JSObject* type means that JSObject* is on the stack as a raw pointer
-  // while destructors are running.
   JS::Rooted<JS::Value> val(aCx);
   {
+    // RefPtr<File> and RefPtr<BlobImpl> need to go out of scope before
+    // toObject() is called because the static analysis thinks releasing XPCOM
+    // objects can GC (because in some cases it can!), and a return statement
+    // with a JSObject* type means that JSObject* is on the stack as a raw
+    // pointer while destructors are running.
+    RefPtr<BlobImpl> blobImpl = aHolder->BlobImpls()[aIndex];
+
+    MOZ_ALWAYS_SUCCEEDS(blobImpl->SetMutable(false));
+
     RefPtr<Blob> blob = Blob::Create(aHolder->ParentDuringRead(), blobImpl);
     if (!ToJSValue(aCx, blob, &val)) {
       return nullptr;
     }
   }
 
   return &val.toObject();
 }
@@ -826,23 +828,25 @@ JSObject* ReadInputStream(JSContext* aCx
                           StructuredCloneHolder* aHolder) {
   MOZ_ASSERT(aHolder);
 #ifdef FUZZING
   if (aIndex >= aHolder->InputStreams().Length()) {
     return nullptr;
   }
 #endif
   MOZ_ASSERT(aIndex < aHolder->InputStreams().Length());
-  nsCOMPtr<nsIInputStream> inputStream = aHolder->InputStreams()[aIndex];
+  JS::RootedValue result(aCx);
+  {
+    nsCOMPtr<nsIInputStream> inputStream = aHolder->InputStreams()[aIndex];
 
-  JS::RootedValue result(aCx);
-  nsresult rv = nsContentUtils::WrapNative(
+    nsresult rv = nsContentUtils::WrapNative(
       aCx, inputStream, &NS_GET_IID(nsIInputStream), &result);
-  if (NS_FAILED(rv)) {
-    return nullptr;
+    if (NS_FAILED(rv)) {
+      return nullptr;
+    }
   }
 
   return &result.toObject();
 }
 
 bool WriteInputStream(JSStructuredCloneWriter* aWriter,
                       nsIInputStream* aInputStream,
                       StructuredCloneHolder* aHolder) {
@@ -884,20 +888,24 @@ JSObject* StructuredCloneHolder::CustomR
   }
 
   if (aTag == SCTAG_DOM_IMAGEBITMAP &&
       (mStructuredCloneScope == StructuredCloneScope::SameProcessSameThread ||
        mStructuredCloneScope ==
            StructuredCloneScope::SameProcessDifferentThread)) {
     // Get the current global object.
     // This can be null.
-    nsCOMPtr<nsIGlobalObject> parent = do_QueryInterface(mParent);
-    // aIndex is the index of the cloned image.
-    return ImageBitmap::ReadStructuredClone(aCx, aReader, parent, GetSurfaces(),
-                                            aIndex);
+    JS::RootedObject result(aCx);
+    {
+      nsCOMPtr<nsIGlobalObject> parent = do_QueryInterface(mParent);
+      // aIndex is the index of the cloned image.
+      result = ImageBitmap::ReadStructuredClone(aCx, aReader, parent,
+                                                GetSurfaces(), aIndex);
+    }
+    return result;
   }
 
   if (aTag == SCTAG_DOM_STRUCTURED_CLONE_HOLDER) {
     return StructuredCloneBlob::ReadStructuredClone(aCx, aReader, this);
   }
 
   if (aTag == SCTAG_DOM_WASM &&
       (mStructuredCloneScope == StructuredCloneScope::SameProcessSameThread ||
--- a/dom/base/test/gtest/TestContentUtils.cpp
+++ b/dom/base/test/gtest/TestContentUtils.cpp
@@ -8,33 +8,37 @@
 
 #include "jsapi.h"
 #include "nsContentUtils.h"
 #include "mozilla/CycleCollectedJSContext.h"
 #include "mozilla/dom/SimpleGlobalObject.h"
 
 TEST(DOM_Base_ContentUtils, StringifyJSON_EmptyValue)
 {
-  JSObject* globalObject = mozilla::dom::SimpleGlobalObject::Create(
-      mozilla::dom::SimpleGlobalObject::GlobalType::BindingDetail);
+  JS::RootedObject globalObject(
+    mozilla::dom::RootingCx(),
+    mozilla::dom::SimpleGlobalObject::Create(
+      mozilla::dom::SimpleGlobalObject::GlobalType::BindingDetail));
   mozilla::dom::AutoJSAPI jsAPI;
   ASSERT_TRUE(jsAPI.Init(globalObject));
   JSContext* cx = jsAPI.cx();
   nsAutoString serializedValue;
 
   JS::RootedValue jsValue(cx);
   ASSERT_TRUE(nsContentUtils::StringifyJSON(cx, &jsValue, serializedValue));
 
   ASSERT_TRUE(serializedValue.EqualsLiteral("null"));
 }
 
 TEST(DOM_Base_ContentUtils, StringifyJSON_Object)
 {
-  JSObject* globalObject = mozilla::dom::SimpleGlobalObject::Create(
-      mozilla::dom::SimpleGlobalObject::GlobalType::BindingDetail);
+  JS::RootedObject globalObject(
+    mozilla::dom::RootingCx(),
+    mozilla::dom::SimpleGlobalObject::Create(
+      mozilla::dom::SimpleGlobalObject::GlobalType::BindingDetail));
   mozilla::dom::AutoJSAPI jsAPI;
   ASSERT_TRUE(jsAPI.Init(globalObject));
   JSContext* cx = jsAPI.cx();
   nsAutoString serializedValue;
 
   JS::RootedObject jsObj(cx, JS_NewPlainObject(cx));
   JS::RootedString valueStr(cx, JS_NewStringCopyZ(cx, "Hello World!"));
   ASSERT_TRUE(JS_DefineProperty(cx, jsObj, "key1", valueStr, JSPROP_ENUMERATE));
--- a/dom/bindings/BindingUtils.h
+++ b/dom/bindings/BindingUtils.h
@@ -261,16 +261,22 @@ MOZ_ALWAYS_INLINE nsresult UnwrapObjectI
   // Unwrap into a temporary pointer, because in general unwrapping into
   // something of type U might trigger GC (e.g. release the value currently
   // stored in there, with arbitrary consequences) and invalidate the
   // "unwrappedObj" pointer.
   T* tempValue = nullptr;
   nsresult rv = UnwrapObjectInternal<T, false>(unwrappedObj, tempValue, protoID,
                                                protoDepth, nullptr);
   if (NS_SUCCEEDED(rv)) {
+    // Suppress a hazard related to keeping tempValue alive across
+    // UnwrapObjectInternal, because the analysis can't tell that this function
+    // will not GC if maybeWrapped=False and we've already gone through a level
+    // of unwrapping so unwrappedObj will be !IsWrapper.
+    JS::AutoSuppressGCAnalysis suppress;
+
     // It's very important to not update "obj" with the "unwrappedObj" value
     // until we know the unwrap has succeeded.  Otherwise, in a situation in
     // which we have an overload of object and primitive we could end up
     // converting to the primitive from the unwrappedObj, whereas we want to do
     // it from the original object.
     obj = unwrappedObj;
     // And now assign to "value"; at this point we don't care if a GC happens
     // and invalidates unwrappedObj.
@@ -1511,22 +1517,26 @@ bool WrapObject(JSContext* cx, const Win
 
 // Given an object "p" that inherits from nsISupports, wrap it and return the
 // result.  Null is returned on wrapping failure.  This is somewhat similar to
 // WrapObject() above, but does NOT allow Xrays around the result, since we
 // don't want those for our parent object.
 template <typename T>
 static inline JSObject* WrapNativeISupports(JSContext* cx, T* p,
                                             nsWrapperCache* cache) {
-  xpcObjectHelper helper(ToSupports(p), cache);
-  JS::Rooted<JSObject*> scope(cx, JS::CurrentGlobalOrNull(cx));
-  JS::Rooted<JS::Value> v(cx);
-  return XPCOMObjectToJsval(cx, scope, helper, nullptr, false, &v)
-             ? v.toObjectOrNull()
-             : nullptr;
+  JS::Rooted<JSObject*> retval(cx);
+  {
+    xpcObjectHelper helper(ToSupports(p), cache);
+    JS::Rooted<JSObject*> scope(cx, JS::CurrentGlobalOrNull(cx));
+    JS::Rooted<JS::Value> v(cx);
+    retval = XPCOMObjectToJsval(cx, scope, helper, nullptr, false, &v)
+      ? v.toObjectOrNull()
+      : nullptr;
+  }
+  return retval;
 }
 
 // Wrapping of our native parent, for cases when it's a WebIDL object.
 template <typename T, bool hasWrapObject = NativeHasMember<T>::WrapObject>
 struct WrapNativeHelper {
   static inline JSObject* Wrap(JSContext* cx, T* parent,
                                nsWrapperCache* cache) {
     MOZ_ASSERT(cache);
--- a/dom/script/ScriptSettings.cpp
+++ b/dom/script/ScriptSettings.cpp
@@ -290,21 +290,21 @@ void AutoJSAPI::InitInternal(nsIGlobalOb
   MOZ_ASSERT_IF(aGlobalObject,
                 aGlobalObject->GetGlobalJSObjectPreserveColor() == aGlobal);
 #ifdef DEBUG
   bool haveException = JS_IsExceptionPending(aCx);
 #endif  // DEBUG
 
   mCx = aCx;
   mIsMainThread = aIsMainThread;
-  mGlobalObject = aGlobalObject;
   if (aGlobal) {
     JS::AssertObjectIsNotGray(aGlobal);
   }
   mAutoNullableRealm.emplace(mCx, aGlobal);
+  mGlobalObject = aGlobalObject;
 
   ScriptSettingsStack::Push(this);
 
   mOldWarningReporter.emplace(JS::GetWarningReporter(aCx));
 
   JS::SetWarningReporter(aCx, WarningOnlyErrorReporter);
 
 #ifdef DEBUG
--- a/dom/workers/RuntimeService.cpp
+++ b/dom/workers/RuntimeService.cpp
@@ -809,25 +809,25 @@ bool InitJSContextForWorker(WorkerPrivat
 static bool PreserveWrapper(JSContext* cx, JS::HandleObject obj) {
   MOZ_ASSERT(cx);
   MOZ_ASSERT(obj);
   MOZ_ASSERT(mozilla::dom::IsDOMObject(obj));
 
   return mozilla::dom::TryPreserveWrapper(obj);
 }
 
-static bool IsWorkerDebuggerGlobalOrSandbox(JSObject* aGlobal) {
+static bool IsWorkerDebuggerGlobalOrSandbox(JS::HandleObject aGlobal) {
   return IsWorkerDebuggerGlobal(aGlobal) || IsWorkerDebuggerSandbox(aGlobal);
 }
 
 JSObject* Wrap(JSContext* cx, JS::HandleObject existing, JS::HandleObject obj) {
-  JSObject* targetGlobal = JS::CurrentGlobalOrNull(cx);
+  JS::RootedObject targetGlobal(cx, JS::CurrentGlobalOrNull(cx));
 
   // Note: the JS engine unwraps CCWs before calling this callback.
-  JSObject* originGlobal = JS::GetNonCCWObjectGlobal(obj);
+  JS::RootedObject originGlobal(cx, JS::GetNonCCWObjectGlobal(obj));
 
   const js::Wrapper* wrapper = nullptr;
   if (IsWorkerDebuggerGlobalOrSandbox(targetGlobal) &&
       IsWorkerDebuggerGlobalOrSandbox(originGlobal)) {
     wrapper = &js::CrossCompartmentWrapper::singleton;
   } else {
     wrapper = &js::OpaqueCrossCompartmentWrapper::singleton;
   }
--- a/js/src/gc/Zone.cpp
+++ b/js/src/gc/Zone.cpp
@@ -622,16 +622,20 @@ void Zone::addSizeOfIncludingThis(
 }
 
 void* ZoneAllocator::onOutOfMemory(js::AllocFunction allocFunc,
                                    arena_id_t arena, size_t nbytes,
                                    void* reallocPtr) {
   if (!js::CurrentThreadCanAccessRuntime(runtime_)) {
     return nullptr;
   }
+  // The analysis sees that JSRuntime::onOutOfMemory could report an error,
+  // which with a JSErrorInterceptor could GC. But we're passing a null cx (to
+  // a default parameter) so the error will not be reported.
+  JS::AutoSuppressGCAnalysis suppress;
   return runtimeFromMainThread()->onOutOfMemory(allocFunc, arena, nbytes,
                                                 reallocPtr);
 }
 
 void ZoneAllocator::reportAllocationOverflow() const {
   js::ReportAllocationOverflow(nullptr);
 }
 
--- a/js/xpconnect/loader/mozJSComponentLoader.cpp
+++ b/js/xpconnect/loader/mozJSComponentLoader.cpp
@@ -687,37 +687,42 @@ JSObject* mozJSComponentLoader::PrepareO
     NS_ENSURE_TRUE(thisObj, nullptr);
   }
 
   *aRealFile = false;
 
   // need to be extra careful checking for URIs pointing to files
   // EnsureFile may not always get called, especially on resource URIs
   // so we need to call GetFile to make sure this is a valid file
-  nsresult rv = NS_OK;
-  nsCOMPtr<nsIFileURL> fileURL = do_QueryInterface(aURI, &rv);
-  nsCOMPtr<nsIFile> testFile;
-  if (NS_SUCCEEDED(rv)) {
-    fileURL->GetFile(getter_AddRefs(testFile));
-  }
-
-  if (testFile) {
-    *aRealFile = true;
+  {
+    // Create an extra scope so that ~nsCOMPtr will run before the returned
+    // JSObject* is placed on the stack, since otherwise a GC in the destructor
+    // would invalidate the return value.
+    nsresult rv = NS_OK;
+    nsCOMPtr<nsIFileURL> fileURL = do_QueryInterface(aURI, &rv);
+    nsCOMPtr<nsIFile> testFile;
+    if (NS_SUCCEEDED(rv)) {
+      fileURL->GetFile(getter_AddRefs(testFile));
+    }
 
-    if (XRE_IsParentProcess()) {
-      RootedObject locationObj(aCx);
+    if (testFile) {
+      *aRealFile = true;
+
+      if (XRE_IsParentProcess()) {
+        RootedObject locationObj(aCx);
 
-      rv = nsXPConnect::XPConnect()->WrapNative(aCx, thisObj, aComponentFile,
-                                                NS_GET_IID(nsIFile),
-                                                locationObj.address());
-      NS_ENSURE_SUCCESS(rv, nullptr);
-      NS_ENSURE_TRUE(locationObj, nullptr);
+        rv = nsXPConnect::XPConnect()->WrapNative(aCx, thisObj, aComponentFile,
+                                                  NS_GET_IID(nsIFile),
+                                                  locationObj.address());
+        NS_ENSURE_SUCCESS(rv, nullptr);
+        NS_ENSURE_TRUE(locationObj, nullptr);
 
-      if (!JS_DefineProperty(aCx, thisObj, "__LOCATION__", locationObj, 0)) {
-        return nullptr;
+        if (!JS_DefineProperty(aCx, thisObj, "__LOCATION__", locationObj, 0)) {
+          return nullptr;
+        }
       }
     }
   }
 
   // Expose the URI from which the script was imported through a special
   // variable that we insert into the JSM.
   RootedString exposedUri(
       aCx, JS_NewStringCopyN(aCx, nativePath.get(), nativePath.Length()));
--- a/js/xpconnect/loader/mozJSSubScriptLoader.cpp
+++ b/js/xpconnect/loader/mozJSSubScriptLoader.cpp
@@ -237,17 +237,18 @@ static bool EvalScript(JSContext* cx, Ha
       JSAutoRealm ar(cx, script);
       WriteCachedScript(StartupCache::GetSingleton(), cachePath, cx, script);
     }
   }
 
   return true;
 }
 
-JSScript* mozJSSubScriptLoader::ReadScript(
+bool mozJSSubScriptLoader::ReadScript(
+    JS::MutableHandle<JSScript*> script,
     nsIURI* uri, JSContext* cx, HandleObject targetObj, const char* uriStr,
     nsIIOService* serv, bool wantReturnValue, bool useCompilationScope) {
   // We create a channel and call SetContentType, to avoid expensive MIME type
   // lookups (bug 632490).
   nsCOMPtr<nsIChannel> chan;
   nsCOMPtr<nsIInputStream> instream;
   nsresult rv;
   rv = NS_NewChannel(getter_AddRefs(chan), uri,
@@ -262,52 +263,58 @@ JSScript* mozJSSubScriptLoader::ReadScri
 
   if (NS_SUCCEEDED(rv)) {
     chan->SetContentType(NS_LITERAL_CSTRING("application/javascript"));
     rv = chan->Open(getter_AddRefs(instream));
   }
 
   if (NS_FAILED(rv)) {
     ReportError(cx, LOAD_ERROR_NOSTREAM, uri);
-    return nullptr;
+    return false;
   }
 
   int64_t len = -1;
 
   rv = chan->GetContentLength(&len);
   if (NS_FAILED(rv) || len == -1) {
     ReportError(cx, LOAD_ERROR_NOCONTENT, uri);
-    return nullptr;
+    return false;
   }
 
   if (len > INT32_MAX) {
     ReportError(cx, LOAD_ERROR_CONTENTTOOBIG, uri);
-    return nullptr;
+    return false;
   }
 
   nsCString buf;
   rv = NS_ReadInputStreamToString(instream, buf, len);
-  NS_ENSURE_SUCCESS(rv, nullptr);
+  NS_ENSURE_SUCCESS(rv, false);
 
   Maybe<JSAutoRealm> ar;
 
   // Note that when using the ScriptPreloader cache with loadSubScript, there
   // will be a side-effect of keeping the global that the script was compiled
   // for alive. See note above in EvalScript().
   //
   // This will compile the script in XPConnect compilation scope. When the
   // script is evaluated, it will be cloned into the target scope to be
   // executed, avoiding leaks on the first session when we don't have a
   // startup cache.
   if (useCompilationScope) {
     ar.emplace(cx, xpc::CompilationScope());
   }
 
-  return PrepareScript(uri, cx, JS_IsGlobalObject(targetObj), uriStr, buf.get(),
-                       len, wantReturnValue);
+  JSScript* ret = PrepareScript(uri, cx, JS_IsGlobalObject(targetObj), uriStr, buf.get(),
+                                len, wantReturnValue);
+  if (!ret) {
+    return false;
+  }
+
+  script.set(ret);
+  return true;
 }
 
 NS_IMETHODIMP
 mozJSSubScriptLoader::LoadSubScript(const nsAString& url, HandleValue target,
                                     JSContext* cx, MutableHandleValue retval) {
   /*
    * Loads a local url, referring to UTF-8-encoded data, and evals it into the
    * current cx.  Synchronous. ChromeUtils.compileScript() should be used for
@@ -460,20 +467,18 @@ nsresult mozJSSubScriptLoader::DoLoadSub
     }
   }
 
   if (script) {
     // |script| came from the cache, so don't bother writing it
     // |back there.
     cache = nullptr;
   } else {
-    script =
-        ReadScript(uri, cx, targetObj, static_cast<const char*>(uriStr.get()),
-                   serv, options.wantReturnValue, useCompilationScope);
-    if (!script) {
+    if (!ReadScript(&script, uri, cx, targetObj, static_cast<const char*>(uriStr.get()),
+                    serv, options.wantReturnValue, useCompilationScope)) {
       return NS_OK;
     }
   }
 
   Unused << EvalScript(cx, targetObj, loadScope, retval, uri, !!cache,
                        !ignoreCache && !options.wantReturnValue, &script);
   return NS_OK;
 }
--- a/js/xpconnect/loader/mozJSSubScriptLoader.h
+++ b/js/xpconnect/loader/mozJSSubScriptLoader.h
@@ -26,19 +26,20 @@ class mozJSSubScriptLoader : public mozI
 
   // all the interface method declarations...
   NS_DECL_ISUPPORTS
   NS_DECL_MOZIJSSUBSCRIPTLOADER
 
  private:
   virtual ~mozJSSubScriptLoader();
 
-  JSScript* ReadScript(nsIURI* uri, JSContext* cx, JS::HandleObject targetObj,
-                       const char* uriStr, nsIIOService* serv,
-                       bool wantReturnValue, bool useCompilationScope);
+  bool ReadScript(JS::MutableHandle<JSScript*> script,
+                  nsIURI* uri, JSContext* cx, JS::HandleObject targetObj,
+                  const char* uriStr, nsIIOService* serv,
+                  bool wantReturnValue, bool useCompilationScope);
 
   nsresult ReadScriptAsync(nsIURI* uri, JS::HandleObject targetObj,
                            JS::HandleObject loadScope, nsIIOService* serv,
                            bool wantReturnValue, bool cache,
                            JS::MutableHandleValue retval);
 
   nsresult DoLoadSubScriptWithOptions(const nsAString& url,
                                       LoadSubScriptOptions& options,
--- a/js/xpconnect/src/XPCComponents.cpp
+++ b/js/xpconnect/src/XPCComponents.cpp
@@ -875,28 +875,30 @@ struct MOZ_STACK_CLASS ExceptionArgParse
 
   bool parseStack(HandleValue v) {
     if (!v.isObject()) {
       // eStack has already been initialized to null, which is what we want
       // for any non-object values (including null).
       return true;
     }
 
+    RootedObject stackObj(cx, &v.toObject());
     return NS_SUCCEEDED(xpc->WrapJS(
-        cx, &v.toObject(), NS_GET_IID(nsIStackFrame), getter_AddRefs(eStack)));
+        cx, stackObj, NS_GET_IID(nsIStackFrame), getter_AddRefs(eStack)));
   }
 
   bool parseData(HandleValue v) {
     if (!v.isObject()) {
       // eData has already been initialized to null, which is what we want
       // for any non-object values (including null).
       return true;
     }
 
-    return NS_SUCCEEDED(xpc->WrapJS(cx, &v.toObject(), NS_GET_IID(nsISupports),
+    RootedObject obj(cx, &v.toObject());
+    return NS_SUCCEEDED(xpc->WrapJS(cx, obj, NS_GET_IID(nsISupports),
                                     getter_AddRefs(eData)));
   }
 
   bool parseOptionsObject(HandleObject obj) {
     RootedValue v(cx);
 
     if (!getOption(obj, "result", &v) || (!v.isUndefined() && !parseResult(v)))
       return false;
@@ -2031,19 +2033,20 @@ nsXPCComponents_Utils::Dispatch(HandleVa
     }
   }
 
   // Get an XPCWrappedJS for |runnable|.
   if (!runnable.isObject()) {
     return NS_ERROR_INVALID_ARG;
   }
 
+  RootedObject runnableObj(cx, &runnable.toObject());
   nsCOMPtr<nsIRunnable> run;
   nsresult rv = nsXPConnect::XPConnect()->WrapJS(
-      cx, &runnable.toObject(), NS_GET_IID(nsIRunnable), getter_AddRefs(run));
+      cx, runnableObj, NS_GET_IID(nsIRunnable), getter_AddRefs(run));
   NS_ENSURE_SUCCESS(rv, rv);
   MOZ_ASSERT(run);
 
   // Dispatch.
   return NS_DispatchToMainThread(run);
 }
 
 #define GENERATE_JSCONTEXTOPTION_GETTER_SETTER(_attr, _getter, _setter) \
--- a/js/xpconnect/src/XPCConvert.cpp
+++ b/js/xpconnect/src/XPCConvert.cpp
@@ -45,16 +45,19 @@ using namespace JS;
 
 #define ILLEGAL_CHAR_RANGE(c) (0 != ((c)&0x80))
 
 /***********************************************************/
 
 static JSObject* UnwrapNativeCPOW(nsISupports* wrapper) {
   nsCOMPtr<nsIXPConnectWrappedJS> underware = do_QueryInterface(wrapper);
   if (underware) {
+    // The analysis falsely believes that ~nsCOMPtr can GC because it could
+    // drop the refcount to zero, but that can't happen here.
+    JS::AutoSuppressGCAnalysis nogc;
     JSObject* mainObj = underware->GetJSObject();
     if (mainObj && mozilla::jsipc::IsWrappedCPOW(mainObj)) {
       return mainObj;
     }
   }
   return nullptr;
 }
 
--- a/js/xpconnect/src/XPCJSRuntime.cpp
+++ b/js/xpconnect/src/XPCJSRuntime.cpp
@@ -42,16 +42,17 @@
 
 #include "nsContentUtils.h"
 #include "nsCCUncollectableMarker.h"
 #include "nsCycleCollectionNoteRootCallback.h"
 #include "nsCycleCollector.h"
 #include "jsapi.h"
 #include "js/BuildId.h"  // JS::BuildIdCharVector, JS::SetProcessBuildIdOp
 #include "js/experimental/SourceHook.h"  // js::{,Set}SourceHook
+#include "js/GCAPI.h"
 #include "js/MemoryFunctions.h"
 #include "js/MemoryMetrics.h"
 #include "js/UbiNode.h"
 #include "js/UbiNodeUtils.h"
 #include "mozilla/dom/GeneratedAtomList.h"
 #include "mozilla/dom/BindingUtils.h"
 #include "mozilla/dom/Element.h"
 #include "mozilla/dom/WindowBinding.h"
@@ -3017,16 +3018,18 @@ js::UniquePtr<EdgeRange> ReflectorNode::
     return nullptr;
   }
   // UNWRAP_NON_WRAPPER_OBJECT assumes the object is completely initialized,
   // but ours may not be. Luckily, UnwrapDOMObjectToISupports checks for the
   // uninitialized case (and returns null if uninitialized), so we can use that
   // to guard against uninitialized objects.
   nsISupports* supp = UnwrapDOMObjectToISupports(&get());
   if (supp) {
+    JS::AutoSuppressGCAnalysis nogc; // bug 1582326
+
     nsINode* node;
     // UnwrapDOMObjectToISupports can only return non-null if its argument is
     // an actual DOM object, not a cross-compartment wrapper.
     if (NS_SUCCEEDED(UNWRAP_NON_WRAPPER_OBJECT(Node, &get(), node))) {
       char16_t* edgeName = nullptr;
       if (wantNames) {
         edgeName = NS_xstrdup(u"Reflected Node");
       }
@@ -3271,44 +3274,48 @@ void XPCJSRuntime::RemoveGCCallback(xpcG
   }
 }
 
 JSObject* XPCJSRuntime::GetUAWidgetScope(JSContext* cx,
                                          nsIPrincipal* principal) {
   MOZ_ASSERT(!nsContentUtils::IsSystemPrincipal(principal),
              "Running UA Widget in chrome");
 
-  RefPtr<BasePrincipal> key = BasePrincipal::Cast(principal);
-  if (Principal2JSObjectMap::Ptr p = mUAWidgetScopeMap.lookup(key)) {
-    return p->value();
-  }
-
-  SandboxOptions options;
-  options.sandboxName.AssignLiteral("UA Widget Scope");
-  options.wantXrays = false;
-  options.wantComponents = false;
-  options.isUAWidgetScope = true;
-
-  // Use an ExpandedPrincipal to create asymmetric security.
-  MOZ_ASSERT(!nsContentUtils::IsExpandedPrincipal(principal));
-  nsTArray<nsCOMPtr<nsIPrincipal>> principalAsArray(1);
-  principalAsArray.AppendElement(principal);
-  RefPtr<ExpandedPrincipal> ep = ExpandedPrincipal::Create(
+  RootedObject scope(cx);
+  do {
+    RefPtr<BasePrincipal> key = BasePrincipal::Cast(principal);
+    if (Principal2JSObjectMap::Ptr p = mUAWidgetScopeMap.lookup(key)) {
+      scope = p->value();
+      break; // Need ~RefPtr to run, and potentially GC, before returning.
+    }
+
+    SandboxOptions options;
+    options.sandboxName.AssignLiteral("UA Widget Scope");
+    options.wantXrays = false;
+    options.wantComponents = false;
+    options.isUAWidgetScope = true;
+
+    // Use an ExpandedPrincipal to create asymmetric security.
+    MOZ_ASSERT(!nsContentUtils::IsExpandedPrincipal(principal));
+    nsTArray<nsCOMPtr<nsIPrincipal>> principalAsArray(1);
+    principalAsArray.AppendElement(principal);
+    RefPtr<ExpandedPrincipal> ep = ExpandedPrincipal::Create(
       principalAsArray, principal->OriginAttributesRef());
 
-  // Create the sandbox.
-  RootedValue v(cx);
-  nsresult rv = CreateSandboxObject(
+    // Create the sandbox.
+    RootedValue v(cx);
+    nsresult rv = CreateSandboxObject(
       cx, &v, static_cast<nsIExpandedPrincipal*>(ep), options);
-  NS_ENSURE_SUCCESS(rv, nullptr);
-  JSObject* scope = &v.toObject();
-
-  MOZ_ASSERT(xpc::IsInUAWidgetScope(js::UncheckedUnwrap(scope)));
-
-  MOZ_ALWAYS_TRUE(mUAWidgetScopeMap.putNew(key, scope));
+    NS_ENSURE_SUCCESS(rv, nullptr);
+    scope = &v.toObject();
+
+    MOZ_ASSERT(xpc::IsInUAWidgetScope(js::UncheckedUnwrap(scope)));
+
+    MOZ_ALWAYS_TRUE(mUAWidgetScopeMap.putNew(key, scope));
+  } while (false);
 
   return scope;
 }
 
 void XPCJSRuntime::InitSingletonScopes() {
   // This all happens very early, so we don't bother with cx pushing.
   JSContext* cx = XPCJSContext::Get()->Context();
   RootedValue v(cx);
--- a/js/xpconnect/src/XPCVariant.cpp
+++ b/js/xpconnect/src/XPCVariant.cpp
@@ -279,17 +279,17 @@ bool XPCVariant::InitializeData(JSContex
     mData.SetToVoid();
     return true;
   }
   if (val.isNull()) {
     mData.SetToEmpty();
     return true;
   }
   if (val.isString()) {
-    JSString* str = val.toString();
+    RootedString str(cx, val.toString());
     if (!str) {
       return false;
     }
 
     MOZ_ASSERT(mData.GetType() == nsIDataType::VTYPE_EMPTY,
                "Why do we already have data?");
 
     size_t length = JS_GetStringLength(str);
--- a/js/xpconnect/src/XPCWrappedNativeJSOps.cpp
+++ b/js/xpconnect/src/XPCWrappedNativeJSOps.cpp
@@ -187,19 +187,22 @@ static bool XPC_WN_Shared_toPrimitive(JS
  *        scriptable type.
  *   ii)  Avoiding the explicit interface makes it easier for both the caller
  *        and the component.
  */
 
 static JSObject* GetDoubleWrappedJSObject(XPCCallContext& ccx,
                                           XPCWrappedNative* wrapper) {
   RootedObject obj(ccx);
-  nsCOMPtr<nsIXPConnectWrappedJS> underware =
+  {
+    nsCOMPtr<nsIXPConnectWrappedJS> underware =
       do_QueryInterface(wrapper->GetIdentityObject());
-  if (underware) {
+    if (!underware) {
+      return nullptr;
+    }
     RootedObject mainObj(ccx, underware->GetJSObject());
     if (mainObj) {
       JSAutoRealm ar(ccx, underware->GetJSObjectGlobal());
 
       // We don't have to root this ID, as it's already rooted by our context.
       HandleId id =
           ccx.GetContext()->GetStringID(XPCJSContext::IDX_WRAPPED_JSOBJECT);
 
--- a/js/xpconnect/src/nsXPConnect.cpp
+++ b/js/xpconnect/src/nsXPConnect.cpp
@@ -432,52 +432,54 @@ JSObject* CreateGlobalObject(JSContext* 
                              JS::RealmOptions& aOptions) {
   MOZ_ASSERT(NS_IsMainThread(), "using a principal off the main thread?");
   MOZ_ASSERT(principal);
 
   MOZ_RELEASE_ASSERT(
       principal != nsContentUtils::GetNullSubjectPrincipal(),
       "The null subject principal is getting inherited - fix that!");
 
-  SiteIdentifier site;
-  nsresult rv = BasePrincipal::Cast(principal)->GetSiteIdentifier(site);
-  NS_ENSURE_SUCCESS(rv, nullptr);
+  RootedObject global(cx);
+  {
+    SiteIdentifier site;
+    nsresult rv = BasePrincipal::Cast(principal)->GetSiteIdentifier(site);
+    NS_ENSURE_SUCCESS(rv, nullptr);
 
-  RootedObject global(
-      cx, JS_NewGlobalObject(cx, clasp, nsJSPrincipals::get(principal),
-                             JS::DontFireOnNewGlobalHook, aOptions));
-  if (!global) {
-    return nullptr;
-  }
-  JSAutoRealm ar(cx, global);
+    global = JS_NewGlobalObject(cx, clasp, nsJSPrincipals::get(principal),
+                                JS::DontFireOnNewGlobalHook, aOptions);
+    if (!global) {
+      return nullptr;
+    }
+    JSAutoRealm ar(cx, global);
 
-  RealmPrivate::Init(global, site);
+    RealmPrivate::Init(global, site);
 
-  if (clasp->flags & JSCLASS_DOM_GLOBAL) {
+    if (clasp->flags & JSCLASS_DOM_GLOBAL) {
 #ifdef DEBUG
-    // Verify that the right trace hook is called. Note that this doesn't
-    // work right for wrapped globals, since the tracing situation there is
-    // more complicated. Manual inspection shows that they do the right
-    // thing.  Also note that we only check this for JSCLASS_DOM_GLOBAL
-    // classes because xpc::TraceXPCGlobal won't call
-    // TraceProtoAndIfaceCache unless that flag is set.
-    if (!((const JSClass*)clasp)->isWrappedNative()) {
-      VerifyTraceProtoAndIfaceCacheCalledTracer trc(cx);
-      TraceChildren(&trc, GCCellPtr(global.get()));
-      MOZ_ASSERT(trc.ok,
-                 "Trace hook on global needs to call TraceXPCGlobal for "
-                 "XPConnect compartments.");
-    }
+      // Verify that the right trace hook is called. Note that this doesn't
+      // work right for wrapped globals, since the tracing situation there is
+      // more complicated. Manual inspection shows that they do the right
+      // thing. Also note that we only check this for JSCLASS_DOM_GLOBAL
+      // classes because xpc::TraceXPCGlobal won't call TraceProtoAndIfaceCache
+      // unless that flag is set.
+      if (!((const JSClass*)clasp)->isWrappedNative()) {
+        VerifyTraceProtoAndIfaceCacheCalledTracer trc(cx);
+        TraceChildren(&trc, GCCellPtr(global.get()));
+        MOZ_ASSERT(trc.ok,
+                   "Trace hook on global needs to call TraceXPCGlobal for "
+                   "XPConnect compartments.");
+      }
 #endif
 
-    const char* className = clasp->name;
-    AllocateProtoAndIfaceCache(global, (strcmp(className, "Window") == 0 ||
-                                        strcmp(className, "ChromeWindow") == 0)
+      const char* className = clasp->name;
+      AllocateProtoAndIfaceCache(global, (strcmp(className, "Window") == 0 ||
+                                          strcmp(className, "ChromeWindow") == 0)
                                            ? ProtoAndIfaceCache::WindowLike
                                            : ProtoAndIfaceCache::NonWindowLike);
+    }
   }
 
   return global;
 }
 
 void InitGlobalObjectOptions(JS::RealmOptions& aOptions,
                              nsIPrincipal* aPrincipal) {
   bool shouldDiscardSystemSource = ShouldDiscardSystemSource();
--- a/storage/mozStorageStatementJSHelper.cpp
+++ b/storage/mozStorageStatementJSHelper.cpp
@@ -33,17 +33,17 @@ static bool stepFunc(JSContext* aCtx, ui
   nsCOMPtr<nsIXPConnect> xpc(nsIXPConnect::XPConnect());
   nsCOMPtr<nsIXPConnectWrappedNative> wrapper;
 
   if (!args.thisv().isObject()) {
     ::JS_ReportErrorASCII(aCtx, "mozIStorageStatement::step() requires object");
     return false;
   }
 
-  JSObject* obj = &args.thisv().toObject();
+  JS::RootedObject obj(aCtx, &args.thisv().toObject());
   nsresult rv =
       xpc->GetWrappedNativeOfJSObject(aCtx, obj, getter_AddRefs(wrapper));
   if (NS_FAILED(rv)) {
     ::JS_ReportErrorASCII(
         aCtx, "mozIStorageStatement::step() could not obtain native statement");
     return false;
   }
 
--- a/toolkit/components/places/History.cpp
+++ b/toolkit/components/places/History.cpp
@@ -221,17 +221,18 @@ nsresult GetJSArrayFromJSValue(JS::Handl
  * @return the nsIURI object, or null if aValue is not a nsIURI object.
  */
 already_AddRefed<nsIURI> GetJSValueAsURI(JSContext* aCtx,
                                          const JS::Value& aValue) {
   if (!aValue.isPrimitive()) {
     nsCOMPtr<nsIXPConnect> xpc = nsIXPConnect::XPConnect();
 
     nsCOMPtr<nsIXPConnectWrappedNative> wrappedObj;
-    nsresult rv = xpc->GetWrappedNativeOfJSObject(aCtx, aValue.toObjectOrNull(),
+    JS::Rooted<JSObject*> obj(aCtx, aValue.toObjectOrNull());
+    nsresult rv = xpc->GetWrappedNativeOfJSObject(aCtx, obj,
                                                   getter_AddRefs(wrappedObj));
     NS_ENSURE_SUCCESS(rv, nullptr);
     nsCOMPtr<nsIURI> uri = do_QueryInterface(wrappedObj->Native());
     return uri.forget();
   }
   return nullptr;
 }
 
--- a/toolkit/components/telemetry/geckoview/TelemetryGeckoViewPersistence.cpp
+++ b/toolkit/components/telemetry/geckoview/TelemetryGeckoViewPersistence.cpp
@@ -238,18 +238,18 @@ void MainThreadArmPersistenceTimer() {
  * the native JS JSON parser.
  */
 void MainThreadParsePersistedProbes(const nsACString& aProbeData) {
   // We're required to run on the main thread since we're using JS.
   MOZ_ASSERT(NS_IsMainThread());
   ANDROID_LOG("MainThreadParsePersistedProbes");
 
   // We need a JS context to run the parsing stuff in.
-  JSObject* cleanGlobal =
-      SimpleGlobalObject::Create(SimpleGlobalObject::GlobalType::BindingDetail);
+  JS::Rooted<JSObject*> cleanGlobal(mozilla::dom::RootingCx(),
+                                    SimpleGlobalObject::Create(SimpleGlobalObject::GlobalType::BindingDetail));
   if (!cleanGlobal) {
     ANDROID_LOG(
         "MainThreadParsePersistedProbes - Failed to create a JS global object");
     return;
   }
 
   AutoJSAPI jsapi;
   if (NS_WARN_IF(!jsapi.Init(cleanGlobal))) {
--- a/toolkit/components/telemetry/tests/gtest/TelemetryFixture.cpp
+++ b/toolkit/components/telemetry/tests/gtest/TelemetryFixture.cpp
@@ -16,12 +16,13 @@ void TelemetryTestFixture::SetUp() {
   // The test must fail if we failed getting the global.
   ASSERT_NE(mCleanGlobal, nullptr)
       << "SimpleGlobalObject must return a valid global object.";
 }
 
 AutoJSContextWithGlobal::AutoJSContextWithGlobal(JSObject* aGlobalObject)
     : mCx(nullptr) {
   // The JS API must initialize correctly.
-  MOZ_ALWAYS_TRUE(mJsAPI.Init(aGlobalObject));
+  JS::Rooted<JSObject*> globalObject(dom::RootingCx(), aGlobalObject);
+  MOZ_ALWAYS_TRUE(mJsAPI.Init(globalObject));
 }
 
 JSContext* AutoJSContextWithGlobal::GetJSContext() const { return mJsAPI.cx(); }
--- a/xpcom/base/CycleCollectedJSRuntime.cpp
+++ b/xpcom/base/CycleCollectedJSRuntime.cpp
@@ -655,54 +655,56 @@ void CycleCollectedJSRuntime::NoteGCThin
 }
 
 void CycleCollectedJSRuntime::NoteGCThingXPCOMChildren(
     const JSClass* aClasp, JSObject* aObj,
     nsCycleCollectionTraversalCallback& aCb) const {
   MOZ_ASSERT(aClasp);
   MOZ_ASSERT(aClasp == js::GetObjectClass(aObj));
 
-  if (NoteCustomGCThingXPCOMChildren(aClasp, aObj, aCb)) {
+  JS::Rooted<JSObject*> obj(RootingCx(), aObj);
+
+  if (NoteCustomGCThingXPCOMChildren(aClasp, obj, aCb)) {
     // Nothing else to do!
     return;
   }
 
   // XXX This test does seem fragile, we should probably whitelist classes
   //     that do hold a strong reference, but that might not be possible.
   if (aClasp->flags & JSCLASS_HAS_PRIVATE &&
       aClasp->flags & JSCLASS_PRIVATE_IS_NSISUPPORTS) {
     NS_CYCLE_COLLECTION_NOTE_EDGE_NAME(aCb, "js::GetObjectPrivate(obj)");
-    aCb.NoteXPCOMChild(static_cast<nsISupports*>(js::GetObjectPrivate(aObj)));
+    aCb.NoteXPCOMChild(static_cast<nsISupports*>(js::GetObjectPrivate(obj)));
     return;
   }
 
   const DOMJSClass* domClass = GetDOMClass(aClasp);
   if (domClass) {
     NS_CYCLE_COLLECTION_NOTE_EDGE_NAME(aCb, "UnwrapDOMObject(obj)");
     // It's possible that our object is an unforgeable holder object, in
     // which case it doesn't actually have a C++ DOM object associated with
     // it.  Use UnwrapPossiblyNotInitializedDOMObject, which produces null in
     // that case, since NoteXPCOMChild/NoteNativeChild are null-safe.
     if (domClass->mDOMObjectIsISupports) {
       aCb.NoteXPCOMChild(
-          UnwrapPossiblyNotInitializedDOMObject<nsISupports>(aObj));
+          UnwrapPossiblyNotInitializedDOMObject<nsISupports>(obj));
     } else if (domClass->mParticipant) {
-      aCb.NoteNativeChild(UnwrapPossiblyNotInitializedDOMObject<void>(aObj),
+      aCb.NoteNativeChild(UnwrapPossiblyNotInitializedDOMObject<void>(obj),
                           domClass->mParticipant);
     }
     return;
   }
 
-  if (IsRemoteObjectProxy(aObj)) {
+  if (IsRemoteObjectProxy(obj)) {
     auto handler =
-        static_cast<const RemoteObjectProxyBase*>(js::GetProxyHandler(aObj));
-    return handler->NoteChildren(aObj, aCb);
+        static_cast<const RemoteObjectProxyBase*>(js::GetProxyHandler(obj));
+    return handler->NoteChildren(obj, aCb);
   }
 
-  JS::Value value = js::MaybeGetScriptPrivate(aObj);
+  JS::Value value = js::MaybeGetScriptPrivate(obj);
   if (!value.isUndefined()) {
     aCb.NoteXPCOMChild(static_cast<nsISupports*>(value.toPrivate()));
   }
 }
 
 void CycleCollectedJSRuntime::TraverseGCThing(
     TraverseSelect aTs, JS::GCCellPtr aThing,
     nsCycleCollectionTraversalCallback& aCb) {