Bug 989373, part 1 - Remove pref for reusing globals with JSMs. r=kmag
authorAndrew McCreight <continuation@gmail.com>
Mon, 15 May 2017 14:33:21 -0700
changeset 359832 b2b8fadcfa753e9a1571c935a415d076306c0204
parent 359831 373092e83b736f09e2e060aaf7fae71e54816efd
child 359833 36bb469c723c7400668f6aae028d93178704578f
push id31859
push userihsiao@mozilla.com
push dateMon, 22 May 2017 03:28:26 +0000
treeherdermozilla-central@367944041b55 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerskmag
bugs989373
milestone55.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 989373, part 1 - Remove pref for reusing globals with JSMs. r=kmag Removing support for this preference means that mReuseLoaderGlobal will always be false, which lets us eliminate that field, and remove a lot of code. This also means that false is always passed to PrepareObjectForLocation for aReuseLoaderGlobal. ReadCachedFunction is no longer used, so it is deleted. MozReview-Commit-ID: 5JD24EYVcQf
dom/ipc/ContentPrefs.cpp
js/xpconnect/loader/mozJSComponentLoader.cpp
js/xpconnect/loader/mozJSComponentLoader.h
js/xpconnect/loader/mozJSLoaderUtils.cpp
js/xpconnect/loader/mozJSLoaderUtils.h
modules/libpref/init/all.js
--- a/dom/ipc/ContentPrefs.cpp
+++ b/dom/ipc/ContentPrefs.cpp
@@ -108,17 +108,16 @@ const char* mozilla::dom::ContentPrefs::
   "javascript.options.strict",
   "javascript.options.strict.debug",
   "javascript.options.throw_on_asmjs_validation_failure",
   "javascript.options.throw_on_debuggee_would_run",
   "javascript.options.wasm",
   "javascript.options.wasm_baselinejit",
   "javascript.options.werror",
   "javascript.use_us_english_locale",
-  "jsloader.reuseGlobal",
   "layout.idle_period.required_quiescent_frames",
   "layout.idle_period.time_limit",
   "layout.interruptible-reflow.enabled",
   "mathml.disabled",
   "media.apple.forcevda",
   "media.clearkey.persistent-license.enabled",
   "media.cubeb.backend",
   "media.cubeb_latency_msg_frames",
--- a/js/xpconnect/loader/mozJSComponentLoader.cpp
+++ b/js/xpconnect/loader/mozJSComponentLoader.cpp
@@ -49,21 +49,16 @@
 #include "mozilla/UniquePtrExtensions.h"
 #include "mozilla/Unused.h"
 
 using namespace mozilla;
 using namespace mozilla::scache;
 using namespace xpc;
 using namespace JS;
 
-// This JSClass exists to trick silly code that expects toString()ing the
-// global in a component scope to return something with "BackstagePass" in it
-// to continue working.
-static const JSClass kFakeBackstagePassJSClass = { "FakeBackstagePass" };
-
 static const char kObserverServiceContractID[] = "@mozilla.org/observer-service;1";
 static const char kJSCachePrefix[] = "jsloader";
 
 /**
  * Buffer sizes for serialization and deserialization of scripts.
  * FIXME: bug #411579 (tune this macro!) Last updated: Jan 2008
  */
 #define XPC_SERIALIZATION_BUFFER_SIZE   (64 * 1024)
@@ -191,18 +186,17 @@ ReportOnCallerUTF8(JSCLContextHelper& he
     va_end(ap);
     return NS_OK;
 }
 
 mozJSComponentLoader::mozJSComponentLoader()
     : mModules(16),
       mImports(16),
       mInProgressImports(16),
-      mInitialized(false),
-      mReuseLoaderGlobal(false)
+      mInitialized(false)
 {
     MOZ_ASSERT(!sSelf, "mozJSComponentLoader should be a singleton");
 
     sSelf = this;
 }
 
 #define ENSURE_DEP(name) { nsresult rv = Ensure##name(); NS_ENSURE_SUCCESS(rv, rv); }
 #define ENSURE_DEPS(...) MOZ_FOR_EACH(ENSURE_DEP, (), (__VA_ARGS__));
