Bug 1176341 - De-holder nsIXPConnect::CreateSandbox. r=baku,gabor
☠☠ backed out by b35af146ed92 ☠ ☠
authorAndrew McCreight <continuation@gmail.com>
Fri, 10 Jul 2015 07:41:33 -0700
changeset 252290 74293125739a75c17f146f594ea533ebb7800124
parent 252289 9814f77d258b2b6b292fb3170b2416e965dc0599
child 252291 3a9b803a09a8e1db74af7a2d582f21c43e33de7f
push id62112
push useramccreight@mozilla.com
push dateFri, 10 Jul 2015 14:41:52 +0000
treeherdermozilla-inbound@74293125739a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbaku, gabor
bugs1176341
milestone42.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 1176341 - De-holder nsIXPConnect::CreateSandbox. r=baku,gabor
dom/base/Console.cpp
dom/base/Console.h
dom/datastore/DataStoreDB.cpp
dom/workers/ScriptLoader.cpp
dom/workers/ServiceWorkerScriptCache.cpp
extensions/pref/autoconfig/src/nsJSConfigTriggers.cpp
js/xpconnect/idl/nsIXPConnect.idl
js/xpconnect/src/nsXPConnect.cpp
--- a/dom/base/Console.cpp
+++ b/dom/base/Console.cpp
@@ -398,23 +398,17 @@ private:
     while (wp->GetParent()) {
       wp = wp->GetParent();
     }
 
     MOZ_ASSERT(!wp->GetWindow());
 
     AutoSafeJSContext cx;
 
-    nsCOMPtr<nsIXPConnectJSObjectHolder> sandbox =
-      mConsole->GetOrCreateSandbox(cx, wp->GetPrincipal());
-    if (NS_WARN_IF(!sandbox)) {
-       return;
-    }
-
-    JS::Rooted<JSObject*> global(cx, sandbox->GetJSObject());
+    JS::Rooted<JSObject*> global(cx, mConsole->GetOrCreateSandbox(cx, wp->GetPrincipal()));
     if (NS_WARN_IF(!global)) {
       return;
     }
 
     // The CreateSandbox call returns a proxy to the actual sandbox object. We
     // don't need a proxy here.
     global = js::UncheckedUnwrap(global);
 
@@ -684,17 +678,17 @@ private:
   Sequence<JS::Value> mArguments;
 
   JSAutoStructuredCloneBuffer mBuffer;
   ConsoleStructuredCloneData mData;
 };
 
 NS_IMPL_CYCLE_COLLECTION_CLASS(Console)
 
-// We don't need to traverse/unlink mStorage and mSanbox because they are not
+// We don't need to traverse/unlink mStorage and mSandbox because they are not
 // CCed objects and they are only used on the main thread, even when this
 // Console object is used on workers.
 
 NS_IMPL_CYCLE_COLLECTION_UNLINK_BEGIN(Console)
   NS_IMPL_CYCLE_COLLECTION_UNLINK(mWindow)
   NS_IMPL_CYCLE_COLLECTION_UNLINK_PRESERVED_WRAPPER
 NS_IMPL_CYCLE_COLLECTION_UNLINK_END
 
@@ -738,29 +732,22 @@ Console::Console(nsPIDOMWindow* aWindow)
   }
 
   mozilla::HoldJSObjects(this);
 }
 
 Console::~Console()
 {
   if (!NS_IsMainThread()) {
-    nsCOMPtr<nsIThread> mainThread;
-    NS_GetMainThread(getter_AddRefs(mainThread));
-
     if (mStorage) {
-      nsIConsoleAPIStorage* storage;
-      mStorage.forget(&storage);
-      NS_ProxyRelease(mainThread, storage, false);
+      NS_ReleaseOnMainThread(mStorage);
     }
 
     if (mSandbox) {
-      nsIXPConnectJSObjectHolder* sandbox;
-      mSandbox.forget(&sandbox);
-      NS_ProxyRelease(mainThread, sandbox, false);
+      NS_ReleaseOnMainThread(mSandbox);
     }
   }
 
   mozilla::DropJSObjects(this);
 }
 
 NS_IMETHODIMP
 Console::Observe(nsISupports* aSubject, const char* aTopic,
@@ -1899,29 +1886,31 @@ Console::ShouldIncludeStackTrace(MethodN
     case MethodAssert:
     case MethodTrace:
       return true;
     default:
       return false;
   }
 }
 
