Bug 1607545 - Don't record about_home_topsites_first_paint scalar unless the first windows initial tab is about:home. r=k88hudson
authorMike Conley <mconley@mozilla.com>
Wed, 12 Feb 2020 19:17:12 +0000
changeset 513619 7be6903300cfb83220957c288700846566752fa8
parent 513618 b3f0df8ad260f8bf5a0e0e76d9c6d3d4fffd8043
child 513620 0b633de1ffd949dc2faaa297d905a64777c18173
push id107140
push usermconley@mozilla.com
push dateWed, 12 Feb 2020 19:18:40 +0000
treeherderautoland@7be6903300cf [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersk88hudson
bugs1607545
milestone75.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 1607545 - Don't record about_home_topsites_first_paint scalar unless the first windows initial tab is about:home. r=k88hudson The test fix was reviewed by Ed Lee (edilee@mozilla.com) in https://phabricator.services.mozilla.com/D61401, and then that change was rolled into this commit. Differential Revision: https://phabricator.services.mozilla.com/D60416
browser/base/content/browser.js
browser/components/newtab/AboutNewTabService.jsm
browser/components/newtab/lib/TelemetryFeed.jsm
browser/components/newtab/test/unit/lib/TelemetryFeed.test.js
--- a/browser/base/content/browser.js
+++ b/browser/base/content/browser.js
@@ -10,16 +10,17 @@ var { Services } = ChromeUtils.import("r
 var { AppConstants } = ChromeUtils.import(
   "resource://gre/modules/AppConstants.jsm"
 );
 ChromeUtils.import("resource://gre/modules/NotificationDB.jsm");
 
 // lazy module getters
 
 XPCOMUtils.defineLazyModuleGetters(this, {
+  AboutNewTabStartupRecorder: "resource:///modules/AboutNewTabService.jsm",
   AddonManager: "resource://gre/modules/AddonManager.jsm",
   AMTelemetry: "resource://gre/modules/AddonManager.jsm",
   NewTabPagePreloading: "resource:///modules/NewTabPagePreloading.jsm",
   BrowserUsageTelemetry: "resource:///modules/BrowserUsageTelemetry.jsm",
   BrowserUtils: "resource://gre/modules/BrowserUtils.jsm",
   BrowserWindowTracker: "resource:///modules/BrowserWindowTracker.jsm",
   CFRPageActions: "resource://activity-stream/lib/CFRPageActions.jsm",
   CharsetMenu: "resource://gre/modules/CharsetMenu.jsm",
@@ -2339,16 +2340,18 @@ var gBrowserInit = {
 
       let uri = window.arguments[0];
       let defaultArgs = Cc["@mozilla.org/browser/clh;1"].getService(
         Ci.nsIBrowserHandler
       ).defaultArgs;
 
       // If the given URI is different from the homepage, we want to load it.
       if (uri != defaultArgs) {
+        AboutNewTabStartupRecorder.noteNonDefaultStartup();
+
         if (uri instanceof Ci.nsIArray) {
           // Transform the nsIArray of nsISupportsString's into a JS Array of
           // JS strings.
           return Array.from(
             uri.enumerate(Ci.nsISupportsString),
             supportStr => supportStr.data
           );
         } else if (uri instanceof Ci.nsISupportsString) {
--- a/browser/components/newtab/AboutNewTabService.jsm
+++ b/browser/components/newtab/AboutNewTabService.jsm
@@ -46,17 +46,16 @@ function AboutNewTabService() {
   );
   if (!IS_RELEASE_OR_BETA) {
     Services.prefs.addObserver(PREF_ACTIVITY_STREAM_DEBUG, this);
   }
 
   // More initialization happens here
   this.toggleActivityStream(true);
   this.initialized = true;
-  this.alreadyRecordedTopsitesPainted = false;
 
   if (IS_MAIN_PROCESS) {
     AboutNewTab.init();
   } else if (IS_PRIVILEGED_PROCESS) {
     Services.obs.addObserver(this, TOPIC_CONTENT_DOCUMENT_INTERACTIVE);
   }
 }
 
@@ -292,39 +291,54 @@ AboutNewTabService.prototype = {
 
   resetNewTabURL() {
     this._overridden = false;
     this._newTabURL = ABOUT_URL;
     this.toggleActivityStream(true, true);
     this.notifyChange();
   },
 
-  maybeRecordTopsitesPainted(timestamp) {
-    if (this.alreadyRecordedTopsitesPainted) {
-      return;
-    }
-
-    const SCALAR_KEY = "timestamps.about_home_topsites_first_paint";
-
-    let startupInfo = Services.startup.getStartupInfo();
-    let processStartTs = startupInfo.process.getTime();
-    let delta = Math.round(timestamp - processStartTs);
-    Services.telemetry.scalarSet(SCALAR_KEY, delta);
-    this.alreadyRecordedTopsitesPainted = true;
-  },
-
   uninit() {
     if (!this.initialized) {
       return;
     }
     Services.obs.removeObserver(this, TOPIC_APP_QUIT);
     Services.prefs.removeObserver(
       PREF_SEPARATE_PRIVILEGEDABOUT_CONTENT_PROCESS,
       this
     );
     if (!IS_RELEASE_OR_BETA) {
       Services.prefs.removeObserver(PREF_ACTIVITY_STREAM_DEBUG, this);
     }
     this.initialized = false;
   },
 };
 
-const EXPORTED_SYMBOLS = ["AboutNewTabService"];
+/**
+ * We split out the definition of AboutNewTabStartupRecorder from
+ * AboutNewTabService to avoid initializing the AboutNewTabService
+ * unnecessarily early when we just want to record some startup
+ * data.
+ */
+const AboutNewTabStartupRecorder = {
+  _alreadyRecordedTopsitesPainted: false,
+  _nonDefaultStartup: false,
+
+  noteNonDefaultStartup() {
+    this._nonDefaultStartup = true;
+  },
+
+  maybeRecordTopsitesPainted(timestamp) {
+    if (this._alreadyRecordedTopsitesPainted || this._nonDefaultStartup) {
+      return;
+    }
+
+    const SCALAR_KEY = "timestamps.about_home_topsites_first_paint";
+
+    let startupInfo = Services.startup.getStartupInfo();
+    let processStartTs = startupInfo.process.getTime();
+    let delta = Math.round(timestamp - processStartTs);
+    Services.telemetry.scalarSet(SCALAR_KEY, delta);
+    this._alreadyRecordedTopsitesPainted = true;
+  },
+};
+
+const EXPORTED_SYMBOLS = ["AboutNewTabService", "AboutNewTabStartupRecorder"];
--- a/browser/components/newtab/lib/TelemetryFeed.jsm
+++ b/browser/components/newtab/lib/TelemetryFeed.jsm
@@ -26,16 +26,21 @@ ChromeUtils.defineModuleGetter(
 );
 ChromeUtils.defineModuleGetter(
   this,
   "perfService",
   "resource://activity-stream/common/PerfService.jsm"
 );
 ChromeUtils.defineModuleGetter(
   this,
+  "AboutNewTabStartupRecorder",
+  "resource:///modules/AboutNewTabService.jsm"
+);
+ChromeUtils.defineModuleGetter(
+  this,
   "PingCentre",
   "resource:///modules/PingCentre.jsm"
 );
 ChromeUtils.defineModuleGetter(
   this,
   "UTEventReporting",
   "resource://activity-stream/lib/UTEventReporting.jsm"
 );
@@ -1068,17 +1073,17 @@ this.TelemetryFeed = class TelemetryFeed
     let timestamp = data.topsites_first_painted_ts;
 
     if (
       timestamp &&
       session.page === "about:home" &&
       !HomePage.overridden &&
       Services.prefs.getIntPref("browser.startup.page") === 1
     ) {
-      aboutNewTabService.maybeRecordTopsitesPainted(timestamp);
+      AboutNewTabStartupRecorder.maybeRecordTopsitesPainted(timestamp);
     }
 
     Object.assign(session.perf, data);
   }
 
   uninit() {
     try {
       Services.obs.removeObserver(
--- a/browser/components/newtab/test/unit/lib/TelemetryFeed.test.js
+++ b/browser/components/newtab/test/unit/lib/TelemetryFeed.test.js
@@ -1254,19 +1254,17 @@ describe("TelemetryFeed", () => {
     });
 
     it("should call maybeRecordTopsitesPainted when url is about:home and topsites_first_painted_ts is given", () => {
       const topsites_first_painted_ts = 44455;
       const data = { topsites_first_painted_ts };
       const spy = sandbox.spy();
 
       sandbox.stub(Services.prefs, "getIntPref").returns(1);
-      globals.set("aboutNewTabService", {
-        overridden: false,
-        newTabURL: "",
+      globals.set("AboutNewTabStartupRecorder", {
         maybeRecordTopsitesPainted: spy,
       });
       instance.addSession("port123", "about:home");
       instance.saveSessionPerfData("port123", data);
 
       assert.calledOnce(spy);
       assert.calledWith(spy, topsites_first_painted_ts);
     });