Bug 1246091 - patch 7/7 - Correct use of JSCompartment in Console.cpp, r=bz
authorAndrea Marchesini <amarchesini@mozilla.com>
Wed, 23 Mar 2016 22:55:07 +0100
changeset 290199 cb19fc3357e279a886abeaee475dfdb966fcd11c
parent 290198 94c7a77724d36b22b64afb24f50ff3e719de2b23
child 290200 edd188f125c5e8d7f1b2ff2a03b0e57d249afbc6
push id19656
push usergwagner@mozilla.com
push dateMon, 04 Apr 2016 13:43:23 +0000
treeherderb2g-inbound@e99061fde28a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbz
bugs1246091
milestone48.0a1
Bug 1246091 - patch 7/7 - Correct use of JSCompartment in Console.cpp, r=bz
dom/base/Console.cpp
dom/base/Console.h
--- a/dom/base/Console.cpp
+++ b/dom/base/Console.cpp
@@ -112,16 +112,18 @@ public:
 
     // We must be registered before doing any JS operation otherwise it can
     // happen that mCopiedArguments are not correctly traced.
     aConsole->StoreCallData(this);
 
     mMethodName = aName;
     mMethodString = aString;
 
+    mGlobal = JS::CurrentGlobalOrNull(aCx);
+
     for (uint32_t i = 0; i < aArguments.Length(); ++i) {
       if (NS_WARN_IF(!mCopiedArguments.AppendElement(aArguments[i]))) {
         aConsole->UnstoreCallData(this);
         return false;
       }
     }
 
     return true;
@@ -166,25 +168,29 @@ public:
   Trace(const TraceCallbacks& aCallbacks, void* aClosure)
   {
     AssertIsOnOwningThread();
 
     ConsoleCallData* tmp = this;
     for (uint32_t i = 0; i < mCopiedArguments.Length(); ++i) {
       NS_IMPL_CYCLE_COLLECTION_TRACE_JS_MEMBER_CALLBACK(mCopiedArguments[i])
     }
+
+    NS_IMPL_CYCLE_COLLECTION_TRACE_JS_MEMBER_CALLBACK(mGlobal);
   }
 
   void
   AssertIsOnOwningThread() const
   {
     MOZ_ASSERT(mOwningThread);
     MOZ_ASSERT(PR_GetCurrentThread() == mOwningThread);
   }
 
+  JS::Heap<JSObject*> mGlobal;
+
   // This is a copy of the arguments we received from the DOM bindings. Console
   // object traces them because this ConsoleCallData calls
   // RegisterConsoleCallData() in the Initialize().
   nsTArray<JS::Heap<JS::Value>> mCopiedArguments;
 
   Console::MethodName mMethodName;
   bool mPrivate;
   int64_t mTimeStamp;
@@ -312,19 +318,19 @@ public:
   virtual
   ~ConsoleRunnable()
   {
     // Clear the StructuredCloneHolderBase class.
     Clear();
   }
 
   bool
-  Dispatch(JS::Handle<JSObject*> aGlobal)
+  Dispatch(JSContext* aCx)
   {
-    if (!DispatchInternal(aGlobal)) {
+    if (!DispatchInternal(aCx)) {
       ReleaseData();
       return false;
     }
 
     return true;
   }
 
   virtual bool Notify(JSContext* aCx, workers::Status aStatus) override
@@ -353,23 +359,21 @@ private:
       RunWithWindow(window);
     }
 
     PostDispatch();
     return NS_OK;
   }
 
   bool