@@ -295,18 +289,16 @@ NS_IMPL_ISUPPORTS(mozJSComponentLoader,
                   xpcIJSModuleLoader,
                   nsIObserver)
 
 nsresult
 mozJSComponentLoader::ReallyInit()
 {
     nsresult rv;
 
-    mReuseLoaderGlobal = Preferences::GetBool("jsloader.reuseGlobal");
-
     nsCOMPtr<nsIScriptSecurityManager> secman =
         do_GetService(NS_SCRIPTSECURITYMANAGER_CONTRACTID);
     if (!secman)
         return NS_ERROR_FAILURE;
 
     rv = secman->GetSystemPrincipal(getter_AddRefs(mSystemPrincipal));
     if (NS_FAILED(rv) || !mSystemPrincipal)
         return NS_ERROR_FAILURE;
@@ -425,49 +417,27 @@ mozJSComponentLoader::LoadModule(FileLoc
         return nullptr;
     }
 
     // Cache this module for later
     mModules.Put(spec, entry);
 
     // Set the location information for the new global, so that tools like
     // about:memory may use that information
-    if (!mReuseLoaderGlobal) {
-        xpc::SetLocationForGlobal(entryObj, spec);
-    }
+    xpc::SetLocationForGlobal(entryObj, spec);
 
     // The hash owns the ModuleEntry now, forget about it
     return entry.forget();
 }
 
 void
 mozJSComponentLoader::FindTargetObject(JSContext* aCx,
                                        MutableHandleObject aTargetObject)
 {
-    aTargetObject.set(nullptr);
-
-    RootedObject targetObject(aCx);
-    if (mReuseLoaderGlobal) {
-        JSFunction* fun = js::GetOutermostEnclosingFunctionOfScriptedCaller(aCx);
-        if (fun) {
-            JSObject* funParent = js::GetNearestEnclosingWithEnvironmentObjectForFunction(fun);
-            if (JS_GetClass(funParent) == &kFakeBackstagePassJSClass)
-                targetObject = funParent;
-        }
-    }
-
-    // The above could fail, even if mReuseLoaderGlobal, if the scripted
-    // caller is not a component/JSM (it could be a DOM scope, for
-    // instance).
-    if (!targetObject) {
-        // Our targetObject is the caller's global object. Let's get it.
-        targetObject = CurrentGlobalOrNull(aCx);
-    }
-
-    aTargetObject.set(targetObject);
+    aTargetObject.set(CurrentGlobalOrNull(aCx));
 }
 
 // This requires that the keys be strings and the values be pointers.
 template <class Key, class Data, class UserData>
 static size_t
 SizeOfTableExcludingThis(const nsBaseHashtable<Key, Data, UserData>& aTable,
                          MallocSizeOf aMallocSizeOf)
 {
@@ -507,37 +477,32 @@ class FileMapAutoCloser
  private:
     PRFileMap* mMap;
 };
 
 JSObject*
 mozJSComponentLoader::PrepareObjectForLocation(JSContext* aCx,
                                                nsIFile* aComponentFile,
                                                nsIURI* aURI,
-                                               bool aReuseLoaderGlobal,
                                                bool* aRealFile)
 {
     nsCOMPtr<nsIXPConnectJSObjectHolder> holder;
-    if (aReuseLoaderGlobal) {
-        holder = mLoaderGlobal;
-    }
-
     nsresult rv = NS_OK;
     bool createdNewGlobal = false;
 
     if (!mLoaderGlobal) {
         RefPtr<BackstagePass> backstagePass;
         rv = NS_NewBackstagePass(getter_AddRefs(backstagePass));
         NS_ENSURE_SUCCESS(rv, nullptr);
 
         CompartmentOptions options;
 
         options.creationOptions()
                .setSystemZone()
-               .setAddonId(aReuseLoaderGlobal ? nullptr : MapURIToAddonID(aURI));
+               .setAddonId(MapURIToAddonID(aURI));
 
         options.behaviors().setVersion(JSVERSION_LATEST);
 
         if (xpc::SharedMemoryEnabled())
             options.creationOptions().setSharedMemoryAndAtomicsEnabled(true);
 
         // Defer firing OnNewGlobalObject until after the __URI__ property has
         // been defined so the JS debugger can tell what module the global is
@@ -557,34 +522,23 @@ mozJSComponentLoader::PrepareObjectForLo
 
         backstagePass->SetGlobalObject(global);
 
         JSAutoCompartment ac(aCx, global);
         if (!JS_DefineFunctions(aCx, global, gGlobalFun) ||
             !JS_DefineProfilingFunctions(aCx, global)) {
             return nullptr;
         }
-
-        if (aReuseLoaderGlobal) {
-            mLoaderGlobal = holder;
-        }
     }
 
     RootedObject obj(aCx, holder->GetJSObject());
     NS_ENSURE_TRUE(obj, nullptr);
 
     JSAutoCompartment ac(aCx, obj);
 
-    if (aReuseLoaderGlobal) {
-        // If we're reusing the loader global, we don't actually use the
-        // global, but rather we use a different object as the 'this' object.
-        obj = JS_NewObject(aCx, &kFakeBackstagePassJSClass);
-        NS_ENSURE_TRUE(obj, nullptr);
-    }
-
     *aRealFile = false;
 
     // need to be extra careful checking for URIs pointing to files
     // EnsureFile may not always get called, especially on resource URIs
     // so we need to call GetFile to make sure this is a valid file
     nsCOMPtr<nsIFileURL> fileURL = do_QueryInterface(aURI, &rv);
     nsCOMPtr<nsIFile> testFile;
     if (NS_SUCCEEDED(rv)) {
@@ -646,19 +600,19 @@ mozJSComponentLoader::ObjectForLocation(
     dom::AutoJSAPI jsapi;
     jsapi.Init();
     JSContext* cx = jsapi.cx();
 
     bool realFile = false;
     nsresult rv = aInfo.EnsureURI();
     NS_ENSURE_SUCCESS(rv, rv);
     RootedObject obj(cx, PrepareObjectForLocation(cx, aComponentFile, aInfo.URI(),
-                                                  mReuseLoaderGlobal, &realFile));
+                                                  &realFile));
     NS_ENSURE_TRUE(obj, NS_ERROR_FAILURE);
-    MOZ_ASSERT(JS_IsGlobalObject(obj) == !mReuseLoaderGlobal);
+    MOZ_ASSERT(JS_IsGlobalObject(obj));
 
     JSAutoCompartment ac(cx, obj);
 
     RootedScript script(cx);
     RootedFunction function(cx);
 
     nsAutoCString nativePath;
     rv = aInfo.URI()->GetSpec(nativePath);
@@ -670,62 +624,47 @@ mozJSComponentLoader::ObjectForLocation(
 
     bool writeToCache = false;
     StartupCache* cache = StartupCache::GetSingleton();
 
     nsAutoCString cachePath(kJSCachePrefix);
     rv = PathifyURI(aInfo.URI(), cachePath);
     NS_ENSURE_SUCCESS(rv, rv);
 
-    if (!mReuseLoaderGlobal) {
-        script = ScriptPreloader::GetSingleton().GetCachedScript(cx, cachePath);
-        if (!script && cache) {
-            ReadCachedScript(cache, cachePath, cx, mSystemPrincipal, &script);
-        }
-    } else if (cache) {
-        ReadCachedFunction(cache, cachePath, cx, mSystemPrincipal,
-                           function.address());
+    script = ScriptPreloader::GetSingleton().GetCachedScript(cx, cachePath);
+    if (!script && cache) {
+        ReadCachedScript(cache, cachePath, cx, mSystemPrincipal, &script);
     }
 
     if (script || function) {
         LOG(("Successfully loaded %s from startupcache\n", nativePath.get()));
     } else if (cache) {
         // This is ok, it just means the script is not yet in the
         // cache. Could mean that the cache was corrupted and got removed,
         // but either way we're going to write this out.
         writeToCache = true;
-        // ReadCachedScript and ReadCachedFunction may have set a pending
-        // exception.
+        // ReadCachedScript may have set a pending exception.
         JS_ClearPendingException(cx);
     }
 
     if (!script && !function) {
         // The script wasn't in the cache , so compile it now.
         LOG(("Slow loading %s\n", nativePath.get()));
 
-        // Use lazy source if both of these conditions hold:
-        //
-        // (1) mReuseLoaderGlobal is false. If mReuseLoaderGlobal is true, we
-        //     can't do lazy source because we compile things as functions
-        //     (rather than script), and lazy source isn't supported in that
-        //     configuration. That's ok though, because we only do
-        //     mReuseLoaderGlobal on b2g, where we invoke setDiscardSource(true)
-        //     on the entire global.
-        //
-        // (2) We're using the startup cache. Non-lazy source + startup cache
-        //     regresses installer size (due to source code stored in XDR
-        //     encoded modules in omni.ja). Also, XDR decoding is relatively
-        //     fast. Content processes don't use the startup cache, so we want
-        //     them to use non-lazy source code to enable lazy parsing.
-        //     See bug 1303754.
+        // Use lazy source if we're using the startup cache. Non-lazy source +
+        // startup cache regresses installer size (due to source code stored in
+        // XDR encoded modules in omni.ja). Also, XDR decoding is relatively
+        // fast. When we're not using the startup cache, we want to use non-lazy
+        // source code so that we can use lazy parsing.
+        // See bug 1303754.
         CompileOptions options(cx);
-        options.setNoScriptRval(mReuseLoaderGlobal ? false : true)
+        options.setNoScriptRval(true)
                .setVersion(JSVERSION_LATEST)
                .setFileAndLine(nativePath.get(), 1)
-               .setSourceIsLazy(!mReuseLoaderGlobal && !!cache);
+               .setSourceIsLazy(!!cache);
 
         if (realFile) {
             int64_t fileSize;
             rv = aComponentFile->GetFileSize(&fileSize);
             if (NS_FAILED(rv)) {
                 return rv;
             }
 
@@ -761,28 +700,19 @@ mozJSComponentLoader::ObjectForLocation(
             uint32_t fileSize32 = fileSize;
 
             char* buf = static_cast<char*>(PR_MemMap(map, 0, fileSize32));
             if (!buf) {
                 NS_WARNING("Failed to map file");
                 return NS_ERROR_FAILURE;
             }
 
-            if (!mReuseLoaderGlobal) {
-                Compile(cx, options, buf, fileSize32, &script);
-            } else {
-                // Note: exceptions will get handled further down;
-                // don't early return for them here.
-                AutoObjectVector envChain(cx);
-                if (envChain.append(obj)) {
-                    CompileFunction(cx, envChain,
-                                    options, nullptr, 0, nullptr,
-                                    buf, fileSize32, &function);
-                }
-            }
+            // Note: exceptions will get handled further down;
+            // don't early return for them here.
+            Compile(cx, options, buf, fileSize32, &script);
 
             PR_MemUnmap(buf, fileSize32);
         } else {
             rv = aInfo.EnsureScriptChannel();
             NS_ENSURE_SUCCESS(rv, rv);
             nsCOMPtr<nsIInputStream> scriptStream;
             rv = NS_MaybeOpenChannelUsingOpen2(aInfo.ScriptChannel(),
                    getter_AddRefs(scriptStream));
@@ -805,28 +735,17 @@ mozJSComponentLoader::ObjectForLocation(
 
             /* read the file in one swoop */
             rv = scriptStream->Read(buf.get(), len, &bytesRead);
             if (bytesRead != len)
                 return NS_BASE_STREAM_OSERROR;
 
             buf[len] = '\0';
 
-            if (!mReuseLoaderGlobal) {
-                Compile(cx, options, buf.get(), bytesRead, &script);
-            } else {
-                // Note: exceptions will get handled further down;
-                // don't early return for them here.
-                AutoObjectVector envChain(cx);
-                if (envChain.append(obj)) {
-                    CompileFunction(cx, envChain,
-                                    options, nullptr, 0, nullptr,
-                                    buf.get(), bytesRead, &function);
-                }
-            }
+            Compile(cx, options, buf.get(), bytesRead, &script);
         }
         // Propagate the exception, if one exists. Also, don't leave the stale
         // exception on this context.
         if (!script && !function && aPropagateExceptions &&
             jsapi.HasException()) {
             if (!jsapi.StealException(aException))
                 return NS_ERROR_OUT_OF_MEMORY;
         }
@@ -919,36 +838,16 @@ mozJSComponentLoader::ObjectForLocation(
     return NS_OK;
 }
 
 void
 mozJSComponentLoader::UnloadModules()
 {
     mInitialized = false;
 
-    if (mLoaderGlobal) {
-        MOZ_ASSERT(mReuseLoaderGlobal, "How did this happen?");
-
-        dom::AutoJSAPI jsapi;
-        jsapi.Init();
-        JSContext* cx = jsapi.cx();
-        RootedObject global(cx, mLoaderGlobal->GetJSObject());
-        if (global) {
-            JSAutoCompartment ac(cx, global);
-            if (JS_HasExtensibleLexicalEnvironment(global)) {
-                JS_SetAllNonReservedSlotsToUndefined(cx, JS_ExtensibleLexicalEnvironment(global));
-            }
-            JS_SetAllNonReservedSlotsToUndefined(cx, global);
-        } else {
-            NS_WARNING("Going to leak!");
-        }
-
-        mLoaderGlobal = nullptr;
-    }
-
     mInProgressImports.Clear();
     mImports.Clear();
 
     for (auto iter = mModules.Iter(); !iter.Done(); iter.Next()) {
         iter.Data()->Clear();
         iter.Remove();
     }
 }
@@ -1136,19 +1035,17 @@ mozJSComponentLoader::ImportInto(const n
             }
 
             // Something failed, but we don't know what it is, guess.
             return NS_ERROR_FILE_NOT_FOUND;
         }
 
         // Set the location information for the new global, so that tools like
         // about:memory may use that information
-        if (!mReuseLoaderGlobal) {
-            xpc::SetLocationForGlobal(newEntry->obj, aLocation);
-        }
+        xpc::SetLocationForGlobal(newEntry->obj, aLocation);
 
         mod = newEntry;
     }
 
     MOZ_ASSERT(mod->obj, "Import table contains entry with no object");
     vp.set(mod->obj);
 
     if (targetObj) {
@@ -1281,19 +1178,16 @@ NS_IMETHODIMP
 mozJSComponentLoader::Unload(const nsACString & aLocation)
 {
     nsresult rv;
 
     if (!mInitialized) {
         return NS_OK;
     }
 
-    MOZ_RELEASE_ASSERT(!mReuseLoaderGlobal, "Module unloading not supported when "
-                                            "compartment sharing is enabled");
-
     ComponentLoaderInfo info(aLocation);
     rv = info.EnsureKey();
     NS_ENSURE_SUCCESS(rv, rv);
     ModuleEntry* mod;
     if (mImports.Get(info.Key(), &mod)) {
         mImports.Remove(info.Key());
     }
 
--- a/js/xpconnect/loader/mozJSComponentLoader.h
+++ b/js/xpconnect/loader/mozJSComponentLoader.h
@@ -60,17 +60,16 @@ class mozJSComponentLoader : public mozi
     static mozJSComponentLoader* sSelf;
 
     nsresult ReallyInit();
     void UnloadModules();
 
     JSObject* PrepareObjectForLocation(JSContext* aCx,
                                        nsIFile* aComponentFile,
                                        nsIURI* aComponent,
-                                       bool aReuseLoaderGlobal,
                                        bool* aRealFile);
 
     nsresult ObjectForLocation(ComponentLoaderInfo& aInfo,
                                nsIFile* aComponentFile,
                                JS::MutableHandleObject aObject,
                                JS::MutableHandleScript aTableScript,
                                char** location,
                                bool aCatchException,
@@ -150,12 +149,11 @@ class mozJSComponentLoader : public mozi
 
     // Modules are intentionally leaked, but still cleared.
     nsDataHashtable<nsCStringHashKey, ModuleEntry*> mModules;
 
     nsClassHashtable<nsCStringHashKey, ModuleEntry> mImports;
     nsDataHashtable<nsCStringHashKey, ModuleEntry*> mInProgressImports;
 
     bool mInitialized;
-    bool mReuseLoaderGlobal;
 };
 
 #endif
--- a/js/xpconnect/loader/mozJSLoaderUtils.cpp
+++ b/js/xpconnect/loader/mozJSLoaderUtils.cpp
@@ -39,24 +39,16 @@ ReadCachedScript(StartupCache* cache, ns
         return NS_ERROR_FAILURE;
 
     MOZ_ASSERT((code & JS::TranscodeResult_Throw) != 0);
     JS_ClearPendingException(cx);
     return NS_ERROR_OUT_OF_MEMORY;
 }
 
 nsresult
-ReadCachedFunction(StartupCache* cache, nsACString& uri, JSContext* cx,
-                   nsIPrincipal* systemPrincipal, JSFunction** functionp)
-{
-    // This doesn't actually work ...
-    return NS_ERROR_NOT_IMPLEMENTED;
-}
-
-nsresult
 WriteCachedScript(StartupCache* cache, nsACString& uri, JSContext* cx,
                   nsIPrincipal* systemPrincipal, HandleScript script)
 {
     MOZ_ASSERT(JS_GetScriptPrincipals(script) == nsJSPrincipals::get(systemPrincipal));
 
     JS::TranscodeBuffer buffer;
     JS::TranscodeResult code = JS::EncodeScript(cx, buffer, script);
     if (code != JS::TranscodeResult_Ok) {
--- a/js/xpconnect/loader/mozJSLoaderUtils.h
+++ b/js/xpconnect/loader/mozJSLoaderUtils.h
@@ -16,21 +16,16 @@ class StartupCache;
 } // namespace mozilla
 
 nsresult
 ReadCachedScript(mozilla::scache::StartupCache* cache, nsACString& uri,
                  JSContext* cx, nsIPrincipal* systemPrincipal,
                  JS::MutableHandleScript scriptp);
 
 nsresult
-ReadCachedFunction(mozilla::scache::StartupCache* cache, nsACString& uri,
-                   JSContext* cx, nsIPrincipal* systemPrincipal,
-                   JSFunction** function);
-
-nsresult
 WriteCachedScript(mozilla::scache::StartupCache* cache, nsACString& uri,
                   JSContext* cx, nsIPrincipal* systemPrincipal,
                   JS::HandleScript script);
 nsresult
 WriteCachedFunction(mozilla::scache::StartupCache* cache, nsACString& uri,
                     JSContext* cx, nsIPrincipal* systemPrincipal,
                     JSFunction* function);
 
--- a/modules/libpref/init/all.js
+++ b/modules/libpref/init/all.js
@@ -5067,22 +5067,16 @@ pref("social.toast-notifications.enabled
 pref("dom.idle-observers-api.fuzz_time.disabled", true);
 
 // Minimum delay in milliseconds between network activity notifications (0 means
 // no notifications). The delay is the same for both download and upload, though
 // they are handled separately. This pref is only read once at startup:
 // a restart is required to enable a new value.
 pref("network.activity.blipIntervalMilliseconds", 0);
 
-// If true, reuse the same global for everything loaded by the component loader
-// (JS components, JSMs, etc).  This saves memory, but makes it possible for
-// the scripts to interfere with each other.  A restart is required for this
-// to take effect.
-pref("jsloader.reuseGlobal", false);
-
 // When we're asked to take a screenshot, don't wait more than 2000ms for the
 // event loop to become idle before actually taking the screenshot.
 pref("dom.browserElement.maxScreenshotDelayMS", 2000);
 
 // Whether we should show the placeholder when the element is focused but empty.
 pref("dom.placeholder.show_on_focus", true);
 
 // WebVR is enabled by default in beta and release for Windows and for all