Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[DO NOT MERGE] Expose webext-storage bridged engine logic #6180

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 7 additions & 2 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,11 @@
### Suggest
- Removed the deprecated `remote_settings_config` method. No consumers were using this.

### Webext-Storage
- Exposed the bridged engine logic for use in desktop ([#6180](https://github.com/mozilla/application-services/pull/6180)).
- This updates the signature of the `bridged_engine` function technically making this a breaking change though the
references to function with the old signature are being removed in a desktop patch.

## ✨ What's New ✨

### Suggest
Expand Down Expand Up @@ -4260,7 +4265,7 @@ call `PushManager.unsubscribeAll()`.

### What's Fixed

- Megazords and requests should work again. ([#946](https://github.com/mozilla/application-services/pull/946))
- Megazords and requests should work again. ([#946](https://github.com/mozilla/application-services/pull/946))
- The vestigial `reqwest` backend is no longer compiled into the megazords ([#937](https://github.com/mozilla/application-services/pull/937)).
- Note that prior to this it was present, but unused.

Expand Down Expand Up @@ -4740,7 +4745,7 @@ and working around subtle build issues.

- In most cases, opaque integer handles are now used to pass data over the FFI ([#567](https://github.com/mozilla/application-services/issues/567)). This should be more robust, and allow detection of many types of errors that would previously cause silent memory corruption.

This should be mostly transparent, but is a semi-breaking semantic change in the case that something throws an exception indicating that the Rust code panicked (which should only occur due to bugs anyway). If this occurs, all subsequent operations on that object (except `close`/`lock`) will cause errors. It is "poisoned", in Rust terminology. (In the future, this may be handled automatically)
This should be mostly transparent, but is a semi-breaking semantic change in the case that something throws an exception indicating that the Rust code panicked (which should only occur due to bugs anyway). If this occurs, all subsequent operations on that object (except `close`/`lock`) will cause errors. It is "poisoned", in Rust terminology. (In the future, this may be handled automatically)

This may seem inconvenient, but it should be an improvement over the previous version, where we instead would simply carry on despite potentially having corrupted internal state.

Expand Down
1 change: 1 addition & 0 deletions components/webext-storage/src/db.rs
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,7 @@ impl ThreadSafeStorageDb {
Arc::clone(&self.interrupt_handle)
}

#[allow(dead_code)]
pub fn begin_interrupt_scope(&self) -> Result<SqlInterruptScope> {
Ok(self.interrupt_handle.begin_interrupt_scope()?)
}
Expand Down
8 changes: 8 additions & 0 deletions components/webext-storage/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -143,3 +143,11 @@ impl From<serde_json::Error> for WebExtStorageApiError {
}
}
}

impl From<anyhow::Error> for WebExtStorageApiError {
fn from(value: anyhow::Error) -> Self {
WebExtStorageApiError::UnexpectedError {
reason: value.to_string(),
}
}
}
15 changes: 15 additions & 0 deletions components/webext-storage/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ pub use api::SYNC_QUOTA_BYTES_PER_ITEM;

pub use crate::error::{QuotaReason, WebExtStorageApiError};
pub use crate::store::WebExtStorageStore;
pub use crate::sync::{bridge::WebExtStorageBridgedEngine, SyncedExtensionChange};
pub use api::UsageInfo;
pub use api::{StorageChanges, StorageValueChange};

Expand All @@ -42,3 +43,17 @@ impl UniffiCustomTypeConverter for JsonValue {
obj.to_string()
}
}

// Our UDL uses a `Guid` type.
lougeniaC64 marked this conversation as resolved.
Show resolved Hide resolved
use sync_guid::Guid;
impl UniffiCustomTypeConverter for Guid {
type Builtin = String;

fn into_custom(val: Self::Builtin) -> uniffi::Result<Guid> {
Ok(Guid::new(val.as_str()))
}

fn from_custom(obj: Self) -> Self::Builtin {
obj.into()
}
}
11 changes: 3 additions & 8 deletions components/webext-storage/src/store.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ use serde_json::Value as JsonValue;
/// connection with our sync engines - ie, these engines also hold an Arc<>
/// around the same object.
pub struct WebExtStorageStore {
db: Arc<ThreadSafeStorageDb>,
pub(crate) db: Arc<ThreadSafeStorageDb>,
}

impl WebExtStorageStore {
Expand Down Expand Up @@ -119,14 +119,9 @@ impl WebExtStorageStore {

/// Returns the bytes in use for the specified items (which can be null,
/// a string, or an array)
pub fn get_bytes_in_use(&self, ext_id: &str, keys: JsonValue) -> Result<usize> {
pub fn get_bytes_in_use(&self, ext_id: &str, keys: JsonValue) -> Result<u64> {
let db = self.db.lock();
api::get_bytes_in_use(&db, ext_id, keys)
}

/// Returns a bridged sync engine for Desktop for this store.
pub fn bridged_engine(&self) -> sync::BridgedEngine {
sync::BridgedEngine::new(&self.db)
Ok(api::get_bytes_in_use(&db, ext_id, keys)? as u64)
}

/// Closes the store and its database connection. See the docs for
Expand Down
100 changes: 97 additions & 3 deletions components/webext-storage/src/sync/bridge.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,18 +5,30 @@
use anyhow::Result;
use rusqlite::Transaction;
use std::sync::{Arc, Weak};
use sync15::bso::IncomingBso;
use sync15::engine::ApplyResults;
use sync15::bso::{IncomingBso, OutgoingBso};
use sync15::engine::{ApplyResults, BridgedEngine as Sync15BridgedEngine};
use sync_guid::Guid as SyncGuid;

use crate::db::{delete_meta, get_meta, put_meta, ThreadSafeStorageDb};
use crate::schema;
use crate::sync::incoming::{apply_actions, get_incoming, plan_incoming, stage_incoming};
use crate::sync::outgoing::{get_outgoing, record_uploaded, stage_outgoing};
use crate::WebExtStorageStore;

const LAST_SYNC_META_KEY: &str = "last_sync_time";
const SYNC_ID_META_KEY: &str = "sync_id";

impl WebExtStorageStore {
// Returns a bridged sync engine for this store.
pub fn bridged_engine(self: Arc<Self>) -> Arc<WebExtStorageBridgedEngine> {
let engine = Box::new(BridgedEngine::new(&self.db));
let bridged_engine = WebExtStorageBridgedEngine {
bridge_impl: engine,
};
Arc::new(bridged_engine)
}
}

/// A bridged engine implements all the methods needed to make the
/// `storage.sync` store work with Desktop's Sync implementation.
/// Conceptually, it's similar to `sync15::Store`, which we
Expand Down Expand Up @@ -54,7 +66,7 @@ impl BridgedEngine {
}
}

impl sync15::engine::BridgedEngine for BridgedEngine {
impl Sync15BridgedEngine for BridgedEngine {
fn last_sync(&self) -> Result<i64> {
let shared_db = self.thread_safe_storage_db()?;
let db = shared_db.lock();
Expand Down Expand Up @@ -182,6 +194,88 @@ impl sync15::engine::BridgedEngine for BridgedEngine {
}
}

pub struct WebExtStorageBridgedEngine {
bridge_impl: Box<dyn Sync15BridgedEngine>,
}

impl WebExtStorageBridgedEngine {
pub fn new(bridge_impl: Box<dyn Sync15BridgedEngine>) -> Self {
Self { bridge_impl }
}

pub fn last_sync(&self) -> Result<i64> {
self.bridge_impl.last_sync()
}

pub fn set_last_sync(&self, last_sync: i64) -> Result<()> {
self.bridge_impl.set_last_sync(last_sync)
}

pub fn sync_id(&self) -> Result<Option<String>> {
self.bridge_impl.sync_id()
}

pub fn reset_sync_id(&self) -> Result<String> {
self.bridge_impl.reset_sync_id()
}

pub fn ensure_current_sync_id(&self, sync_id: &str) -> Result<String> {
self.bridge_impl.ensure_current_sync_id(sync_id)
}

pub fn prepare_for_sync(&self, client_data: &str) -> Result<()> {
self.bridge_impl.prepare_for_sync(client_data)
}

pub fn store_incoming(&self, incoming: Vec<String>) -> Result<()> {
self.bridge_impl
.store_incoming(self.convert_incoming_bsos(incoming)?)
}

pub fn apply(&self) -> Result<Vec<String>> {
let apply_results = self.bridge_impl.apply()?;
self.convert_outgoing_bsos(apply_results.records)
}

pub fn set_uploaded(&self, server_modified_millis: i64, guids: Vec<SyncGuid>) -> Result<()> {
self.bridge_impl
.set_uploaded(server_modified_millis, &guids)
}

pub fn sync_started(&self) -> Result<()> {
self.bridge_impl.sync_started()
}

pub fn sync_finished(&self) -> Result<()> {
self.bridge_impl.sync_finished()
}

pub fn reset(&self) -> Result<()> {
self.bridge_impl.reset()
}

pub fn wipe(&self) -> Result<()> {
self.bridge_impl.wipe()
}

fn convert_incoming_bsos(&self, incoming: Vec<String>) -> Result<Vec<IncomingBso>> {
let mut bsos = Vec::with_capacity(incoming.len());
for inc in incoming {
bsos.push(serde_json::from_str::<IncomingBso>(&inc)?);
}
Ok(bsos)
}

// Encode OutgoingBso's into JSON for UniFFI
fn convert_outgoing_bsos(&self, outgoing: Vec<OutgoingBso>) -> Result<Vec<String>> {
let mut bsos = Vec::with_capacity(outgoing.len());
for e in outgoing {
bsos.push(serde_json::to_string(&e)?);
}
Ok(bsos)
}
}

impl From<anyhow::Error> for crate::error::Error {
fn from(value: anyhow::Error) -> Self {
crate::error::Error::SyncError(value.to_string())
Expand Down
3 changes: 1 addition & 2 deletions components/webext-storage/src/sync/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
* 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/. */

mod bridge;
pub(crate) mod bridge;
mod incoming;
mod outgoing;

Expand All @@ -17,7 +17,6 @@ use serde_derive::*;
use sql_support::ConnExt;
use sync_guid::Guid as SyncGuid;

pub use bridge::BridgedEngine;
use incoming::IncomingAction;

type JsonMap = serde_json::Map<String, serde_json::Value>;
Expand Down
63 changes: 63 additions & 0 deletions components/webext-storage/src/webext-storage.udl
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,9 @@
[Custom]
typedef string JsonValue;

[Custom]
typedef string Guid;

namespace webextstorage {

};
Expand All @@ -22,6 +25,11 @@ interface WebExtStorageApiError {
QuotaError(QuotaReason reason);
};

dictionary SyncedExtensionChange {
string ext_id;
string changes;
};

dictionary StorageValueChange {
string key;
JsonValue? old_value;
Expand All @@ -32,6 +40,9 @@ dictionary StorageChanges {
sequence<StorageValueChange> changes;
};

// Note that the `close` function has been intentionally excluded from `WebExtStorageStore` because at present
// it is not necessary for our current use in mozilla central and converting `close` to pass a reference to
// the store resulted in errors.
interface WebExtStorageStore {
[Throws=WebExtStorageApiError]
constructor(string path);
Expand All @@ -42,9 +53,61 @@ interface WebExtStorageStore {
[Throws=WebExtStorageApiError]
JsonValue get([ByRef] string ext_id, JsonValue keys);

[Throws=WebExtStorageApiError]
u64 get_bytes_in_use([ByRef] string ext_id, JsonValue keys);

[Throws=WebExtStorageApiError]
StorageChanges remove([ByRef] string ext_id, JsonValue keys);

[Throws=WebExtStorageApiError]
StorageChanges clear([ByRef] string ext_id);

[Self=ByArc]
WebExtStorageBridgedEngine bridged_engine();

[Throws=WebExtStorageApiError]
sequence<SyncedExtensionChange> get_synced_changes();
};

// Note the canonical docs for this are in https://github.com/mozilla/application-services/blob/main/components/sync15/src/engine/bridged_engine.rs
// NOTE: all timestamps here are milliseconds.
interface WebExtStorageBridgedEngine {
[Throws=WebExtStorageApiError]
i64 last_sync();

[Throws=WebExtStorageApiError]
void set_last_sync(i64 last_sync);

[Throws=WebExtStorageApiError]
string? sync_id();

[Throws=WebExtStorageApiError]
string reset_sync_id();

[Throws=WebExtStorageApiError]
string ensure_current_sync_id([ByRef]string new_sync_id);

[Throws=WebExtStorageApiError]
void prepare_for_sync([ByRef]string client_data);

[Throws=WebExtStorageApiError]
void sync_started();

[Throws=WebExtStorageApiError]
void store_incoming(sequence<string> incoming);

[Throws=WebExtStorageApiError]
sequence<string> apply();

[Throws=WebExtStorageApiError]
void set_uploaded(i64 server_modified_millis, sequence<Guid> guids);

[Throws=WebExtStorageApiError]
void sync_finished();

[Throws=WebExtStorageApiError]
void reset();

[Throws=WebExtStorageApiError]
void wipe();
};