Backed out changeset 5336b29e9db0 (bug 1504370). a=backout
authorBrindusan Cristian <cbrindusan@mozilla.com>
Tue, 13 Nov 2018 14:56:15 +0200
changeset 501179 c1babc4f5dfc82eaf9edf4377c87d029dd6564c1
parent 501178 fd2d2838220419e3e2682889598cfcded38211fd
child 501180 072d56f53c88aa83db40ccb4be38226bf24ebddc
push id1864
push userffxbld-merge
push dateMon, 03 Dec 2018 15:51:40 +0000
treeherdermozilla-release@f040763d99ad [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbackout
bugs1504370
milestone64.0
backs out5336b29e9db097a82a4222e6332ba16063e9836e
Backed out changeset 5336b29e9db0 (bug 1504370). a=backout
browser/base/content/browser.js
browser/components/extensions/parent/ext-search.js
browser/modules/BrowserUsageTelemetry.jsm
browser/modules/test/browser/browser_UsageTelemetry_urlbar.js
dom/media/platforms/agnostic/OpusDecoder.cpp
toolkit/components/search/nsSearchService.js
--- a/browser/base/content/browser.js
+++ b/browser/base/content/browser.js
@@ -4229,17 +4229,17 @@ const BrowserSearch = {
    *        An optional parameter passed to |BrowserUsageTelemetry.recordSearch|.
    *        See its documentation for allowed options.
    *        Additionally, if the search was a suggested search, |details.selection|
    *        indicates where the item was in the suggestion list and how the user
    *        selected it: {selection: {index: The selected index, kind: "key" or "mouse"}}
    */
   recordSearchInTelemetry(engine, source, details = {}) {
     try {
-      BrowserUsageTelemetry.recordSearch(gBrowser, engine, source, details);
+      BrowserUsageTelemetry.recordSearch(engine, source, details);
     } catch (ex) {
       Cu.reportError(ex);
     }
   },
 
   /**
    * Helper to record a one-off search with Telemetry.
    *
@@ -4253,17 +4253,17 @@ const BrowserSearch = {
    * @param type
    *        (string) Indicates how the user selected the search item.
    * @param where
    *        (string) Where was the search link opened (e.g. new tab, current tab, ..).
    */
   recordOneoffSearchInTelemetry(engine, source, type, where) {
     try {
       const details = {type, isOneOff: true};
-      BrowserUsageTelemetry.recordSearch(gBrowser, engine, source, details);
+      BrowserUsageTelemetry.recordSearch(engine, source, details);
     } catch (ex) {
       Cu.reportError(ex);
     }
   },
 };
 
 XPCOMUtils.defineConstant(this, "BrowserSearch", BrowserSearch);
 
--- a/browser/components/extensions/parent/ext-search.js
+++ b/browser/components/extensions/parent/ext-search.js
@@ -71,27 +71,24 @@ this.search = class extends ExtensionAPI
           } else {
             engine = Services.search.currentEngine;
           }
           let submission = engine.getSubmission(searchProperties.query, null, "webextension");
           let options = {
             postData: submission.postData,
             triggeringPrincipal: context.principal,
           };
-          let tabbrowser;
           if (searchProperties.tabId === null) {
             let {gBrowser} = windowTracker.topWindow;
             let nativeTab = gBrowser.addTab(submission.uri.spec, options);
             if (!searchLoadInBackground) {
               gBrowser.selectedTab = nativeTab;
             }
-            tabbrowser = gBrowser;
           } else {
             let tab = tabTracker.getTab(searchProperties.tabId);
             tab.linkedBrowser.loadURI(submission.uri.spec, options);
-            tabbrowser = tab.linkedBrowser.getTabBrowser();
           }
-          BrowserUsageTelemetry.recordSearch(tabbrowser, engine, "webextension");
+          BrowserUsageTelemetry.recordSearch(engine, "webextension");
         },
       },
     };
   }
 };
