Bug 1475571 - Replace follow-on addon with in tree telemetry. r=mikedeboer,florian, a=RyanVM FENNEC_61_0_2_BUILD1 FENNEC_61_0_2_RELEASE FIREFOX_61_0_2_BUILD1 FIREFOX_61_0_2_RELEASE
authorMichael Kaply <mozilla@kaply.com>
Tue, 07 Aug 2018 11:53:14 -0500
changeset 473827 975058795980
parent 473826 0be81adef007
child 473828 82d9460bcb1e
push id1751
push userryanvm@gmail.com
push dateTue, 07 Aug 2018 17:02:31 +0000
treeherdermozilla-release@975058795980 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmikedeboer, florian, RyanVM
bugs1475571
milestone61.0.2
Bug 1475571 - Replace follow-on addon with in tree telemetry. r=mikedeboer,florian, a=RyanVM
browser/base/content/test/performance/browser_preferences_usage.js
browser/extensions/moz.build
browser/modules/BrowserUsageTelemetry.jsm
toolkit/components/search/nsSearchService.js
toolkit/components/telemetry/Histograms.json
--- a/browser/base/content/test/performance/browser_preferences_usage.js
+++ b/browser/base/content/test/performance/browser_preferences_usage.js
@@ -85,17 +85,17 @@ add_task(async function startup() {
       min: 100,
       max: 150,
     },
     "layout.css.dpi": {
       min: 45,
       max: 75,
     },
     "extensions.getAddons.cache.enabled": {
-      min: 10,
+      min: 9,
       max: 55,
     },
   };
 
   let startupRecorder = Cc["@mozilla.org/test/startuprecorder;1"].getService().wrappedJSObject;
   await startupRecorder.done;
 
   ok(startupRecorder.data.prefStats, "startupRecorder has prefStats");
--- a/browser/extensions/moz.build
+++ b/browser/extensions/moz.build
@@ -2,17 +2,16 @@
 # vim: set filetype=python:
 # 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/.
 
 DIRS += [
     'activity-stream',
     'aushelper',
-    'followonsearch',
     'formautofill',
     'onboarding',
     'pdfjs',
     'pocket',
     'screenshots',
     'webcompat',
     'webcompat-reporter'
 ]
--- a/browser/modules/BrowserUsageTelemetry.jsm
+++ b/browser/modules/BrowserUsageTelemetry.jsm
@@ -13,16 +13,18 @@ var EXPORTED_SYMBOLS = [
  ];
 
 ChromeUtils.import("resource://gre/modules/Services.jsm");
 ChromeUtils.import("resource://gre/modules/XPCOMUtils.jsm");
 
 ChromeUtils.defineModuleGetter(this, "PrivateBrowsingUtils",
                                "resource://gre/modules/PrivateBrowsingUtils.jsm");
 
+Cu.importGlobalProperties(["URLSearchParams"]);
+
 // The upper bound for the count of the visited unique domain names.
 const MAX_UNIQUE_VISITED_DOMAINS = 100;
 
 // Observed topic names.
 const TAB_RESTORING_TOPIC = "SSTabRestoring";
 const TELEMETRY_SUBSESSIONSPLIT_TOPIC = "internal-telemetry-after-subsession-split";
 const DOMWINDOW_OPENED_TOPIC = "domwindowopened";
 const AUTOCOMPLETE_ENTER_TEXT_TOPIC = "autocomplete-did-enter-text";
@@ -84,17 +86,16 @@ const URLBAR_SELECTED_RESULT_METHODS = {
   arrowEnterSelection: 3,
   tabEnterSelection: 4,
   rightClickEnter: 5,
 };
 
 
 const MINIMUM_TAB_COUNT_INTERVAL_MS = 5 * 60 * 1000; // 5 minutes, in ms
 
