Bug 1389847: Don't add caller location to sandbox name if an explicit name is provided. r?krizsa draft
authorKris Maglione <maglione.k@gmail.com>
Sat, 12 Aug 2017 16:01:00 -0700
changeset 645473 ca237e2a9f3c82743d1834adcdb230f17fe37b57
parent 645465 a2ba607e48e4a0f62494618802f64e6944ff978b
child 645474 9990d47ac1bf949d2bb147c40a8d55a973ba8d2d
push id73758
push usermaglione.k@gmail.com
push dateSat, 12 Aug 2017 23:01:32 +0000
reviewerskrizsa
bugs1389847
milestone57.0a1
Bug 1389847: Don't add caller location to sandbox name if an explicit name is provided. r?krizsa MozReview-Commit-ID: KOGrrMurs6X
js/xpconnect/src/Sandbox.cpp
js/xpconnect/src/XPCJSRuntime.cpp
toolkit/components/extensions/ExtensionCommon.jsm
toolkit/components/extensions/ExtensionParent.jsm
--- a/js/xpconnect/src/Sandbox.cpp
+++ b/js/xpconnect/src/Sandbox.cpp
@@ -1701,16 +1701,22 @@ SandboxOptions::Parse()
 }
 
 static nsresult
 AssembleSandboxMemoryReporterName(JSContext* cx, nsCString& sandboxName)
 {
     // Use a default name when the caller did not provide a sandboxName.
     if (sandboxName.IsEmpty())
         sandboxName = NS_LITERAL_CSTRING("[anonymous sandbox]");
+#ifndef DEBUG
+    // Adding the caller location is fairly expensive, so in non-debug builds,
+    // only add it if we don't have an explicit sandbox name.
+    else
+        return NS_OK;
+#endif
 
     // Get the xpconnect native call context.
     XPCCallContext* cc = XPCJSContext::Get()->GetCallContext();
     NS_ENSURE_TRUE(cc, NS_ERROR_INVALID_ARG);
 
     // Get the current source info from xpc.
     nsCOMPtr<nsIStackFrame> frame;
     nsXPConnect::XPConnect()->GetCurrentJSStack(getter_AddRefs(frame));
--- a/js/xpconnect/src/XPCJSRuntime.cpp
+++ b/js/xpconnect/src/XPCJSRuntime.cpp
@@ -246,26 +246,33 @@ bool CompartmentPrivate::TryParseLocatio
     if (!aURI)
         return false;
 
     // Need to parse the URI.
     if (location.IsEmpty())
         return false;
 
     // Handle Sandbox location strings.
-    // A sandbox string looks like this:
+    // A sandbox string looks like this, for anonymous sandboxes, and builds
+    // where Sandbox location tagging is enabled:
+    //
     // <sandboxName> (from: <js-stack-frame-filename>:<lineno>)
+    //
     // where <sandboxName> is user-provided via Cu.Sandbox()
     // and <js-stack-frame-filename> and <lineno> is the stack frame location
     // from where Cu.Sandbox was called.
+    //
+    // Otherwise, it is simply the caller-provided name, which is usually a URI.
+    //
     // <js-stack-frame-filename> furthermore is "free form", often using a
     // "uri -> uri -> ..." chain. The following code will and must handle this
     // common case.
+    //
     // It should be noted that other parts of the code may already rely on the
-    // "format" of these strings, such as the add-on SDK.
+    // "format" of these strings.
 
     static const nsDependentCString from("(from: ");
     static const nsDependentCString arrow(" -> ");
     static const size_t fromLength = from.Length();
     static const size_t arrowLength = arrow.Length();
 
     // See: XPCComponents.cpp#AssembleSandboxMemoryReporterName
     int32_t idx = location.Find(from);
--- a/toolkit/components/extensions/ExtensionCommon.jsm
+++ b/toolkit/components/extensions/ExtensionCommon.jsm
@@ -1240,17 +1240,17 @@ class SchemaAPIManager extends EventEmit
    * Create a global object that is used as the shared global for all ext-*.js
    * scripts that are loaded via `loadScript`.
    *
    * @returns {object} A sandbox that is used as the global by `loadScript`.
    */
   _createExtGlobal() {
     let global = Cu.Sandbox(Services.scriptSecurityManager.getSystemPrincipal(), {
       wantXrays: false,
-      sandboxName: `Namespace of ext-*.js scripts for ${this.processType}`,
+      sandboxName: `Namespace of ext-*.js scripts for ${this.processType} (from: resource://gre/modules/ExtensionCommon.jsm)`,
     });
 
     Object.assign(global, {global, Cc, Ci, Cu, Cr, XPCOMUtils, ChromeWorker, ExtensionAPI, ExtensionCommon, MatchPattern, MatchPatternSet, StructuredCloneHolder, extensions: this});
 
     Cu.import("resource://gre/modules/AppConstants.jsm", global);
 
     XPCOMUtils.defineLazyGetter(global, "console", getConsole);
 
--- a/toolkit/components/extensions/ExtensionParent.jsm
+++ b/toolkit/components/extensions/ExtensionParent.jsm
@@ -435,17 +435,17 @@ defineLazyGetter(ProxyContextParent.prot
   return can;
 });
 
 defineLazyGetter(ProxyContextParent.prototype, "apiObj", function() {
   return this.apiCan.root;
 });
 
 defineLazyGetter(ProxyContextParent.prototype, "sandbox", function() {
-  return Cu.Sandbox(this.principal);
+  return Cu.Sandbox(this.principal, {sandboxName: this.uri.spec});
 });
 
 /**
  * The parent side of proxied API context for extension content script
  * running in ExtensionContent.jsm.
  */
 class ContentScriptContextParent extends ProxyContextParent {
 }