Bug 1536465 - remove expiring FX_TAB_REMOTE_NAVIGATION_DELAY_MS telemetry probe. r=mconley
authorJulien Cristau <jcristau@mozilla.com>
Mon, 20 May 2019 11:57:42 +0200
changeset 474480 ab585bce1f5bee00fa4dcad39f7701359ceba6a0
parent 474479 acb185a74efcf22bb8491f8eb95cedff393c6184
child 474481 0e88a0a379f37595e17d7b516c90d8d2962798d0
push id36040
push userrgurzau@mozilla.com
push dateMon, 20 May 2019 13:43:21 +0000
treeherdermozilla-central@319a369ccde4 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmconley
bugs1536465
milestone68.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 1536465 - remove expiring FX_TAB_REMOTE_NAVIGATION_DELAY_MS telemetry probe. r=mconley Differential Revision: https://phabricator.services.mozilla.com/D27328
browser/components/sessionstore/ContentSessionStore.jsm
browser/components/sessionstore/SessionStore.jsm
devtools/client/responsive.html/browser/web-navigation.js
dom/ipc/tests/browser.ini
dom/ipc/tests/browser_remote_navigation_delay_telemetry.js
toolkit/actors/WebNavigationChild.jsm
toolkit/components/remotebrowserutils/RemoteWebNavigation.jsm
toolkit/components/telemetry/Histograms.json
--- a/browser/components/sessionstore/ContentSessionStore.jsm
+++ b/browser/components/sessionstore/ContentSessionStore.jsm
@@ -748,21 +748,16 @@ class ContentSessionStore {
       this.epoch = data.epoch;
     }
 
     switch (name) {
       case "SessionStore:restoreHistory":
         this.restoreHistory(data);
         break;
       case "SessionStore:restoreTabContent":
-        if (data.isRemotenessUpdate) {
-          let histogram = Services.telemetry.getKeyedHistogramById("FX_TAB_REMOTE_NAVIGATION_DELAY_MS");
-          histogram.add("SessionStore:restoreTabContent",
-                        Services.telemetry.msSystemNow() - data.requestTime);
-        }
         this.restoreTabContent(data);
         break;
       case "SessionStore:resetRestore":
         this.contentRestore.resetRestore();
         break;
       case "SessionStore:flush":
         this.flush(data);
         break;
--- a/browser/components/sessionstore/SessionStore.jsm
+++ b/browser/components/sessionstore/SessionStore.jsm
@@ -4305,18 +4305,17 @@ var SessionStoreInternal = {
     // view source browser that will load the required frame script.
     if (uri && ViewSourceBrowser.isViewSource(uri)) {
       new ViewSourceBrowser(browser);
     }
 
     browser.messageManager.sendAsyncMessage("SessionStore:restoreTabContent",
       {loadArguments, isRemotenessUpdate,
        reason: aOptions.restoreContentReason ||
-               RESTORE_TAB_CONTENT_REASON.SET_STATE,
-       requestTime: Services.telemetry.msSystemNow()});
+               RESTORE_TAB_CONTENT_REASON.SET_STATE});
 
     // Focus the tab's content area.
     if (aTab.selected && !window.isBlankPageURL(uri)) {
       browser.focus();
     }
   },
 
   /**
--- a/devtools/client/responsive.html/browser/web-navigation.js
+++ b/devtools/client/responsive.html/browser/web-navigation.js
@@ -4,19 +4,16 @@
 
 "use strict";
 
 const { Cc, Ci, Cu, Cr } = require("chrome");
 const ChromeUtils = require("ChromeUtils");
 const Services = require("Services");
 const { NetUtil } = require("resource://gre/modules/NetUtil.jsm");
 const { E10SUtils } = require("resource://gre/modules/E10SUtils.jsm");
-const Telemetry = require("devtools/client/shared/telemetry");
-
-const telemetry = new Telemetry();
 
 function readInputStreamToString(stream) {
   return NetUtil.readInputStreamToString(stream, stream.available());
 }
 
 /**
  * This object aims to provide the nsIWebNavigation interface for mozbrowser elements.
  * nsIWebNavigation is one of the interfaces expected on <xul:browser>s, so this wrapper
@@ -80,17 +77,16 @@ BrowserElementWebNavigation.prototype = 
       flags,
       referrerInfo,
       postData: postData ? readInputStreamToString(postData) : null,
       headers: headers ? readInputStreamToString(headers) : null,
       baseURI: baseURI ? baseURI.spec : null,
       triggeringPrincipal: E10SUtils.serializePrincipal(
                            triggeringPrincipal ||
                            Services.scriptSecurityManager.createNullPrincipal({})),
-      requestTime: telemetry.msSystemNow(),
     });
   },
 
   setOriginAttributesBeforeLoading(originAttributes) {
     // No equivalent in the current BrowserElement API
     this._sendMessage("WebNavigation:SetOriginAttributes", {
       originAttributes,
     });
--- a/dom/ipc/tests/browser.ini
+++ b/dom/ipc/tests/browser.ini
@@ -2,12 +2,10 @@
 support-files =
   file_disableScript.html
   file_domainPolicy_base.html
   file_cancel_content_js.html
 
 [browser_domainPolicy.js]
 [browser_memory_distribution_telemetry.js]
 skip-if = !e10 # This is an e10s only probe.
-[browser_remote_navigation_delay_telemetry.js]
+[browser_cancel_content_js.js]
 skip-if = !e10s # This is an e10s only probe.
-[browser_cancel_content_js.js]
-skip-if = !e10s # This is an e10s only probe.
\ No newline at end of file
deleted file mode 100644
--- a/dom/ipc/tests/browser_remote_navigation_delay_telemetry.js
+++ /dev/null
@@ -1,48 +0,0 @@
-"use strict";
-
-var session = ChromeUtils.import("resource://gre/modules/TelemetrySession.jsm", null);
-
-add_task(async function test_memory_distribution() {
-  if (Services.prefs.getIntPref("dom.ipc.processCount", 1) < 2) {
-    ok(true, "Skip this test if e10s-multi is disabled.");
-    return;
-  }
-
-  let canRecordExtended = Services.telemetry.canRecordExtended;
-  Services.telemetry.canRecordExtended = true;
-  registerCleanupFunction(() => Services.telemetry.canRecordExtended = canRecordExtended);
-
-  Services.telemetry.getSnapshotForKeyedHistograms("main", true /* clear */);
-
-  // Open a remote page in a new tab to trigger the WebNavigation:LoadURI.
-  let tab1 = await BrowserTestUtils.openNewForegroundTab(gBrowser, "http://example.com");
-  ok(tab1.linkedBrowser.isRemoteBrowser, "|tab1| should have a remote browser.");
-
-  // Open a new tab with about:robots, so it ends up in the parent process with a non-remote browser.
-  let tab2 = await BrowserTestUtils.openNewForegroundTab(gBrowser, "about:robots");
-  ok(!tab2.linkedBrowser.isRemoteBrowser, "|tab2| should have a non-remote browser.");
-  // Navigate the tab, so it will change remotness and it triggers the SessionStore:restoreTabContent case.
-  await BrowserTestUtils.loadURI(tab2.linkedBrowser, "http://example.com");
-  ok(tab2.linkedBrowser.isRemoteBrowser, "|tab2| should have a remote browser by now.");
-
-  // There is no good way to make sure that the parent received the histogram entries from the child processes.
-  // Let's stick to the ugly, spinning the event loop until we have a good approach (Bug 1357509).
-  await BrowserTestUtils.waitForCondition(() => {
-    let s = Services.telemetry.getSnapshotForKeyedHistograms("main", false).content.FX_TAB_REMOTE_NAVIGATION_DELAY_MS;
-    return s && "WebNavigation:LoadURI" in s && "SessionStore:restoreTabContent" in s;
-  });
-
-  let s = Services.telemetry.getSnapshotForKeyedHistograms("main", false).content.FX_TAB_REMOTE_NAVIGATION_DELAY_MS;
-  let restoreTabSnapshot = s["SessionStore:restoreTabContent"];
-  ok(restoreTabSnapshot.sum > 0, "Zero delay for the restoreTabContent case is unlikely.");
-  ok(restoreTabSnapshot.sum < 10000, "More than 10 seconds delay for the restoreTabContent case is unlikely.");
-
-  let loadURISnapshot = s["WebNavigation:LoadURI"];
-  ok(loadURISnapshot.sum > 0, "Zero delay for the LoadURI case is unlikely.");
-  ok(loadURISnapshot.sum < 10000, "More than 10 seconds delay for the LoadURI case is unlikely.");
-
-  Services.telemetry.getSnapshotForKeyedHistograms("main", true /* clear */);
-
-  BrowserTestUtils.removeTab(tab2);
-  BrowserTestUtils.removeTab(tab1);
-});
--- a/toolkit/actors/WebNavigationChild.jsm
+++ b/toolkit/actors/WebNavigationChild.jsm
@@ -31,22 +31,17 @@ class WebNavigationChild extends ActorCh
         break;
       case "WebNavigation:GoForward":
         this.goForward(message.data);
         break;
       case "WebNavigation:GotoIndex":
         this.gotoIndex(message.data);
         break;
       case "WebNavigation:LoadURI":
-        let histogram = Services.telemetry.getKeyedHistogramById("FX_TAB_REMOTE_NAVIGATION_DELAY_MS");
-        histogram.add("WebNavigation:LoadURI",
-                      Services.telemetry.msSystemNow() - message.data.requestTime);
-
         this.loadURI(message.data);
-
         break;
       case "WebNavigation:SetOriginAttributes":
         this.setOriginAttributes(message.data.originAttributes);
         break;
       case "WebNavigation:Reload":
         this.reload(message.data.flags);
         break;
       case "WebNavigation:Stop":
--- a/toolkit/components/remotebrowserutils/RemoteWebNavigation.jsm
+++ b/toolkit/components/remotebrowserutils/RemoteWebNavigation.jsm
@@ -111,17 +111,16 @@ RemoteWebNavigation.prototype = {
       flags: aLoadURIOptions.loadFlags,
       referrerInfo: E10SUtils.serializeReferrerInfo(aLoadURIOptions.referrerInfo),
       postData: aLoadURIOptions.postData ? Utils.serializeInputStream(aLoadURIOptions.postData) : null,
       headers: aLoadURIOptions.headers ? Utils.serializeInputStream(aLoadURIOptions.headers) : null,
       baseURI: aLoadURIOptions.baseURI ? aLoadURIOptions.baseURI.spec : null,
       triggeringPrincipal: E10SUtils.serializePrincipal(
                            aLoadURIOptions.triggeringPrincipal || Services.scriptSecurityManager.createNullPrincipal({})),
       csp: aLoadURIOptions.csp ? E10SUtils.serializeCSP(aLoadURIOptions.csp) : null,
-      requestTime: Services.telemetry.msSystemNow(),
       cancelContentJSEpoch,
     });
   },
   setOriginAttributesBeforeLoading(aOriginAttributes) {
     this._sendMessage("WebNavigation:SetOriginAttributes", {
       originAttributes: aOriginAttributes,
     });
   },