-nsIXPConnectJSObjectHolder*
+JSObject*
 Console::GetOrCreateSandbox(JSContext* aCx, nsIPrincipal* aPrincipal)
 {
   MOZ_ASSERT(NS_IsMainThread());
 
   if (!mSandbox) {
     nsIXPConnect* xpc = nsContentUtils::XPConnect();
     MOZ_ASSERT(xpc, "This should never be null!");
 
-    nsresult rv = xpc->CreateSandbox(aCx, aPrincipal,
-                                     getter_AddRefs(mSandbox));
+    JS::Rooted<JSObject*> sandbox(aCx);
+    nsresult rv = xpc->CreateSandbox(aCx, aPrincipal, sandbox.address());
     if (NS_WARN_IF(NS_FAILED(rv))) {
       return nullptr;
     }
+
+    mSandbox = new JSObjectHolder(aCx, sandbox);
   }
 
-  return mSandbox;
+  return mSandbox->GetJSObject();
 }
 
 } // namespace dom
 } // namespace mozilla
--- a/dom/base/Console.h
+++ b/dom/base/Console.h
@@ -4,28 +4,28 @@
  * 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/. */
 
 #ifndef mozilla_dom_Console_h
 #define mozilla_dom_Console_h
 
 #include "mozilla/dom/BindingDeclarations.h"
 #include "mozilla/ErrorResult.h"
+#include "mozilla/JSObjectHolder.h"
 #include "nsCycleCollectionParticipant.h"
 #include "nsDataHashtable.h"
 #include "nsHashKeys.h"
 #include "nsIObserver.h"
 #include "nsWrapperCache.h"
 #include "nsDOMNavigationTiming.h"
 #include "nsPIDOMWindow.h"
 
 class nsIConsoleAPIStorage;
 class nsIPrincipal;
 class nsIProfiler;
