Commit 14c03db3 authored by Mark Hammond's avatar Mark Hammond
Browse files

Bug 1635348 - Migrate browser.storage.sync data from kinto to the new world. r=rpl,lina a=jcristau

Differential Revision: https://phabricator.services.mozilla.com/D76094
parent baceee17
......@@ -36,6 +36,17 @@ XPCOMUtils.defineLazyGetter(this, "storageSvc", () =>
.getInterface(Ci.mozIExtensionStorageArea)
);
// We might end up falling back to kinto...
XPCOMUtils.defineLazyGetter(
this,
"extensionStorageSyncKinto",
() =>
ChromeUtils.import(
"resource://gre/modules/ExtensionStorageSyncKinto.jsm",
{}
).extensionStorageSync
);
// The interfaces which define the callbacks used by the bridge. There's a
// callback for success, failure, and to record data changes.
function ExtensionStorageApiCallback(resolve, reject, extId, changeCallback) {
......@@ -59,15 +70,7 @@ ExtensionStorageApiCallback.prototype = {
let e = new Error(message);
e.code = code;
Cu.reportError(e);
// The only "public" exception here is for quota failure - all others are
// sanitized.
let sanitized =
code == NS_ERROR_DOM_QUOTA_EXCEEDED_ERR
? // The same message as the local IDB implementation
`QuotaExceededError: storage.sync API call exceeded its quota limitations.`
: // The standard, generic extension error.
"An unexpected error occurred";
this.reject(new ExtensionUtils.ExtensionError(sanitized));
this.reject(e);
},
onChanged(json) {
......@@ -85,52 +88,79 @@ ExtensionStorageApiCallback.prototype = {
class ExtensionStorageSync {
constructor() {
this.listeners = new Map();
// We are optimistic :) If we ever see the special nsresult which indicates
// migration failure, it will become false. In practice, this will only ever
// happen on the first operation.
this.migrationOk = true;
}
// The main entry-point to our bridge. It performs some important roles:
// * Ensures the API is allowed to be used.
// * Works out what "extension id" to use.
// * Turns the callback API into a promise API.
async _promisify(fn, extension, ...args) {
async _promisify(fnName, extension, context, ...args) {
let extId = extension.id;
if (prefPermitsStorageSync !== true) {
throw new ExtensionUtils.ExtensionError(
`Please set ${STORAGE_SYNC_ENABLED_PREF} to true in about:config`
);
}
return new Promise((resolve, reject) => {
let callback = new ExtensionStorageApiCallback(
resolve,
reject,
extId,
(extId, changes) => this.notifyListeners(extId, changes)
);
fn(extId, ...args, callback);
});
if (this.migrationOk) {
// We can call ours.
try {
return await new Promise((resolve, reject) => {
let callback = new ExtensionStorageApiCallback(
resolve,
reject,
extId,
(extId, changes) => this.notifyListeners(extId, changes)
);
let sargs = args.map(JSON.stringify);
storageSvc[fnName](extId, ...sargs, callback);
});
} catch (ex) {
if (ex.code != Cr.NS_ERROR_CANNOT_CONVERT_DATA) {
// Some non-migration related error we want to sanitize and propagate.
// The only "public" exception here is for quota failure - all others
// are sanitized.
let sanitized =
ex.code == NS_ERROR_DOM_QUOTA_EXCEEDED_ERR
? // The same message as the local IDB implementation
`QuotaExceededError: storage.sync API call exceeded its quota limitations.`
: // The standard, generic extension error.
"An unexpected error occurred";
throw new ExtensionUtils.ExtensionError(sanitized);
}
// This means "migrate failed" so we must fall back to kinto.
Cu.reportError(
"migration of extension-storage failed - will fall back to kinto"
);
this.migrationOk = false;
}
}
// We've detected failure to migrate, so we want to use kinto.
return extensionStorageSyncKinto[fnName](extension, ...args, context);
}
set(extension, items, context) {
return this._promisify(storageSvc.set, extension, JSON.stringify(items));
return this._promisify("set", extension, context, items);
}
remove(extension, keys, context) {
return this._promisify(storageSvc.remove, extension, JSON.stringify(keys));
return this._promisify("remove", extension, context, keys);
}
clear(extension, context) {
return this._promisify(storageSvc.clear, extension);
return this._promisify("clear", extension, context);
}
get(extension, spec, context) {
return this._promisify(storageSvc.get, extension, JSON.stringify(spec));
return this._promisify("get", extension, context, spec);
}
getBytesInUse(extension, keys, context) {
return this._promisify(
storageSvc.getBytesInUse,
extension,
JSON.stringify(keys)
);
return this._promisify("getBytesInUse", extension, context, keys);
}
addOnChangedListener(extension, listener, context) {
......
......@@ -64,8 +64,9 @@ function StorageSyncService() {
return StorageSyncService._singleton;
}
let file = FileUtils.getFile("ProfD", ["storage-sync2.sqlite"]);
this._storageArea = new StorageSyncArea(file);
let file = FileUtils.getFile("ProfD", ["storage-sync-v2.sqlite"]);
let kintoFile = FileUtils.getFile("ProfD", ["storage-sync.sqlite"]);
this._storageArea = new StorageSyncArea(file, kintoFile);
// Register a blocker to close the storage connection on shutdown.
this._shutdownBound = () => this._shutdown();
......
......@@ -69,7 +69,10 @@ interface mozIConfigurableExtensionStorageArea : mozIExtensionStorageArea {
// `configure` multiple times will throw. `configure` must also be called
// before any of the `mozIExtensionStorageArea` methods, or they'll fail
// with errors.
void configure(in nsIFile databaseFile);
// The second param is the path to the kinto database file from which we
// should migrate. This should always be specified even when there's a
// chance the file doesn't exist.
void configure(in nsIFile databaseFile, in nsIFile kintoFile);
// Tears down the storage area, closing the backing database connection.
// This is called automatically when Firefox shuts down. Once a storage area
......
......@@ -6,7 +6,9 @@ use std::{
cell::{Ref, RefCell},
convert::TryInto,
ffi::OsString,
mem, str,
mem,
path::PathBuf,
str,
sync::Arc,
};
......@@ -28,6 +30,29 @@ use crate::error::{Error, Result};
use crate::punt::{Punt, PuntTask, TeardownTask};
use crate::store::{LazyStore, LazyStoreConfig};
fn path_from_nsifile(file: &nsIFile) -> Result<PathBuf> {
let mut raw_path = nsString::new();
// `nsIFile::GetPath` gives us a UTF-16-encoded version of its
// native path, which we must turn back into a platform-native
// string. We can't use `nsIFile::nativePath()` here because
// it's marked as `nostdcall`, which Rust doesn't support.
unsafe { file.GetPath(&mut *raw_path) }.to_result()?;
let native_path = {
// On Windows, we can create a native string directly from the
// encoded path.
#[cfg(windows)]
{
use std::os::windows::prelude::*;
OsString::from_wide(&*raw_path)
}
// On other platforms, we must first decode the raw path from
// UTF-16, and then create our native string.
#[cfg(not(windows))]
OsString::from(String::from_utf16(&*raw_path)?)
};
Ok(native_path.into())
}
/// An XPCOM component class for the Rust extension storage API. This class
/// implements the interfaces needed for syncing and storage.
///
......@@ -87,32 +112,15 @@ impl StorageSyncArea {
xpcom_method!(
configure => Configure(
database_file: *const nsIFile
database_file: *const nsIFile,
kinto_file: *const nsIFile
)
);
/// Sets up the storage area.
fn configure(&self, database_file: &nsIFile) -> Result<()> {
let mut raw_path = nsString::new();
// `nsIFile::GetPath` gives us a UTF-16-encoded version of its
// native path, which we must turn back into a platform-native
// string. We can't use `nsIFile::nativePath()` here because
// it's marked as `nostdcall`, which Rust doesn't support.
unsafe { database_file.GetPath(&mut *raw_path) }.to_result()?;
let native_path = {
// On Windows, we can create a native string directly from the
// encoded path.
#[cfg(windows)]
{
use std::os::windows::prelude::*;
OsString::from_wide(&*raw_path)
}
// On other platforms, we must first decode the raw path from
// UTF-16, and then create our native string.
#[cfg(not(windows))]
OsString::from(String::from_utf16(&*raw_path)?)
};
fn configure(&self, database_file: &nsIFile, kinto_file: &nsIFile) -> Result<()> {
self.store()?.configure(LazyStoreConfig {
path: native_path.into(),
path: path_from_nsifile(database_file)?,
kinto_path: path_from_nsifile(kinto_file)?,
})?;
Ok(())
}
......
......@@ -6,8 +6,9 @@ use std::{error, fmt, result, str::Utf8Error, string::FromUtf16Error};
use golden_gate::Error as GoldenGateError;
use nserror::{
nsresult, NS_ERROR_ALREADY_INITIALIZED, NS_ERROR_DOM_QUOTA_EXCEEDED_ERR, NS_ERROR_FAILURE,
NS_ERROR_INVALID_ARG, NS_ERROR_NOT_IMPLEMENTED, NS_ERROR_NOT_INITIALIZED, NS_ERROR_UNEXPECTED,
nsresult, NS_ERROR_ALREADY_INITIALIZED, NS_ERROR_CANNOT_CONVERT_DATA,
NS_ERROR_DOM_QUOTA_EXCEEDED_ERR, NS_ERROR_FAILURE, NS_ERROR_INVALID_ARG,
NS_ERROR_NOT_IMPLEMENTED, NS_ERROR_NOT_INITIALIZED, NS_ERROR_UNEXPECTED,
};
use serde_json::error::Error as JsonError;
use webext_storage::error::Error as WebextStorageError;
......@@ -23,6 +24,7 @@ pub type Result<T> = result::Result<T, Error>;
pub enum Error {
Nsresult(nsresult),
WebextStorage(WebextStorageError),
MigrationFailed(WebextStorageError),
GoldenGate(GoldenGateError),
MalformedString(Box<dyn error::Error + Send + Sync + 'static>),
AlreadyConfigured,
......@@ -86,6 +88,7 @@ impl From<Error> for nsresult {
WebextStorageErrorKind::QuotaError(_) => NS_ERROR_DOM_QUOTA_EXCEEDED_ERR,
_ => NS_ERROR_FAILURE,
},
Error::MigrationFailed(_) => NS_ERROR_CANNOT_CONVERT_DATA,
Error::GoldenGate(error) => error.into(),
Error::MalformedString(_) => NS_ERROR_INVALID_ARG,
Error::AlreadyConfigured => NS_ERROR_ALREADY_INITIALIZED,
......@@ -103,6 +106,7 @@ impl fmt::Display for Error {
match self {
Error::Nsresult(result) => write!(f, "Operation failed with {}", result),
Error::WebextStorage(error) => error.fmt(f),
Error::MigrationFailed(error) => write!(f, "Migration failed with {}", error),
Error::GoldenGate(error) => error.fmt(f),
Error::MalformedString(error) => error.fmt(f),
Error::AlreadyConfigured => write!(f, "The storage area is already configured"),
......
......@@ -3,6 +3,7 @@
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */
use std::{
fs::remove_file,
mem,
path::PathBuf,
sync::{Mutex, MutexGuard},
......@@ -19,6 +20,11 @@ use crate::error::{Error, Result};
pub struct LazyStoreConfig {
/// The path to the database file for this storage area.
pub path: PathBuf,
/// The path to the old kinto database. If it exists, we should attempt to
/// migrate from this database as soon as we open our DB. It's not Option<>
/// because the caller will not have checked whether it exists or not, so
/// will assume it might.
pub kinto_path: PathBuf,
}
/// A lazy store is automatically initialized on a background thread with its
......@@ -68,7 +74,7 @@ impl LazyStore {
.store
.get_or_try_init(|| match self.config.get() {
Some(config) => {
let store = Store::new(&config.path)?;
let store = init_store(config)?;
let handle = store.interrupt_handle();
Ok(InterruptStore {
inner: Mutex::new(store),
......@@ -189,3 +195,51 @@ impl BridgedEngine for LazyStore {
Ok(self.get()?.bridged_engine().wipe()?)
}
}
// Initialize the store, performing a migration if necessary.
// The requirements for migration are, roughly:
// * If kinto_path doesn't exist, we don't try to migrate.
// * If our DB path exists, we assume we've already migrated and don't try again
// * If the migration fails, we close our store and delete the DB, then return
// a special error code which tells our caller about the failure. It's then
// expected to fallback to the "old" kinto store and we'll try next time.
// Note that the migrate() method on the store is written such that is should
// ignore all "read" errors from the source, but propagate "write" errors on our
// DB - the intention is that things like corrupted source databases never fail,
// but disk-space failures on our database does.
fn init_store(config: &LazyStoreConfig) -> Result<Store> {
let should_migrate = config.kinto_path.exists() && !config.path.exists();
let store = Store::new(&config.path)?;
if should_migrate {
match store.migrate(&config.kinto_path) {
Ok(num) => {
// need logging, but for now let's print to stdout.
println!("extension-storage: migrated {} records", num);
Ok(store)
}
Err(e) => {
println!("extension-storage: migration failure: {}", e);
if let Err((store, e)) = store.close() {
// welp, this probably isn't going to end well...
println!(
"extension-storage: failed to close the store after migration failure: {}",
e
);
// I don't think we should hit this in this case - I guess we
// could sleep and retry if we thought we were.
mem::drop(store);
}
if let Err(e) = remove_file(&config.path) {
// this is bad - if it happens regularly it will defeat
// out entire migration strategy - we'll assume it
// worked.
// So it's desirable to make noise if this happens.
println!("Failed to remove file after failed migration: {}", e);
}
Err(Error::MigrationFailed(e))
}
}
} else {
Ok(store)
}
}
/* -*- Mode: indent-tabs-mode: nil; js-indent-level: 2 -*- */
/* vim: set sts=2 sw=2 et tw=80: */
"use strict";
// Import the rust-based and kinto-based implementations
const { extensionStorageSync: rustImpl } = ChromeUtils.import(
"resource://gre/modules/ExtensionStorageSync.jsm"
);
const { extensionStorageSync: kintoImpl } = ChromeUtils.import(
"resource://gre/modules/ExtensionStorageSyncKinto.jsm"
);
Services.prefs.setBoolPref("webextensions.storage.sync.kinto", false);
add_task(async function test_sync_migration() {
// There's no good reason to perform this test via test extensions - we just
// call the underlying APIs directly.
// Set some stuff using the kinto-based impl.
let e1 = { id: "test@mozilla.com" };
let c1 = { extension: e1, callOnClose() {} };
await kintoImpl.set(e1, { foo: "bar" }, c1);
let e2 = { id: "test-2@mozilla.com" };
let c2 = { extension: e2, callOnClose() {} };
await kintoImpl.set(e2, { second: "2nd" }, c2);
let e3 = { id: "test-3@mozilla.com" };
let c3 = { extension: e3, callOnClose() {} };
// And all the data should be magically migrated.
Assert.deepEqual(await rustImpl.get(e1, "foo", c1), { foo: "bar" });
Assert.deepEqual(await rustImpl.get(e2, null, c2), { second: "2nd" });
// Sanity check we really are doing what we think we are - set a value in our
// new one, it should not be reflected by kinto.
await rustImpl.set(e3, { third: "3rd" }, c3);
Assert.deepEqual(await rustImpl.get(e3, null, c3), { third: "3rd" });
Assert.deepEqual(await kintoImpl.get(e3, null, c3), {});
// cleanup.
await kintoImpl.clear(e1, c1);
await kintoImpl.clear(e2, c2);
await kintoImpl.clear(e3, c3);
await rustImpl.clear(e1, c1);
await rustImpl.clear(e2, c2);
await rustImpl.clear(e3, c3);
});
// It would be great to have failure tests, but that seems impossible to have
// in automated tests given the conditions under which we migrate - it would
// basically require us to arrange for zero free disk space or to somehow
// arrange for sqlite to see an io error. Specially crafted "corrupt"
// sqlite files doesn't help because that file must not exist for us to even
// attempt migration.
//
// But - what we can test is that if .migratedOk on the new impl ever goes to
// false we delegate correctly.
add_task(async function test_sync_migration_delgates() {
let e1 = { id: "test@mozilla.com" };
let c1 = { extension: e1, callOnClose() {} };
await kintoImpl.set(e1, { foo: "bar" }, c1);
// We think migration went OK - `get` shouldn't see kinto.
Assert.deepEqual(rustImpl.get(e1, null, c1), {});
info(
"Setting migration failure flag to ensure we delegate to kinto implementation"
);
rustImpl.migrationOk = false;
// get should now be seeing kinto.
Assert.deepEqual(await rustImpl.get(e1, null, c1), { foo: "bar" });
// check everything else delegates.
await rustImpl.set(e1, { foo: "foo" }, c1);
Assert.deepEqual(await kintoImpl.get(e1, null, c1), { foo: "foo" });
Assert.equal(await rustImpl.getBytesInUse(e1, null, c1), 8);
await rustImpl.remove(e1, "foo", c1);
Assert.deepEqual(await kintoImpl.get(e1, null, c1), {});
await rustImpl.set(e1, { foo: "foo" }, c1);
Assert.deepEqual(await kintoImpl.get(e1, null, c1), { foo: "foo" });
await rustImpl.clear(e1, c1);
Assert.deepEqual(await kintoImpl.get(e1, null, c1), {});
});
......@@ -33,6 +33,8 @@ prefs =
# For tests which rely on conetn pages, and should only run with remote content
# but in-process extensions.
[test_ExtensionStorageSync_migration_kinto.js]
skip-if = os == 'android' # Not shipped on Android
[test_MatchPattern.js]
[test_StorageSyncService.js]
skip-if = os == 'android' && processor == 'x86_64'
......
Supports Markdown
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment