Bug 1681382 - Drop BrowserSearch.record*SearchInTelemetry and call BrowserSearchTelemetry direct. r=daleharvey

Depends on D99206

Differential Revision: https://phabricator.services.mozilla.com/D99207
This commit is contained in:
Mark Banner 2020-12-10 23:40:58 +00:00
parent 7fdf013dc5
commit 3f5df73069
8 changed files with 85 additions and 117 deletions

View File

@ -12,21 +12,13 @@ const { XPCOMUtils } = ChromeUtils.import(
XPCOMUtils.defineLazyGlobalGetters(this, ["XMLHttpRequest"]);
ChromeUtils.defineModuleGetter(
this,
"FormHistory",
"resource://gre/modules/FormHistory.jsm"
);
ChromeUtils.defineModuleGetter(
this,
"PrivateBrowsingUtils",
"resource://gre/modules/PrivateBrowsingUtils.jsm"
);
ChromeUtils.defineModuleGetter(
this,
"SearchSuggestionController",
"resource://gre/modules/SearchSuggestionController.jsm"
);
XPCOMUtils.defineLazyModuleGetters(this, {
BrowserSearchTelemetry: "resource:///modules/BrowserSearchTelemetry.jsm",
FormHistory: "resource://gre/modules/FormHistory.jsm",
PrivateBrowsingUtils: "resource://gre/modules/PrivateBrowsingUtils.jsm",
SearchSuggestionController:
"resource://gre/modules/SearchSuggestionController.jsm",
});
const MAX_LOCAL_SUGGESTIONS = 3;
const MAX_SUGGESTIONS = 6;
@ -250,7 +242,7 @@ let ContentSearch = {
};
win.openTrustedLinkIn(submission.uri.spec, where, params);
}
win.BrowserSearch.recordSearchInTelemetry(engine, data.healthReportKey, {
BrowserSearchTelemetry.recordSearch(browser, engine, data.healthReportKey, {
selection: data.selection,
url: submission.uri,
});

View File

@ -4425,7 +4425,9 @@ const BrowserSearch = {
csp
);
if (engine) {
BrowserSearch.recordSearchInTelemetry(engine, "contextmenu", { url });
BrowserSearchTelemetry.recordSearch(gBrowser, engine, "contextmenu", {
url,
});
}
},
@ -4442,7 +4444,7 @@ const BrowserSearch = {
csp
);
if (engine) {
BrowserSearch.recordSearchInTelemetry(engine, "system", { url });
BrowserSearchTelemetry.recordSearch(gBrowser, engine, "system", { url });
}
},
@ -4461,9 +4463,14 @@ const BrowserSearch = {
tab
);
BrowserSearch.recordSearchInTelemetry(result.engine, "webextension", {
url: result.url,
});
BrowserSearchTelemetry.recordSearch(
gBrowser,
result.engine,
"webextension",
{
url: result.url,
}
);
},
pasteAndSearch(event) {
@ -4490,53 +4497,6 @@ const BrowserSearch = {
var where = newWindowPref == 3 ? "tab" : "window";
openTrustedLinkIn(this.searchEnginesURL, where);
},
/**
* Helper to record a search with Telemetry.
*
* Telemetry records only search counts and nothing pertaining to the search itself.
*
* @param engine
* (nsISearchEngine) The engine handling the search.
* @param source
* (string) Where the search originated from. See BrowserUsageTelemetry for
* allowed values.
* @param details [optional]
* 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 {
BrowserSearchTelemetry.recordSearch(gBrowser, engine, source, details);
} catch (ex) {
Cu.reportError(ex);
}
},
/**
* Helper to record a one-off search with Telemetry.
*
* Telemetry records only search counts and nothing pertaining to the search itself.
*
* @param engine
* (nsISearchEngine) The engine handling the search.
* @param source
* (string) Where the search originated from. See BrowserUsageTelemetry for
* allowed values.
* @param type
* (string) Indicates how the user selected the search item.
*/
recordOneoffSearchInTelemetry(engine, source, type) {
try {
const details = { type, isOneOff: true };
BrowserSearchTelemetry.recordSearch(gBrowser, engine, source, details);
} catch (ex) {
Cu.reportError(ex);
}
},
};
XPCOMUtils.defineConstant(this, "BrowserSearch", BrowserSearch);

View File

