Bug 1929307: Import prefs from SharedPrefs at startup rather than clobbering them. r=jhirsch,niklas

This also stops sharing browser.profiles.enabled and toolkit.profiles.storeID. In the first case
I want to minimise any risk of this preference getting set to false by accident because then a
user loses all of their profiles. In the latter case there is no point storing it in the database,
we already have a backup mechanism for it and if we have lost the storeID we can't load the database
anyway.

Differential Revision: https://phabricator.services.mozilla.com/D228174
This commit is contained in:
Dave Townsend 2024-11-12 15:16:46 +00:00
parent f478854a4b
commit 0b8abf4e93
6 changed files with 366 additions and 189 deletions

View File

@ -110,15 +110,33 @@ class SelectableProfileServiceClass {
];
#initPromise = null;
#notifyTask = null;
#observedPrefs = null;
static #dirSvc = null;
// The initial preferences that will be shared amongst profiles. Only used during database
// creation, after that the set in the database is used.
static initialSharedPrefs = ["toolkit.telemetry.cachedProfileGroupID"];
// Preferences that were previously shared but should now be ignored.
static ignoredSharedPrefs = [
"browser.profiles.enabled",
"toolkit.profiles.storeID",
];
// Preferences that need to be set in newly created profiles.
static profileInitialPrefs = [
"browser.profiles.enabled",
"toolkit.profiles.storeID",
];
constructor() {
this.themeObserver = this.themeObserver.bind(this);
this.prefObserver = (subject, topic, prefName) =>
this.flushSharedPrefToDatabase(prefName);
this.#profileService = Cc[
"@mozilla.org/toolkit/profile-service;1"
].getService(Ci.nsIToolkitProfileService);
this.#asyncShutdownBlocker = () => this.uninit();
this.#observedPrefs = new Set();
}
get isEnabled() {
@ -337,8 +355,6 @@ class SelectableProfileServiceClass {
);
} catch {}
this.setSharedPrefs();
// The 'activate' event listeners use #currentProfile, so this line has
// to come after #currentProfile has been set.
this.initWindowTracker();
@ -349,6 +365,13 @@ class SelectableProfileServiceClass {
);
this.#initialized = true;
// this.#currentProfile is unset in the case that the database has only just been created. We
// don't need to import from the database in this case.
if (this.#currentProfile) {
// Assume that settings in the database may have changed while we weren't running.
await this.databaseChanged("startup");
}
}
async uninit() {
@ -369,6 +392,11 @@ class SelectableProfileServiceClass {
);
} catch (e) {}
for (let prefName of this.#observedPrefs) {
Services.prefs.removeObserver(prefName, this.prefObserver);
}
this.#observedPrefs.clear();
// During shutdown we don't need to notify ourselves, just other instances
// so rather than finalizing the task just disarm it and do the notification
// manually.
@ -472,22 +500,36 @@ class SelectableProfileServiceClass {
}
/**
* Set the shared prefs that will be needed when creating a
* new selectable profile.
* Flushes the value of a preference to the database.
*
* @param {string} prefName the name of the preference.
*/
setSharedPrefs() {
this.setPref(
"toolkit.profiles.storeID",
Services.prefs.getStringPref("toolkit.profiles.storeID", "")
);
this.setPref(
"browser.profiles.enabled",
Services.prefs.getBoolPref("browser.profiles.enabled", true)
);
this.setPref(
"toolkit.telemetry.cachedProfileGroupID",
Services.prefs.getStringPref("toolkit.telemetry.cachedProfileGroupID", "")
);
async flushSharedPrefToDatabase(prefName) {
if (!this.#observedPrefs.has(prefName)) {
Services.prefs.addObserver(prefName, this.prefObserver);
this.#observedPrefs.add(prefName);
}
if (!Services.prefs.prefHasUserValue(prefName)) {
await this.#deleteDBPref(prefName);
return;
}
let value;
switch (Services.prefs.getPrefType(prefName)) {
case Ci.nsIPrefBranch.PREF_BOOL:
value = Services.prefs.getBoolPref(prefName);
break;
case Ci.nsIPrefBranch.PREF_INT:
value = Services.prefs.getIntPref(prefName);
break;
case Ci.nsIPrefBranch.PREF_STRING:
value = Services.prefs.getCharPref(prefName);
break;
}
await this.#setDBPref(prefName, value);
}
/**
@ -615,16 +657,20 @@ class SelectableProfileServiceClass {
* Invoked when changes have been made to the database. Sends the observer
* notification "sps-profiles-updated" indicating that something has changed.
*
* @param {"local"|"remote"} source The source of the notification. Either
* "local" meaning that the change was made in this process or "remote"
* meaning the change was made by a different Firefox instance.
* @param {"local"|"remote"|"startup"} source The source of the notification.
* Either "local" meaning that the change was made in this process, "remote"
* meaning the change was made by a different Firefox instance or "startup"
* meaning the application has just launched and we may need to reload
* changes from the database.
*/
async databaseChanged(source) {
if (source == "remote") {
await this.refreshPrefs();
if (source != "local") {
await this.loadSharedPrefsFromDatabase();
}
Services.obs.notifyObservers(null, "sps-profiles-updated", source);
if (source != "startup") {
Services.obs.notifyObservers(null, "sps-profiles-updated", source);
}
}
/**
@ -677,19 +723,40 @@ class SelectableProfileServiceClass {
/**
* Fetch all prefs from the DB and write to the current instance.
*/
async refreshPrefs() {
for (let { name, value, type } of await this.getAllPrefs()) {
switch (type) {
case "boolean":
Services.prefs.setBoolPref(name, value);
break;
case "string":
Services.prefs.setCharPref(name, value);
break;
case "number":
Services.prefs.setIntPref(name, value);
break;
async loadSharedPrefsFromDatabase() {
// This stops us from observing the change during the load and means we stop observing any prefs
// no longer in the database.
for (let prefName of this.#observedPrefs) {
Services.prefs.removeObserver(prefName, this.prefObserver);
}
this.#observedPrefs.clear();
for (let { name, value, type } of await this.getAllDBPrefs()) {
if (SelectableProfileServiceClass.ignoredSharedPrefs.includes(name)) {
continue;
}
if (value === null) {
Services.prefs.clearUserPref(name);
} else {
switch (type) {
case "boolean":
Services.prefs.setBoolPref(name, value);
break;
case "string":
Services.prefs.setCharPref(name, value);
break;
case "number":
Services.prefs.setIntPref(name, value);
break;
case "null":
Services.prefs.clearUserPref(name);
break;
}
}
Services.prefs.addObserver(name, this.prefObserver);
this.#observedPrefs.add(name);
}
}
@ -793,14 +860,14 @@ class SelectableProfileServiceClass {
let prefsJsFilePath = await IOUtils.createUniqueFile(
profileDir.path,
"prefs.js",
0o700
0o600
);
const sharedPrefs = await this.getAllPrefs();
const sharedPrefs = await this.getAllDBPrefs();
const LINEBREAK = AppConstants.platform === "win" ? "\r\n" : "\n";
const prefsJsHeader = [
const prefsJs = [
"// Mozilla User Preferences",
LINEBREAK,
"// DO NOT EDIT THIS FILE.",
@ -812,19 +879,33 @@ class SelectableProfileServiceClass {
"// - modify it via the UI (e.g. via about:config in the browser); or",
"// - set it within a user.js file in your profile.",
LINEBREAK,
'user_pref("browser.profiles.profile-name.updated", false);',
];
const prefsJsContent = sharedPrefs.map(
pref =>
for (let pref of sharedPrefs) {
prefsJs.push(
`user_pref("${pref.name}", ${
pref.type === "string" ? `"${pref.value}"` : `${pref.value}`
});`
);
);
}
const prefsJs = prefsJsHeader.concat(
prefsJsContent,
'user_pref("browser.profiles.profile-name.updated", false);'
);
for (let prefName of SelectableProfileServiceClass.profileInitialPrefs) {
let value;
switch (Services.prefs.getPrefType(prefName)) {
case Ci.nsIPrefBranch.PREF_STRING:
value = `"${Services.prefs.getCharPref(prefName)}"`;
break;
case Ci.nsIPrefBranch.PREF_BOOL:
value = Services.prefs.getBoolPref(prefName);
break;
case Ci.nsIPrefBranch.PREF_INT:
value = Services.prefs.getIntPref(prefName);
break;
}
prefsJs.push(`user_pref("${prefName}", ${value});`);
}
await IOUtils.writeUTF8(prefsJsFilePath, prefsJs.join(LINEBREAK));
}
@ -899,15 +980,23 @@ class SelectableProfileServiceClass {
* be added to the datastore.
*/
async maybeSetupDataStore() {
if (this.#connection) {
return;
}
// Create the profiles db and set the storeID on the toolkit profile if it
// doesn't exist so we can init the service.
await this.maybeCreateProfilesStorePath();
await this.init();
// Flush our shared prefs into the database.
for (let prefName of SelectableProfileServiceClass.initialSharedPrefs) {
await this.flushSharedPrefToDatabase(prefName);
}
// If this is the first time the user has created a selectable profile,
// add the current toolkit profile to the datastore.
let profiles = await this.getAllProfiles();
if (!profiles.length) {
if (!this.#currentProfile) {
let path = this.#profileService.currentProfile.rootDir;
this.#currentProfile = await this.#createProfile(path);
}
@ -1154,7 +1243,7 @@ class SelectableProfileServiceClass {
*
* @returns {{name: string, value: *, type: string}}
*/
async getAllPrefs() {
async getAllDBPrefs() {
return (
await this.#connection.executeCached("SELECT * FROM SharedPrefs;")
).map(row => {
@ -1168,80 +1257,34 @@ class SelectableProfileServiceClass {
}
/**
* Get the value of a specific shared pref.
* Get the value of a specific shared pref from the database.
*
* @param {string} aPrefName The name of the pref to get
*
* @returns {any} Value of the pref
*/
async getPref(aPrefName) {
let row = (
await this.#connection.execute(
"SELECT value, isBoolean FROM SharedPrefs WHERE name = :name;",
{
name: aPrefName,
}
)
)[0];
async getDBPref(aPrefName) {
let rows = await this.#connection.execute(
"SELECT value, isBoolean FROM SharedPrefs WHERE name = :name;",
{
name: aPrefName,
}
);
return this.getPrefValueFromRow(row);
}
/**
* Get the value of a specific shared pref.
*
* @param {string} aPrefName The name of the pref to get
*
* @returns {boolean} Value of the pref
*/
async getBoolPref(aPrefName) {
let prefValue = await this.getPref(aPrefName);
if (typeof prefValue !== "boolean") {
return null;
if (!rows.length) {
throw new Error(`Unknown preference '${aPrefName}'`);
}
return prefValue;
return this.getPrefValueFromRow(rows[0]);
}
/**
* Get the value of a specific shared pref.
*
* @param {string} aPrefName The name of the pref to get
*
* @returns {number} Value of the pref
*/
async getIntPref(aPrefName) {
let prefValue = await this.getPref(aPrefName);
if (typeof prefValue !== "number") {
return null;
}
return prefValue;
}
/**
* Get the value of a specific shared pref.
*
* @param {string} aPrefName The name of the pref to get
*
* @returns {string} Value of the pref
*/
async getStringPref(aPrefName) {
let prefValue = await this.getPref(aPrefName);
if (typeof prefValue !== "string") {
return null;
}
return prefValue;
}
/**
* Insert or update a pref value, then notify() other running instances.
* Insert or update a pref value in the database, then notify() other running instances.
*
* @param {string} aPrefName The name of the pref
* @param {any} aPrefValue The value of the pref
*/
async setPref(aPrefName, aPrefValue) {
async #setDBPref(aPrefName, aPrefValue) {
await this.#connection.execute(
"INSERT INTO SharedPrefs(id, name, value, isBoolean) VALUES (NULL, :name, :value, :isBoolean) ON CONFLICT(name) DO UPDATE SET value=excluded.value, isBoolean=excluded.isBoolean;",
{
@ -1254,53 +1297,21 @@ class SelectableProfileServiceClass {
this.#notifyTask.arm();
}
/**
* Insert or update a pref value, then notify() other running instances.
*
* @param {string} aPrefName The name of the pref
* @param {boolean} aPrefValue The value of the pref
*/
async setBoolPref(aPrefName, aPrefValue) {
if (typeof aPrefValue !== "boolean") {
throw new Error("aPrefValue must be of type boolean");
}
await this.setPref(aPrefName, aPrefValue);
// Starts tracking a new shared pref across the profiles.
async trackPref(aPrefName) {
await this.flushSharedPrefToDatabase(aPrefName);
}
/**
* Insert or update a pref value, then notify() other running instances.
*
* @param {string} aPrefName The name of the pref
* @param {number} aPrefValue The value of the pref
*/
async setIntPref(aPrefName, aPrefValue) {
if (typeof aPrefValue !== "number") {
throw new Error("aPrefValue must be of type number");
}
await this.setPref(aPrefName, aPrefValue);
}
/**
* Insert or update a pref value, then notify() other running instances.
*
* @param {string} aPrefName The name of the pref
* @param {string} aPrefValue The value of the pref
*/
async setStringPref(aPrefName, aPrefValue) {
if (typeof aPrefValue !== "string") {
throw new Error("aPrefValue must be of type string");
}
await this.setPref(aPrefName, aPrefValue);
}
/**
* Remove a shared pref, then notify() other running instances.
* Remove a shared pref from the database, then notify() other running instances.
*
* @param {string} aPrefName The name of the pref to delete
*/
async deletePref(aPrefName) {
await this.#connection.execute(
"DELETE FROM SharedPrefs WHERE name = :name;",
async #deleteDBPref(aPrefName) {
// We mark the value as null if it already exists in the database so other profiles know what
// preference to remove.
await this.#connection.executeCached(
"UPDATE SharedPrefs SET value=NULL, isBoolean=FALSE WHERE name=:name;",
{
name: aPrefName,
}
@ -1342,7 +1353,9 @@ export class CommandLineHandler {
handle(cmdLine) {
// This is only ever sent when the application is already running.
if (cmdLine.handleFlag(COMMAND_LINE_UPDATE, true)) {
SelectableProfileService.databaseChanged("remote").catch(console.error);
if (SelectableProfileService.initialized) {
SelectableProfileService.databaseChanged("remote").catch(console.error);
}
cmdLine.preventDefault = true;
return;
}

View File

@ -81,9 +81,8 @@ add_task(async function test_selector_window() {
// Setting shared prefs should notify.
notifications = [];
await SelectableProfileService.setPref("test.pref1", "hello");
await SelectableProfileService.setPref("test.pref2", 5);
await SelectableProfileService.setPref("test.pref3", true);
Services.prefs.setCharPref("test.pref1", "hello");
await SelectableProfileService.trackPref("test.pref1");
await TestUtils.waitForCondition(() => notifications.length >= 2);
@ -92,11 +91,6 @@ add_task(async function test_selector_window() {
// Notifications should be debounced
Assert.equal(notifications.length, 2);
// Should have actually set the prefs.
Assert.equal(Services.prefs.getCharPref("test.pref1", null), "hello");
Assert.equal(Services.prefs.getIntPref("test.pref2", 0), 5);
Assert.equal(Services.prefs.getBoolPref("test.pref3", false), true);
// Properly simulate a set from another instance.
notifications = [];
await connection.execute(

View File

@ -7,6 +7,9 @@
const { SelectableProfile } = ChromeUtils.importESModule(
"resource:///modules/profiles/SelectableProfile.sys.mjs"
);
const { Sqlite } = ChromeUtils.importESModule(
"resource://gre/modules/Sqlite.sys.mjs"
);
const lazy = {};
@ -65,6 +68,16 @@ function getRelativeProfilePath(path) {
return relativePath;
}
async function openDatabase() {
let dbFile = Services.dirsvc.get("UAppData", Ci.nsIFile);
dbFile.append("Profile Groups");
dbFile.append(`${getProfileService().currentProfile.storeID}.sqlite`);
return Sqlite.openConnection({
path: dbFile.path,
openNotExclusive: true,
});
}
async function createTestProfile(profileData = {}) {
const SelectableProfileService = getSelectableProfileService();

View File

@ -3,10 +3,6 @@ https://creativecommons.org/publicdomain/zero/1.0/ */
"use strict";
const { Sqlite } = ChromeUtils.importESModule(
"resource://gre/modules/Sqlite.sys.mjs"
);
add_task(async function test_recover_storeID() {
startProfileService();
Services.prefs.setCharPref("toolkit.profiles.storeID", "foobar");

View File

@ -100,6 +100,38 @@ add_task(async function test_SelectableProfileLifecycle() {
"Profile local dir was successfully created"
);
let times = PathUtils.join(rootDir.path, "times.json");
Assert.ok(await IOUtils.exists(times), "times.json should exist");
let json = await IOUtils.readJSON(times);
Assert.ok(
json.created <= Date.now() && json.created >= Date.now() - 30000,
"Should have been created roughly now."
);
let prefs = PathUtils.join(rootDir.path, "prefs.js");
Assert.ok(await IOUtils.exists(prefs), "prefs.js should exist");
let contents = (await IOUtils.readUTF8(prefs)).split("\n");
let sawStoreID = false;
let sawEnabled = false;
for (let line of contents) {
if (line == `user_pref("browser.profiles.enabled", true);`) {
sawEnabled = true;
}
if (
line ==
`user_pref("toolkit.profiles.storeID", "${
getProfileService().currentProfile.storeID
}");`
) {
sawStoreID = true;
}
}
Assert.ok(sawStoreID, "Should have seen the store ID defined in prefs.js");
Assert.ok(sawEnabled, "Should have seen the service enabled in prefs.js");
profiles = await SelectableProfileService.getAllProfiles();
Assert.equal(profiles.length, 2, "Should now be two profiles.");

View File

@ -3,66 +3,195 @@ https://creativecommons.org/publicdomain/zero/1.0/ */
"use strict";
add_setup(initSelectableProfileService);
add_setup(async () => {
startProfileService();
const SelectableProfileService = getSelectableProfileService();
await SelectableProfileService.init();
Services.prefs.setIntPref("testPrefInt0", 5);
Services.prefs.setBoolPref("testBoolPref", true);
Services.prefs.setCharPref("testCharPref", "hello");
SelectableProfileService.constructor.initialSharedPrefs.splice(
0,
SelectableProfileService.constructor.initialSharedPrefs.length,
"testPrefInt0",
"testBoolPref",
"testCharPref"
);
await SelectableProfileService.maybeSetupDataStore();
});
// Waits for the profile service to update about a change
async function updateNotified() {
let { resolve, promise } = Promise.withResolvers();
let observer = (subject, topic, data) => {
Services.obs.removeObserver(observer, "sps-profiles-updated");
resolve(data);
};
Services.obs.addObserver(observer, "sps-profiles-updated");
await promise;
}
add_task(async function test_SharedPrefsLifecycle() {
const SelectableProfileService = getSelectableProfileService();
let prefs = await SelectableProfileService.getAllPrefs();
Assert.equal(prefs.length, 3, "The default shared prefs exist");
await SelectableProfileService.setIntPref("testPrefInt0", 0);
await SelectableProfileService.setIntPref("testPrefInt1", 1);
await SelectableProfileService.setPref("testPrefInt2", 2);
await SelectableProfileService.setStringPref(
"testPrefString0",
"Hello world!"
);
await SelectableProfileService.setPref("testPrefString1", "Hello world 2!");
await SelectableProfileService.setBoolPref("testPrefBoolTrue", true);
await SelectableProfileService.setPref("testPrefBoolFalse", false);
prefs = await SelectableProfileService.getAllPrefs();
Assert.equal(prefs.length, 10, "10 shared prefs exist");
let prefs = await SelectableProfileService.getAllDBPrefs();
Assert.equal(
await SelectableProfileService.getIntPref("testPrefInt0"),
prefs.length,
3,
"Shoulds have stored the default prefs into the database."
);
Assert.equal(
prefs.find(p => p.name == "testPrefInt0")?.value,
5,
"testPrefInt0 should be correct"
);
Assert.equal(
prefs.find(p => p.name == "testBoolPref")?.value,
true,
"testBoolPref should be correct"
);
Assert.equal(
prefs.find(p => p.name == "testCharPref")?.value,
"hello",
"testCharPref should be correct"
);
Services.prefs.setIntPref("testPrefInt0", 2);
await updateNotified();
Services.prefs.setBoolPref("testBoolPref", false);
await updateNotified();
Services.prefs.setCharPref("testCharPref", "goodbye");
await updateNotified();
prefs = await SelectableProfileService.getAllDBPrefs();
Assert.equal(
prefs.length,
3,
"Shoulds have stored the default prefs into the database."
);
Assert.equal(
prefs.find(p => p.name == "testPrefInt0")?.value,
2,
"testPrefInt0 should be correct"
);
Assert.equal(
prefs.find(p => p.name == "testBoolPref")?.value,
false,
"testBoolPref should be correct"
);
Assert.equal(
prefs.find(p => p.name == "testCharPref")?.value,
"goodbye",
"testCharPref should be correct"
);
Services.prefs.setIntPref("testPrefInt0", 0);
Services.prefs.setIntPref("testPrefInt1", 1);
await SelectableProfileService.trackPref("testPrefInt1");
Services.prefs.setIntPref("testPrefInt2", 2);
await SelectableProfileService.trackPref("testPrefInt2");
// Notifications are deferred so we should only see one.
await updateNotified();
await Services.prefs.setCharPref("testPrefString0", "Hello world!");
await SelectableProfileService.trackPref("testPrefString0");
await Services.prefs.setCharPref("testPrefString1", "Hello world 2!");
await SelectableProfileService.trackPref("testPrefString1");
await Services.prefs.setBoolPref("testPrefBoolTrue", true);
await SelectableProfileService.trackPref("testPrefBoolTrue");
await Services.prefs.setBoolPref("testPrefBoolFalse", false);
await SelectableProfileService.trackPref("testPrefBoolFalse");
await updateNotified();
prefs = await SelectableProfileService.getAllDBPrefs();
Assert.equal(prefs.length, 9, "The right number of shared prefs exist");
Assert.equal(
await SelectableProfileService.getDBPref("testPrefInt0"),
0,
"testPrefInt0 value is 0"
);
Assert.equal(
await SelectableProfileService.getIntPref("testPrefInt1"),
await SelectableProfileService.getDBPref("testPrefInt1"),
1,
"testPrefInt1 value is 1"
);
Assert.equal(
await SelectableProfileService.getPref("testPrefInt2"),
await SelectableProfileService.getDBPref("testPrefInt2"),
2,
"testPrefInt2 value is 2"
);
Assert.equal(
await SelectableProfileService.getStringPref("testPrefString0"),
await SelectableProfileService.getDBPref("testPrefString0"),
"Hello world!",
'testPrefString0 value is "Hello world!"'
);
Assert.equal(
await SelectableProfileService.getPref("testPrefString1"),
await SelectableProfileService.getDBPref("testPrefString1"),
"Hello world 2!",
'testPrefString1 value is "Hello world 2!"'
);
Assert.equal(
await SelectableProfileService.getBoolPref("testPrefBoolTrue"),
await SelectableProfileService.getDBPref("testPrefBoolTrue"),
true,
"testPrefBoolTrue value is true"
);
Assert.equal(
await SelectableProfileService.getPref("testPrefBoolFalse"),
await SelectableProfileService.getDBPref("testPrefBoolFalse"),
false,
"testPrefBoolFalse value is false"
);
await SelectableProfileService.uninit();
// Make some changes to the database while the service is shutdown.
let db = await openDatabase();
await db.execute(
"UPDATE SharedPrefs SET value=NULL, isBoolean=FALSE WHERE name=:name;",
{ name: "testPrefInt0" }
);
await db.execute(
"UPDATE SharedPrefs SET value=6, isBoolean=FALSE WHERE name=:name;",
{ name: "testPrefInt1" }
);
await db.execute(
"UPDATE SharedPrefs SET value=FALSE, isBoolean=TRUE WHERE name=:name;",
{ name: "testPrefBoolTrue" }
);
await db.close();
await SelectableProfileService.init();
Assert.equal(
Services.prefs.getPrefType("testPrefInt0"),
Ci.nsIPrefBranch.PREF_INVALID,
"Should have cleared the testPrefInt0 pref"
);
Assert.equal(
Services.prefs.getIntPref("testPrefInt1"),
6,
"Should have updated testPrefInt1"
);
Assert.equal(
Services.prefs.getBoolPref("testPrefBoolTrue"),
false,
"Should have updated testPrefBoolTrue"
);
await SelectableProfileService.deleteProfileGroup();
});