Bug 1487167 - Various DOM rooting issues. r=bz, a=RyanVM
authorSteve Fink <sfink@mozilla.com>
Tue, 28 Aug 2018 21:26:50 -0700
changeset 490184 3e4a2d14ca356812851c177c5d7fed4b32566e6f
parent 490183 b1b3f1e5ec42a960fddf10f10ae8a0ecba93f784
child 490185 c3df2456a08175e477a13e322b577a045d132cb9
push id9938
push userryanvm@gmail.com
push dateFri, 05 Oct 2018 15:41:38 +0000
treeherdermozilla-beta@3e4a2d14ca35 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbz, RyanVM
bugs1487167
milestone63.0
Bug 1487167 - Various DOM rooting issues. r=bz, a=RyanVM
dom/base/ContentFrameMessageManager.cpp
dom/base/ContentProcessMessageManager.cpp
dom/base/CustomElementRegistry.cpp
dom/bindings/DOMJSProxyHandler.cpp
dom/serviceworkers/ServiceWorkerContainer.cpp
--- a/dom/base/ContentFrameMessageManager.cpp
+++ b/dom/base/ContentFrameMessageManager.cpp
@@ -1,25 +1,29 @@
 /* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
 /* vim: set ts=8 sts=2 et sw=2 tw=80: */
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #include "ContentFrameMessageManager.h"
+#include "js/RootingAPI.h"
 #include "mozilla/dom/ScriptSettings.h"
 
 using namespace mozilla;
 using namespace mozilla::dom;
 
 JSObject*
 ContentFrameMessageManager::GetOrCreateWrapper()
 {
-  AutoJSAPI jsapi;
-  jsapi.Init();
+  JS::RootedValue val(RootingCx());
+  {
+    // Scope to run ~AutoJSAPI before working with a raw JSObject*.
+    AutoJSAPI jsapi;
+    jsapi.Init();
 
-  JS::RootedValue val(jsapi.cx());
-  if (!GetOrCreateDOMReflectorNoWrap(jsapi.cx(), this, &val)) {
-    return nullptr;
+    if (!GetOrCreateDOMReflectorNoWrap(jsapi.cx(), this, &val)) {
+      return nullptr;
+    }
   }
   MOZ_ASSERT(val.isObject());
   return &val.toObject();
 }
--- a/dom/base/ContentProcessMessageManager.cpp
+++ b/dom/base/ContentProcessMessageManager.cpp
@@ -109,23 +109,27 @@ ContentProcessMessageManager::WrapObject
                                          JS::Handle<JSObject*> aGivenProto)
 {
   return ContentProcessMessageManager_Binding::Wrap(aCx, this, aGivenProto);
 }
 
 JSObject*
 ContentProcessMessageManager::GetOrCreateWrapper()
 {
-  AutoJSAPI jsapi;
-  jsapi.Init();
+  JS::RootedValue val(RootingCx());
+  {
+    // Scope to run ~AutoJSAPI before working with a raw JSObject*.
+    AutoJSAPI jsapi;
+    jsapi.Init();
 
-  JS::RootedValue val(jsapi.cx());
-  if (!GetOrCreateDOMReflectorNoWrap(jsapi.cx(), this, &val)) {
-    return nullptr;
+    if (!GetOrCreateDOMReflectorNoWrap(jsapi.cx(), this, &val)) {
+      return nullptr;
+    }
   }
+  MOZ_ASSERT(val.isObject());
   return &val.toObject();
 }
 
 void
 ContentProcessMessageManager::LoadScript(const nsAString& aURL)
 {
   Init();
   JS::Rooted<JSObject*> messageManager(mozilla::dom::RootingCx(), GetOrCreateWrapper());
--- a/dom/base/CustomElementRegistry.cpp
+++ b/dom/base/CustomElementRegistry.cpp
@@ -747,17 +747,17 @@ CustomElementRegistry::GetDocGroup() con
 {
   return mWindow ? mWindow->GetDocGroup() : nullptr;
 }
 
 int32_t
 CustomElementRegistry::InferNamespace(JSContext* aCx,
                                       JS::Handle<JSObject*> constructor)
 {
-  JSObject* XULConstructor = XULElement_Binding::GetConstructorObject(aCx);
+  JS::Rooted<JSObject*> XULConstructor(aCx, XULElement_Binding::GetConstructorObject(aCx));
 
   JS::Rooted<JSObject*> proto(aCx, constructor);
   while (proto) {
     if (proto == XULConstructor) {
       return kNameSpaceID_XUL;
     }
 
     JS_GetPrototype(aCx, proto, &proto);
--- a/dom/bindings/DOMJSProxyHandler.cpp
+++ b/dom/bindings/DOMJSProxyHandler.cpp
@@ -100,16 +100,19 @@ CheckExpandoAndGeneration(JSObject* prox
 static inline void
 CheckDOMProxy(JSObject* proxy)
 {
 #ifdef DEBUG
   MOZ_ASSERT(IsDOMProxy(proxy), "expected a DOM proxy object");
   MOZ_ASSERT(!js::gc::EdgeNeedsSweepUnbarriered(&proxy));
   nsISupports* native = UnwrapDOMObject<nsISupports>(proxy);
   nsWrapperCache* cache;
+  // QI to nsWrapperCache cannot GC for very non-obvious reasons; see
+  // https://searchfox.org/mozilla-central/rev/55da592d85c2baf8d8818010c41d9738c97013d2/js/xpconnect/src/XPCWrappedJSClass.cpp#521,545-548
+  JS::AutoSuppressGCAnalysis nogc;
   CallQueryInterface(native, &cache);
   MOZ_ASSERT(cache->GetWrapperPreserveColor() == proxy);
 #endif
 }
 
 // static
 JSObject*
 DOMProxyHandler::GetAndClearExpandoObject(JSObject* obj)
--- a/dom/serviceworkers/ServiceWorkerContainer.cpp
+++ b/dom/serviceworkers/ServiceWorkerContainer.cpp
@@ -82,21 +82,21 @@ ServiceWorkerContainer::IsEnabled(JSCont
   if (!DOMPrefs::ServiceWorkersEnabled()) {
     return false;
   }
 
   if (IsInPrivateBrowsing(aCx)) {
     return false;
   }
 
-  if (IsSecureContextOrObjectIsFromSecureContext(aCx, aGlobal)) {
+  if (IsSecureContextOrObjectIsFromSecureContext(aCx, global)) {
     return true;
   }
 
-  const bool isTestingEnabledInWindow = IsServiceWorkersTestingEnabledInWindow(aGlobal);
+  const bool isTestingEnabledInWindow = IsServiceWorkersTestingEnabledInWindow(global);
   const bool isTestingEnabledByPref = DOMPrefs::ServiceWorkersTestingEnabled();
   const bool isTestingEnabled = isTestingEnabledByPref || isTestingEnabledInWindow;
 
   return isTestingEnabled;
 }
 
 // static
 already_AddRefed<ServiceWorkerContainer>