--- a/browser/modules/BrowserUsageTelemetry.jsm
+++ b/browser/modules/BrowserUsageTelemetry.jsm
@@ -112,21 +112,16 @@ function getSearchEngineId(engine) {
     }
     if (engine.name) {
       return "other-" + engine.name;
     }
   }
   return "other";
 }
 
-function shouldRecordSearchCount(tabbrowser) {
-  return !PrivateBrowsingUtils.isWindowPrivate(tabbrowser.ownerGlobal) ||
-         !Services.prefs.getBoolPref("browser.engagement.search_counts.pbm", false);
-}
-
 let URICountListener = {
   // A set containing the visited domains, see bug 1271310.
   _domainSet: new Set(),
   // A map to keep track of the URIs loaded from the restored tabs.
   _restoredURIsMap: new WeakMap(),
 
   isHttpURI(uri) {
     // Only consider http(s) schemas.
@@ -200,19 +195,17 @@ let URICountListener = {
     if (shouldCountURI) {
       Services.telemetry.scalarAdd(UNFILTERED_URI_COUNT_SCALAR_NAME, 1);
     }
 
     if (!this.isHttpURI(uri)) {
       return;
     }
 
-    if (shouldRecordSearchCount(browser.getTabBrowser())) {
-      Services.search.recordSearchURLTelemetry(uriSpec);
-    }
+    Services.search.recordSearchURLTelemetry(uriSpec);
 
     if (!shouldCountURI) {
       return;
     }
 
     // Update the URI counts.
     Services.telemetry.scalarAdd(TOTAL_URI_COUNT_SCALAR_NAME, 1);
 
@@ -402,39 +395,33 @@ let BrowserUsageTelemetry = {
 
   /**
    * The main entry point for recording search related Telemetry. This includes
    * search counts and engagement measurements.
    *
    * Telemetry records only search counts per engine and action origin, but
    * nothing pertaining to the search contents themselves.
    *
-   * @param {tabbrowser} tabbrowser
-   *        The tabbrowser where the search was loaded.
    * @param {nsISearchEngine} engine
    *        The engine handling the search.
    * @param {String} source
    *        Where the search originated from. See KNOWN_SEARCH_SOURCES for allowed
    *        values.
    * @param {Object} [details] Options object.
    * @param {Boolean} [details.isOneOff=false]
    *        true if this event was generated by a one-off search.
    * @param {Boolean} [details.isSuggestion=false]
    *        true if this event was generated by a suggested search.
-   * @param {Boolean} [details.Alias=null]
-   *        The search engine alias used in the search, if any.
+   * @param {Boolean} [details.isAlias=false]
+   *        true if this event was generated by a search using an alias.
    * @param {Object} [details.type=null]
    *        The object describing the event that triggered the search.
    * @throws if source is not in the known sources list.
    */
-recordSearch(tabbrowser, engine, source, details = {}) {
-    if (!shouldRecordSearchCount(tabbrowser)) {
-      return;
-    }
-
+  recordSearch(engine, source, details = {}) {
     const isOneOff = !!details.isOneOff;
     const countId = getSearchEngineId(engine) + "." + source;
 
     if (isOneOff) {
       if (!KNOWN_ONEOFF_SOURCES.includes(source)) {
         // Silently drop the error if this bogus call
         // came from 'urlbar' or 'searchbar'. They're
         // calling |recordSearch| twice from two different
--- a/browser/modules/test/browser/browser_UsageTelemetry_urlbar.js
+++ b/browser/modules/test/browser/browser_UsageTelemetry_urlbar.js
@@ -527,132 +527,8 @@ add_task(async function test_suggestion_
     let resultMethods = resultMethodHist.snapshot();
     checkHistogramResults(resultMethods,
       URLBAR_SELECTED_RESULT_METHODS.rightClickEnter,
       "FX_URLBAR_SELECTED_RESULT_METHOD");
 
     BrowserTestUtils.removeTab(tab);
   });
 });
-
-add_task(async function test_privateWindow() {
-  // Mock the search service's search provider info so that its
-  // recordSearchURLTelemetry() function adds the in-content SEARCH_COUNTS
-  // telemetry for our test engine.
-  Services.search.QueryInterface(Ci.nsIObserver).observe(
-    null,
-    "test:setSearchProviderInfo",
-    JSON.stringify({
-      "example": {
-        "regexp": "^http://example\\.com/",
-        "queryParam": "q",
-      },
-    })
-  );
-
-  let search_hist = getAndClearKeyedHistogram("SEARCH_COUNTS");
-
-  let engine = Services.search.getEngineByName("MozSearch");
-  let expectedURL = engine.getSubmission("query").uri.spec;
-
-  // First, do a bunch of searches in a private window.
-  let win = await BrowserTestUtils.openNewBrowserWindow({ private: true });
-
-  info("Search in a private window and the pref does not exist");
-  let p = BrowserTestUtils.browserLoaded(win.gBrowser.selectedBrowser, false,
-                                         expectedURL);
-  await searchInAwesomebar("query", win);
-  EventUtils.synthesizeKey("KEY_Enter", undefined, win);
-  await p;
-
-  // SEARCH_COUNTS should be incremented.
-  checkKeyedHistogram(search_hist, "other-MozSearch.urlbar", 1);
-  checkKeyedHistogram(search_hist, "example.in-content:organic:none", 1);
-
-  info("Search again in a private window after setting the pref to true");
-  Services.prefs.setBoolPref("browser.engagement.search_counts.pbm", true);
-  p = BrowserTestUtils.browserLoaded(win.gBrowser.selectedBrowser);
-  await searchInAwesomebar("another query", win);
-  EventUtils.synthesizeKey("KEY_Enter", undefined, win);
-  await p;
-
-  // SEARCH_COUNTS should *not* be incremented.
-  checkKeyedHistogram(search_hist, "other-MozSearch.urlbar", 1);
-  checkKeyedHistogram(search_hist, "example.in-content:organic:none", 1);
-
-  info("Search again in a private window after setting the pref to false");
-  Services.prefs.setBoolPref("browser.engagement.search_counts.pbm", false);
-  p = BrowserTestUtils.browserLoaded(win.gBrowser.selectedBrowser);
-  await searchInAwesomebar("another query", win);
-  EventUtils.synthesizeKey("KEY_Enter", undefined, win);
-  await p;
-
-  // SEARCH_COUNTS should be incremented.
-  checkKeyedHistogram(search_hist, "other-MozSearch.urlbar", 2);
-  checkKeyedHistogram(search_hist, "example.in-content:organic:none", 2);
-
-  info("Search again in a private window after clearing the pref");
-  Services.prefs.clearUserPref("browser.engagement.search_counts.pbm");
-  p = BrowserTestUtils.browserLoaded(win.gBrowser.selectedBrowser);
-  await searchInAwesomebar("another query", win);
-  EventUtils.synthesizeKey("KEY_Enter", undefined, win);
-  await p;
-
-  // SEARCH_COUNTS should be incremented.
-  checkKeyedHistogram(search_hist, "other-MozSearch.urlbar", 3);
-  checkKeyedHistogram(search_hist, "example.in-content:organic:none", 3);
-
-  await BrowserTestUtils.closeWindow(win);
-
-  // Now, do a bunch of searches in a non-private window.  Telemetry should
-  // always be recorded regardless of the pref's value.
-  win = await BrowserTestUtils.openNewBrowserWindow();
-
-  info("Search in a non-private window and the pref does not exist");
-  p = BrowserTestUtils.browserLoaded(win.gBrowser.selectedBrowser, false,
-                                     expectedURL);
-  await searchInAwesomebar("query", win);
-  EventUtils.synthesizeKey("KEY_Enter", undefined, win);
-  await p;
-
-  // SEARCH_COUNTS should be incremented.
-  checkKeyedHistogram(search_hist, "other-MozSearch.urlbar", 4);
-  checkKeyedHistogram(search_hist, "example.in-content:organic:none", 4);
-
-  info("Search again in a non-private window after setting the pref to true");
-  Services.prefs.setBoolPref("browser.engagement.search_counts.pbm", true);
-  p = BrowserTestUtils.browserLoaded(win.gBrowser.selectedBrowser);
-  await searchInAwesomebar("another query", win);
-  EventUtils.synthesizeKey("KEY_Enter", undefined, win);
-  await p;
-
-  // SEARCH_COUNTS should be incremented.
-  checkKeyedHistogram(search_hist, "other-MozSearch.urlbar", 5);
-  checkKeyedHistogram(search_hist, "example.in-content:organic:none", 5);
-
-  info("Search again in a non-private window after setting the pref to false");
-  Services.prefs.setBoolPref("browser.engagement.search_counts.pbm", false);
-  p = BrowserTestUtils.browserLoaded(win.gBrowser.selectedBrowser);
-  await searchInAwesomebar("another query", win);
-  EventUtils.synthesizeKey("KEY_Enter", undefined, win);
-  await p;
-
-  // SEARCH_COUNTS should be incremented.
-  checkKeyedHistogram(search_hist, "other-MozSearch.urlbar", 6);
-  checkKeyedHistogram(search_hist, "example.in-content:organic:none", 6);
-
-  info("Search again in a non-private window after clearing the pref");
-  Services.prefs.clearUserPref("browser.engagement.search_counts.pbm");
-  p = BrowserTestUtils.browserLoaded(win.gBrowser.selectedBrowser);
-  await searchInAwesomebar("another query", win);
-  EventUtils.synthesizeKey("KEY_Enter", undefined, win);
-  await p;
-
-  // SEARCH_COUNTS should be incremented.
-  checkKeyedHistogram(search_hist, "other-MozSearch.urlbar", 7);
-  checkKeyedHistogram(search_hist, "example.in-content:organic:none", 7);
-
-  await BrowserTestUtils.closeWindow(win);
-
-  // Reset the search provider info.
-  Services.search.QueryInterface(Ci.nsIObserver)
-    .observe(null, "test:setSearchProviderInfo", "");
-});
--- a/dom/media/platforms/agnostic/OpusDecoder.cpp
+++ b/dom/media/platforms/agnostic/OpusDecoder.cpp
@@ -149,23 +149,23 @@ OpusDataDecoder::DecodeHeader(const unsi
   if (vorbisLayout.IsValid()) {
     mChannelMap = vorbisLayout.Map();
 
     AudioConfig::ChannelLayout smpteLayout(
       AudioConfig::ChannelLayout::SMPTEDefault(vorbisLayout));
 
     AutoTArray<uint8_t, 8> map;
     map.SetLength(channels);
-    if (mOpusParser->mChannelMapping == 1 &&
-        vorbisLayout.MappingTable(smpteLayout, &map)) {
+    if (vorbisLayout.MappingTable(smpteLayout, &map)) {
       for (int i = 0; i < channels; i++) {
         mMappingTable[i] = mOpusParser->mMappingTable[map[i]];
       }
     } else {
-      // Use Opus set channel mapping and return channels as-is.
+      // Should never get here as vorbis layout is always convertible to SMPTE
+      // default layout.
       PodCopy(mMappingTable.Elements(), mOpusParser->mMappingTable, channels);
     }
   } else {
     // Create a dummy mapping table so that channel ordering stay the same
     // during decoding.
     for (int i = 0; i < channels; i++) {
       mMappingTable[i] = i;
     }
--- a/toolkit/components/search/nsSearchService.js
+++ b/toolkit/components/search/nsSearchService.js
@@ -185,44 +185,48 @@ var OS_UNSUPPORTED_PARAMS = [
 // specifies an updateURL, but not an updateInterval.
 const SEARCH_DEFAULT_UPDATE_INTERVAL = 7;
 
 // The default interval before checking again for the name of the
 // default engine for the region, in seconds. Only used if the response
 // from the server doesn't specify an interval.
 const SEARCH_GEO_DEFAULT_UPDATE_INTERVAL = 2592000; // 30 days.
 
+// Regular expressions used to identify common search URLS.
+const SEARCH_URL_REGEX = new RegExp([
+  /^https:\/\/www\.(google)\.(?:.+)\/search/,
+  /^https:\/\/(?:.*)search\.(yahoo)\.com\/search/,
+  /^https:\/\/www\.(bing)\.com\/search/,
+  /^https:\/\/(duckduckgo)\.com\//,
+  /^https:\/\/www\.(baidu)\.com\/(?:s|baidu)/,
+].map(regex => regex.source).join("|"));
+
 // Used to identify various parameters (query, code, etc.) in search URLS.
 const SEARCH_PROVIDER_INFO = {
   "google": {
-    "regexp": /^https:\/\/www\.(google)\.(?:.+)\/search/,
     "queryParam": "q",
     "codeParam": "client",
     "codePrefixes": ["firefox"],
     "followonParams": ["oq", "ved", "ei"],
   },
   "duckduckgo": {
-    "regexp": /^https:\/\/(duckduckgo)\.com\//,
     "queryParam": "q",
     "codeParam": "t",
     "codePrefixes": ["ff"],
   },
   "yahoo": {
-    "regexp": /^https:\/\/(?:.*)search\.(yahoo)\.com\/search/,
     "queryParam": "p",
   },
   "baidu": {
-    "regexp": /^https:\/\/www\.(baidu)\.com\/(?:s|baidu)/,
     "queryParam": "wd",
     "codeParam": "tn",
     "codePrefixes": ["monline_dg"],
     "followonParams": ["oq"],
   },
   "bing": {
-    "regexp": /^https:\/\/www\.(bing)\.com\/search/,
     "queryParam": "q",
     "codeParam": "pc",
     "codePrefixes": ["MOZ", "MZ"],
   },
 };
 
 const SEARCH_COUNTS_HISTOGRAM_KEY = "SEARCH_COUNTS";
 /**
@@ -4558,32 +4562,23 @@ SearchService.prototype = {
     } catch (ex) {
       // Decoding errors will cause this match to be ignored.
       return gEmptyParseSubmissionResult;
     }
 
     return new ParseSubmissionResult(mapEntry.engine, terms, offset, length);
   },
 
-  __searchProviderInfo: null,
-  get _searchProviderInfo() {
-    if (!this.__searchProviderInfo) {
-      this.__searchProviderInfo = SEARCH_PROVIDER_INFO;
-    }
-    return this.__searchProviderInfo;
-  },
-
   recordSearchURLTelemetry(url) {
-    let entry = Object.entries(this._searchProviderInfo).find(
-      ([_, info]) => info.regexp.test(url)
-    );
-    if (!entry) {
+    let matches = url.match(SEARCH_URL_REGEX);
+    if (!matches) {
       return;
     }
-    let [provider, searchProviderInfo] = entry;
+    let provider = matches.filter((e, i) => e && i != 0)[0];
+    let searchProviderInfo = SEARCH_PROVIDER_INFO[provider];
     let queries = new URLSearchParams(url.split("#")[0].split("?")[1]);
     if (!queries.get(searchProviderInfo.queryParam)) {
       return;
     }
     // Default to organic to simplify things.
     // We override type in the sap cases.
     let type = "organic";
     let code;
@@ -4660,28 +4655,16 @@ SearchService.prototype = {
         // Locale changed. Re-init. We rely on observers, because we can't
         // return this promise to anyone.
         // FYI, This is also used by the search tests to do an async reinit.
         // Locales are removed during shutdown, so ignore this message
         if (!Services.startup.shuttingDown) {
           this._asyncReInit();
         }
         break;
-
-      case "test:setSearchProviderInfo":
-        if (aVerb) {
-          let infoByProvider = JSON.parse(aVerb);
-          for (let info of Object.values(infoByProvider)) {
-            info.regexp = new RegExp(info.regexp);
-          }
-          this.__searchProviderInfo = infoByProvider;
-        } else {
-          this.__searchProviderInfo = SEARCH_PROVIDER_INFO;
-        }
-        break;
     }
   },
 
   // nsITimerCallback
   notify: function SRCH_SVC_notify(aTimer) {
     LOG("_notify: checking for updates");
 
     if (!Services.prefs.getBoolPref(BROWSER_SEARCH_PREF + "update", true))