--- a/toolkit/components/telemetry/Histograms.json
+++ b/toolkit/components/telemetry/Histograms.json
@@ -6277,27 +6277,16 @@
   "FX_TAB_CLICK_MS": {
     "record_in_processes": ["main", "content"],
     "expires_in_version": "default",
     "kind": "exponential",
     "high": 1000,
     "n_buckets": 20,
     "description": "Firefox: Time in ms spent on switching tabs in response to a tab click"
   },
-  "FX_TAB_REMOTE_NAVIGATION_DELAY_MS": {
-    "record_in_processes": ["content"],
-    "alert_emails": ["mconley@mozilla.com"],
-    "bug_numbers": [1352961, 1501295],
-    "expires_in_version": "69",
-    "kind": "exponential",
-    "high": 4000,
-    "n_buckets": 100,
-    "keyed": true,
-    "description": "Time taken (in ms) from the point of the parent sending the naviagion triggering message to the content and the content receiving it. This message can be either SessionStore:restoreTabContent or WebNavigation:LoadURI and these names are used as keys for this histogram. This is e10s only and recorded in the content process."
-  },
   "FX_BOOKMARKS_TOOLBAR_INIT_MS": {
     "record_in_processes": ["main", "content"],
     "expires_in_version": "never",
     "kind": "exponential",
     "low": 50,
     "high": 5000,
     "n_buckets": 10,
     "description": "Firefox: Time to initialize the bookmarks toolbar view (ms)"