Skip to content

Commit

Permalink
Bug 1870138 - Add remote settings support for search overrides. r=Sta…
Browse files Browse the repository at this point in the history
…ndard8

Differential Revision: https://phabricator.services.mozilla.com/D196490
  • Loading branch information
mkaply committed Jan 5, 2024
1 parent e093365 commit 05f593b
Show file tree
Hide file tree
Showing 21 changed files with 650 additions and 10 deletions.
42 changes: 42 additions & 0 deletions browser/components/search/BrowserSearchTelemetry.sys.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,17 @@ ChromeUtils.defineESModuleGetters(lazy, {
UrlbarSearchUtils: "resource:///modules/UrlbarSearchUtils.sys.mjs",
});

// `contextId` is a unique identifier used by Contextual Services
const CONTEXT_ID_PREF = "browser.contextual-services.contextId";
ChromeUtils.defineLazyGetter(lazy, "contextId", () => {
let _contextId = Services.prefs.getStringPref(CONTEXT_ID_PREF, null);
if (!_contextId) {
_contextId = Services.uuid.generateUUID().toString();
Services.prefs.setStringPref(CONTEXT_ID_PREF, _contextId);
}
return _contextId;
});

// A map of known search origins.
// The keys of this map are used in the calling code to recordSearch, and in
// the SEARCH_COUNTS histogram.
Expand Down Expand Up @@ -171,6 +182,10 @@ class BrowserSearchTelemetryHandler {
* @throws if source is not in the known sources list.
*/
recordSearch(browser, engine, source, details = {}) {
if (engine.clickUrl) {
this.#reportSearchInGlean(engine.clickUrl);
}

try {
if (!this.shouldRecordSearchCount(browser)) {
return;
Expand Down Expand Up @@ -281,6 +296,33 @@ class BrowserSearchTelemetryHandler {
}
);
}

/**
* Records the search in Glean for contextual services.
*
* @param {string} reportingUrl
* The url to be sent to contextual services.
*/
#reportSearchInGlean(reportingUrl) {
let defaultValuesByGleanKey = {
contextId: lazy.contextId,
};

let sendGleanPing = valuesByGleanKey => {
valuesByGleanKey = { ...defaultValuesByGleanKey, ...valuesByGleanKey };
for (let [gleanKey, value] of Object.entries(valuesByGleanKey)) {
let glean = Glean.searchWith[gleanKey];
if (value !== undefined && value !== "") {
glean.set(value);
}
}
GleanPings.searchWith.submit();
};

sendGleanPing({
reportingUrl,
});
}
}

export var BrowserSearchTelemetry = new BrowserSearchTelemetryHandler();
35 changes: 35 additions & 0 deletions browser/components/search/metrics.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -318,3 +318,38 @@ serp:
notification_emails:
- [email protected]
expires: never

search_with:
reporting_url:
type: url
description: >
The external url to report this interaction to.
bugs:
- https://bugzilla.mozilla.org/show_bug.cgi?id=1870138
data_reviews:
- https://bugzilla.mozilla.org/show_bug.cgi?id=1870138
data_sensitivity:
- web_activity
notification_emails:
- [email protected]
expires: never
send_in_pings:
- search-with

context_id:
type: uuid
description: >
An identifier for Contextual Services user interaction pings. This is
used internally for counting unique users as well as for anti-fraud. It
is shared with other Contextual Services. It is not shared externally.
bugs:
- https://bugzilla.mozilla.org/show_bug.cgi?id=1870138
data_reviews:
- https://bugzilla.mozilla.org/show_bug.cgi?id=1870138#c3
data_sensitivity:
- technical
notification_emails:
- [email protected]
expires: never
send_in_pings:
- search-with
22 changes: 22 additions & 0 deletions browser/components/search/pings.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
# 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/.

---
$schema: moz://mozilla.org/schemas/glean/pings/2-0-0

search-with:
description: |
A ping representing a "This time, search with" event with a partner search.
Does not contain a `client_id`, preferring a `context_id` instead.
The `context_id` is used internally for counting unique sers as well as for
anti-fraud. It is shared with other Contextual Services. It is not shared
externally.
include_client_id: false
bugs:
- https://bugzilla.mozilla.org/show_bug.cgi?id=1870138
data_reviews:
- https://bugzilla.mozilla.org/show_bug.cgi?id=1870138
notification_emails:
- [email protected]
2 changes: 2 additions & 0 deletions services/settings/dumps/main/moz.build
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ FINAL_TARGET_FILES.defaults.settings.main += [
"language-dictionaries.json",
"password-recipes.json",
"password-rules.json",
"search-config-overrides-v2.json",
"search-config-overrides.json",
"search-config-v2.json",
"search-config.json",
"search-default-override-allowlist.json",
Expand Down
4 changes: 4 additions & 0 deletions services/settings/dumps/main/search-config-overrides-v2.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
{
"data": [],
"timestamp": 1704379586890
}
4 changes: 4 additions & 0 deletions services/settings/dumps/main/search-config-overrides.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
{
"data": [],
"timestamp": 1704379586619
}
1 change: 1 addition & 0 deletions toolkit/components/glean/metrics_index.py
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,7 @@
firefox_desktop_pings = [
"browser/components/newtab/pings.yaml",
"browser/components/pocket/pings.yaml",
"browser/components/search/pings.yaml",
"browser/components/urlbar/pings.yaml",
"toolkit/components/crashes/pings.yaml",
"toolkit/components/telemetry/pings.yaml",
Expand Down
4 changes: 4 additions & 0 deletions toolkit/components/search/SearchEngine.sys.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -1008,6 +1008,10 @@ export class SearchEngine {
this._urls.push(trending);
}

if (configuration.clickUrl) {
this.clickUrl = configuration.clickUrl;
}

if (details.encoding) {
this._queryCharset = details.encoding;
}
Expand Down
68 changes: 66 additions & 2 deletions toolkit/components/search/SearchEngineSelector.sys.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,13 @@ export class SearchEngineSelector {
*/
constructor(listener) {
this._remoteConfig = lazy.RemoteSettings(lazy.SearchUtils.NEW_SETTINGS_KEY);
this._remoteConfigOverrides = lazy.RemoteSettings(
lazy.SearchUtils.NEW_SETTINGS_OVERRIDES_KEY
);
this._listenerAdded = false;
this._onConfigurationUpdated = this._onConfigurationUpdated.bind(this);
this._onConfigurationOverridesUpdated =
this._onConfigurationOverridesUpdated.bind(this);
this._changeListener = listener;
}

Expand All @@ -41,8 +46,13 @@ export class SearchEngineSelector {
return this._getConfigurationPromise;
}

this._configuration = await (this._getConfigurationPromise =
this._getConfiguration());
this._getConfigurationPromise = Promise.all([
this._getConfiguration(),
this._getConfigurationOverrides(),
]);
let remoteSettingsData = await this._getConfigurationPromise;
this._configuration = remoteSettingsData[0];
this._configurationOverrides = remoteSettingsData[1];
delete this._getConfigurationPromise;

if (!this._configuration?.length) {
Expand All @@ -54,12 +64,24 @@ export class SearchEngineSelector {

if (!this._listenerAdded) {
this._remoteConfig.on("sync", this._onConfigurationUpdated);
this._remoteConfigOverrides.on(
"sync",
this._onConfigurationOverridesUpdated
);
this._listenerAdded = true;
}

return this._configuration;
}

/**
* Used by tests to get the configuration overrides.
*/
async getEngineConfigurationOverrides() {
await this.getEngineConfiguration();
return this._configurationOverrides;
}

/**
* Obtains the configuration from remote settings. This includes
* verifying the signature of the record within the database.
Expand Down Expand Up @@ -119,6 +141,42 @@ export class SearchEngineSelector {
}
}

/**
* Handles updating of the configuration. Note that the search service is
* only updated after a period where the user is observed to be idle.
*
* @param {object} options
* The options object
* @param {object} options.data
* The data to update
* @param {Array} options.data.current
* The new configuration object
*/
_onConfigurationOverridesUpdated({ data: { current } }) {
this._configurationOverrides = current;
lazy.logConsole.debug("Search configuration overrides updated remotely");
if (this._changeListener) {
this._changeListener();
}
}

/**
* Obtains the configuration overrides from remote settings.
*
* @returns {Array}
* An array of objects in the database, or an empty array if none
* could be obtained.
*/
async _getConfigurationOverrides() {
let result = [];
try {
result = await this._remoteConfigOverrides.get();
} catch (ex) {
// This data is remote only, so we just return an empty array if it fails.
}
return result;
}

/**
* @param {object} options
* The options object
Expand Down Expand Up @@ -205,6 +263,12 @@ export class SearchEngineSelector {
engine = this.#deepCopyObject(engine, variant);
}

for (let override of this._configurationOverrides) {
if (override.identifier == engine.identifier) {
engine = this.#deepCopyObject(engine, override);
}
}

engines.push(engine);
}

Expand Down
Loading

0 comments on commit 05f593b

Please sign in to comment.