Bug 1475571 - Replace follow-on addon with in tree telemetry. r=mikedeboer,florian
☠☠ backed out by 46cf10902086 ☠ ☠
authorMichael Kaply <mozilla@kaply.com>
Mon, 06 Aug 2018 22:02:02 +0000
changeset 827166 502893089232b37d57c44e2a3c045dca80fff266
parent 827165 53682999921900d9d8cc0f43cd8d5022cf0e5696
child 827167 136c3952bbe087092358341988d2e368a0431071
push id118488
push userbmo:hsivonen@hsivonen.fi
push dateTue, 07 Aug 2018 12:28:14 +0000
reviewersmikedeboer, florian
bugs1475571
milestone63.0a1
Bug 1475571 - Replace follow-on addon with in tree telemetry. r=mikedeboer,florian Differential Revision: https://phabricator.services.mozilla.com/D2125
browser/extensions/moz.build
browser/modules/BrowserUsageTelemetry.jsm
toolkit/components/search/nsSearchService.js
toolkit/components/telemetry/Histograms.json
--- a/browser/extensions/moz.build
+++ b/browser/extensions/moz.build
@@ -1,17 +1,16 @@
 # -*- Mode: python; indent-tabs-mode: nil; tab-width: 40 -*-
 # 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 += [
     'aushelper',
-    'followonsearch',
     'formautofill',
     'onboarding',
     'pdfjs',
     'pocket',
     'screenshots',
     'webcompat',
     'webcompat-reporter'
 ]
--- a/browser/modules/BrowserUsageTelemetry.jsm
+++ b/browser/modules/BrowserUsageTelemetry.jsm
@@ -8,20 +8,23 @@
 var EXPORTED_SYMBOLS = [
   "BrowserUsageTelemetry",
   "URLBAR_SELECTED_RESULT_TYPES",
   "URLBAR_SELECTED_RESULT_METHODS",
   "MINIMUM_TAB_COUNT_INTERVAL_MS",
  ];
 
 ChromeUtils.import("resource://gre/modules/Services.jsm");
+ChromeUtils.import("resource://gre/modules/XPCOMUtils.jsm");
 
 ChromeUtils.defineModuleGetter(this, "PrivateBrowsingUtils",
                                "resource://gre/modules/PrivateBrowsingUtils.jsm");
 
+XPCOMUtils.defineLazyGlobalGetters(this, ["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";
@@ -83,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++;
@@ -154,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;
@@ -183,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) {
@@ -221,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,
@@ -609,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
@@ -4326,17 +4326,22 @@ SearchService.prototype = {
         }
         // Else we're a match.
         return true;
       });
     });
   },
 
   parseSubmissionURL: function SRCH_SVC_parseSubmissionURL(aURL) {
-    this._ensureInitialized();
+    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;
+    }
     LOG("parseSubmissionURL: Parsing \"" + aURL + "\".");
 
     if (!this._parseSubmissionMap) {
       this._buildParseSubmissionMap();
     }
 
     // Extract the elements of the provided URL first.
     let soughtKey, soughtQuery;
--- a/toolkit/components/telemetry/Histograms.json
+++ b/toolkit/components/telemetry/Histograms.json
@@ -7822,17 +7822,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,