Bug 1645510: Part 2 - Avoid using the unprivileged junk scope where possible. r=bholley
authorKris Maglione <maglione.k@gmail.com>
Sat, 27 Jun 2020 03:06:28 +0000
changeset 537675 93bca0225002f5dae30b60b823cb73684031a348
parent 537674 821960c5fa97cbe6fabaa26182c35d1e76a3ae4e
child 537676 a6b021b3776ab954b0bf56d7a5641b072a719e2f
push id37545
push usernerli@mozilla.com
push dateSat, 27 Jun 2020 09:38:32 +0000
treeherdermozilla-central@0a4b3f99d2d1 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbholley
bugs1645510
milestone79.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 1645510: Part 2 - Avoid using the unprivileged junk scope where possible. r=bholley Differential Revision: https://phabricator.services.mozilla.com/D79720
caps/ContentPrincipal.cpp
dom/base/WindowDestroyedEvent.cpp
dom/base/nsContentUtils.cpp
dom/base/nsJSEnvironment.cpp
dom/console/Console.cpp
dom/ipc/BrowserChild.cpp
dom/ipc/BrowserChild.h
dom/ipc/MMPrinter.cpp
dom/script/ScriptSettings.cpp
dom/security/nsContentSecurityUtils.cpp
dom/workers/WorkerDebugger.cpp
js/xpconnect/src/XPCJSRuntime.cpp
--- a/caps/ContentPrincipal.cpp
+++ b/caps/ContentPrincipal.cpp
@@ -358,18 +358,20 @@ ContentPrincipal::SetDomain(nsIURI* aDom
   // Set the changed-document-domain flag on compartments containing realms
   // using this principal.
   auto cb = [](JSContext*, void*, JS::Handle<JS::Realm*> aRealm) {
     JS::Compartment* comp = JS::GetCompartmentForRealm(aRealm);
     xpc::SetCompartmentChangedDocumentDomain(comp);
   };
   JSPrincipals* principals =
       nsJSPrincipals::get(static_cast<nsIPrincipal*>(this));
-  AutoSafeJSContext cx;
-  JS::IterateRealmsWithPrincipals(cx, principals, nullptr, cb);
+
+  dom::AutoJSAPI jsapi;
+  jsapi.Init();
+  JS::IterateRealmsWithPrincipals(jsapi.cx(), principals, nullptr, cb);
 
   return NS_OK;
 }
 
 static nsresult GetSpecialBaseDomain(const nsCOMPtr<nsIURI>& aURI,
                                      bool* aHandled, nsACString& aBaseDomain) {
   *aHandled = false;
 
--- a/dom/base/WindowDestroyedEvent.cpp
+++ b/dom/base/WindowDestroyedEvent.cpp
@@ -100,17 +100,19 @@ WindowDestroyedEvent::Run() {
           currentInner = nsGlobalWindowInner::FromSupports(window);
         } else {
           nsGlobalWindowOuter* outer =
               nsGlobalWindowOuter::FromSupports(window);
           currentInner = outer->GetCurrentInnerWindowInternal();
         }
         NS_ENSURE_TRUE(currentInner, NS_OK);
 
-        AutoSafeJSContext cx;
+        dom::AutoJSAPI jsapi;
+        jsapi.Init();
+        JSContext* cx = jsapi.cx();
         JS::Rooted<JSObject*> obj(cx, currentInner->GetGlobalJSObject());
         if (obj && !js::IsSystemRealm(js::GetNonCCWObjectRealm(obj))) {
           JS::Realm* realm = js::GetNonCCWObjectRealm(obj);
 
           xpc::NukeJSStackFrames(realm);
 
           nsCOMPtr<nsIPrincipal> pc =
               nsJSPrincipals::get(JS::GetRealmPrincipals(realm));
--- a/dom/base/nsContentUtils.cpp
+++ b/dom/base/nsContentUtils.cpp
@@ -6424,19 +6424,20 @@ Maybe<bool> nsContentUtils::IsPatternMat
   // The fact that we're using a JS regexp under the hood should not be visible
   // to things like window onerror handlers, so we don't initialize our JSAPI
   // with the document's window (which may not exist anyway).
   AutoJSAPI jsapi;
   jsapi.Init();
   JSContext* cx = jsapi.cx();
   AutoDisableJSInterruptCallback disabler(cx);
 
-  // We can use the junk scope here, because we're just using it for
-  // regexp evaluation, not actual script execution.
-  JSAutoRealm ar(cx, xpc::UnprivilegedJunkScope());
+  // We can use the junk scope here, because we're just using it for regexp
+  // evaluation, not actual script execution, and we disable statics so that the
+  // evaluation does not interact with the execution global.
+  JSAutoRealm ar(cx, xpc::PrivilegedJunkScope());
 
   // Check if the pattern by itself is valid first, and not that it only becomes
   // valid once we add ^(?: and )$.
   JS::RootedValue error(cx);
   if (!JS::CheckRegExpSyntax(
           cx, static_cast<char16_t*>(aPattern.BeginWriting()),
           aPattern.Length(), JS::RegExpFlag::Unicode, &error)) {
     return Nothing();
--- a/dom/base/nsJSEnvironment.cpp
+++ b/dom/base/nsJSEnvironment.cpp
@@ -2697,18 +2697,24 @@ void AsyncErrorReporter::SetException(JS
                                       JS::Handle<JS::Value> aException) {
   MOZ_ASSERT(NS_IsMainThread());
   mException.init(aCx, aException);
   mHasException = true;
 }
 
 NS_IMETHODIMP AsyncErrorReporter::Run() {
   AutoJSAPI jsapi;
-  DebugOnly<bool> ok = jsapi.Init(xpc::UnprivilegedJunkScope());
-  MOZ_ASSERT(ok, "Problem with junk scope?");
+  // We're only using this context to deserialize a stack to report to the
+  // console, so the scope we use doesn't matter. Stack frame filtering happens
+  // based on the principal encoded into the frame and the caller compartment,
+  // not the compartment of the frame object, and the console reporting code
+  // will not be using our context, and therefore will not care what compartment
+  // it has entered.
+  DebugOnly<bool> ok = jsapi.Init(xpc::PrivilegedJunkScope());
+  MOZ_ASSERT(ok, "Problem with system global?");
   JSContext* cx = jsapi.cx();
   JS::Rooted<JSObject*> stack(cx);
   JS::Rooted<JSObject*> stackGlobal(cx);
   if (mStackHolder) {
     stack = mStackHolder->ReadStack(cx);
     if (stack) {
       stackGlobal = JS::CurrentGlobalOrNull(cx);
     }
--- a/dom/console/Console.cpp
+++ b/dom/console/Console.cpp
@@ -493,17 +493,19 @@ class ConsoleCallDataWorkletRunnable fin
     const WorkletLoadInfo& loadInfo = mWorkletImpl->LoadInfo();
     mCallData->SetIDs(loadInfo.OuterWindowID(), loadInfo.InnerWindowID());
   }
 
   ~ConsoleCallDataWorkletRunnable() override = default;
 
   NS_IMETHOD Run() override {
     AssertIsOnMainThread();
-    AutoSafeJSContext cx;
+    AutoJSAPI jsapi;
+    jsapi.Init();
+    JSContext* cx = jsapi.cx();
 
     JSObject* sandbox =
         mConsoleData->GetOrCreateSandbox(cx, mWorkletImpl->Principal());
     JS::Rooted<JSObject*> global(cx, sandbox);
     if (NS_WARN_IF(!global)) {
       return NS_ERROR_FAILURE;
     }
 
@@ -724,17 +726,19 @@ class ConsoleProfileWorkletRunnable fina
                                 const nsAString& aAction)
       : ConsoleWorkletRunnable(aConsole), mName(aName), mAction(aAction) {
     MOZ_ASSERT(aConsole);
   }
 
   NS_IMETHOD Run() override {
     AssertIsOnMainThread();
 
-    AutoSafeJSContext cx;
+    AutoJSAPI jsapi;
+    jsapi.Init();
+    JSContext* cx = jsapi.cx();
 
     JSObject* sandbox =
         mConsoleData->GetOrCreateSandbox(cx, mWorkletImpl->Principal());
     JS::Rooted<JSObject*> global(cx, sandbox);
     if (NS_WARN_IF(!global)) {
       return NS_ERROR_FAILURE;
     }
 
--- a/dom/ipc/BrowserChild.cpp
+++ b/dom/ipc/BrowserChild.cpp
@@ -173,38 +173,16 @@ already_AddRefed<Document> BrowserChild:
 
 PresShell* BrowserChild::GetTopLevelPresShell() const {
   if (RefPtr<Document> doc = GetTopLevelDocument()) {
     return doc->GetPresShell();
   }
   return nullptr;
 }
 
-void BrowserChild::DispatchMessageManagerMessage(const nsAString& aMessageName,
-                                                 const nsAString& aJSONData) {
-  AutoSafeJSContext cx;
-  JS::Rooted<JS::Value> json(cx, JS::NullValue());
-  dom::ipc::StructuredCloneData data;
-  if (JS_ParseJSON(cx, static_cast<const char16_t*>(aJSONData.BeginReading()),
-                   aJSONData.Length(), &json)) {
-    ErrorResult rv;
-    data.Write(cx, json, rv);
-    if (NS_WARN_IF(rv.Failed())) {
-      rv.SuppressException();
-      return;
-    }
-  }
-
-  RefPtr<BrowserChildMessageManager> kungFuDeathGrip(
-      mBrowserChildMessageManager);
-  RefPtr<nsFrameMessageManager> mm = kungFuDeathGrip->GetMessageManager();
-  mm->ReceiveMessage(static_cast<EventTarget*>(kungFuDeathGrip), nullptr,
-                     aMessageName, false, &data, nullptr, IgnoreErrors());
-}
-
 bool BrowserChild::UpdateFrame(const RepaintRequest& aRequest) {
   MOZ_ASSERT(aRequest.GetScrollId() != ScrollableLayerGuid::NULL_SCROLL_ID);
 
   if (aRequest.IsRootContent()) {
     if (PresShell* presShell = GetTopLevelPresShell()) {
       // Guard against stale updates (updates meant for a pres shell which
       // has since been torn down and destroyed).
       if (aRequest.GetPresShellId() == presShell->GetPresShellId()) {
--- a/dom/ipc/BrowserChild.h
+++ b/dom/ipc/BrowserChild.h
@@ -732,24 +732,16 @@ class BrowserChild final : public nsMess
   mozilla::ipc::IPCResult RecvStopIMEStateManagement();
 
   mozilla::ipc::IPCResult RecvAllowScriptsToClose();
 
   mozilla::ipc::IPCResult RecvSetWidgetNativeData(
       const WindowsHandle& aWidgetNativeData);
 
  private:
-  // Wraps up a JSON object as a structured clone and sends it to the browser
-  // chrome script.
-  //
-  // XXX/bug 780335: Do the work the browser chrome script does in C++ instead
-  // so we don't need things like this.
-  void DispatchMessageManagerMessage(const nsAString& aMessageName,
-                                     const nsAString& aJSONData);
-
   void HandleDoubleTap(const CSSPoint& aPoint, const Modifiers& aModifiers,
                        const ScrollableLayerGuid& aGuid);
 
   // Notify others that our TabContext has been updated.
   //
   // You should call this after calling TabContext::SetTabContext().  We also
   // call this during Init().
   void NotifyTabContextUpdated();
--- a/dom/ipc/MMPrinter.cpp
+++ b/dom/ipc/MMPrinter.cpp
@@ -47,17 +47,21 @@ void MMPrinter::PrintImpl(char const* aL
 
   if (!MOZ_LOG_TEST(sMMLog, LogLevel::Verbose)) {
     return;
   }
 
   ErrorResult rv;
 
   AutoJSAPI jsapi;
-  MOZ_ALWAYS_TRUE(jsapi.Init(xpc::UnprivilegedJunkScope()));
+  // We're using this context to deserialize, stringify, and print a message
+  // manager message here. Since the messages are always sent from and to system
+  // scopes, we need to do this in a system scope, or attempting to deserialize
+  // certain privileged objects will fail.
+  MOZ_ALWAYS_TRUE(jsapi.Init(xpc::PrivilegedJunkScope()));
   JSContext* cx = jsapi.cx();
 
   ipc::StructuredCloneData data;
   ipc::UnpackClonedMessageDataForChild(aData, data);
 
   /* Read original StructuredCloneData. */
   JS::RootedValue scdContent(cx);
   data.Read(cx, &scdContent, rv);
--- a/dom/script/ScriptSettings.cpp
+++ b/dom/script/ScriptSettings.cpp
@@ -742,17 +742,22 @@ AutoSlowOperation::AutoSlowOperation(
   if (mIsMainThread) {
     mScriptActivity.emplace(true);
   }
 }
 
 void AutoSlowOperation::CheckForInterrupt() {
   // For now we support only main thread!
   if (mIsMainThread) {
-    // JS_CheckForInterrupt expects us to be in a realm.
-    AutoJSAPI jsapi;
-    if (jsapi.Init(xpc::UnprivilegedJunkScope())) {
-      JS_CheckForInterrupt(jsapi.cx());
-    }
+    // JS_CheckForInterrupt expects us to be in a realm, so we use a junk scope.
+    // In principle, it doesn't matter which one we use, since we aren't really
+    // running scripts here, and none of our interrupt callbacks can stop
+    // scripts in a junk scope anyway. In practice, though, the privileged junk
+    // scope is the same as the JSM global, and therefore always exists, while
+    // the unprivileged junk scope is created lazily, and may not exist until we
+    // try to use it. So we use the former for the sake of efficiency.
+    dom::AutoJSAPI jsapi;
+    MOZ_ALWAYS_TRUE(jsapi.Init(xpc::PrivilegedJunkScope()));
+    JS_CheckForInterrupt(jsapi.cx());
   }
 }
 
 }  // namespace mozilla
--- a/dom/security/nsContentSecurityUtils.cpp
+++ b/dom/security/nsContentSecurityUtils.cpp
@@ -103,17 +103,20 @@ nsresult RegexEval(const nsAString& aPat
   aMatchResult = false;
 
   AutoJSAPI jsapi;
   jsapi.Init();
 
   JSContext* cx = jsapi.cx();
   AutoDisableJSInterruptCallback disabler(cx);
 
-  JSAutoRealm ar(cx, xpc::UnprivilegedJunkScope());
+  // We can use the junk scope here, because we're just using it for regexp
+  // evaluation, not actual script execution, and we disable statics so that the
+  // evaluation does not interact with the execution global.
+  JSAutoRealm ar(cx, xpc::PrivilegedJunkScope());
 
   JS::RootedObject regexp(
       cx, JS::NewUCRegExpObject(cx, aPattern.BeginReading(), aPattern.Length(),
                                 JS::RegExpFlag::Unicode));
   if (!regexp) {
     return NS_ERROR_ILLEGAL_VALUE;
   }
 
--- a/dom/workers/WorkerDebugger.cpp
+++ b/dom/workers/WorkerDebugger.cpp
@@ -435,21 +435,25 @@ void WorkerDebugger::ReportErrorToDebugg
 void WorkerDebugger::ReportErrorToDebuggerOnMainThread(
     const nsAString& aFilename, uint32_t aLineno, const nsAString& aMessage) {
   AssertIsOnMainThread();
 
   for (const auto& listener : mListeners.Clone()) {
     listener->OnError(aFilename, aLineno, aMessage);
   }
 
-  // We need a JSContext to be able to read any stack associated with the error.
-  // This will not run any scripts.
   AutoJSAPI jsapi;
-  DebugOnly<bool> ok = jsapi.Init(xpc::UnprivilegedJunkScope());
-  MOZ_ASSERT(ok, "UnprivilegedJunkScope should exist");
+  // We're only using this context to deserialize a stack to report to the
+  // console, so the scope we use doesn't matter. Stack frame filtering happens
+  // based on the principal encoded into the frame and the caller compartment,
+  // not the compartment of the frame object, and the console reporting code
+  // will not be using our context, and therefore will not care what compartment
+  // it has entered.
+  DebugOnly<bool> ok = jsapi.Init(xpc::PrivilegedJunkScope());
+  MOZ_ASSERT(ok, "PrivilegedJunkScope should exist");
 
   WorkerErrorReport report;
   report.mMessage = aMessage;
   report.mFilename = aFilename;
   WorkerErrorReport::LogErrorToConsole(jsapi.cx(), report, 0);
 }
 
 RefPtr<PerformanceInfoPromise> WorkerDebugger::ReportPerformanceInfo() {
--- a/js/xpconnect/src/XPCJSRuntime.cpp
+++ b/js/xpconnect/src/XPCJSRuntime.cpp
@@ -2191,24 +2191,23 @@ class XPCJSRuntimeStats : public JS::Run
 
     for (size_t i = 0; i != zoneStatsVector.length(); ++i) {
       delete static_cast<xpc::ZoneStatsExtras*>(zoneStatsVector[i].extra);
     }
   }
 
   virtual void initExtraZoneStats(JS::Zone* zone,
                                   JS::ZoneStats* zStats) override {
-    AutoSafeJSContext cx;
     xpc::ZoneStatsExtras* extras = new xpc::ZoneStatsExtras;
     extras->pathPrefix.AssignLiteral("explicit/js-non-window/zones/");
 
     // Get some global in this zone.
-    Rooted<Realm*> realm(cx, js::GetAnyRealmInZone(zone));
+    Rooted<Realm*> realm(dom::RootingCx(), js::GetAnyRealmInZone(zone));
     if (realm) {
-      RootedObject global(cx, JS::GetRealmGlobalOrNull(realm));
+      RootedObject global(dom::RootingCx(), JS::GetRealmGlobalOrNull(realm));
       if (global) {
         RefPtr<nsGlobalWindowInner> window;
         if (NS_SUCCEEDED(UNWRAP_NON_WRAPPER_OBJECT(Window, global, window))) {
           // The global is a |window| object.  Use the path prefix that
           // we should have already created for it.
           if (mTopWindowPaths->Get(window->WindowID(), &extras->pathPrefix))
             extras->pathPrefix.AppendLiteral("/js-");
         }
@@ -2224,19 +2223,18 @@ class XPCJSRuntimeStats : public JS::Run
 
   virtual void initExtraRealmStats(Handle<Realm*> realm,
                                    JS::RealmStats* realmStats) override {
     xpc::RealmStatsExtras* extras = new xpc::RealmStatsExtras;
     nsCString rName;
     GetRealmName(realm, rName, &mAnonymizeID, /* replaceSlashes = */ true);
 
     // Get the realm's global.
-    AutoSafeJSContext cx;
     bool needZone = true;
-    RootedObject global(cx, JS::GetRealmGlobalOrNull(realm));
+    RootedObject global(dom::RootingCx(), JS::GetRealmGlobalOrNull(realm));
     if (global) {
       RefPtr<nsGlobalWindowInner> window;
       if (NS_SUCCEEDED(UNWRAP_NON_WRAPPER_OBJECT(Window, global, window))) {
         // The global is a |window| object.  Use the path prefix that
         // we should have already created for it.
         if (mWindowPaths->Get(window->WindowID(), &extras->jsPathPrefix)) {
           extras->domPathPrefix.Assign(extras->jsPathPrefix);
           extras->domPathPrefix.AppendLiteral("/dom/");