Bug 1358798 - add a test preventing us from loading scripts unintentionally during startup, r=mconley,mccr8.
authorFlorian Quèze <florian@queze.net>
Wed, 31 May 2017 23:00:43 +0200
changeset 361682 ef0ddbc20a37650c964cd6d6e0547b9339868f16
parent 361681 65251b0ed973675e2d490458fedfc5cb75753abb
child 361683 b94150e0a2991fe40a3504355fad38969b413da0
push id43854
push userryanvm@gmail.com
push dateThu, 01 Jun 2017 00:50:28 +0000
treeherderautoland@5ecd65c9136b [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmconley, mccr8
bugs1358798
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 1358798 - add a test preventing us from loading scripts unintentionally during startup, r=mconley,mccr8.
browser/base/content/test/performance/browser.ini
browser/base/content/test/performance/browser_startup.js
browser/components/moz.build
browser/components/tests/startupRecorder.js
browser/components/tests/testComponents.manifest
browser/installer/package-manifest.in
js/xpconnect/idl/xpcIJSModuleLoader.idl
js/xpconnect/loader/mozJSComponentLoader.cpp
--- a/browser/base/content/test/performance/browser.ini
+++ b/browser/base/content/test/performance/browser.ini
@@ -1,7 +1,8 @@
 [DEFAULT]
 support-files =
   head.js
+[browser_startup.js]
 [browser_tabclose_reflows.js]
 [browser_tabopen_reflows.js]
 [browser_toolbariconcolor_restyles.js]
 [browser_windowopen_reflows.js]
new file mode 100644
--- /dev/null
+++ b/browser/base/content/test/performance/browser_startup.js
@@ -0,0 +1,135 @@
+/* Any copyright is dedicated to the Public Domain.
+   http://creativecommons.org/publicdomain/zero/1.0/ */
+
+/* This test records at which phase of startup the JS components and modules
+ * are first loaded.
+ * If you made changes that cause this test to fail, it's likely because you
+ * are loading more JS code during startup.
+ * Most code has no reason to run off of the app-startup notification
+ * (this is very early, before we have selected the user profile, so
+ *  preferences aren't accessible yet).
+ * If your code isn't strictly required to show the first browser window,
+ * it shouldn't be loaded before we are done with first paint.
+ * Finally, if your code isn't really needed during startup, it should not be
+ * loaded before we have started handling user events.
+ */
+
+"use strict";
+
+const startupPhases = {
+  // For app-startup, we have a whitelist of acceptable JS files.
+  // Anything loaded during app-startup must have a compelling reason
+  // to run before we have even selected the user profile.
+  // Consider loading your code after first paint instead,
+  // eg. from nsBrowserGlue.js' _onFirstWindowLoaded method).
+  "before profile selection": {whitelist: {
+    components: new Set([
+      "nsBrowserGlue.js",
+      "MainProcessSingleton.js",
+
+      // Bugs to fix: The following components shouldn't be initialized that early.
+      "WebContentConverter.js",
+      "nsSessionStartup.js",
+      "PushComponents.js",
+    ]),
+    modules: new Set([
+      "resource://gre/modules/AppConstants.jsm",
+      "resource://gre/modules/XPCOMUtils.jsm",
+      "resource://gre/modules/Services.jsm",
+
+      // Bugs to fix: Probably loaded too early, needs investigation.
+      "resource://gre/modules/Log.jsm",
+      "resource://gre/modules/AsyncPrefs.jsm",
+      "resource://gre/modules/RemotePageManager.jsm",
+      "resource://gre/modules/TelemetryStopwatch.jsm",
+      "resource://gre/modules/PrivateBrowsingUtils.jsm",
+      "resource://gre/modules/Promise.jsm"
+    ])
+  }},
+
+  // For the following phases of startup we have only a black list for now
+
+  // We are at this phase after creating the first browser window (ie. after final-ui-startup).
+  "before opening first browser window": {blacklist: {
+    components: new Set([
+      "nsSearchService.js",
+    ])
+  }},
+
+  // We reach this phase right after showing the first browser window.
+  // This means that anything already loaded at this point has been loaded
+  // before first paint and delayed it.
+  "before first paint": {},
+
+  // We are at this phase once we are ready to handle user events.
+  // Anything loaded at this phase or before gets in the way of the user
+  // interacting with the first browser window.
+  "before handling user events": {},
+}
+
+function test() {
+  if (!AppConstants.NIGHTLY_BUILD && !AppConstants.DEBUG) {
+    ok(!("@mozilla.org/test/startuprecorder;1" in Cc),
+       "the startup recorder component shouldn't exist in this non-nightly non-debug build.");
+    return;
+  }
+
+  let data = Cc["@mozilla.org/test/startuprecorder;1"].getService().wrappedJSObject.data;
+  // Keep only the file name for components, as the path is an absolute file
+  // URL rather than a resource:// URL like for modules.
+  for (let phase in data) {
+    data[phase].components =
+      data[phase].components.map(f => f.replace(/.*\//, ""))
+                            .filter(c => c != "startupRecorder.js");
+  }
+
+  // This block only adds debug output to help find the next bugs to file,
+  // it doesn't contribute to the actual test.
+  SimpleTest.requestCompleteLog();
+  let previous;
+  for (let phase in data) {
+    for (let scriptType in data[phase]) {
+      for (let f of data[phase][scriptType]) {
+        // phases are ordered, so if a script wasn't loaded yet at the immediate
+        // previous phase, it wasn't loaded during any of the previous phases
+        // either, and is new in the current phase.
+        if (!previous || !data[previous][scriptType].includes(f))
+          info(`${scriptType} loaded ${phase}: ${f}`);
+      }
+    }
+    previous = phase;
+  }
+
+  for (let phase in startupPhases) {
+    let loadedList = data[phase];
+    let whitelist = startupPhases[phase].whitelist || null;
+    if (whitelist) {
+      for (let scriptType in loadedList) {
+        loadedList[scriptType] = loadedList[scriptType].filter(c => {
+          if (!whitelist[scriptType].has(c))
+            return true;
+          whitelist[scriptType].delete(c);
+          return false;
+        });
+        is(loadedList[scriptType].length, 0,
+           `should have no unexpected ${scriptType} loaded ${phase}`);
+        for (let script of loadedList[scriptType]) {
+          ok(false, `unexpected ${scriptType}: ${script}`);
+        }
+        is(whitelist[scriptType].size, 0,
+           `all ${scriptType} whitelist entries should have been used`);
+        for (let script of whitelist[scriptType]) {
+          ok(false, `unused ${scriptType} whitelist entry: ${script}`);
+        }
+      }
+    }
+    let blacklist = startupPhases[phase].blacklist || null;
+    if (blacklist) {
+      for (let scriptType in blacklist) {
+        for (let file of blacklist[scriptType]) {
+          ok(!loadedList[scriptType].includes(file), `${file} is not allowed ${phase}`);
+        }
+      }
+    }
+  }
+}
--- a/browser/components/moz.build
+++ b/browser/components/moz.build
@@ -61,16 +61,18 @@ XPIDL_MODULE = 'browsercompsbase'
 
 EXTRA_PP_COMPONENTS += [
     'BrowserComponents.manifest',
 ]
 
 EXTRA_COMPONENTS += [
     'nsBrowserContentHandler.js',
     'nsBrowserGlue.js',
+    'tests/startupRecorder.js',
+    'tests/testComponents.manifest',
 ]
 
 EXTRA_JS_MODULES += [
     'distribution.js',
 ]
 
 BROWSER_CHROME_MANIFESTS += [
     'safebrowsing/content/test/browser.ini',
new file mode 100644
--- /dev/null
+++ b/browser/components/tests/startupRecorder.js
@@ -0,0 +1,71 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * 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/. */
+
+const {classes: Cc, utils: Cu, interfaces: Ci} = Components;
+
+Cu.import("resource://gre/modules/XPCOMUtils.jsm");
+Cu.import("resource://gre/modules/Services.jsm");
+
+/**
+  * The startupRecorder component observes notifications at various stages of
+  * startup and records the set of JS components and modules that were already
+  * loaded at each of these points.
+  * The records are meant to be used by startup tests in
+  * browser/base/content/test/performance
+  * This component only exists in nightly and debug builds, it doesn't ship in
+  * our release builds.
+  */
+function startupRecorder() {
+  this.wrappedJSObject = this;
+  this.loader = Cc["@mozilla.org/moz/jsloader;1"].getService(Ci.xpcIJSModuleLoader);
+  this.data = {};
+}
+startupRecorder.prototype = {
+  classID: Components.ID("{11c095b2-e42e-4bdf-9dd0-aed87595f6a4}"),
+
+  QueryInterface: XPCOMUtils.generateQI([Ci.nsIObserver]),
+
+  record(name) {
+    this.data[name] = {
+      components: this.loader.loadedComponents(),
+      modules: this.loader.loadedModules()
+    };
+  },
+
+  observe(subject, topic, data) {
+
+    if (topic == "app-startup") {
+      // We can't ensure our observer will be called first or last, so the list of
+      // topics we observe here should avoid the topics used to trigger things
+      // during startup (eg. the topics observed by nsBrowserGlue.js).
+      let topics = [
+        "profile-do-change", // This catches stuff loaded during app-startup
+        "toplevel-window-ready", // Catches stuff from final-ui-startup
+        "widget-first-paint",
+        "sessionstore-windows-restored",
+      ];
+      for (let t of topics)
+        Services.obs.addObserver(this, t);
+      return;
+    }
+
+    Services.obs.removeObserver(this, topic);
+
+    if (topic == "sessionstore-windows-restored") {
+      // We use idleDispatch here to record the set of loaded scripts after we
+      // are fully done with startup and ready to react to user events.
+      Services.tm.mainThread.idleDispatch(
+        this.record.bind(this, "before handling user events"));
+    } else {
+      const topicsToNames = {
+        "profile-do-change": "before profile selection",
+        "toplevel-window-ready": "before opening first browser window",
+        "widget-first-paint": "before first paint",
+      };
+      this.record(topicsToNames[topic]);
+    }
+  }
+};
+
+this.NSGetFactory = XPCOMUtils.generateNSGetFactory([startupRecorder]);
new file mode 100644
--- /dev/null
+++ b/browser/components/tests/testComponents.manifest
@@ -0,0 +1,5 @@
+# This component restricts its registration for the app-startup category
+# to the browser app so it doesn't get loaded in xpcshell.
+component {11c095b2-e42e-4bdf-9dd0-aed87595f6a4} startupRecorder.js
+contract @mozilla.org/test/startuprecorder;1 {11c095b2-e42e-4bdf-9dd0-aed87595f6a4}
+category app-startup startupRecorder service,@mozilla.org/test/startuprecorder;1 application={ec8030f7-c20a-464f-9b0e-13a3a9e97384}
--- a/browser/installer/package-manifest.in
+++ b/browser/installer/package-manifest.in
@@ -563,16 +563,21 @@
 #endif
 
 #if defined(ENABLE_TESTS) && defined(MOZ_DEBUG)
 @RESPATH@/components/TestInterfaceJS.js
 @RESPATH@/components/TestInterfaceJS.manifest
 @RESPATH@/components/TestInterfaceJSMaplike.js
 #endif
 
+#if defined(MOZ_DEBUG) || defined(NIGHTLY_BUILD)
+@RESPATH@/browser/components/testComponents.manifest
+@RESPATH@/browser/components/startupRecorder.js
+#endif
+
 ; [Extensions]
 @RESPATH@/components/extensions-toolkit.manifest
 @RESPATH@/browser/components/extensions-browser.manifest
 
 ; Modules
 @RESPATH@/browser/modules/*
 @RESPATH@/modules/*
 
--- a/js/xpconnect/idl/xpcIJSModuleLoader.idl
+++ b/js/xpconnect/idl/xpcIJSModuleLoader.idl
@@ -74,9 +74,17 @@ interface xpcIJSModuleLoader : nsISuppor
    * otherwise.
    *
    * @param resourceURI A resource:// URI string representing the location of
    *        the js file to be checked if it is already loaded or not.
    * @returns boolean, true if the js file has been loaded via import. false
    *          otherwise
    */
   boolean isModuleLoaded(in AUTF8String aResourceURI);
+
+  // These 2 functions are for startup testing purposes. They are not expected
+  // to be used for production code.
+  void loadedModules([optional] out unsigned long length,
+                     [retval, array, size_is(length)] out string aModules);
+
+  void loadedComponents([optional] out unsigned long length,
+                        [retval, array, size_is(length)] out string aComponents);
 };
--- a/js/xpconnect/loader/mozJSComponentLoader.cpp
+++ b/js/xpconnect/loader/mozJSComponentLoader.cpp
@@ -919,16 +919,46 @@ mozJSComponentLoader::IsModuleLoaded(con
     ComponentLoaderInfo info(aLocation);
     rv = info.EnsureKey();
     NS_ENSURE_SUCCESS(rv, rv);
 
     *retval = !!mImports.Get(info.Key());
     return NS_OK;
 }
 
+NS_IMETHODIMP mozJSComponentLoader::LoadedModules(uint32_t* length,
+                                                  char*** aModules)
+{
+    char** modules = new char*[mImports.Count()];
+    *length = mImports.Count();
+    *aModules = modules;
+
+    for (auto iter = mImports.Iter(); !iter.Done(); iter.Next()) {
+        *modules = NS_strdup(iter.Data()->location);
+        modules++;
+    }
+
+    return NS_OK;
+}
+
+NS_IMETHODIMP mozJSComponentLoader::LoadedComponents(uint32_t* length,
+                                                     char*** aComponents)
+{
+    char** comp = new char*[mModules.Count()];
+    *length = mModules.Count();
+    *aComponents = comp;
+
+    for (auto iter = mModules.Iter(); !iter.Done(); iter.Next()) {
+        *comp = NS_strdup(iter.Data()->location);
+        comp++;
+    }
+
+    return NS_OK;
+}
+
 static JSObject*
 ResolveModuleObjectPropertyById(JSContext* aCx, HandleObject aModObj, HandleId id)
 {
     if (JS_HasExtensibleLexicalEnvironment(aModObj)) {
         RootedObject lexical(aCx, JS_ExtensibleLexicalEnvironment(aModObj));
         bool found;
         if (!JS_HasOwnPropertyById(aCx, lexical, id, &found)) {
             return nullptr;