Skip to content

Commit

Permalink
Bug 1790790 - Add Nimbus flag to control Search Private Default. r=St…
Browse files Browse the repository at this point in the history
…andard8

Differential Revision: https://phabricator.services.mozilla.com/D157605
  • Loading branch information
daleharvey committed Sep 24, 2022
1 parent fbd9515 commit 7c34f31
Show file tree
Hide file tree
Showing 8 changed files with 200 additions and 7 deletions.
4 changes: 4 additions & 0 deletions browser/components/search/test/browser/browser.ini
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,10 @@ support-files =
[browser_contextmenu.js]
[browser_contextmenu_whereToOpenLink.js]
[browser_contextSearchTabPosition.js]
[browser_defaultPrivate_nimbus.js]
support-files =
search-engines/basic/manifest.json
search-engines/private/manifest.json
[browser_google_behavior.js]
skip-if = (os == 'linux') && ccov # Bug 1511273
[browser_healthreport.js]
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,100 @@
/* 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/. */

const { ExperimentAPI, NimbusFeatures } = ChromeUtils.import(
"resource://nimbus/ExperimentAPI.jsm"
);
const { ExperimentFakes } = ChromeUtils.import(
"resource://testing-common/NimbusTestUtils.jsm"
);

ChromeUtils.defineESModuleGetters(this, {
SearchTestUtils: "resource://testing-common/SearchTestUtils.sys.mjs",
});

const CONFIG_DEFAULT = [
{
webExtension: { id: "[email protected]" },
appliesTo: [{ included: { everywhere: true } }],
default: "yes",
},
{
webExtension: { id: "[email protected]" },
appliesTo: [
{
experiment: "testing",
included: { everywhere: true },
},
],
defaultPrivate: "yes",
},
];

SearchTestUtils.init(this);

add_setup(async () => {
// Use engines in test directory
let searchExtensions = getChromeDir(getResolvedURI(gTestPath));
searchExtensions.append("search-engines");
await SearchTestUtils.useMochitestEngines(searchExtensions);

// Current default values.
await SpecialPowers.pushPrefEnv({
set: [
["browser.search.separatePrivateDefault.ui.enabled", false],
["browser.search.separatePrivateDefault.urlbarResult.enabled", false],
["browser.search.separatePrivateDefault", true],
["browser.urlbar.suggest.searches", true],
],
});

SearchTestUtils.useMockIdleService();
await SearchTestUtils.updateRemoteSettingsConfig(CONFIG_DEFAULT);

registerCleanupFunction(async () => {
let settingsWritten = SearchTestUtils.promiseSearchNotification(
"write-settings-to-disk-complete"
);
await SearchTestUtils.updateRemoteSettingsConfig();
await settingsWritten;
});
});

add_task(async function test_nimbus_experiment() {
Assert.equal(
Services.search.defaultPrivateEngine.name,
"basic",
"Should have basic as private default while not in experiment"
);
await ExperimentAPI.ready();

let reloadObserved = SearchTestUtils.promiseSearchNotification(
"engines-reloaded"
);

let doExperimentCleanup = await ExperimentFakes.enrollWithFeatureConfig({
featureId: "search",
value: {
seperatePrivateDefaultUIEnabled: true,
sseperatePrivateDefaultUrlbarResultEnabled: false,
experiment: "testing",
},
});
await reloadObserved;
Assert.equal(
Services.search.defaultPrivateEngine.name,
"private",
"Should have private as private default while in experiment"
);
reloadObserved = SearchTestUtils.promiseSearchNotification(
"engines-reloaded"
);
await doExperimentCleanup();
await reloadObserved;
Assert.equal(
Services.search.defaultPrivateEngine.name,
"basic",
"Should turn off private default and restore default engine after experiment"
);
});
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ add_task(async function test_engines_reloaded_nimbus() {
);
Assert.equal(
getVariableSpy.callCount,
2,
4,
"Called by update function to fetch engines and by ParamPreferenceCache"
);
Assert.ok(
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
{
"name": "basic",
"manifest_version": 2,
"version": "1.0",
"description": "basic",
"applications": {
"gecko": {
"id": "[email protected]"
}
},
"hidden": true,
"chrome_settings_overrides": {
"search_provider": {
"name": "basic",
"search_url": "https://mochi.test:8888/browser/browser/components/search/test/browser/?search={searchTerms}&foo=1",
"suggest_url": "https://mochi.test:8888/browser/browser/modules/test/browser/usageTelemetrySearchSuggestions.sjs?{searchTerms}"
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
{
"name": "private",
"manifest_version": 2,
"version": "1.0",
"description": "A test private engine",
"applications": {
"gecko": {
"id": "[email protected]"
}
},
"hidden": true,
"chrome_settings_overrides": {
"search_provider": {
"name": "private",
"search_url": "https://example.com",
"suggest_url": "https://example.com?search={searchTerms}"
}
}
}
6 changes: 6 additions & 0 deletions toolkit/components/nimbus/FeatureManifest.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,12 @@ search:
extraParams:
type: json
description: Query parameters values for search engine configurations.
seperatePrivateDefaultUIEnabled:
type: boolean
description: Whether the UI for the separate private default feature is enabled.
seperatePrivateDefaultUrlbarResultEnabled:
type: boolean
description: Whether the urlbar result for the separate private default is shown.
urlbar:
description: The Address Bar
owner: [email protected]
Expand Down
49 changes: 43 additions & 6 deletions toolkit/components/search/SearchService.sys.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -1309,6 +1309,14 @@ export class SearchService {
);

lazy.logConsole.debug("Completed #init");

// It is possible that Nimbus could have called onUpdate before
// we started listening, so do a check on startup.
Services.tm.dispatchToMainThread(async () => {
await lazy.NimbusFeatures.search.ready();
this.#nimbusSearchUpdated();
});

return this.#initRV;
}

Expand Down Expand Up @@ -3024,6 +3032,38 @@ export class SearchService {
return policy;
}

#nimbusSearchUpdatedFun = null;

async #nimbusSearchUpdated(event, reason) {
let nimbusPrivateDefaultUIEnabled = lazy.NimbusFeatures.search.getVariable(
"seperatePrivateDefaultUIEnabled"
);
let nimbusPrivateDefaultUrlbarResultEnabled = lazy.NimbusFeatures.search.getVariable(
"seperatePrivateDefaultUrlbarResultEnabled"
);

if (
this._separatePrivateDefaultEnabledPrefValue !=
nimbusPrivateDefaultUIEnabled
) {
Services.prefs.setBoolPref(
`${lazy.SearchUtils.BROWSER_SEARCH_PREF}separatePrivateDefault.ui.enabled`,
nimbusPrivateDefaultUIEnabled
);
}
if (
this.separatePrivateDefaultUrlbarResultEnabled !=
nimbusPrivateDefaultUrlbarResultEnabled
) {
Services.prefs.setBoolPref(
`${lazy.SearchUtils.BROWSER_SEARCH_PREF}separatePrivateDefaultUrlbarResultEnabled`,
nimbusPrivateDefaultUrlbarResultEnabled
);
}

Services.search.wrappedJSObject._maybeReloadEngines();
}

#addObservers() {
if (this.#observersAdded) {
// There might be a race between synchronous and asynchronous
Expand All @@ -3032,9 +3072,8 @@ export class SearchService {
}
this.#observersAdded = true;

lazy.NimbusFeatures.search.onUpdate(() =>
Services.search.wrappedJSObject._maybeReloadEngines()
);
this.#nimbusSearchUpdatedFun = this.#nimbusSearchUpdated.bind(this);
lazy.NimbusFeatures.search.onUpdate(this.#nimbusSearchUpdatedFun);

Services.obs.addObserver(this, lazy.SearchUtils.TOPIC_ENGINE_MODIFIED);
Services.obs.addObserver(this, QUIT_APPLICATION_TOPIC);
Expand Down Expand Up @@ -3097,9 +3136,7 @@ export class SearchService {

this._settings.removeObservers();

lazy.NimbusFeatures.search.off(() =>
Services.search.wrappedJSObject._maybeReloadEngines()
);
lazy.NimbusFeatures.search.off(this.#nimbusSearchUpdatedFun);

Services.obs.removeObserver(this, lazy.SearchUtils.TOPIC_ENGINE_MODIFIED);
Services.obs.removeObserver(this, QUIT_APPLICATION_TOPIC);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,14 @@ async function switchExperiment(newExperiment) {
if (name == "experiment") {
return newExperiment;
}
if (
[
"seperatePrivateDefaultUrlbarResultEnabled",
"seperatePrivateDefaultUIEnabled",
].includes(name)
) {
return true;
}
return undefined;
});
for (let call of NimbusFeatures.search.onUpdate.getCalls()) {
Expand Down

0 comments on commit 7c34f31

Please sign in to comment.