Add LoginsBridgedEngine to expose logins sync to Desktop via UniFFI

components/logins/src/error.rs

@@ -195,6 +195,17 @@ impl GetErrorHandling for Error {
     }
 }
 
+// The bridged sync engine (`sync::bridge`) deals in `anyhow::Result`, as that's
+// what the `sync15` BridgedEngine traits use. This lets UniFFI map those errors
+// onto our public error type when the bridge methods are exposed via the UDL.
+impl From<anyhow::Error> for LoginsApiError {
+    fn from(value: anyhow::Error) -> Self {
+        LoginsApiError::UnexpectedLoginsApiError {
+            reason: value.to_string(),
+        }
+    }
+}
+
 impl From<uniffi::UnexpectedUniFFICallbackError> for LoginsApiError {
     fn from(error: uniffi::UnexpectedUniFFICallbackError) -> Self {
         LoginsApiError::UnexpectedLoginsApiError {

components/logins/src/lib.rs

@@ -29,7 +29,7 @@ use crate::encryption::{check_canary, create_canary, create_key};
 pub use crate::error::*;
 pub use crate::login::*;
 pub use crate::store::*;
-pub use crate::sync::LoginsSyncEngine;
+pub use crate::sync::{LoginsBridgedEngine, LoginsSyncEngine};
 use std::sync::Arc;
 
 // Utility function to create a StaticKeyManager to be used for the time being until support lands

components/logins/src/logins.udl

@@ -301,10 +301,58 @@ interface LoginStore {
     [Self=ByArc]
     void register_with_sync_manager();
 
+    /// Returns a bridged sync engine for Desktop's Sync framework.
+    /// Without this UDL entry the engine is invisible to JS: UniFFI generates
+    /// the XPCOM glue that lets JS call `rustStore.bridgedEngine()`.
+    [Throws=LoginsApiError, Self=ByArc]
+    LoginsBridgedEngine bridged_engine();
+
     [Self=ByArc]
     void shutdown();
 };
 
+/// The Desktop-facing bridged sync engine. The canonical docs are in
+/// https://searchfox.org/mozilla-central/source/services/interfaces/mozIBridgedSyncEngine.idl
+/// It's only actually used on Desktop, but it's fine to expose this everywhere.
+/// NOTE: all timestamps here are milliseconds.
+interface LoginsBridgedEngine {
+    [Throws=LoginsApiError]
+    i64 last_sync();
+
+    [Throws=LoginsApiError]
+    void set_last_sync(i64 last_sync);
+
+    [Throws=LoginsApiError]
+    string? sync_id();
+
+    [Throws=LoginsApiError]
+    string reset_sync_id();
+
+    [Throws=LoginsApiError]
+    string ensure_current_sync_id([ByRef]string new_sync_id);
+
+    [Throws=LoginsApiError]
+    void sync_started();
+
+    [Throws=LoginsApiError]
+    void store_incoming(sequence<string> incoming_envelopes_as_json);
+
+    [Throws=LoginsApiError]
+    sequence<string> apply();
+
+    [Throws=LoginsApiError]
+    void set_uploaded(i64 new_timestamp, sequence<string> uploaded_ids);
+
+    [Throws=LoginsApiError]
+    void sync_finished();
+
+    [Throws=LoginsApiError]
+    void reset();
+
+    [Throws=LoginsApiError]
+    void wipe();
+};
+
 dictionary RunMaintenanceOptions {
     // Wipe un-decryptable logins.  These will hopefully come back on the next sync.
     boolean delete_undecryptable_records_for_remote_replacement=true;

components/logins/src/sync/bridge.rs

@@ -0,0 +1,193 @@
+/* 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/. */
+
+use crate::sync::engine::LoginsSyncEngine;
+use crate::LoginStore;
+use anyhow::Result;
+use std::sync::Arc;
+use sync15::bso::{IncomingBso, OutgoingBso};
+use sync15::engine::{BridgedEngine, BridgedEngineAdaptor};
+use sync15::ServerTimestamp;
+use sync_guid::Guid as SyncGuid;
+
+impl LoginStore {
+    /// Returns a bridged sync engine for Desktop for this store.
+    ///
+    /// Unlike Tabs, constructing a `LoginsSyncEngine` locks the DB and can
+    /// fail, so this is fallible (and exposed as `[Throws]` in the UDL). The
+    /// internal error is surfaced via `anyhow`, which UniFFI maps onto
+    /// `LoginsApiError` through `From<anyhow::Error>`.
+    pub fn bridged_engine(self: Arc<Self>) -> Result<Arc<LoginsBridgedEngine>> {
+        let engine = LoginsSyncEngine::new(self)?;
+        let bridged_engine = LoginsBridgedEngineAdaptor { engine };
+        Ok(Arc::new(LoginsBridgedEngine::new(Box::new(bridged_engine))))
+    }
+}
+
+/// `LoginsSyncEngine` only implements the internal `sync15::SyncEngine` trait,
+/// which is what the mobile (Android/iOS) sync manager drives. Desktop's Sync
+/// framework instead speaks the `mozIBridgedSyncEngine` interface, whose Rust
+/// shape is `sync15::BridgedEngine`. This adaptor wraps our `SyncEngine` and,
+/// via the blanket `impl<A: BridgedEngineAdaptor> BridgedEngine for A`, gives
+/// us a `BridgedEngine` for free. The adaptor exists only because these two
+/// sync-engine traits still live side by side; it can go away if they're ever
+/// unified.
+struct LoginsBridgedEngineAdaptor {
+    engine: LoginsSyncEngine,
+}
+
+/// see sync15/src/engine/bridged_engine.rs for required functions for the trait
+impl BridgedEngineAdaptor for LoginsBridgedEngineAdaptor {
+    fn last_sync(&self) -> Result<i64> {
+        // `get_last_sync` takes the `&LoginDb` to avoid deadlocking when called
+        // mid-sync (while the lock is already held). The bridge methods are
+        // always called outside a sync transaction, so we can lock here.
+        let db = self.engine.store.lock_db()?;
+        Ok(self
+            .engine
+            .get_last_sync(&db)?
+            .unwrap_or_default()
+            .as_millis())
+    }
+
+    fn set_last_sync(&self, last_sync_millis: i64) -> Result<()> {
+        let db = self.engine.store.lock_db()?;
+        self.engine
+            .set_last_sync(&db, ServerTimestamp::from_millis(last_sync_millis))?;
+        Ok(())
+    }
+
+    fn engine(&self) -> &dyn sync15::engine::SyncEngine {
+        &self.engine
+    }
+}
+
+// This is what UniFFI exposes; it does nothing other than delegate back to the
+// `BridgedEngine` trait object (and handle the JSON (de)serialization of BSOs
+// that crosses the FFI boundary).
+/// see services/interfaces/mozIBridgedSyncEngine.idl for contract
+pub struct LoginsBridgedEngine {
+    bridge_impl: Box<dyn BridgedEngine>,
+}
+
+impl LoginsBridgedEngine {
+    pub fn new(bridge_impl: Box<dyn BridgedEngine>) -> 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 sync_started(&self) -> Result<()> {
+        self.bridge_impl.sync_started()
+    }
+
+    // Decode the JSON-encoded IncomingBso's that UniFFI passes to us
+    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)
+    }
+
+    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<String>) -> Result<()> {
+        // UniFFI hands us plain strings; the bridge works in terms of `Guid`.
+        let guids: Vec<SyncGuid> = guids.into_iter().map(SyncGuid::from).collect();
+        self.bridge_impl
+            .set_uploaded(server_modified_millis, &guids)
+    }
+
+    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()
+    }
+}
+
+#[cfg(not(feature = "keydb"))]
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use nss_as::ensure_initialized;
+
+    // Exercises the sync-metadata plumbing (last_sync / sync_id / reset) that
+    // Desktop's Sync framework drives through the bridge, mirroring the Tabs
+    // `test_sync_meta` test.
+    #[test]
+    fn test_sync_meta() {
+        ensure_initialized();
+        error_support::init_for_tests();
+
+        let store = Arc::new(LoginStore::new_in_memory());
+        let bridge = store.bridged_engine().expect("should create bridge");
+
+        // Fresh DB: never synced.
+        assert_eq!(bridge.last_sync().unwrap(), 0);
+        bridge.set_last_sync(3).unwrap();
+        assert_eq!(bridge.last_sync().unwrap(), 3);
+
+        assert!(bridge.sync_id().unwrap().is_none());
+
+        bridge.ensure_current_sync_id("some_guid").unwrap();
+        assert_eq!(bridge.sync_id().unwrap(), Some("some_guid".to_string()));
+        // changing the sync ID should reset the timestamp
+        assert_eq!(bridge.last_sync().unwrap(), 0);
+        bridge.set_last_sync(3).unwrap();
+
+        bridge.reset_sync_id().unwrap();
+        // should now be a random guid.
+        assert_ne!(bridge.sync_id().unwrap(), Some("some_guid".to_string()));
+        // should have reset the last sync timestamp.
+        assert_eq!(bridge.last_sync().unwrap(), 0);
+        bridge.set_last_sync(3).unwrap();
+
+        // `reset` clears the guid and the timestamp
+        bridge.reset().unwrap();
+        assert_eq!(bridge.last_sync().unwrap(), 0);
+        assert!(bridge.sync_id().unwrap().is_none());
+    }
+}

components/logins/src/sync/engine.rs

@@ -16,9 +16,8 @@ use crate::LoginStore;
 use interrupt_support::SqlInterruptScope;
 use rusqlite::named_params;
 use sql_support::ConnExt;
-use std::cell::RefCell;
 use std::collections::HashSet;
-use std::sync::Arc;
+use std::sync::{Arc, Mutex};
 use std::time::{Duration, UNIX_EPOCH};
 use sync15::bso::{IncomingBso, OutgoingBso, OutgoingEnvelope};
 use sync15::engine::{CollSyncIds, CollectionRequest, EngineSyncAssociation, SyncEngine};
@@ -30,7 +29,9 @@ pub struct LoginsSyncEngine {
     pub store: Arc<LoginStore>,
     pub scope: SqlInterruptScope,
     pub encdec: Arc<dyn EncryptorDecryptor>,
-    pub staged: RefCell<Vec<IncomingBso>>,
+    // `Mutex` (rather than `RefCell`) so the engine is `Sync`, which the
+    // Desktop `BridgedEngineAdaptor` requires. Only ever locked briefly.
+    pub staged: Mutex<Vec<IncomingBso>>,
 }
 
 impl LoginsSyncEngine {
@@ -43,7 +44,7 @@ impl LoginsSyncEngine {
             store,
             encdec,
             scope,
-            staged: RefCell::new(vec![]),
+            staged: Mutex::new(vec![]),
         })
     }
 
@@ -292,9 +293,13 @@ impl LoginsSyncEngine {
         db.put_meta(schema::LAST_SYNC_META_KEY, &last_sync_millis)
     }
 
-    fn get_last_sync(&self, db: &LoginDb) -> Result<Option<ServerTimestamp>> {
-        let millis = db.get_meta::<i64>(schema::LAST_SYNC_META_KEY)?.unwrap();
-        Ok(Some(ServerTimestamp(millis)))
+    // Public so the bridged engine (`sync::bridge`) can read the last-sync
+    // timestamp without needing access to the private internals here. Returns
+    // `None` when we've never synced, rather than panicking on a fresh DB.
+    pub fn get_last_sync(&self, db: &LoginDb) -> Result<Option<ServerTimestamp>> {
+        Ok(db
+            .get_meta::<i64>(schema::LAST_SYNC_META_KEY)?
+            .map(ServerTimestamp))
     }
 
     fn mark_as_synchronized(&self, guids: &[&str], ts: ServerTimestamp) -> Result<()> {
@@ -424,7 +429,7 @@ impl SyncEngine for LoginsSyncEngine {
     ) -> anyhow::Result<()> {
         // We don't have cross-item dependencies like bookmarks does, so we can
         // just apply now instead of "staging"
-        self.staged.borrow_mut().append(&mut inbound);
+        self.staged.lock().unwrap().append(&mut inbound);
         Ok(())
     }
 
@@ -433,7 +438,7 @@ impl SyncEngine for LoginsSyncEngine {
         timestamp: ServerTimestamp,
         telem: &mut telemetry::Engine,
     ) -> anyhow::Result<Vec<OutgoingBso>> {
-        let inbound = (*self.staged.borrow_mut()).drain(..).collect();
+        let inbound = self.staged.lock().unwrap().drain(..).collect();
         Ok(self.do_apply_incoming(inbound, timestamp, telem)?)
     }
 

components/logins/src/sync/mod.rs

@@ -2,11 +2,13 @@
  * 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;
 mod engine;
 pub(crate) mod merge;
 mod payload;
 mod update_plan;
 
+pub use bridge::LoginsBridgedEngine;
 pub use engine::LoginsSyncEngine;
 use payload::{IncomingLogin, LoginPayload};