Bug 1276276 part 1. Make AutoJSAPI hold a strong ref to the nsIGlobalObject it's initialized with, so it won't go away while we're working with it. r=smaug
authorBoris Zbarsky <bzbarsky@mit.edu>
Thu, 02 Jun 2016 10:34:39 -0400
changeset 341199 20835baaf80fb66a98261e0399b4a4ac5dd2672b
parent 341198 508c43bf55788e76bd05ee5bdffdbd3711d19132
child 341200 f70ca2787f508594e19363128987431552b94bd9
push id1183
push userraliiev@mozilla.com
push dateMon, 05 Sep 2016 20:01:49 +0000
treeherdermozilla-release@3148731bed45 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssmaug
bugs1276276
milestone49.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 1276276 part 1. Make AutoJSAPI hold a strong ref to the nsIGlobalObject it's initialized with, so it won't go away while we're working with it. r=smaug
dom/base/ScriptSettings.cpp
dom/base/ScriptSettings.h
--- a/dom/base/ScriptSettings.cpp
+++ b/dom/base/ScriptSettings.cpp
@@ -339,26 +339,30 @@ AutoJSAPI::~AutoJSAPI()
   }
 }
 
 void
 WarningOnlyErrorReporter(JSContext* aCx, const char* aMessage,
                          JSErrorReport* aRep);
 
 void
-AutoJSAPI::InitInternal(JSObject* aGlobal, JSContext* aCx, bool aIsMainThread)
+AutoJSAPI::InitInternal(nsIGlobalObject* aGlobalObject, JSObject* aGlobal,
+                        JSContext* aCx, bool aIsMainThread)
 {
   MOZ_ASSERT(aCx);
   MOZ_ASSERT(aIsMainThread == NS_IsMainThread());
+  MOZ_ASSERT(bool(aGlobalObject) == bool(aGlobal));
+  MOZ_ASSERT_IF(aGlobalObject, aGlobalObject->GetGlobalJSObject() == aGlobal);
 #ifdef DEBUG
   bool haveException = JS_IsExceptionPending(aCx);
 #endif // DEBUG
 
   mCx = aCx;
   mIsMainThread = aIsMainThread;
+  mGlobalObject = aGlobalObject;
   if (aIsMainThread) {
     // This Rooted<> is necessary only as long as AutoCxPusher::AutoCxPusher
     // can GC, which is only possible because XPCJSContextStack::Push calls
     // nsIPrincipal.Equals. Once that is removed, the Rooted<> will no longer
     // be necessary.
     JS::Rooted<JSObject*> global(JS_GetRuntime(aCx), aGlobal);
     mCxPusher.emplace(mCx);
     mAutoNullableCompartment.emplace(mCx, global);
@@ -446,25 +450,26 @@ AutoJSAPI::AutoJSAPI(nsIGlobalObject* aG
   : mOldAutoJSAPIOwnsErrorReporting(false)
   , mIsMainThread(aIsMainThread)
 {
   MOZ_ASSERT(aGlobalObject);
   MOZ_ASSERT(aGlobalObject->GetGlobalJSObject(), "Must have a JS global");
   MOZ_ASSERT(aCx);
   MOZ_ASSERT(aIsMainThread == NS_IsMainThread());
 
-  InitInternal(aGlobalObject->GetGlobalJSObject(), aCx, aIsMainThread);
+  InitInternal(aGlobalObject, aGlobalObject->GetGlobalJSObject(), aCx,
+               aIsMainThread);
 }
 
 void
 AutoJSAPI::Init()
 {
   MOZ_ASSERT(!mCx, "An AutoJSAPI should only be initialised once");
 
-  InitInternal(/* aGlobal */ nullptr,
+  InitInternal(/* aGlobalObject */ nullptr, /* aGlobal */ nullptr,
                nsContentUtils::GetDefaultJSContextForThread(),
                NS_IsMainThread());
 }
 
 bool
 AutoJSAPI::Init(nsIGlobalObject* aGlobalObject, JSContext* aCx)
 {
   MOZ_ASSERT(!mCx, "An AutoJSAPI should only be initialised once");
@@ -474,17 +479,17 @@ AutoJSAPI::Init(nsIGlobalObject* aGlobal
     return false;
   }
 
   JSObject* global = aGlobalObject->GetGlobalJSObject();
   if (NS_WARN_IF(!global)) {
     return false;
   }
 
-  InitInternal(global, aCx, NS_IsMainThread());
+  InitInternal(aGlobalObject, global, aCx, NS_IsMainThread());
   return true;
 }
 
 bool
 AutoJSAPI::Init(nsIGlobalObject* aGlobalObject)
 {
   return Init(aGlobalObject, nsContentUtils::GetDefaultJSContextForThread());
 }
--- a/dom/base/ScriptSettings.h
+++ b/dom/base/ScriptSettings.h
@@ -300,27 +300,33 @@ protected:
   // Protected constructor, allowing subclasses to specify a particular cx to
   // be used. This constructor initialises the AutoJSAPI, so Init must NOT be
   // called on subclasses that use this.
   // If aGlobalObject, its associated JS global or aCx are null this will cause
   // an assertion, as will setting aIsMainThread incorrectly.
   AutoJSAPI(nsIGlobalObject* aGlobalObject, bool aIsMainThread, JSContext* aCx);
 
 private:
+  // We need to hold a strong ref to our global object, so it won't go away
+  // while we're being used.  This _could_ become a JS::Rooted<JSObject*> if we
+  // grabbed our JSContext in our constructor instead of waiting for Init(), so
+  // we could construct this at that point.  It might be worth it do to that.
+  RefPtr<nsIGlobalObject> mGlobalObject;
   mozilla::Maybe<danger::AutoCxPusher> mCxPusher;
   mozilla::Maybe<JSAutoNullableCompartment> mAutoNullableCompartment;
   JSContext *mCx;
 
   // Track state between the old and new error reporting modes.
   bool mOldAutoJSAPIOwnsErrorReporting;
   // Whether we're mainthread or not; set when we're initialized.
   bool mIsMainThread;
   Maybe<JSErrorReporter> mOldErrorReporter;
 
-  void InitInternal(JSObject* aGlobal, JSContext* aCx, bool aIsMainThread);
+  void InitInternal(nsIGlobalObject* aGlobalObject, JSObject* aGlobal,
+                    JSContext* aCx, bool aIsMainThread);
 
   AutoJSAPI(const AutoJSAPI&) = delete;
   AutoJSAPI& operator= (const AutoJSAPI&) = delete;
 };
 
 /*
  * A class that represents a new script entry point.
  *