-class nsIXPConnectJSObjectHolder;
 
 namespace mozilla {
 namespace dom {
 
 class ConsoleCallData;
 struct ConsoleStackEntry;
 
 class Console final : public nsIObserver
@@ -194,22 +194,22 @@ private:
 
   JS::Value
   IncreaseCounter(JSContext* aCx, const ConsoleStackEntry& aFrame,
                    const nsTArray<JS::Heap<JS::Value>>& aArguments);
 
   bool
   ShouldIncludeStackTrace(MethodName aMethodName);
 
-  nsIXPConnectJSObjectHolder*
+  JSObject*
   GetOrCreateSandbox(JSContext* aCx, nsIPrincipal* aPrincipal);
 
   nsCOMPtr<nsPIDOMWindow> mWindow;
   nsCOMPtr<nsIConsoleAPIStorage> mStorage;
-  nsCOMPtr<nsIXPConnectJSObjectHolder> mSandbox;
+  nsRefPtr<JSObjectHolder> mSandbox;
 #ifdef MOZ_ENABLE_PROFILER_SPS
   nsCOMPtr<nsIProfiler> mProfiler;
 #endif
 
   nsDataHashtable<nsStringHashKey, DOMHighResTimeStamp> mTimerRegistry;
   nsDataHashtable<nsStringHashKey, uint32_t> mCounterRegistry;
 
   uint64_t mOuterID;
--- a/dom/datastore/DataStoreDB.cpp
+++ b/dom/datastore/DataStoreDB.cpp
@@ -108,28 +108,22 @@ DataStoreDB::CreateFactoryIfNeeded()
     if (!principal) {
       return NS_ERROR_FAILURE;
     }
 
     nsIXPConnect* xpc = nsContentUtils::XPConnect();
     MOZ_ASSERT(xpc);
 
     AutoSafeJSContext cx;
-
-    nsCOMPtr<nsIXPConnectJSObjectHolder> globalHolder;
-    rv = xpc->CreateSandbox(cx, principal, getter_AddRefs(globalHolder));
+    JS::Rooted<JSObject*> global(cx);
+    rv = xpc->CreateSandbox(cx, principal, global.address());
     if (NS_WARN_IF(NS_FAILED(rv))) {
       return rv;
     }
 
-    JS::Rooted<JSObject*> global(cx, globalHolder->GetJSObject());
-    if (NS_WARN_IF(NS_FAILED(rv))) {
-      return NS_ERROR_UNEXPECTED;
-    }
-
     // The CreateSandbox call returns a proxy to the actual sandbox object. We
     // don't need a proxy here.
     global = js::UncheckedUnwrap(global);
 
     JSAutoCompartment ac(cx, global);
 
     rv = IDBFactory::CreateForDatastore(cx, global, getter_AddRefs(mFactory));
     if (NS_WARN_IF(NS_FAILED(rv))) {
--- a/dom/workers/ScriptLoader.cpp
+++ b/dom/workers/ScriptLoader.cpp
@@ -1252,23 +1252,23 @@ CacheCreator::CreateCacheStorage(nsIPrin
   AssertIsOnMainThread();
   MOZ_ASSERT(!mCacheStorage);
   MOZ_ASSERT(aPrincipal);
 
   nsIXPConnect* xpc = nsContentUtils::XPConnect();
   MOZ_ASSERT(xpc, "This should never be null!");
 
   mozilla::AutoSafeJSContext cx;
-  nsCOMPtr<nsIXPConnectJSObjectHolder> sandbox;
-  nsresult rv = xpc->CreateSandbox(cx, aPrincipal, getter_AddRefs(sandbox));
+  JS::Rooted<JSObject*> sandbox(cx);
+  nsresult rv = xpc->CreateSandbox(cx, aPrincipal, sandbox.address());
   if (NS_WARN_IF(NS_FAILED(rv))) {
     return rv;
   }
 
-  mSandboxGlobalObject = xpc::NativeGlobal(sandbox->GetJSObject());
+  mSandboxGlobalObject = xpc::NativeGlobal(sandbox);
   if (NS_WARN_IF(!mSandboxGlobalObject)) {
     return NS_ERROR_FAILURE;
   }
 
   // If we're in private browsing mode, don't even try to create the
   // CacheStorage.  Instead, just fail immediately to terminate the
   // ServiceWorker load.
   if (NS_WARN_IF(mPrivateBrowsing)) {
--- a/dom/workers/ServiceWorkerScriptCache.cpp
+++ b/dom/workers/ServiceWorkerScriptCache.cpp
@@ -29,41 +29,40 @@ using mozilla::dom::cache::CacheStorage;
 BEGIN_WORKERS_NAMESPACE
 
 namespace serviceWorkerScriptCache {
 
 namespace {
 
 already_AddRefed<CacheStorage>
 CreateCacheStorage(nsIPrincipal* aPrincipal, ErrorResult& aRv,
-                   nsIXPConnectJSObjectHolder** aHolder = nullptr)
+                   JS::MutableHandle<JSObject*>* aSandbox = nullptr)
 {
   AssertIsOnMainThread();
   MOZ_ASSERT(aPrincipal);
 
   nsIXPConnect* xpc = nsContentUtils::XPConnect();
   MOZ_ASSERT(xpc, "This should never be null!");
 
   AutoJSAPI jsapi;
   jsapi.Init();
-  nsCOMPtr<nsIXPConnectJSObjectHolder> sandbox;
-  aRv = xpc->CreateSandbox(jsapi.cx(), aPrincipal, getter_AddRefs(sandbox));
+  JS::Rooted<JSObject*> sandbox(jsapi.cx());
+  aRv = xpc->CreateSandbox(jsapi.cx(), aPrincipal, sandbox.address());
   if (NS_WARN_IF(aRv.Failed())) {
     return nullptr;
   }
 
-  nsCOMPtr<nsIGlobalObject> sandboxGlobalObject =
-    xpc::NativeGlobal(sandbox->GetJSObject());
+  nsCOMPtr<nsIGlobalObject> sandboxGlobalObject = xpc::NativeGlobal(sandbox);
   if (!sandboxGlobalObject) {
     aRv.Throw(NS_ERROR_FAILURE);
     return nullptr;
   }
 
-  if (aHolder) {
-    sandbox.forget(aHolder);
+  if (aSandbox) {
+    aSandbox->set(sandbox);
   }
 
   // We assume private browsing is not enabled here.  The ScriptLoader
   // explicitly fails for private browsing so there should never be
   // a service worker running in private browsing mode.  Therefore if
   // we are purging scripts or running a comparison algorithm we cannot
   // be in private browing.
   //
@@ -315,17 +314,18 @@ public:
     AssertIsOnMainThread();
     MOZ_ASSERT(aPrincipal);
 
     mURL = aURL;
 
     // Always create a CacheStorage since we want to write the network entry to
     // the cache even if there isn't an existing one.
     ErrorResult result;
-    mCacheStorage = CreateCacheStorage(aPrincipal, result, getter_AddRefs(mSandbox));
+    JS::MutableHandle<JSObject*> sandboxHandle(&mSandbox);
+    mCacheStorage = CreateCacheStorage(aPrincipal, result, &sandboxHandle);
     if (NS_WARN_IF(result.Failed())) {
       MOZ_ASSERT(!result.IsErrorWithMessage());
       return result.StealNSResult();
     }
 
     mCN = new CompareNetwork(this);
     nsresult rv = mCN->Initialize(aPrincipal, aURL, aLoadGroup);
     if (NS_WARN_IF(NS_FAILED(rv))) {
@@ -616,17 +616,17 @@ private:
       return;
     }
 
     mState = WaitingForPut;
     cachePromise->AppendNativeHandler(this);
   }
 
   nsRefPtr<CompareCallback> mCallback;
-  nsCOMPtr<nsIXPConnectJSObjectHolder> mSandbox;
+  JS::PersistentRooted<JSObject*> mSandbox;
   nsRefPtr<CacheStorage> mCacheStorage;
 
   nsRefPtr<CompareNetwork> mCN;
   nsRefPtr<CompareCache> mCC;
 
   nsString mURL;
   // Only used if the network script has changed and needs to be cached.
   nsString mNewCacheName;
--- a/extensions/pref/autoconfig/src/nsJSConfigTriggers.cpp
+++ b/extensions/pref/autoconfig/src/nsJSConfigTriggers.cpp
@@ -41,23 +41,23 @@ nsresult CentralizedAdminPrefManagerInit
 
     // Grab the system principal.
     nsCOMPtr<nsIPrincipal> principal;
     nsContentUtils::GetSecurityManager()->GetSystemPrincipal(getter_AddRefs(principal));
 
 
     // Create a sandbox.
     AutoSafeJSContext cx;
-    nsCOMPtr<nsIXPConnectJSObjectHolder> sandbox;
-    rv = xpc->CreateSandbox(cx, principal, getter_AddRefs(sandbox));
+    JS::Rooted<JSObject*> sandbox(cx);
+    rv = xpc->CreateSandbox(cx, principal, sandbox.address());
     NS_ENSURE_SUCCESS(rv, rv);
 
     // Unwrap, store and root the sandbox.
-    NS_ENSURE_STATE(sandbox->GetJSObject());
-    autoconfigSb.init(cx, js::UncheckedUnwrap(sandbox->GetJSObject()));
+    NS_ENSURE_STATE(sandbox);
+    autoconfigSb.init(cx, js::UncheckedUnwrap(sandbox));
 
     return NS_OK;
 }
 
 nsresult CentralizedAdminPrefManagerFinish()
 {
     if (autoconfigSb.initialized()) {
         AutoSafeJSContext cx;
--- a/js/xpconnect/idl/nsIXPConnect.idl
+++ b/js/xpconnect/idl/nsIXPConnect.idl
@@ -261,17 +261,17 @@ interface nsIXPCFunctionThisTranslator :
 %{ C++
 // For use with the service manager
 // {CB6593E0-F9B2-11d2-BDD6-000064657374}
 #define NS_XPCONNECT_CID \
 { 0xcb6593e0, 0xf9b2, 0x11d2, \
     { 0xbd, 0xd6, 0x0, 0x0, 0x64, 0x65, 0x73, 0x74 } }
 %}
 
-[noscript, uuid(b91f1eeb-2fe4-44cc-9983-abcc06d69a94)]
+[noscript, uuid(0e415848-65b6-4235-b5b1-ec90509d1133)]
 interface nsIXPConnect : nsISupports
 {
 %{ C++
   NS_DEFINE_STATIC_CID_ACCESSOR(NS_XPCONNECT_CID)
 %}
 
     /**
      * Creates a new global object using the given aCOMObj as the global
@@ -469,18 +469,17 @@ interface nsIXPConnect : nsISupports
     /**
      * Create a sandbox for evaluating code in isolation using
      * evalInSandboxObject().
      *
      * @param cx A context to use when creating the sandbox object.
      * @param principal The principal (or NULL to use the null principal)
      *                  to use when evaluating code in this sandbox.
      */
-    [noscript] nsIXPConnectJSObjectHolder createSandbox(in JSContextPtr cx,
-                                                        in nsIPrincipal principal);
+    [noscript] JSObjectPtr createSandbox(in JSContextPtr cx, in nsIPrincipal principal);
 
     /**
      * Evaluate script in a sandbox, completely isolated from all
      * other running scripts.
      *
      * @param source The source of the script to evaluate.
      * @param filename The filename of the script. May be null.
      * @param cx The context to use when setting up the evaluation of
--- a/js/xpconnect/src/nsXPConnect.cpp
+++ b/js/xpconnect/src/nsXPConnect.cpp
@@ -732,30 +732,28 @@ nsXPConnect::SetFunctionThisTranslator(c
     XPCJSRuntime* rt = GetRuntime();
     IID2ThisTranslatorMap* map = rt->GetThisTranslatorMap();
     map->Add(aIID, aTranslator);
     return NS_OK;
 }
 
 NS_IMETHODIMP
 nsXPConnect::CreateSandbox(JSContext* cx, nsIPrincipal* principal,
-                           nsIXPConnectJSObjectHolder** _retval)
+                           JSObject** _retval)
 {
     *_retval = nullptr;
 
     RootedValue rval(cx);
     SandboxOptions options;
     nsresult rv = CreateSandboxObject(cx, &rval, principal, options);
     MOZ_ASSERT(NS_FAILED(rv) || !rval.isPrimitive(),
                "Bad return value from xpc_CreateSandboxObject()!");
 
     if (NS_SUCCEEDED(rv) && !rval.isPrimitive()) {
-        JSObject* obj = rval.toObjectOrNull();
-        nsRefPtr<XPCJSObjectHolder> rval = new XPCJSObjectHolder(obj);
-        rval.forget(_retval);
+        *_retval = rval.toObjectOrNull();
     }
 
     return rv;
 }
 
 NS_IMETHODIMP
 nsXPConnect::EvalInSandboxObject(const nsAString& source, const char* filename,
                                  JSContext* cx, JSObject* sandboxArg,