Bug 1444965 - Fix search composition migration for beta and nightly. r?mak,mythmon
MozReview-Commit-ID: IpQdSQV7YF4
--- a/browser/components/nsBrowserGlue.js
+++ b/browser/components/nsBrowserGlue.js
@@ -104,16 +104,17 @@ XPCOMUtils.defineLazyModuleGetters(this,
Integration: "resource://gre/modules/Integration.jsm",
L10nRegistry: "resource://gre/modules/L10nRegistry.jsm",
LanguagePrompt: "resource://gre/modules/LanguagePrompt.jsm",
LightweightThemeManager: "resource://gre/modules/LightweightThemeManager.jsm",
LoginHelper: "resource://gre/modules/LoginHelper.jsm",
LoginManagerParent: "resource://gre/modules/LoginManagerParent.jsm",
NewTabUtils: "resource://gre/modules/NewTabUtils.jsm",
Normandy: "resource://normandy/Normandy.jsm",
+ ObjectUtils: "resource://gre/modules/ObjectUtils.jsm",
OS: "resource://gre/modules/osfile.jsm",
PageActions: "resource:///modules/PageActions.jsm",
PageThumbs: "resource://gre/modules/PageThumbs.jsm",
PdfJs: "resource://pdf.js/PdfJs.jsm",
PermissionUI: "resource:///modules/PermissionUI.jsm",
PingCentre: "resource:///modules/PingCentre.jsm",
PlacesBackups: "resource://gre/modules/PlacesBackups.jsm",
PlacesUtils: "resource://gre/modules/PlacesUtils.jsm",
@@ -478,18 +479,21 @@ BrowserGlue.prototype = {
} else if (data == "mock-alerts-service") {
Object.defineProperty(this, "AlertsService", {
value: subject.wrappedJSObject
});
} else if (data == "places-browser-init-complete") {
if (this._placesBrowserInitComplete) {
Services.obs.notifyObservers(null, "places-browser-init-complete");
}
- } else if (data == "migrateMatchBucketsPrefForUIVersion60") {
- this._migrateMatchBucketsPrefForUIVersion60(true);
+ } else if (data == "migrateMatchBucketsPrefForUI66") {
+ this._migrateMatchBucketsPrefForUI66().then(() => {
+ Services.obs.notifyObservers(null, "browser-glue-test",
+ "migrateMatchBucketsPrefForUI66-done");
+ });
}
break;
case "initial-migration-will-import-default-bookmarks":
this._migrationImportsDefaultBookmarks = true;
break;
case "initial-migration-did-import-default-bookmarks":
this._initPlaces(true);
break;
@@ -562,16 +566,17 @@ BrowserGlue.prototype = {
// extension is installed the init method below will be overridden
// leaving initialization to the extension.
// parent only: configure default prefs, set up pref observers, register
// pdf content handler, and initializes parent side message manager
// shim for privileged api access.
PdfJs.init(true);
break;
case "shield-init-complete":
+ this._shieldInitComplete = true;
this._sendMainPingCentrePing();
break;
}
},
// initialization (called on application startup)
_init: function BG__init() {
let os = Services.obs;
@@ -1828,17 +1833,17 @@ BrowserGlue.prototype = {
if (toolbarIsCustomized || getToolbarFolderCount() > NUM_TOOLBAR_BOOKMARKS_TO_UNHIDE) {
xulStore.setValue(BROWSER_DOCURL, "PersonalToolbar", "collapsed", "false");
}
}
},
// eslint-disable-next-line complexity
_migrateUI: function BG__migrateUI() {
- const UI_VERSION = 65;
+ const UI_VERSION = 66;
const BROWSER_DOCURL = "chrome://browser/content/browser.xul";
let currentUIVersion;
if (Services.prefs.prefHasUserValue("browser.migration.version")) {
currentUIVersion = Services.prefs.getIntPref("browser.migration.version");
} else {
// This is a new profile, nothing to migrate.
Services.prefs.setIntPref("browser.migration.version", UI_VERSION);
@@ -2119,19 +2124,17 @@ BrowserGlue.prototype = {
}
}
Services.prefs.clearUserPref(SELECTED_LOCALE_PREF);
Services.prefs.clearUserPref(MATCHOS_LOCALE_PREF);
}
}
if (currentUIVersion < 60) {
- // Set whether search suggestions or history results come first in the
- // urlbar results.
- this._migrateMatchBucketsPrefForUIVersion60();
+ // This version is superseded by version 66. See bug 1444965.
}
if (currentUIVersion < 61) {
// Remove persisted toolbarset from navigator toolbox
xulStore.removeValue(BROWSER_DOCURL, "navigator-toolbox", "toolbarset");
}
if (currentUIVersion < 62) {
@@ -2173,16 +2176,22 @@ BrowserGlue.prototype = {
let engine = Services.search.getEngineByName(engineName);
if (engine) {
Services.search.removeEngine(engine);
}
}
});
}
+ if (currentUIVersion < 66) {
+ // Set whether search suggestions or history/bookmarks results come first
+ // in the urlbar results, and uninstall a related Shield study.
+ this._migrateMatchBucketsPrefForUI66();
+ }
+
// Update the migration version.
Services.prefs.setIntPref("browser.migration.version", UI_VERSION);
},
_checkForDefaultBrowser() {
// Perform default browser checking.
if (!ShellService) {
return;
@@ -2254,54 +2263,95 @@ BrowserGlue.prototype = {
.add(promptCount);
} catch (ex) { /* Don't break the default prompt if telemetry is broken. */ }
if (willPrompt) {
DefaultBrowserCheck.prompt(RecentWindow.getMostRecentBrowserWindow());
}
},
- _migrateMatchBucketsPrefForUIVersion60(forceCheck = false) {
- function check() {
- if (CustomizableUI.getPlacementOfWidget("search-container")) {
- Services.prefs.setCharPref(prefName,
- "general:5,suggestion:Infinity");
+ async _migrateMatchBucketsPrefForUI66() {
+ // This does two related things.
+ //
+ // (1) Profiles created on or after Firefox 57's release date were eligible
+ // for a Shield study that changed the browser.urlbar.matchBuckets pref in
+ // order to show search suggestions above history/bookmarks in the urlbar
+ // popup. This uninstalls that study. (It's actually slightly more
+ // complex. The study set the pref to several possible values, but the
+ // overwhelming number of profiles in the study got search suggestions
+ // first, followed by history/bookmarks.)
+ //
+ // (2) This also ensures that (a) new users see search suggestions above
+ // history/bookmarks, thereby effectively making the study permanent, and
+ // (b) old users (including those in the study) continue to see whatever
+ // they were seeing before. This works together with UnifiedComplete.js.
+ // By default, the browser.urlbar.matchBuckets pref does not exist, and
+ // UnifiedComplete.js internally hardcodes a default value for it. Before
+ // Firefox 60, the hardcoded default was to show history/bookmarks first.
+ // After 60, it's to show search suggestions first.
+
+ // Wait for Shield init to complete.
+ await new Promise(resolve => {
+ if (this._shieldInitComplete) {
+ resolve();
+ return;
+ }
+ let topic = "shield-init-complete";
+ Services.obs.addObserver(function obs() {
+ Services.obs.removeObserver(obs, topic);
+ resolve();
+ }, topic);
+ });
+
+ // Now get the pref's value. If the study is active, the value will have
+ // just been set (on the default branch) as part of Shield's init. The pref
+ // should not exist otherwise (normally).
+ let prefName = "browser.urlbar.matchBuckets";
+ let prefValue = Services.prefs.getCharPref(prefName, "");
+
+ // Get the study (aka experiment). It may not be installed.
+ let experiment = null;
+ let experimentName = "pref-flip-search-composition-57-release-1413565";
+ let {PreferenceExperiments} =
+ ChromeUtils.import("resource://normandy/lib/PreferenceExperiments.jsm", {});
+ try {
+ experiment = await PreferenceExperiments.get(experimentName);
+ } catch (e) {}
+
+ // Uninstall the study, resetting the pref to its state before the study.
+ if (experiment && !experiment.expired) {
+ await PreferenceExperiments.stop(experimentName, {
+ resetValue: true,
+ reason: "external:search-ui-migration",
+ });
+ }
+
+ // At this point, normally the pref should not exist. If it does, then it
+ // either has a user value, or something unexpectedly set its value on the
+ // default branch. Either way, preserve that value.
+ if (Services.prefs.getCharPref(prefName, "")) {
+ return;
+ }
+
+ // The new default is "suggestion:4,general:5" (show search suggestions
+ // before history/bookmarks), but we implement that by leaving the pref
+ // undefined, and UnifiedComplete.js hardcodes that value internally. So if
+ // the pref was "suggestion:4,general:5" (modulo whitespace), we're done.
+ if (prefValue) {
+ let buckets = PlacesUtils.convertMatchBucketsStringToArray(prefValue);
+ if (ObjectUtils.deepEqual(buckets, [["suggestion", 4], ["general", 5]])) {
+ return;
}
}
- let prefName = "browser.urlbar.matchBuckets";
- let pref = Services.prefs.getCharPref(prefName, "");
- if (!pref) {
- // Set the pref based on the search bar's current placement. If it's
- // placed (the urlbar and search bar are not unified), then set the pref
- // (so that history results will come before search suggestions). If it's
- // not placed (the urlbar and search bar are unified), then leave the pref
- // cleared so that UnifiedComplete.js uses the default value (so that
- // search suggestions will come before history results).
- if (forceCheck) {
- // This is the case when this is called by the test.
- check();
- } else {
- // This is the normal, non-test case. At this point the first window
- // has not been set up yet, so use a CUI listener to get the placement
- // when the nav-bar is first registered.
- let listener = {
- onAreaNodeRegistered(area, container) {
- if (CustomizableUI.AREA_NAVBAR == area) {
- check();
- CustomizableUI.removeListener(listener);
- }
- },
- };
- CustomizableUI.addListener(listener);
- }
- }
- // Else, the pref has already been set. Normally this pref does not exist.
- // Either the user customized it, or they were enrolled in the Shield study
- // in Firefox 57 that effectively already migrated the pref. Either way,
- // leave it at its current value.
+
+ // Set the pref on the user branch. If the pref had a value, then preserve
+ // it. Otherwise, set the previous default value, which was to show history
+ // and bookmarks before search suggestions.
+ prefValue = prefValue || "general:5,suggestion:Infinity";
+ Services.prefs.setCharPref(prefName, prefValue);
},
async ensurePlacesDefaultQueriesInitialized() {
// This is the current smart bookmarks version, it must be increased every
// time they change.
// When adding a new smart bookmark below, its newInVersion property must
// be set to the version it has been added in. We will compare its value
// to users' smartBookmarksVersion and add new smart bookmarks without
--- a/browser/components/tests/browser/browser.ini
+++ b/browser/components/tests/browser/browser.ini
@@ -1,8 +1,8 @@
[DEFAULT]
[browser_bug538331.js]
skip-if = !updater
reason = test depends on update channel
[browser_contentpermissionprompt.js]
[browser_default_bookmark_toolbar_visibility.js]
-[browser_urlbar_matchBuckets_migration59.js]
+[browser_urlbar_matchBuckets_migration60.js]
deleted file mode 100644
--- a/browser/components/tests/browser/browser_urlbar_matchBuckets_migration59.js
+++ /dev/null
@@ -1,100 +0,0 @@
-/* Any copyright is dedicated to the Public Domain.
- * http://creativecommons.org/publicdomain/zero/1.0/ */
-
-// Makes sure the browser.urlbar.matchBuckets pref is set correctly starting in
-// Firefox 59 (nsBrowserGlue UI version 60).
-
-const SEARCHBAR_WIDGET_ID = "search-container";
-const PREF_NAME = "browser.urlbar.matchBuckets";
-const SEARCHBAR_PRESENT_PREF_VALUE = "general:5,suggestion:Infinity";
-
-add_task(async function test() {
- // Initial checks.
- Assert.equal(CustomizableUI.getPlacementOfWidget(SEARCHBAR_WIDGET_ID), null,
- "Searchbar should not be placed initially");
- Assert.equal(Services.prefs.getCharPref(PREF_NAME, ""), "",
- "Pref should be cleared initially");
-
- // Add the searchbar.
- let widgetPromise = promiseWidget("onWidgetAdded");
- CustomizableUI.addWidgetToArea(SEARCHBAR_WIDGET_ID,
- CustomizableUI.AREA_NAVBAR);
- info("Waiting for searchbar to be added");
- await widgetPromise;
-
- // Force nsBrowserGlue to attempt update the pref again via UI version
- // migration. It shouldn't actually though since the UI version has already
- // been migrated. If it erroneously does, then the matchBuckets pref will be
- // set since the searchbar is now placed.
- messageBrowserGlue("force-ui-migration");
-
- // The pref should remain cleared since the migration already happened even
- // though the searchbar is now present.
- Assert.equal(Services.prefs.getCharPref(PREF_NAME, ""), "",
- "Pref should remain cleared even though searchbar present");
-
- // Force nsBrowserGlue to update the pref.
- forceBrowserGlueUpdatePref();
-
- // The pref should be set since the searchbar is present.
- Assert.equal(Services.prefs.getCharPref(PREF_NAME, ""),
- SEARCHBAR_PRESENT_PREF_VALUE,
- "Pref should be set to show history first");
-
- // Set the pref to something custom.
- let customValue = "test:Infinity";
- Services.prefs.setCharPref(PREF_NAME, customValue);
-
- // Force nsBrowserGlue to update the pref again.
- forceBrowserGlueUpdatePref();
-
- // The pref should remain the custom value.
- Assert.equal(Services.prefs.getCharPref(PREF_NAME, ""), customValue,
- "Pref should remain the custom value");
-
- // Remove the searchbar.
- widgetPromise = promiseWidget("onWidgetRemoved");
- CustomizableUI.removeWidgetFromArea(SEARCHBAR_WIDGET_ID);
- info("Waiting for searchbar to be removed");
- await widgetPromise;
-
- // Force nsBrowserGlue to update the pref again.
- forceBrowserGlueUpdatePref();
-
- // The pref should remain the custom value.
- Assert.equal(Services.prefs.getCharPref(PREF_NAME, ""), customValue,
- "Pref should remain the custom value");
-
- // Clear the pref.
- Services.prefs.clearUserPref(PREF_NAME);
-
- // Force nsBrowserGlue to update the pref again.
- forceBrowserGlueUpdatePref();
-
- // The pref should remain cleared since the searchbar isn't placed.
- Assert.equal(Services.prefs.getCharPref(PREF_NAME, ""), "",
- "Pref should remain cleared");
-});
-
-function promiseWidget(observerName) {
- return new Promise(resolve => {
- let listener = {};
- listener[observerName] = widgetID => {
- if (widgetID == SEARCHBAR_WIDGET_ID) {
- CustomizableUI.removeListener(listener);
- executeSoon(resolve);
- }
- };
- CustomizableUI.addListener(listener);
- });
-}
-
-function messageBrowserGlue(msgName) {
- Cc["@mozilla.org/browser/browserglue;1"]
- .getService(Ci.nsIObserver)
- .observe(null, "browser-glue-test", msgName);
-}
-
-function forceBrowserGlueUpdatePref() {
- messageBrowserGlue("migrateMatchBucketsPrefForUIVersion60");
-}
new file mode 100644
--- /dev/null
+++ b/browser/components/tests/browser/browser_urlbar_matchBuckets_migration60.js
@@ -0,0 +1,228 @@
+/* Any copyright is dedicated to the Public Domain.
+ * http://creativecommons.org/publicdomain/zero/1.0/ */
+
+// Makes sure the browser.urlbar.matchBuckets pref is set correctly starting in
+// Firefox 60, nsBrowserGlue UI version 66.
+
+const PREF_NAME = "browser.urlbar.matchBuckets";
+const PREF_VALUE_SUGGESTIONS_FIRST = "suggestion:4,general:5";
+const PREF_VALUE_GENERAL_FIRST = "general:5,suggestion:Infinity";
+const STUDY_NAME = "pref-flip-search-composition-57-release-1413565";
+
+ChromeUtils.import("resource://normandy/lib/PreferenceExperiments.jsm", this);
+
+
+// Migrates without doing anything else. The pref should be set to show history
+// first.
+add_task(async function migrate() {
+ await sanityCheckInitialState();
+
+ // Trigger migration. The pref is cleared initially, so after migration it
+ // should be set on the user branch to show history first.
+ await promiseMigration();
+ Assert.equal(Services.prefs.getCharPref(PREF_NAME, ""),
+ PREF_VALUE_GENERAL_FIRST,
+ "Pref should be set, general first");
+ Assert.ok(Services.prefs.prefHasUserValue(PREF_NAME),
+ "Pref should be set on user branch");
+
+ Services.prefs.clearUserPref(PREF_NAME);
+});
+
+
+// Sets the pref to a value on the user branch and migrates. The pref shouldn't
+// change.
+add_task(async function setUserPrefAndMigrate() {
+ await sanityCheckInitialState();
+
+ // Set a value for the pref on the user branch.
+ let userValue = "userTest:10";
+ Services.prefs.setCharPref(PREF_NAME, userValue);
+
+ // Trigger migration. The pref should be preserved.
+ await promiseMigration();
+ Assert.equal(Services.prefs.getCharPref(PREF_NAME, ""),
+ userValue,
+ "Pref should remain same");
+ Assert.ok(Services.prefs.prefHasUserValue(PREF_NAME),
+ "Pref should remain on user branch");
+
+ Services.prefs.clearUserPref(PREF_NAME);
+});
+
+
+// Sets the pref to a value on the default branch and migrates. The pref
+// shouldn't change.
+add_task(async function setDefaultPrefAndMigrate() {
+ await sanityCheckInitialState();
+
+ // Set a value for the pref on the default branch.
+ let defaultValue = "defaultTest:10";
+ Services.prefs.getDefaultBranch(PREF_NAME).setCharPref("", defaultValue);
+
+ // Trigger migration. The pref should be preserved.
+ await promiseMigration();
+ Assert.equal(Services.prefs.getCharPref(PREF_NAME, ""),
+ defaultValue,
+ "Pref should remain same");
+ Assert.ok(!Services.prefs.prefHasUserValue(PREF_NAME),
+ "Pref should remain on default branch");
+
+ Services.prefs.deleteBranch(PREF_NAME);
+});
+
+
+// Installs the study using the pref that the overwhelming majority of users
+// will see ("ratio": 97, "value": "suggestion:4,general:5") and migrates. The
+// study should be stopped and the pref should remain cleared.
+add_task(async function installStudyAndMigrate() {
+ await sanityCheckInitialState();
+
+ // Install the study. It should set the pref.
+ await PreferenceExperiments.start(newExperimentOpts());
+ Assert.ok(await PreferenceExperiments.has(STUDY_NAME),
+ "Study installed");
+ Assert.equal(Services.prefs.getCharPref(PREF_NAME, ""),
+ PREF_VALUE_SUGGESTIONS_FIRST,
+ "Pref should be set by study");
+
+ // Trigger migration. The study should be stopped, and the pref should be
+ // cleared since it's the default value.
+ await promiseMigration();
+ Assert.ok(!(await getNonExpiredExperiment()), "Study stopped");
+ Assert.equal(Services.prefs.getCharPref(PREF_NAME, ""), "",
+ "Pref should be cleared");
+
+ await PreferenceExperiments.clearAllExperimentStorage();
+});
+
+
+// Installs the study using the pref that the overwhelming majority of users
+// will see ("ratio": 97, "value": "suggestion:4,general:5"), except that the
+// pref has unnecessary spaces in it, and then migrates. The study should be
+// stopped and the pref should remain cleared. i.e., the migration code should
+// parse the pref value and compare the resulting buckets instead of comparing
+// strings directly.
+add_task(async function installStudyPrefWithSpacesAndMigrate() {
+ await sanityCheckInitialState();
+
+ // Install the study. It should set the pref.
+ let preferenceValue = " suggestion : 4, general : 5 ";
+ await PreferenceExperiments.start(newExperimentOpts({ preferenceValue }));
+ Assert.ok(await PreferenceExperiments.has(STUDY_NAME),
+ "Study installed");
+ Assert.equal(Services.prefs.getCharPref(PREF_NAME, ""), preferenceValue,
+ "Pref should be set by study");
+
+ // Trigger migration. The study should be stopped, and the pref should be
+ // cleared since it's the default value.
+ await promiseMigration();
+ Assert.ok(!(await getNonExpiredExperiment()), "Study stopped");
+ Assert.equal(Services.prefs.getCharPref(PREF_NAME, ""), "",
+ "Pref should be cleared");
+
+ await PreferenceExperiments.clearAllExperimentStorage();
+});
+
+
+// Installs the study using a pref that a tiny number of users will see
+// ("ratio": 1, "value": "general:3,suggestion:6") and migrates. The study
+// should be stopped and the pref should be preserved since it's not the new
+// default.
+add_task(async function installStudyMinorityPrefAndMigrate() {
+ await sanityCheckInitialState();
+
+ // Install the study. It should set the pref.
+ let preferenceValue = "general:3,suggestion:6";
+ await PreferenceExperiments.start(newExperimentOpts({ preferenceValue }));
+ Assert.ok(await PreferenceExperiments.has(STUDY_NAME),
+ "Study installed");
+ Assert.equal(Services.prefs.getCharPref(PREF_NAME, ""), preferenceValue,
+ "Pref should be set by study");
+
+ // Trigger migration. The study should be stopped, and the pref should remain
+ // the same since it's a non-default value. It should be set on the user
+ // branch.
+ await promiseMigration();
+ Assert.ok(!(await getNonExpiredExperiment()), "Study stopped");
+ Assert.equal(Services.prefs.getCharPref(PREF_NAME, ""),
+ preferenceValue,
+ "Pref should remain the same");
+ Assert.ok(Services.prefs.prefHasUserValue(PREF_NAME),
+ "Pref should be set on user branch");
+
+ await PreferenceExperiments.clearAllExperimentStorage();
+ Services.prefs.clearUserPref(PREF_NAME);
+});
+
+
+// Sets the pref to some value on the default branch, installs the study, and
+// migrates. The study should be stopped and the original pref value should be
+// restored.
+add_task(async function setDefaultPrefInstallStudyAndMigrate() {
+ await sanityCheckInitialState();
+
+ // First, set the pref to some value on the default branch. (If the pref is
+ // set on the user branch, starting the study actually throws because the
+ // study's branch is the default branch.)
+ let defaultValue = "test:10";
+ Services.prefs.getDefaultBranch(PREF_NAME).setCharPref("", defaultValue);
+
+ // Install the study. It should set the pref.
+ await PreferenceExperiments.start(newExperimentOpts());
+ Assert.ok(await PreferenceExperiments.has(STUDY_NAME),
+ "Study installed");
+ Assert.equal(Services.prefs.getCharPref(PREF_NAME, ""),
+ PREF_VALUE_SUGGESTIONS_FIRST,
+ "Pref should be set by study");
+
+ // Trigger migration. The study should be stopped, and the pref should be
+ // restored to the value set above.
+ await promiseMigration();
+ Assert.ok(!(await getNonExpiredExperiment()), "Study stopped");
+ Assert.equal(Services.prefs.getCharPref(PREF_NAME, ""), defaultValue,
+ "Pref should be restored to user value");
+
+ await PreferenceExperiments.clearAllExperimentStorage();
+ Services.prefs.deleteBranch(PREF_NAME);
+});
+
+
+async function sanityCheckInitialState() {
+ Assert.equal(Services.prefs.getCharPref(PREF_NAME, ""), "",
+ "Pref should be cleared initially");
+ Assert.ok(!(await PreferenceExperiments.has(STUDY_NAME)),
+ "Study should not be installed initially");
+}
+
+function promiseMigration() {
+ let topic = "browser-glue-test";
+ let donePromise = TestUtils.topicObserved(topic, (subj, data) => {
+ return "migrateMatchBucketsPrefForUI66-done" == data;
+ });
+ Cc["@mozilla.org/browser/browserglue;1"]
+ .getService(Ci.nsIObserver)
+ .observe(null, topic, "migrateMatchBucketsPrefForUI66");
+ return donePromise;
+}
+
+function newExperimentOpts(opts) {
+ return Object.assign({
+ name: STUDY_NAME,
+ branch: "branch",
+ preferenceName: PREF_NAME,
+ preferenceValue: PREF_VALUE_SUGGESTIONS_FIRST,
+ preferenceBranchType: "default",
+ preferenceType: "string",
+ }, opts);
+}
+
+async function getNonExpiredExperiment() {
+ try {
+ let exp = await PreferenceExperiments.get(STUDY_NAME);
+ if (exp.expired) {
+ return null;
+ }
+ } catch (ex) {}
+ return null;
+}