-
 function getOpenTabsAndWinsCounts() {
   let tabCount = 0;
   let winCount = 0;
 
   let browserEnum = Services.wm.getEnumerator("navigator:browser");
   while (browserEnum.hasMoreElements()) {
     let win = browserEnum.getNext();
     winCount++;
@@ -155,24 +156,30 @@ let URICountListener = {
     // loads, we will hit onLocationChange again.
     // We can catch the first case by checking for null requests: be advised that
     // this can also happen when navigating page fragments, so account for it.
     if (!request &&
         !(flags & Ci.nsIWebProgressListener.LOCATION_CHANGE_SAME_DOCUMENT)) {
       return;
     }
 
+    // Don't include URI and domain counts when in private mode.
+    let shouldCountURI = !PrivateBrowsingUtils.isWindowPrivate(browser.ownerGlobal) ||
+                         Services.prefs.getBoolPref("browser.engagement.total_uri_count.pbm", false);
+
     // Track URI loads, even if they're not http(s).
     let uriSpec = null;
     try {
       uriSpec = uri.spec;
     } catch (e) {
       // If we have troubles parsing the spec, still count this as
       // an unfiltered URI.
-      Services.telemetry.scalarAdd(UNFILTERED_URI_COUNT_SCALAR_NAME, 1);
+      if (shouldCountURI) {
+        Services.telemetry.scalarAdd(UNFILTERED_URI_COUNT_SCALAR_NAME, 1);
+      }
       return;
     }
 
 
     // Don't count about:blank and similar pages, as they would artificially
     // inflate the counts.
     if (browser.ownerGlobal.gInitialPages.includes(uriSpec)) {
       return;
@@ -184,22 +191,52 @@ let URICountListener = {
     if (this._restoredURIsMap.get(browser) === uriSpec) {
       this._restoredURIsMap.delete(browser);
       return;
     }
 
     // The URI wasn't from a restored tab. Count it among the unfiltered URIs.
     // If this is an http(s) URI, this also gets counted by the "total_uri_count"
     // probe.
-    Services.telemetry.scalarAdd(UNFILTERED_URI_COUNT_SCALAR_NAME, 1);
+    if (shouldCountURI) {
+      Services.telemetry.scalarAdd(UNFILTERED_URI_COUNT_SCALAR_NAME, 1);
+    }
 
     if (!this.isHttpURI(uri)) {
       return;
     }
 
+    let parseURLResult = Services.search.parseSubmissionURL(uriSpec);
+    if (parseURLResult.engine) {
+      this._recordSearchTelemetry(uriSpec, parseURLResult);
+    } else if (this._urlsQueuedForParsing) {
+      if (Services.search.isInitialized) {
+        this._urlsQueuedForParsing = null;
+      } else {
+        this._urlsQueuedForParsing.push(uriSpec);
+        if (this._urlsQueuedForParsing.length == 1) {
+          Services.search.init(rv => {
+            if (Components.isSuccessCode(rv)) {
+              for (let url of this._urlsQueuedForParsing) {
+                let innerParseURLResult = Services.search.parseSubmissionURL(url);
+                if (innerParseURLResult.engine) {
+                  this._recordSearchTelemetry(url, innerParseURLResult);
+                }
+              }
+            }
+            this._urlsQueuedForParsing = null;
+          });
+        }
+      }
+    }
+
+    if (!shouldCountURI) {
+      return;
+    }
+
     // Update the URI counts.
     Services.telemetry.scalarAdd(TOTAL_URI_COUNT_SCALAR_NAME, 1);
 
     // Update tab count
     BrowserUsageTelemetry._recordTabCount();
 
     // We only want to count the unique domains up to MAX_UNIQUE_VISITED_DOMAINS.
     if (this._domainSet.size == MAX_UNIQUE_VISITED_DOMAINS) {
@@ -222,16 +259,41 @@ let URICountListener = {
 
   /**
    * Reset the counts. This should be called when breaking a session in Telemetry.
    */
   reset() {
     this._domainSet.clear();
   },
 
+  _urlsQueuedForParsing: [],
+
+  _recordSearchTelemetry(url, parseURLResult) {
+    switch (parseURLResult.engine.identifier) {
+      case "google":
+      case "google-2018":
+        let type;
+        let queries = new URLSearchParams(url.split("?")[1]);
+        let code = queries.get("client");
+        if (code) {
+          // Detecting follow-on searches for sap is a little tricky.
+          // There are a few parameters that only show up
+          // with follow-ons, so we look for those. (oq/ved/ei)
+          type = queries.has("oq") || queries.has("ved") || queries.has("ei") ? "sap-follow-on" : "sap";
+        } else {
+          type = "organic";
+        }
+        let payload = `google.in-content.${type}:${code || "none"}`;
+
+        let histogram = Services.telemetry.getKeyedHistogramById("SEARCH_COUNTS");
+        histogram.add(payload);
+        break;
+    }
+  },
+
   QueryInterface: ChromeUtils.generateQI([Ci.nsIWebProgressListener,
                                           Ci.nsISupportsWeakReference]),
 };
 
 let urlbarListener = {
 
   // This is needed for recordUrlbarSelectedResultMethod().
   selectedIndex: -1,
@@ -610,37 +672,27 @@ let BrowserUsageTelemetry = {
 
   /**
    * Adds listeners to a single chrome window.
    */
   _registerWindow(win) {
     win.addEventListener("unload", this);
     win.addEventListener("TabOpen", this, true);
 
-    // Don't include URI and domain counts when in private mode.
-    if (PrivateBrowsingUtils.isWindowPrivate(win) &&
-        !Services.prefs.getBoolPref("browser.engagement.total_uri_count.pbm", false)) {
-      return;
-    }
     win.gBrowser.tabContainer.addEventListener(TAB_RESTORING_TOPIC, this);
     win.gBrowser.addTabsProgressListener(URICountListener);
   },
 
   /**
    * Removes listeners from a single chrome window.
    */
   _unregisterWindow(win) {
     win.removeEventListener("unload", this);
     win.removeEventListener("TabOpen", this, true);
 
-    // Don't include URI and domain counts when in private mode.
-    if (PrivateBrowsingUtils.isWindowPrivate(win.defaultView) &&
-        !Services.prefs.getBoolPref("browser.engagement.total_uri_count.pbm", false)) {
-      return;
-    }
     win.defaultView.gBrowser.tabContainer.removeEventListener(TAB_RESTORING_TOPIC, this);
     win.defaultView.gBrowser.removeTabsProgressListener(URICountListener);
   },
 
   /**
    * Updates the tab counts.
    * @param {Number} [newTabCount=0] The count of the opened tabs across all windows. This
    *        is computed manually if not provided.
--- a/toolkit/components/search/nsSearchService.js
+++ b/toolkit/components/search/nsSearchService.js
@@ -2429,18 +2429,16 @@ Engine.prototype = {
 
   /**
    * Returns URL parsing properties used by _buildParseSubmissionMap.
    */
   getURLParsingInfo() {
     let responseType = AppConstants.platform == "android" ? this._defaultMobileResponseType :
                                                             URLTYPE_SEARCH_HTML;
 
-    LOG("getURLParsingInfo: responseType: \"" + responseType + "\"");
-
     let url = this._getURLOfType(responseType);
     if (!url || url.method != "GET") {
       return null;
     }
 
     let termsParameterName = url._getTermsParameterName();
     if (!termsParameterName) {
       return null;
@@ -4258,62 +4256,52 @@ SearchService.prototype = {
    *   engine: The associated nsISearchEngine.
    *   termsParameterName: Name of the URL parameter containing the search
    *                       terms, for example "q".
    * }
    */
   _parseSubmissionMap: null,
 
   _buildParseSubmissionMap: function SRCH_SVC__buildParseSubmissionMap() {
-    LOG("_buildParseSubmissionMap");
     this._parseSubmissionMap = new Map();
 
     // Used only while building the map, indicates which entries do not refer to
     // the main domain of the engine but to an alternate domain, for example
     // "www.google.fr" for the "www.google.com" search engine.
     let keysOfAlternates = new Set();
 
     for (let engine of this._sortedEngines) {
-      LOG("Processing engine: " + engine.name);
-
       if (engine.hidden) {
-        LOG("Engine is hidden.");
         continue;
       }
 
       let urlParsingInfo = engine.getURLParsingInfo();
       if (!urlParsingInfo) {
-        LOG("Engine does not support URL parsing.");
         continue;
       }
 
       // Store the same object on each matching map key, as an optimization.
       let mapValueForEngine = {
         engine,
         termsParameterName: urlParsingInfo.termsParameterName,
       };
 
       let processDomain = (domain, isAlternate) => {
         let key = domain + urlParsingInfo.path;
 
         // Apply the logic for which main domains take priority over alternate
         // domains, even if they are found later in the ordered engine list.
         let existingEntry = this._parseSubmissionMap.get(key);
         if (!existingEntry) {
-          LOG("Adding new entry: " + key);
           if (isAlternate) {
             keysOfAlternates.add(key);
           }
         } else if (!isAlternate && keysOfAlternates.has(key)) {
-          LOG("Overriding alternate entry: " + key +
-              " (" + existingEntry.engine.name + ")");
           keysOfAlternates.delete(key);
         } else {
-          LOG("Keeping existing entry: " + key +
-              " (" + existingEntry.engine.name + ")");
           return;
         }
 
         this._parseSubmissionMap.set(key, mapValueForEngine);
       };
 
       processDomain(urlParsingInfo.mainDomain, false);
       SearchStaticData.getAlternateDomains(urlParsingInfo.mainDomain)
@@ -4371,64 +4359,64 @@ SearchService.prototype = {
         }
         // Else we're a match.
         return true;
       });
     });
   },
 
   parseSubmissionURL: function SRCH_SVC_parseSubmissionURL(aURL) {
-    this._ensureInitialized();
-    LOG("parseSubmissionURL: Parsing \"" + aURL + "\".");
+    if (!gInitialized) {
+      // If search is not initialized, do nothing.
+      // This allows us to use this function early in telemetry.
+      // The only other consumer of this (places) uses it much later.
+      return gEmptyParseSubmissionResult;
+    }
 
     if (!this._parseSubmissionMap) {
       this._buildParseSubmissionMap();
     }
 
     // Extract the elements of the provided URL first.
     let soughtKey, soughtQuery;
     try {
       let soughtUrl = Services.io.newURI(aURL).QueryInterface(Ci.nsIURL);
 
       // Exclude any URL that is not HTTP or HTTPS from the beginning.
       if (soughtUrl.scheme != "http" && soughtUrl.scheme != "https") {
-        LOG("The URL scheme is not HTTP or HTTPS.");
         return gEmptyParseSubmissionResult;
       }
 
       // Reading these URL properties may fail and raise an exception.
       soughtKey = soughtUrl.host + soughtUrl.filePath.toLowerCase();
       soughtQuery = soughtUrl.query;
     } catch (ex) {
       // Errors while parsing the URL or accessing the properties are not fatal.
-      LOG("The value does not look like a structured URL.");
       return gEmptyParseSubmissionResult;
     }
 
     // Look up the domain and path in the map to identify the search engine.
     let mapEntry = this._parseSubmissionMap.get(soughtKey);
     if (!mapEntry) {
-      LOG("No engine associated with domain and path: " + soughtKey);
       return gEmptyParseSubmissionResult;
     }
 
     // Extract the search terms from the parameter, for example "caff%C3%A8"
     // from the URL "https://www.google.com/search?q=caff%C3%A8&client=firefox".
     let encodedTerms = null;
     for (let param of soughtQuery.split("&")) {
       let equalPos = param.indexOf("=");
       if (equalPos != -1 &&
           param.substr(0, equalPos) == mapEntry.termsParameterName) {
         // This is the parameter we are looking for.
         encodedTerms = param.substr(equalPos + 1);
         break;
       }
     }
     if (encodedTerms === null) {
-      LOG("Missing terms parameter: " + mapEntry.termsParameterName);
       return gEmptyParseSubmissionResult;
     }
 
     let length = 0;
     let offset = aURL.indexOf("?") + 1;
     let query = aURL.slice(offset);
     // Iterate a second time over the original input string to determine the
     // correct search term offset and length in the original encoding.
@@ -4447,22 +4435,19 @@ SearchService.prototype = {
     // Decode the terms using the charset defined in the search engine.
     let terms;
     try {
       terms = Services.textToSubURI.UnEscapeAndConvert(
         mapEntry.engine.queryCharset,
         encodedTerms.replace(/\+/g, " "));
     } catch (ex) {
       // Decoding errors will cause this match to be ignored.
-      LOG("Parameter decoding failed. Charset: " +
-          mapEntry.engine.queryCharset);
       return gEmptyParseSubmissionResult;
     }
 
-    LOG("Match found. Terms: " + terms);
     return new ParseSubmissionResult(mapEntry.engine, terms, offset, length);
   },
 
   // nsIObserver
   observe: function SRCH_SVC_observe(aEngine, aTopic, aVerb) {
     switch (aTopic) {
       case SEARCH_ENGINE_TOPIC:
         switch (aVerb) {
--- a/toolkit/components/telemetry/Histograms.json
+++ b/toolkit/components/telemetry/Histograms.json
@@ -7719,17 +7719,17 @@
     "description": "Time elapsed between before responding to Slow Add-on Warning UI (ms). Not updated if the user doesn't respond at all."
   },
   "SEARCH_COUNTS": {
     "record_in_processes": ["main", "content"],
     "expires_in_version": "never",
     "kind": "count",
     "keyed": true,
     "releaseChannelCollection": "opt-out",
-    "description": "Record the search counts for search engines"
+    "description": "Records search counts for search access points and in content searches. For search access points, the format is: <engine-name>.<search-access-point> For in content searches, the format is <provider>.in-content:[sap|sap-follow-on|organic]:[code|none]"
   },
   "SEARCH_RESET_RESULT": {
     "record_in_processes": ["main", "content"],
     "alert_emails": ["fqueze@mozilla.com"],
     "bug_numbers": [1203168],
     "expires_in_version": "65",
     "kind": "enumerated",
     "n_values": 5,