Bug 1626128 - Change Golden Gate to depend on `sync15_traits`. r=markh

Now that `BridgedEngine` has been moved to `sync15_traits`, we can
remove `golden_gate_traits` from the tree, and change Golden Gate to
depend on `sync15_traits` directly.

This commit also adds a Cargo feature, `services_sync`, which reflects
the `MOZ_SERVICES_SYNC` config option. In the future, we'll use this
feature to gate implementations of `mozIBridgedSyncEngine`.

Differential Revision: https://phabricator.services.mozilla.com/D72784

Cargo.lock

@@ -1827,6 +1827,7 @@ dependencies = [
  "fog",
  "geckoservo",
  "gkrust_utils",
+ "golden_gate",
  "jsrust_shared",
  "kvstore",
  "lmdb-rkv-sys",
@@ -1849,7 +1850,6 @@ dependencies = [
  "shift_or_euc_c",
  "static_prefs",
  "storage",
- "sync15-traits",
  "unic-langid",
  "unic-langid-ffi",
  "webext-storage",
@@ -1961,6 +1961,23 @@ dependencies = [
  "scroll",
 ]
 
+[[package]]
+name = "golden_gate"
+version = "0.1.0"
+dependencies = [
+ "atomic_refcell",
+ "cstr",
+ "interrupt-support",
+ "log",
+ "moz_task",
+ "nserror",
+ "nsstring",
+ "storage_variant",
+ "sync15-traits",
+ "thin-vec",
+ "xpcom",
+]
+
 [[package]]
 name = "guid_win"
 version = "0.1.0"

services/sync/golden_gate/Cargo.toml

@@ -8,12 +8,13 @@ edition = "2018"
 [dependencies]
 atomic_refcell = "0.1"
 cstr = "0.1"
+interrupt-support = { git = "https://github.com/mozilla/application-services", rev = "c17198fa5a88295f2cca722586c539280e10201c" }
 log = "0.4"
-golden_gate_traits = { path = "../golden_gate_traits" }
 moz_task = { path = "../../../xpcom/rust/moz_task" }
 nserror = { path = "../../../xpcom/rust/nserror" }
 nsstring = { path = "../../../xpcom/rust/nsstring" }
 storage_variant = { path = "../../../storage/variant" }
+sync15-traits = { git = "https://github.com/mozilla/application-services", rev = "c17198fa5a88295f2cca722586c539280e10201c" }
 xpcom = { path = "../../../xpcom/rust/xpcom" }
 
 [dependencies.thin-vec]

services/sync/golden_gate/src/lib.rs

@@ -110,5 +110,8 @@ pub mod task;
 
 pub use crate::log::LogSink;
 pub use error::{Error, Result};
-pub use golden_gate_traits::{BridgedEngine, Interrupted, Interruptee};
+// Re-export items from `interrupt-support` and `sync15-traits`, so that
+// consumers of `golden_gate` don't have to depend on them.
+pub use interrupt_support::{Interrupted, Interruptee};
+pub use sync15_traits::BridgedEngine;
 pub use task::{ApplyTask, FerryTask};

services/sync/golden_gate/src/task.rs

@@ -5,10 +5,11 @@
 use std::{fmt::Write, mem, sync::Arc};
 
 use atomic_refcell::AtomicRefCell;
-use golden_gate_traits::{BridgedEngine, Interruptee};
+use interrupt_support::Interruptee;
 use moz_task::{DispatchOptions, Task, TaskRunnable, ThreadPtrHandle, ThreadPtrHolder};
 use nserror::nsresult;
 use nsstring::{nsACString, nsCString};
+use sync15_traits::{ApplyResults, BridgedEngine};
 use thin_vec::ThinVec;
 use xpcom::{
     interfaces::{
@@ -283,7 +284,7 @@ pub struct ApplyTask<N: ?Sized + BridgedEngine, S> {
     engine: Arc<N>,
     signal: Arc<S>,
     callback: ThreadPtrHandle<mozIBridgedSyncEngineApplyCallback>,
-    result: AtomicRefCell<Result<Vec<String>, N::Error>>,
+    result: AtomicRefCell<Result<ApplyResults, N::Error>>,
 }
 
 impl<N, S> ApplyTask<N, S>
@@ -344,8 +345,11 @@ where
             &mut *self.result.borrow_mut(),
             Err(Error::DidNotRun(Self::name()).into()),
         ) {
-            Ok(outgoing) => {
-                let result = outgoing
+            Ok(ApplyResults {
+                records,
+                num_reconciled: _,
+            }) => {
+                let result = records
                     .into_iter()
                     .map(nsCString::from)
                     .collect::<ThinVec<_>>();

services/sync/golden_gate_traits/Cargo.toml

@@ -1,6 +0,0 @@
-[package]
-name = "golden_gate_traits"
-description = "Traits used in Golden Gate and Application Services"
-version = "0.1.0"
-authors = ["The Firefox Sync Developers <sync-team@mozilla.com>"]
-edition = "2018"

services/sync/golden_gate_traits/src/lib.rs

@@ -1,204 +0,0 @@
-/* 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/. */
-
-//! These types should eventually move to the `sync15-traits` crate in
-//! Application Services. They're defined in a separate crate in m-c now so
-//! that Golden Gate doesn't rely on their internals.
-
-use std::{error::Error, fmt, sync::Mutex, sync::MutexGuard, sync::PoisonError};
-
-/// A bridged Sync engine implements all the methods needed to support
-/// Desktop Sync.
-pub trait BridgedEngine {
-    /// The type returned for errors.
-    type Error;
-
-    /// Initializes the engine. This is called once, when the engine is first
-    /// created, and guaranteed to be called before any of the other methods.
-    /// The default implementation does nothing.
-    fn initialize(&self) -> Result<(), Self::Error> {
-        Ok(())
-    }
-
-    /// Returns the last sync time, in milliseconds, for this engine's
-    /// collection. This is called before each sync, to determine the lower
-    /// bound for new records to fetch from the server.
-    fn last_sync(&self) -> Result<i64, Self::Error>;
-
-    /// Sets the last sync time, in milliseconds. This is called throughout
-    /// the sync, to fast-forward the stored last sync time to match the
-    /// timestamp on the uploaded records.
-    fn set_last_sync(&self, last_sync_millis: i64) -> Result<(), Self::Error>;
-
-    /// Returns the sync ID for this engine's collection. This is only used in
-    /// tests.
-    fn sync_id(&self) -> Result<Option<String>, Self::Error>;
-
-    /// Resets the sync ID for this engine's collection, returning the new ID.
-    /// As a side effect, implementations should reset all local Sync state,
-    /// as in `reset`.
-    fn reset_sync_id(&self) -> Result<String, Self::Error>;
-
-    /// Ensures that the locally stored sync ID for this engine's collection
-    /// matches the `new_sync_id` from the server. If the two don't match,
-    /// implementations should reset all local Sync state, as in `reset`.
-    /// This method returns the assigned sync ID, which can be either the
-    /// `new_sync_id`, or a different one if the engine wants to force other
-    /// devices to reset their Sync state for this collection the next time they
-    /// sync.
-    fn ensure_current_sync_id(&self, new_sync_id: &str) -> Result<String, Self::Error>;
-
-    /// Stages a batch of incoming Sync records. This is called multiple
-    /// times per sync, once for each batch. Implementations can use the
-    /// signal to check if the operation was aborted, and cancel any
-    /// pending work.
-    fn store_incoming(
-        &self,
-        incoming_cleartexts: &[String],
-        signal: &dyn Interruptee,
-    ) -> Result<(), Self::Error>;
-
-    /// Applies all staged records, reconciling changes on both sides and
-    /// resolving conflicts. Returns a list of records to upload.
-    fn apply(&self, signal: &dyn Interruptee) -> Result<Vec<String>, Self::Error>;
-
-    /// Indicates that the given record IDs were uploaded successfully to the
-    /// server. This is called multiple times per sync, once for each batch
-    /// upload.
-    fn set_uploaded(
-        &self,
-        server_modified_millis: i64,
-        ids: &[String],
-        signal: &dyn Interruptee,
-    ) -> Result<(), Self::Error>;
-
-    /// Indicates that all records have been uploaded. At this point, any record
-    /// IDs marked for upload that haven't been passed to `set_uploaded`, can be
-    /// assumed to have failed: for example, because the server rejected a record
-    /// with an invalid TTL or sort index.
-    fn sync_finished(&self, signal: &dyn Interruptee) -> Result<(), Self::Error>;
-
-    /// Resets all local Sync state, including any change flags, mirrors, and
-    /// the last sync time, such that the next sync is treated as a first sync
-    /// with all new local data. Does not erase any local user data.
-    fn reset(&self) -> Result<(), Self::Error>;
-
-    /// Erases all local user data for this collection, and any Sync metadata.
-    /// This method is destructive, and unused for most collections.
-    fn wipe(&self) -> Result<(), Self::Error>;
-
-    /// Tears down the engine. The opposite of `initialize`, `finalize` is
-    /// called when an engine is disabled, or otherwise no longer needed. The
-    /// default implementation does nothing.
-    fn finalize(&self) -> Result<(), Self::Error> {
-        Ok(())
-    }
-}
-
-/// An interruptee is an abort signal used to interrupt a running task.
-/// Implementations can store an interrupted flag, usually as an atomic
-/// integer or Boolean, set the flag on abort, and have
-/// `Interruptee::was_interrupted` return the flag's value.
-///
-/// Although it's not required, in practice, an `Interruptee` should be
-/// `Send + Sync`, so that a task running on a background task queue can be
-/// interrupted from the main thread.
-pub trait Interruptee {
-    /// Indicates if the caller signaled to interrupt.
-    fn was_interrupted(&self) -> bool;
-
-    /// Returns an error if the caller signaled to abort. This helper makes it
-    /// easier to use the signal with the `?` operator.
-    fn err_if_interrupted(&self) -> Result<(), Interrupted> {
-        if self.was_interrupted() {
-            Err(Interrupted)
-        } else {
-            Ok(())
-        }
-    }
-}
-
-#[derive(Clone, Copy, Debug, Eq, Hash, PartialEq)]
-pub struct Interrupted;
-
-impl Error for Interrupted {}
-
-impl fmt::Display for Interrupted {
-    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
-        "The operation was interrupted".fmt(f)
-    }
-}
-
-/// A blanket implementation of `BridgedEngine` for any `Mutex<BridgedEngine>`.
-/// This is provided for convenience, since we expect most bridges to hold
-/// their engines in an `Arc<Mutex<impl BridgedEngine>>`.
-impl<E> BridgedEngine for Mutex<E>
-where
-    E: BridgedEngine,
-    E::Error: for<'a> From<PoisonError<MutexGuard<'a, E>>>,
-{
-    type Error = E::Error;
-
-    fn initialize(&self) -> Result<(), Self::Error> {
-        self.lock()?.initialize()
-    }
-
-    fn last_sync(&self) -> Result<i64, Self::Error> {
-        self.lock()?.last_sync()
-    }
-
-    fn set_last_sync(&self, millis: i64) -> Result<(), Self::Error> {
-        self.lock()?.set_last_sync(millis)
-    }
-
-    fn store_incoming(
-        &self,
-        incoming_cleartexts: &[String],
-        signal: &dyn Interruptee,
-    ) -> Result<(), Self::Error> {
-        self.lock()?.store_incoming(incoming_cleartexts, signal)
-    }
-
-    fn apply(&self, signal: &dyn Interruptee) -> Result<Vec<String>, Self::Error> {
-        self.lock()?.apply(signal)
-    }
-
-    fn set_uploaded(
-        &self,
-        server_modified_millis: i64,
-        ids: &[String],
-        signal: &dyn Interruptee,
-    ) -> Result<(), Self::Error> {
-        self.lock()?
-            .set_uploaded(server_modified_millis, ids, signal)
-    }
-
-    fn sync_finished(&self, signal: &dyn Interruptee) -> Result<(), Self::Error> {
-        self.lock()?.sync_finished(signal)
-    }
-
-    fn reset(&self) -> Result<(), Self::Error> {
-        self.lock()?.reset()
-    }
-
-    fn wipe(&self) -> Result<(), Self::Error> {
-        self.lock()?.wipe()
-    }
-
-    fn finalize(&self) -> Result<(), Self::Error> {
-        self.lock()?.finalize()
-    }
-
-    fn sync_id(&self) -> Result<Option<String>, Self::Error> {
-        self.lock()?.sync_id()
-    }
-
-    fn reset_sync_id(&self) -> Result<String, Self::Error> {
-        self.lock()?.reset_sync_id()
-    }
-
-    fn ensure_current_sync_id(&self, new_sync_id: &str) -> Result<String, Self::Error> {
-        self.lock()?.ensure_current_sync_id(new_sync_id)
-    }
-}

toolkit/library/gtest/rust/Cargo.toml

@@ -33,6 +33,7 @@ webgpu = ["gkrust-shared/webgpu"]
 remote_agent = ["gkrust-shared/remote"]
 glean = ["gkrust-shared/glean"]
 new_webext_storage = ["gkrust-shared/new_webext_storage"]
+services_sync = ["gkrust-shared/services_sync"]
 
 [dependencies]
 bench-collections-gtest = { path = "../../../../xpcom/rust/gtest/bench-collections" }

toolkit/library/rust/Cargo.toml

@@ -34,6 +34,7 @@ webgpu = ["gkrust-shared/webgpu"]
 remote_agent = ["gkrust-shared/remote"]
 glean = ["gkrust-shared/glean"]
 new_webext_storage = ["gkrust-shared/new_webext_storage"]
+services_sync = ["gkrust-shared/services_sync"]
 
 [dependencies]
 gkrust-shared = { path = "shared" }

toolkit/library/rust/gkrust-features.mozbuild

@@ -82,3 +82,6 @@ if CONFIG['MOZ_USING_WASM_SANDBOXING']:
 
 if CONFIG['MOZ_NEW_WEBEXT_STORAGE']:
     gkrust_features += ['new_webext_storage']
+
+if CONFIG['MOZ_SERVICES_SYNC']:
+    gkrust_features += ['services_sync']

toolkit/library/rust/shared/Cargo.toml

@@ -55,6 +55,7 @@ unic-langid-ffi = { path = "../../../../intl/locale/rust/unic-langid-ffi" }
 fluent-langneg = { version = "0.12.1", features = ["cldr"] }
 fluent-langneg-ffi = { path = "../../../../intl/locale/rust/fluent-langneg-ffi" }
 webext-storage = { git = "https://github.com/mozilla/application-services", rev = "c17198fa5a88295f2cca722586c539280e10201c", optional = true }
+golden_gate = { path = "../../../../services/sync/golden_gate", optional = true }
 
 # Note: `modern_sqlite` means rusqlite's bindings file be for a sqlite with
 # version less than or equal to what we link to. This isn't a problem because we
@@ -65,8 +66,6 @@ rusqlite = { version = "0.23.1", features = ["modern_sqlite", "in_gecko"] }
 fluent = { version = "0.11" , features = ["fluent-pseudo"] }
 fluent-ffi = { path = "../../../../intl/l10n/rust/fluent-ffi" }
 
-sync15-traits = { git = "https://github.com/mozilla/application-services", rev = "c17198fa5a88295f2cca722586c539280e10201c" }
-
 [build-dependencies]
 rustc_version = "0.2"
 
@@ -99,6 +98,7 @@ webgpu = ["wgpu_bindings"]
 remote_agent = ["remote"]
 glean = ["fog"]
 new_webext_storage = ["webext_storage_bridge"]
+services_sync = ["golden_gate"]
 
 [lib]
 path = "lib.rs"

toolkit/library/rust/shared/lib.rs

@@ -74,7 +74,9 @@ extern crate fluent;
 extern crate fluent_ffi;
 
 extern crate rusqlite;
-extern crate sync15_traits;
+
+#[cfg(feature = "services_sync")]
+extern crate golden_gate;
 
 #[cfg(feature = "remote")]
 extern crate remote;