@ -29,6 +29,7 @@ XPCOMUtils.defineLazyModuleGetters(this, {
Blocklist: "resource://gre/modules/Blocklist.jsm",
BookmarkHTMLUtils: "resource://gre/modules/BookmarkHTMLUtils.jsm",
BookmarkJSONUtils: "resource://gre/modules/BookmarkJSONUtils.jsm",
BrowserSearchTelemetry: "resource:///modules/BrowserSearchTelemetry.jsm",
BrowserUsageTelemetry: "resource:///modules/BrowserUsageTelemetry.jsm",
BrowserUtils: "resource://gre/modules/BrowserUtils.jsm",
BrowserWindowTracker: "resource:///modules/BrowserWindowTracker.jsm",
@ -1133,7 +1134,7 @@ BrowserGlue.prototype = {
Cu.reportError(ex);
}
let win = BrowserWindowTracker.getTopWindow();
win.BrowserSearch.recordSearchInTelemetry(engine, "urlbar");
BrowserSearchTelemetry.recordSearch(win.gBrowser, engine, "urlbar");
break;
case "browser-search-engine-modified":
// Ensure we cleanup the hiddenOneOffs pref when removing

View File

@ -83,50 +83,58 @@ class BrowserSearchTelemetryHandler {
* @throws if source is not in the known sources list.
*/
recordSearch(tabbrowser, engine, source, details = {}) {
if (!this.shouldRecordSearchCount(tabbrowser)) {
return;
}
try {
if (!this.shouldRecordSearchCount(tabbrowser)) {
return;
}
const countIdPrefix = `${engine.telemetryId}.`;
const countIdSource = countIdPrefix + source;
let histogram = Services.telemetry.getKeyedHistogramById("SEARCH_COUNTS");
const countIdPrefix = `${engine.telemetryId}.`;
const countIdSource = countIdPrefix + source;
let histogram = Services.telemetry.getKeyedHistogramById("SEARCH_COUNTS");
if (details.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
// code paths because they want to record the search
// in SEARCH_COUNTS.
if (["urlbar", "searchbar"].includes(source)) {
histogram.add(countIdSource);
PartnerLinkAttribution.makeSearchEngineRequest(
engine,
details.url
).catch(Cu.reportError);
if (details.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
// code paths because they want to record the search
// in SEARCH_COUNTS.
if (["urlbar", "searchbar"].includes(source)) {
histogram.add(countIdSource);
PartnerLinkAttribution.makeSearchEngineRequest(
engine,
details.url
).catch(Cu.reportError);
return;
}
console.trace("Unknown source for one-off search: ", source);
return;
}
throw new Error("Unknown source for one-off search: " + source);
}
} else {
if (!KNOWN_SEARCH_SOURCES.includes(source)) {
throw new Error("Unknown source for search: " + source);
}
if (
details.alias &&
engine.isAppProvided &&
engine.aliases.includes(details.alias)
) {
// This is a keyword search using an AppProvided engine.
// Record the source as "alias", not "urlbar".
histogram.add(countIdPrefix + "alias");
} else {
histogram.add(countIdSource);
if (!KNOWN_SEARCH_SOURCES.includes(source)) {
console.trace("Unknown source for search: ", source);
return;
}
if (
details.alias &&
engine.isAppProvided &&
engine.aliases.includes(details.alias)
) {
// This is a keyword search using an AppProvided engine.
// Record the source as "alias", not "urlbar".
histogram.add(countIdPrefix + "alias");
} else {
histogram.add(countIdSource);
}
}
}
// Dispatch the search signal to other handlers.
this._handleSearchAction(engine, source, details);
// Dispatch the search signal to other handlers.
this._handleSearchAction(engine, source, details);
} catch (ex) {
// Catch any errors here, so that search actions are not broken if
// telemetry is broken for some reason.
console.error(ex);
}
}
_recordSearch(engine, url, source, action = null) {

View File

@ -10,6 +10,7 @@ const { XPCOMUtils } = ChromeUtils.import(
"resource://gre/modules/XPCOMUtils.jsm"
);
XPCOMUtils.defineLazyModuleGetters(this, {
BrowserSearchTelemetry: "resource:///modules/BrowserSearchTelemetry.jsm",
clearTimeout: "resource://gre/modules/Timer.jsm",
PrivateBrowsingUtils: "resource://gre/modules/PrivateBrowsingUtils.jsm",
SearchUIUtils: "resource:///modules/SearchUIUtils.jsm",
@ -1073,11 +1074,10 @@ class SearchOneOffs {
source += "-" + this.telemetryOrigin;
}
this.window.BrowserSearch.recordOneoffSearchInTelemetry(
engine,
source,
type
);
BrowserSearchTelemetry.recordSearch(this.window.gBrowser, engine, source, {
type,
isOneOff: true,
});
return true;
}

View File

@ -380,7 +380,10 @@
if (!aEngine) {
aEngine = this.currentEngine;
}
BrowserSearch.recordOneoffSearchInTelemetry(aEngine, source, type);
BrowserSearchTelemetry.recordSearch(gBrowser, aEngine, source, {
type,
isOneOff: true,
});
}
}
@ -440,7 +443,12 @@
selection: telemetrySearchDetails,
url: submission.uri,
};
BrowserSearch.recordSearchInTelemetry(engine, "searchbar", details);
BrowserSearchTelemetry.recordSearch(
gBrowser,
engine,
"searchbar",
details
);
// null parameter below specifies HTML response for search
let params = {
postData: submission.postData,

View File

@ -2202,7 +2202,8 @@ class UrlbarInput {
details.isOneOff = isOneOff;
details.type = eventType;
this.window.BrowserSearch.recordSearchInTelemetry(
BrowserSearchTelemetry.recordSearch(
this.window.gBrowser,
engine,
// Without checking !isOneOff, we might record the string
// oneoff_urlbar-searchmode in the SEARCH_COUNTS probe (in addition to

View File

@ -12,9 +12,7 @@ Search telemetry
================
This module exposes the ``recordSearch`` method, which serves as the main entry point for recording search related Telemetry. It records only the search *counts* per engine and the origin of the search, but nothing pertaining the search contents themselves.
As the transition to the ``BrowserUsageTelemetry`` happens, the ``recordSearch`` calls are dispatched through `BrowserSearch.recordSearchInTelemetry <https://searchfox.org/mozilla-central/rev/1a973762afcbc5066f73f1508b0c846872fe3952/browser/base/content/browser.js#4498>`_, that is called by all the search related UI components (urlbar, searchbar, context menu and about\:\* pages).
A list of the components recording search Telemetry can be found using the following `Searchfox search <https://searchfox.org/mozilla-central/search?q=recordSearchInTelemetry>`_.
A list of the components recording search Telemetry can be found using the following `Searchfox search <https://searchfox.org/mozilla-central/search?q=recordSearch>`_.
Tab and window interactions
===========================