-  DispatchInternal(JS::Handle<JSObject*> aGlobal)
+  DispatchInternal(JSContext* aCx)
   {
     mWorkerPrivate->AssertIsOnWorkerThread();
 
-    JSContext* cx = mWorkerPrivate->GetJSContext();
-
-    if (NS_WARN_IF(!PreDispatch(cx, aGlobal))) {
+    if (NS_WARN_IF(!PreDispatch(aCx))) {
       return false;
     }
 
     if (NS_WARN_IF(!mWorkerPrivate->AddFeature(this))) {
       return false;
     }
 
     if (NS_WARN_IF(NS_FAILED(NS_DispatchToMainThread(this)))) {
@@ -466,17 +470,17 @@ private:
     JSAutoCompartment ac(cx, global);
 
     RunConsole(cx, nullptr, nullptr);
   }
 
 protected:
   // This method is called in the owning thread of the Console object.
   virtual bool
-  PreDispatch(JSContext* aCx, JS::Handle<JSObject*> aGlobal) = 0;
+  PreDispatch(JSContext* aCx) = 0;
 
   // This method is called in the main-thread.
   virtual void
   RunConsole(JSContext* aCx, nsPIDOMWindowOuter* aOuterWindow,
              nsPIDOMWindowInner* aInnerWindow) = 0;
 
   // This method is called in the owning thread of the Console object.
   virtual void
@@ -562,23 +566,22 @@ public:
 
 private:
   ~ConsoleCallDataRunnable()
   {
     MOZ_ASSERT(!mCallData);
   }
 
   bool
-  PreDispatch(JSContext* aCx, JS::Handle<JSObject*> aGlobal) override
+  PreDispatch(JSContext* aCx) override
   {
     mWorkerPrivate->AssertIsOnWorkerThread();
     mCallData->AssertIsOnOwningThread();
 
     ClearException ce(aCx);
-    JSAutoCompartment ac(aCx, aGlobal);
 
     JS::Rooted<JSObject*> arguments(aCx,
       JS_NewArrayObject(aCx, mCallData->mCopiedArguments.Length()));
     if (NS_WARN_IF(!arguments)) {
       return false;
     }
 
     JS::Rooted<JS::Value> arg(aCx);
@@ -689,19 +692,17 @@ private:
 
       if (!values.AppendElement(value, fallible)) {
         return;
       }
     }
 
     MOZ_ASSERT(values.Length() == length);
 
-    JS::Rooted<JSObject*> global(aCx, JS::CurrentGlobalOrNull(aCx));
-
-    mConsole->ProcessCallData(mCallData, global, values);
+    mConsole->ProcessCallData(aCx, mCallData, values);
   }
 
   RefPtr<ConsoleCallData> mCallData;
 };
 
 // This runnable calls ProfileMethod() on the console on the main-thread.
 class ConsoleProfileRunnable final : public ConsoleRunnable
 {
@@ -712,27 +713,20 @@ public:
     , mAction(aAction)
     , mArguments(aArguments)
   {
     MOZ_ASSERT(aConsole);
   }
 
 private:
   bool
-  PreDispatch(JSContext* aCx, JS::Handle<JSObject*> aGlobal) override
+  PreDispatch(JSContext* aCx) override
   {
     ClearException ce(aCx);
 
-    JS::Rooted<JSObject*> global(aCx, aGlobal);
-    if (NS_WARN_IF(!global)) {
-      return false;
-    }
-
-    JSAutoCompartment ac(aCx, global);
-
     JS::Rooted<JSObject*> arguments(aCx,
       JS_NewArrayObject(aCx, mArguments.Length()));
     if (NS_WARN_IF(!arguments)) {
       return false;
     }
 
     JS::Rooted<JS::Value> arg(aCx);
     for (uint32_t i = 0; i < mArguments.Length(); ++i) {
@@ -1096,17 +1090,17 @@ Console::ProfileMethod(JSContext* aCx, c
   MOZ_ASSERT(mStatus == eInitialized);
 
   if (!NS_IsMainThread()) {
     // Here we are in a worker thread.
     RefPtr<ConsoleProfileRunnable> runnable =
       new ConsoleProfileRunnable(this, aAction, aData);
 
     JS::Rooted<JSObject*> global(aCx, JS::CurrentGlobalOrNull(aCx));
-    runnable->Dispatch(global);
+    runnable->Dispatch(aCx);
     return;
   }
 
   ClearException ce(aCx);
 
   RootedDictionary<ConsoleProfileEvent> event(aCx);
   event.mAction = aAction;
 
@@ -1368,35 +1362,33 @@ Console::Method(JSContext* aCx, MethodNa
     if (callData->mTopStackFrame) {
       frame = *callData->mTopStackFrame;
     }
 
     callData->mCountValue = IncreaseCounter(aCx, frame, aData,
                                             callData->mCountLabel);
   }
 
-  JS::Rooted<JSObject*> global(aCx, JS::CurrentGlobalOrNull(aCx));
-
   if (NS_IsMainThread()) {
     callData->SetIDs(mOuterID, mInnerID);
-    ProcessCallData(callData, global, aData);
+    ProcessCallData(aCx, callData, aData);
 
     // Just because we don't want to expose
     // retrieveConsoleEvents/setConsoleEventHandler to main-thread, we can
     // cleanup the mCallDataStorage:
     UnstoreCallData(callData);
     return;
   }
 
   // We do this only in workers for now.
-  NotifyHandler(aCx, global, aData, callData);
+  NotifyHandler(aCx, aData, callData);
 
   RefPtr<ConsoleCallDataRunnable> runnable =
     new ConsoleCallDataRunnable(this, callData);
-  NS_WARN_IF(!runnable->Dispatch(global));
+  NS_WARN_IF(!runnable->Dispatch(aCx));
 }
 
 // We store information to lazily compute the stack in the reserved slots of
 // LazyStackGetter.  The first slot always stores a JS object: it's either the
 // JS wrapper of the nsIStackFrame or the actual reified stack representation.
 // The second slot is a PrivateValue() holding an nsIStackFrame* when we haven't
 // reified the stack yet, or an UndefinedValue() otherwise.
 enum {
@@ -1435,30 +1427,39 @@ LazyStackGetter(JSContext* aCx, unsigned
   js::SetFunctionNativeReserved(callee, SLOT_STACKOBJ, stackVal);
   js::SetFunctionNativeReserved(callee, SLOT_RAW_STACK, JS::UndefinedValue());
 
   args.rval().set(stackVal);
   return true;
 }
 
 void
-Console::ProcessCallData(ConsoleCallData* aData, JS::Handle<JSObject*> aGlobal,
+Console::ProcessCallData(JSContext* aCx, ConsoleCallData* aData,
                          const Sequence<JS::Value>& aArguments)
 {
   AssertIsOnMainThread();
   MOZ_ASSERT(aData);
 
-  AutoJSAPI jsapi;
-  if (!jsapi.Init(aGlobal)) {
-    return;
-  }
-  JSContext* cx = jsapi.cx();
-
-  JS::Rooted<JS::Value> eventValue(cx);
-  if (NS_WARN_IF(!PopulateEvent(cx, aGlobal, aArguments, &eventValue, aData))) {
+  JS::Rooted<JS::Value> eventValue(aCx);
+
+  // We want to create a console event object and pass it to our
+  // nsIConsoleAPIStorage implementation.  We want to define some accessor
+  // properties on this object, and those will need to keep an nsIStackFrame
+  // alive.  But nsIStackFrame cannot be wrapped in an untrusted scope.  And
+  // further, passing untrusted objects to system code is likely to run afoul of
+  // Object Xrays.  So we want to wrap in a system-principal scope here.  But
+  // which one?  We could cheat and try to get the underlying JSObject* of
+  // mStorage, but that's a bit fragile.  Instead, we just use the junk scope,
+  // with explicit permission from the XPConnect module owner.  If you're
+  // tempted to do that anywhere else, talk to said module owner first.
+
+  // aCx and aArguments are in the same compartment.
+  if (NS_WARN_IF(!PopulateConsoleObjectInTheTargetScope(aCx, aArguments,
+                                                        xpc::PrivilegedJunkScope(),
+                                                        &eventValue, aData))) {
     return;
   }
 
   if (!mStorage) {
     mStorage = do_GetService("@mozilla.org/consoleAPI-storage;1");
   }
 
   if (!mStorage) {
@@ -1479,35 +1480,34 @@ Console::ProcessCallData(ConsoleCallData
   }
 
   if (NS_FAILED(mStorage->RecordEvent(innerID, outerID, eventValue))) {
     NS_WARNING("Failed to record a console event.");
   }
 }
 
 bool
-Console::PopulateEvent(JSContext* aCx,
-                       JS::Handle<JSObject*> aGlobal,
-                       const Sequence<JS::Value>& aArguments,
-                       JS::MutableHandle<JS::Value> aEventValue,
-                       ConsoleCallData* aData) const
+Console::PopulateConsoleObjectInTheTargetScope(JSContext* aCx,
+                                               const Sequence<JS::Value>& aArguments,
+                                               JSObject* aTargetScope,
+                                               JS::MutableHandle<JS::Value> aEventValue,
+                                               ConsoleCallData* aData) const
 {
   MOZ_ASSERT(aCx);
   MOZ_ASSERT(aData);
+  MOZ_ASSERT(aTargetScope);
 
   ConsoleStackEntry frame;
   if (aData->mTopStackFrame) {
     frame = *aData->mTopStackFrame;
   }
 
   ClearException ce(aCx);
   RootedDictionary<ConsoleEvent> event(aCx);
 
-  JSAutoCompartment ac(aCx, aGlobal);
-
   event.mID.Construct();
   event.mInnerID.Construct();
 
   if (aData->mIDType == ConsoleCallData::eString) {
     event.mID.Value().SetAsString() = aData->mOuterIDString;
     event.mInnerID.Value().SetAsString() = aData->mInnerIDString;
   } else if (aData->mIDType == ConsoleCallData::eNumber) {
     event.mID.Value().SetAsUnsignedLongLong() = aData->mOuterIDNumber;
@@ -1585,27 +1585,17 @@ Console::PopulateEvent(JSContext* aCx,
                                         aData->mStopTimerStatus);
   }
 
   else if (aData->mMethodName == MethodCount) {
     event.mCounter = CreateCounterValue(aCx, aData->mCountLabel,
                                         aData->mCountValue);
   }
 
-  // We want to create a console event object and pass it to our
-  // nsIConsoleAPIStorage implementation.  We want to define some accessor
-  // properties on this object, and those will need to keep an nsIStackFrame
-  // alive.  But nsIStackFrame cannot be wrapped in an untrusted scope.  And
-  // further, passing untrusted objects to system code is likely to run afoul of
-  // Object Xrays.  So we want to wrap in a system-principal scope here.  But
-  // which one?  We could cheat and try to get the underlying JSObject* of
-  // mStorage, but that's a bit fragile.  Instead, we just use the junk scope,
-  // with explicit permission from the XPConnect module owner.  If you're
-  // tempted to do that anywhere else, talk to said module owner first.
-  JSAutoCompartment ac2(aCx, xpc::PrivilegedJunkScope());
+  JSAutoCompartment ac2(aCx, aTargetScope);
 
   if (NS_WARN_IF(!ToJSValue(aCx, event, aEventValue))) {
     return false;
   }
 
   JS::Rooted<JSObject*> eventObj(aCx, &aEventValue.toObject());
   if (NS_WARN_IF(!JS_DefineProperty(aCx, eventObj, "wrappedJSObject", eventObj,
                                     JSPROP_ENUMERATE))) {
@@ -2257,64 +2247,73 @@ Console::ReleaseCallData(ConsoleCallData
   MOZ_ASSERT(aCallData);
   MOZ_ASSERT(aCallData->mStatus == ConsoleCallData::eToBeDeleted);
   MOZ_ASSERT(mCallDataStoragePending.Contains(aCallData));
 
   mCallDataStoragePending.RemoveElement(aCallData);
 }
 
 void
-Console::NotifyHandler(JSContext* aCx, JS::Handle<JSObject*> aGlobal,
-                       const Sequence<JS::Value>& aArguments,
+Console::NotifyHandler(JSContext* aCx, const Sequence<JS::Value>& aArguments,
                        ConsoleCallData* aCallData) const
 {
   AssertIsOnOwningThread();
   MOZ_ASSERT(!NS_IsMainThread());
   MOZ_ASSERT(aCallData);
 
   if (!mConsoleEventNotifier) {
     return;
   }
 
-  JSAutoCompartment ac(aCx, mConsoleEventNotifier->Callable());
-
   JS::Rooted<JS::Value> value(aCx);
-  if (NS_WARN_IF(!PopulateEvent(aCx, mConsoleEventNotifier->Callable(),
-                                aArguments, &value, aCallData))) {
+
+  // aCx and aArguments are in the same compartment because this method is
+  // called directly when a Console.something() runs.
+  // mConsoleEventNotifier->Callable() is the scope where value will be sent to.
+  if (NS_WARN_IF(!PopulateConsoleObjectInTheTargetScope(aCx, aArguments,
+                                                        mConsoleEventNotifier->Callable(),
+                                                        &value, aCallData))) {
     return;
   }
 
   JS::Rooted<JS::Value> ignored(aCx);
   mConsoleEventNotifier->Call(value, &ignored);
 }
 
 void
 Console::RetrieveConsoleEvents(JSContext* aCx, nsTArray<JS::Value>& aEvents,
                                ErrorResult& aRv)
 {
   AssertIsOnOwningThread();
 
   // We don't want to expose this functionality to main-thread yet.
   MOZ_ASSERT(!NS_IsMainThread());
 
-  JS::Rooted<JSObject*> global(aCx, JS::CurrentGlobalOrNull(aCx));
+  JS::Rooted<JSObject*> targetScope(aCx, JS::CurrentGlobalOrNull(aCx));
 
   for (uint32_t i = 0; i < mCallDataStorage.Length(); ++i) {
     JS::Rooted<JS::Value> value(aCx);
 
+    JS::Rooted<JSObject*> sequenceScope(aCx, mCallDataStorage[i]->mGlobal);
+    JSAutoCompartment ac(aCx, sequenceScope);
+
     Sequence<JS::Value> sequence;
     SequenceRooter<JS::Value> arguments(aCx, &sequence);
 
     if (!mCallDataStorage[i]->PopulateArgumentsSequence(sequence)) {
       aRv.Throw(NS_ERROR_OUT_OF_MEMORY);
       return;
     }
 
-    if (NS_WARN_IF(!PopulateEvent(aCx, global, sequence, &value,
-                                  mCallDataStorage[i]))) {
+    // Here we have aCx and sequence in the same compartment.
+    // targetScope is the destination scope and value will be populated in its
+    // compartment.
+    if (NS_WARN_IF(!PopulateConsoleObjectInTheTargetScope(aCx, sequence,
+                                                          targetScope, &value,
+                                                          mCallDataStorage[i]))) {
       aRv.Throw(NS_ERROR_FAILURE);
       return;
     }
 
     aEvents.AppendElement(value);
   }
 }
 
--- a/dom/base/Console.h
+++ b/dom/base/Console.h
@@ -158,43 +158,55 @@ private:
     MethodAssert,
     MethodCount
   };
 
   void
   Method(JSContext* aCx, MethodName aName, const nsAString& aString,
          const Sequence<JS::Value>& aData);
 
+  // This method must receive aCx and aArguments in the same JSCompartment.
   void
-  ProcessCallData(ConsoleCallData* aData,
-                  JS::Handle<JSObject*> aGlobal,
+  ProcessCallData(JSContext* aCx,
+                  ConsoleCallData* aData,
                   const Sequence<JS::Value>& aArguments);
 
   void
   StoreCallData(ConsoleCallData* aData);
 
   void
   UnstoreCallData(ConsoleCallData* aData);
 
   // Read in Console.cpp how this method is used.
   void
   ReleaseCallData(ConsoleCallData* aCallData);
 
+  // aCx and aArguments must be in the same JS compartment.
   void
   NotifyHandler(JSContext* aCx,
-                JS::Handle<JSObject*> aGlobal,
                 const Sequence<JS::Value>& aArguments,
                 ConsoleCallData* aData) const;
 
+  // PopulateConsoleObjectInTheTargetScope receives aCx and aArguments in the
+  // same JS compartment and populates the ConsoleEvent object (aValue) in the
+  // aTargetScope.
+  // aTargetScope can be:
+  // - the system-principal scope when we want to dispatch the ConsoleEvent to
+  //   nsIConsoleAPIStorage (See the comment in Console.cpp about the use of
+  //   xpc::PrivilegedJunkScope()
+  // - the mConsoleEventNotifier->Callable() scope when we want to notify this
+  //   handler about a new ConsoleEvent.
+  // - It can be the global from the JSContext when RetrieveConsoleEvents is
+  //   called.
   bool
-  PopulateEvent(JSContext* aCx,
-                JS::Handle<JSObject*> aGlobal,
-                const Sequence<JS::Value>& aArguments,
-                JS::MutableHandle<JS::Value> aValue,
-                ConsoleCallData* aData) const;
+  PopulateConsoleObjectInTheTargetScope(JSContext* aCx,
+                                        const Sequence<JS::Value>& aArguments,
+                                        JSObject* aTargetScope,
+                                        JS::MutableHandle<JS::Value> aValue,
+                                        ConsoleCallData* aData) const;
 
   // If the first JS::Value of the array is a string, this method uses it to
   // format a string. The supported sequences are:
   //   %s    - string
   //   %d,%i - integer
   //   %f    - double
   //   %o,%O - a JS object.
   //   %c    - style string.