[sync] Extend TrustedVaultClient API with more recovery functions

This is a follow-up to crrev.com/c/2329751 to support notifying
observers, along with an API to resolve the recoverability errors.

The actual TrustedVaultClient implementations are left unimplemented,
except for StandaloneTrustedVaultClient where minimal logic is
introduced to support semi-realistic integration tests.

Change-Id: I63acc6196fee59d24edbd1d3816f19ef8e15930a
Bug: 1081649
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2448491
Commit-Queue: Mikel Astiz <mastiz@chromium.org>
Reviewed-by: Maksim Moskvitin <mmoskvitin@google.com>
Cr-Commit-Position: refs/heads/master@{#813716}

chrome/browser/sync/test/integration/single_client_nigori_sync_test.cc

@@ -27,6 +27,7 @@
 #include "chrome/common/chrome_features.h"
 #include "chrome/grit/generated_resources.h"
 #include "components/sync/base/time.h"
+#include "components/sync/driver/sync_driver_switches.h"
 #include "components/sync/engine/sync_engine_switches.h"
 #include "components/sync/nigori/cryptographer_impl.h"
 #include "components/sync/nigori/nigori.h"
@@ -171,6 +172,24 @@ class PageTitleChecker : public StatusChangeChecker,
   DISALLOW_COPY_AND_ASSIGN(PageTitleChecker);
 };
 
+// Used to wait until IsTrustedVaultRecoverabilityDegraded() returns false.
+class TrustedVaultRecoverabilityNotDegradedChecker
+    : public SingleClientStatusChangeChecker {
+ public:
+  explicit TrustedVaultRecoverabilityNotDegradedChecker(
+      syncer::ProfileSyncService* service)
+      : SingleClientStatusChangeChecker(service) {}
+  ~TrustedVaultRecoverabilityNotDegradedChecker() override = default;
+
+ protected:
+  // StatusChangeChecker implementation.
+  bool IsExitConditionSatisfied(std::ostream* os) override {
+    return !service()
+                ->GetUserSettings()
+                ->IsTrustedVaultRecoverabilityDegraded();
+  }
+};
+
 class SingleClientNigoriSyncTest : public SyncTest {
  public:
   SingleClientNigoriSyncTest() : SyncTest(SINGLE_CLIENT) {}
@@ -227,7 +246,7 @@ IN_PROC_BROWSER_TEST_F(SingleClientNigoriSyncTest,
 
   const std::vector<std::vector<uint8_t>>& keystore_keys =
       GetFakeServer()->GetKeystoreKeys();
-  ASSERT_TRUE(keystore_keys.size() == 1);
+  ASSERT_THAT(keystore_keys, SizeIs(1));
   EXPECT_THAT(
       specifics.encryption_keybag(),
       IsDataEncryptedWith(Pbkdf2KeyParamsForTesting(keystore_keys.back())));
@@ -958,6 +977,21 @@ IN_PROC_BROWSER_TEST_F(SingleClientNigoriWithRecoverySyncTest,
       sync_ui_util::GetStatusLabels(GetProfile(0)),
       StatusLabelsMatch(sync_ui_util::SYNCED, IDS_SYNC_ACCOUNT_SYNCING,
                         IDS_SETTINGS_EMPTY_STRING, sync_ui_util::NO_ACTION));
+
+  // Mimic opening a web page where the user can interact with the degraded
+  // recoverability flow.
+  static_cast<syncer::StandaloneTrustedVaultClient*>(
+      GetSyncService(0)->GetSyncClientForTest()->GetTrustedVaultClient())
+      ->ResolveRecoverabilityDegradedForTesting();
+
+  EXPECT_TRUE(
+      TrustedVaultRecoverabilityNotDegradedChecker(GetSyncService(0)).Wait());
+
+#if !defined(OS_CHROMEOS)
+  // Verify the profile-menu error string is empty.
+  EXPECT_EQ(sync_ui_util::NO_SYNC_ERROR,
+            sync_ui_util::GetAvatarSyncErrorType(GetProfile(0)));
+#endif  // !defined(OS_CHROMEOS)
 }
 
 }  // namespace

chrome/browser/sync/test/integration/single_client_status_change_checker.h

@@ -26,10 +26,6 @@ class SingleClientStatusChangeChecker
   ~SingleClientStatusChangeChecker() override;
 
   syncer::ProfileSyncService* service();
-
- protected:
-  // StatusChangeChecker implementations and stubs.
-  bool IsExitConditionSatisfied(std::ostream* os) override = 0;
 };
 
 #endif  // CHROME_BROWSER_SYNC_TEST_INTEGRATION_SINGLE_CLIENT_STATUS_CHANGE_CHECKER_H_

chrome/browser/sync/trusted_vault_client_android.cc

@@ -137,3 +137,20 @@ void TrustedVaultClientAndroid::GetIsRecoverabilityDegraded(
   NOTIMPLEMENTED();
   std::move(cb).Run(false);
 }
+
+std::unique_ptr<TrustedVaultClientAndroid::Subscription>
+TrustedVaultClientAndroid::AddRecoverabilityObserver(
+    const base::RepeatingClosure& cb) {
+  // TODO(crbug.com/1100279): Needs implementation.
+  NOTIMPLEMENTED();
+  return nullptr;
+}
+
+void TrustedVaultClientAndroid::AddTrustedRecoveryMethod(
+    const std::string& gaia_id,
+    const std::vector<uint8_t>& public_key,
+    base::OnceClosure cb) {
+  // TODO(crbug.com/1100279): Needs implementation.
+  NOTIMPLEMENTED();
+  std::move(cb).Run();
+}

chrome/browser/sync/trusted_vault_client_android.h

@@ -60,6 +60,11 @@ class TrustedVaultClientAndroid : public syncer::TrustedVaultClient {
                        base::OnceCallback<void(bool)> cb) override;
   void GetIsRecoverabilityDegraded(const CoreAccountInfo& account_info,
                                    base::OnceCallback<void(bool)> cb) override;
+  std::unique_ptr<Subscription> AddRecoverabilityObserver(
+      const base::RepeatingClosure& cb) override;
+  void AddTrustedRecoveryMethod(const std::string& gaia_id,
+                                const std::vector<uint8_t>& public_key,
+                                base::OnceClosure cb) override;
 
  private:
   // Struct representing an in-flight FetchKeys() call invoked from C++.

components/sync/driver/sync_driver_switches.cc

@@ -67,4 +67,8 @@ const base::Feature kDecoupleSyncFromAndroidMasterSync{
 const base::Feature kFollowTrustedVaultKeyRotation{
     "FollowTrustedVaultKeyRotation", base::FEATURE_DISABLED_BY_DEFAULT};
 
+const base::Feature kSyncSupportTrustedVaultPassphraseRecovery{
+    "SyncSupportTrustedVaultPassphraseRecovery",
+    base::FEATURE_DISABLED_BY_DEFAULT};
+
 }  // namespace switches

components/sync/driver/sync_driver_switches.h

@@ -33,6 +33,8 @@ extern const base::Feature kSyncWifiConfigurations;
 extern const base::Feature kDecoupleSyncFromAndroidMasterSync;
 extern const base::Feature kFollowTrustedVaultKeyRotation;
 
+extern const base::Feature kSyncSupportTrustedVaultPassphraseRecovery;
+
 }  // namespace switches
 
 #endif  // COMPONENTS_SYNC_DRIVER_SYNC_DRIVER_SWITCHES_H_

components/sync/driver/sync_service_crypto.cc

@@ -17,7 +17,6 @@
 #include "components/sync/base/sync_prefs.h"
 #include "components/sync/driver/sync_driver_switches.h"
 #include "components/sync/driver/sync_service.h"
-#include "components/sync/engine/sync_engine_switches.h"
 #include "components/sync/engine/sync_string_conversions.h"
 #include "components/sync/nigori/nigori.h"
 
@@ -72,6 +71,18 @@ class EmptyTrustedVaultClient : public TrustedVaultClient {
                                    base::OnceCallback<void(bool)> cb) override {
     std::move(cb).Run(false);
   }
+
+  std::unique_ptr<Subscription> AddRecoverabilityObserver(
+      const base::RepeatingClosure& cb) override {
+    return nullptr;
+  }
+
+  void AddTrustedRecoveryMethod(const std::string& gaia_id,
+                                const std::vector<uint8_t>& public_key,
+                                base::OnceClosure cb) override {
+    // Never invoked by SyncServiceCrypto.
+    NOTREACHED();
+  }
 };
 
 // A SyncEncryptionHandler::Observer implementation that simply posts all calls
@@ -221,10 +232,15 @@ SyncServiceCrypto::SyncServiceCrypto(
   DCHECK(sync_prefs_);
   DCHECK(trusted_vault_client_);
 
-  trusted_vault_client_subscription_ =
+  trusted_vault_client_keys_subscription_ =
       trusted_vault_client_->AddKeysChangedObserver(base::BindRepeating(
           &SyncServiceCrypto::OnTrustedVaultClientKeysChanged,
           weak_factory_.GetWeakPtr()));
+
+  trusted_vault_client_recoverability_subscription_ =
+      trusted_vault_client_->AddRecoverabilityObserver(base::BindRepeating(
+          &SyncServiceCrypto::OnTrustedVaultClientRecoverabilityChanged,
+          weak_factory_.GetWeakPtr()));
 }
 
 SyncServiceCrypto::~SyncServiceCrypto() = default;
@@ -646,6 +662,10 @@ void SyncServiceCrypto::OnTrustedVaultClientKeysChanged() {
   FetchTrustedVaultKeys(/*is_second_fetch_attempt=*/false);
 }
 
+void SyncServiceCrypto::OnTrustedVaultClientRecoverabilityChanged() {
+  RefreshIsRecoverabilityDegraded();
+}
+
 void SyncServiceCrypto::FetchTrustedVaultKeys(bool is_second_fetch_attempt) {
   DCHECK(state_.engine);
   DCHECK(state_.required_user_action ==
@@ -775,20 +795,37 @@ void SyncServiceCrypto::UpdateRequiredUserActionAndNotify(
   state_.required_user_action = new_required_user_action;
   notify_required_user_action_changed_.Run();
 
-  if (state_.required_user_action == RequiredUserAction::kNone &&
-      state_.cached_passphrase_type ==
-          PassphraseType::kTrustedVaultPassphrase &&
-      base::FeatureList::IsEnabled(
+  RefreshIsRecoverabilityDegraded();
+}
+
+void SyncServiceCrypto::RefreshIsRecoverabilityDegraded() {
+  switch (state_.required_user_action) {
+    case RequiredUserAction::kUnknownDuringInitialization:
+    case RequiredUserAction::kFetchingTrustedVaultKeys:
+    case RequiredUserAction::kTrustedVaultKeyRequired:
+    case RequiredUserAction::kTrustedVaultKeyRequiredButFetching:
+    case RequiredUserAction::kPassphraseRequiredForDecryption:
+    case RequiredUserAction::kPassphraseRequiredForEncryption:
+      return;
+    case RequiredUserAction::kNone:
+    case RequiredUserAction::kTrustedVaultRecoverabilityDegraded:
+      break;
+  }
+
+  if (!base::FeatureList::IsEnabled(
           switches::kSyncSupportTrustedVaultPassphraseRecovery)) {
-    trusted_vault_client_->GetIsRecoverabilityDegraded(
-        state_.account_info,
-        base::BindOnce(&SyncServiceCrypto::GetIsRecoverabilityDegradedCompleted,
-                       weak_factory_.GetWeakPtr()));
+    return;
   }
+
+  trusted_vault_client_->GetIsRecoverabilityDegraded(
+      state_.account_info,
+      base::BindOnce(&SyncServiceCrypto::GetIsRecoverabilityDegradedCompleted,
+                     weak_factory_.GetWeakPtr()));
 }
 
 void SyncServiceCrypto::GetIsRecoverabilityDegradedCompleted(
     bool is_recoverability_degraded) {
+  // The passphrase type could have changed.
   if (state_.cached_passphrase_type !=
       PassphraseType::kTrustedVaultPassphrase) {
     DCHECK_NE(state_.required_user_action,

components/sync/driver/sync_service_crypto.h

@@ -113,8 +113,9 @@ class SyncServiceCrypto : public SyncEncryptionHandler::Observer,
     kTrustedVaultRecoverabilityDegraded,
   };
 
-  // Observer method invoked by TrustedVaultClient when its content changes.
+  // Observer methods invoked by TrustedVaultClient when its content changes.
   void OnTrustedVaultClientKeysChanged();
+  void OnTrustedVaultClientRecoverabilityChanged();
 
   // Reads trusted vault keys from the client and feeds them to the sync engine.
   void FetchTrustedVaultKeys(bool is_second_fetch_attempt);
@@ -135,6 +136,9 @@ class SyncServiceCrypto : public SyncEncryptionHandler::Observer,
   void UpdateRequiredUserActionAndNotify(
       RequiredUserAction new_required_user_action);
 
+  // Invokes TrustedVaultClient::GetIsRecoverabilityDegraded() if needed.
+  void RefreshIsRecoverabilityDegraded();
+
   // Completion callback function for
   // TrustedVaultClient::GetIsRecoverabilityDegraded().
   void GetIsRecoverabilityDegradedCompleted(bool is_recoverability_degraded);
@@ -153,9 +157,11 @@ class SyncServiceCrypto : public SyncEncryptionHandler::Observer,
   // Never null and guaranteed to outlive us.
   TrustedVaultClient* const trusted_vault_client_;
 
-  // Subscription to observe changes in |*trusted_vault_client_|.
+  // Subscriptions to observe changes in |*trusted_vault_client_|.
+  std::unique_ptr<TrustedVaultClient::Subscription>
+      trusted_vault_client_keys_subscription_;
   std::unique_ptr<TrustedVaultClient::Subscription>
-      trusted_vault_client_subscription_;
+      trusted_vault_client_recoverability_subscription_;
 
   // All the mutable state is wrapped in a struct so that it can be easily
   // reset to its default values.

components/sync/driver/sync_service_crypto_unittest.cc

@@ -15,9 +15,9 @@
 #include "base/test/scoped_feature_list.h"
 #include "components/signin/public/identity_manager/account_info.h"
 #include "components/sync/base/sync_prefs.h"
+#include "components/sync/driver/sync_driver_switches.h"
 #include "components/sync/driver/trusted_vault_client.h"
 #include "components/sync/engine/mock_sync_engine.h"
-#include "components/sync/engine/sync_engine_switches.h"
 #include "components/sync/nigori/nigori.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
@@ -151,12 +151,13 @@ class TestTrustedVaultClient : public TrustedVaultClient {
 
   void SetIsRecoverabilityDegraded(bool is_recoverability_degraded) {
     is_recoverability_degraded_ = is_recoverability_degraded;
+    recoverability_observer_list_.Notify();
   }
 
   // TrustedVaultClient implementation.
   std::unique_ptr<Subscription> AddKeysChangedObserver(
       const base::RepeatingClosure& cb) override {
-    return observer_list_.Add(cb);
+    return keys_observer_list_.Add(cb);
   }
 
   void FetchKeys(
@@ -202,12 +203,12 @@ class TestTrustedVaultClient : public TrustedVaultClient {
     CachedKeysPerUser& cached_keys = gaia_id_to_cached_keys_[gaia_id];
     cached_keys.keys = keys;
     cached_keys.marked_as_stale = false;
-    observer_list_.Notify();
+    keys_observer_list_.Notify();
   }
 
   void RemoveAllStoredKeys() override {
     gaia_id_to_cached_keys_.clear();
-    observer_list_.Notify();
+    keys_observer_list_.Notify();
   }
 
   void MarkKeysAsStale(const CoreAccountInfo& account_info,
@@ -236,6 +237,18 @@ class TestTrustedVaultClient : public TrustedVaultClient {
     std::move(cb).Run(is_recoverability_degraded_);
   }
 
+  std::unique_ptr<Subscription> AddRecoverabilityObserver(
+      const base::RepeatingClosure& cb) override {
+    return recoverability_observer_list_.Add(cb);
+  }
+
+  void AddTrustedRecoveryMethod(const std::string& gaia_id,
+                                const std::vector<uint8_t>& public_key,
+                                base::OnceClosure cb) override {
+    // Not relevant in these tests.
+    std::move(cb).Run();
+  }
+
  private:
   struct CachedKeysPerUser {
     bool marked_as_stale = false;
@@ -245,7 +258,8 @@ class TestTrustedVaultClient : public TrustedVaultClient {
   const TestTrustedVaultServer* const server_;
 
   std::map<std::string, CachedKeysPerUser> gaia_id_to_cached_keys_;
-  CallbackList observer_list_;
+  CallbackList keys_observer_list_;
+  CallbackList recoverability_observer_list_;
   int fetch_count_ = 0;
   int keys_marked_as_stale_count_ = 0;
   int get_is_recoverablity_degraded_call_count_ = 0;
@@ -787,7 +801,8 @@ TEST_F(SyncServiceCryptoTest, ShouldNotGetRecoverabilityIfFeatureDisabled) {
   EXPECT_FALSE(crypto_.IsTrustedVaultRecoverabilityDegraded());
 }
 
-TEST_F(SyncServiceCryptoTest, ShouldNotReportDegradedRecoverability) {
+TEST_F(SyncServiceCryptoTest,
+       ShouldNotReportDegradedRecoverabilityUponInitialization) {
   base::test::ScopedFeatureList override_features;
   override_features.InitAndEnableFeature(
       switches::kSyncSupportTrustedVaultPassphraseRecovery);
@@ -804,7 +819,48 @@ TEST_F(SyncServiceCryptoTest, ShouldNotReportDegradedRecoverability) {
   EXPECT_FALSE(crypto_.IsTrustedVaultRecoverabilityDegraded());
 }
 
-TEST_F(SyncServiceCryptoTest, ShouldReportDegradedRecoverability) {
+TEST_F(SyncServiceCryptoTest,
+       ShouldReportDegradedRecoverabilityUponInitialization) {
+  base::test::ScopedFeatureList override_features;
+  override_features.InitAndEnableFeature(
+      switches::kSyncSupportTrustedVaultPassphraseRecovery);
+
+  trusted_vault_client_.SetIsRecoverabilityDegraded(true);
+  crypto_.OnPassphraseTypeChanged(PassphraseType::kTrustedVaultPassphrase,
+                                  base::Time::Now());
+  crypto_.SetSyncEngine(CoreAccountInfo(), &engine_);
+  ASSERT_THAT(crypto_.GetPassphraseType(),
+              Eq(PassphraseType::kTrustedVaultPassphrase));
+  ASSERT_TRUE(crypto_.IsTrustedVaultKeyRequiredStateKnown());
+  ASSERT_FALSE(crypto_.IsTrustedVaultKeyRequired());
+
+  EXPECT_TRUE(crypto_.IsTrustedVaultRecoverabilityDegraded());
+}
+
+TEST_F(SyncServiceCryptoTest, ShouldReportDegradedRecoverabilityUponChange) {
+  base::test::ScopedFeatureList override_features;
+  override_features.InitAndEnableFeature(
+      switches::kSyncSupportTrustedVaultPassphraseRecovery);
+
+  trusted_vault_client_.SetIsRecoverabilityDegraded(false);
+  crypto_.OnPassphraseTypeChanged(PassphraseType::kTrustedVaultPassphrase,
+                                  base::Time::Now());
+  crypto_.SetSyncEngine(CoreAccountInfo(), &engine_);
+  ASSERT_THAT(crypto_.GetPassphraseType(),
+              Eq(PassphraseType::kTrustedVaultPassphrase));
+  ASSERT_TRUE(crypto_.IsTrustedVaultKeyRequiredStateKnown());
+  ASSERT_FALSE(crypto_.IsTrustedVaultKeyRequired());
+  ASSERT_FALSE(crypto_.IsTrustedVaultRecoverabilityDegraded());
+
+  // Changing the state notifies observers and should lead to a change in
+  // IsTrustedVaultRecoverabilityDegraded().
+  EXPECT_CALL(notify_observers_cb_, Run());
+  trusted_vault_client_.SetIsRecoverabilityDegraded(true);
+  EXPECT_TRUE(crypto_.IsTrustedVaultRecoverabilityDegraded());
+}
+
+TEST_F(SyncServiceCryptoTest,
+       ShouldStopReportingDegradedRecoverabilityUponChange) {
   base::test::ScopedFeatureList override_features;
   override_features.InitAndEnableFeature(
       switches::kSyncSupportTrustedVaultPassphraseRecovery);
@@ -817,7 +873,49 @@ TEST_F(SyncServiceCryptoTest, ShouldReportDegradedRecoverability) {
               Eq(PassphraseType::kTrustedVaultPassphrase));
   ASSERT_TRUE(crypto_.IsTrustedVaultKeyRequiredStateKnown());
   ASSERT_FALSE(crypto_.IsTrustedVaultKeyRequired());
+  ASSERT_TRUE(crypto_.IsTrustedVaultRecoverabilityDegraded());
+
+  // Changing the state notifies observers and should lead to a change in
+  // IsTrustedVaultRecoverabilityDegraded().
+  EXPECT_CALL(notify_observers_cb_, Run());
+  trusted_vault_client_.SetIsRecoverabilityDegraded(false);
+  EXPECT_FALSE(crypto_.IsTrustedVaultRecoverabilityDegraded());
+}
+
+TEST_F(SyncServiceCryptoTest, ShouldReportDegradedRecoverabilityUponRetrieval) {
+  base::test::ScopedFeatureList override_features;
+  override_features.InitAndEnableFeature(
+      switches::kSyncSupportTrustedVaultPassphraseRecovery);
+
+  trusted_vault_client_.SetIsRecoverabilityDegraded(true);
+
+  // Mimic startup with trusted vault keys being required.
+  crypto_.OnTrustedVaultKeyRequired();
+  crypto_.OnPassphraseTypeChanged(PassphraseType::kTrustedVaultPassphrase,
+                                  base::Time::Now());
+  crypto_.SetSyncEngine(kSyncingAccount, &engine_);
+  ASSERT_FALSE(crypto_.IsTrustedVaultKeyRequiredStateKnown());
+  ASSERT_FALSE(crypto_.IsTrustedVaultRecoverabilityDegraded());
+
+  // Complete the fetching of initial keys (no keys) from the client.
+  ASSERT_TRUE(trusted_vault_client_.CompleteFetchKeysRequest());
+  ASSERT_TRUE(crypto_.IsTrustedVaultKeyRequiredStateKnown());
+  ASSERT_TRUE(crypto_.IsTrustedVaultKeyRequired());
+  ASSERT_FALSE(crypto_.IsTrustedVaultRecoverabilityDegraded());
+
+  // Mimic a successful key retrieval.
+  ON_CALL(engine_, AddTrustedVaultDecryptionKeys(_, _))
+      .WillByDefault([&](const std::vector<std::vector<uint8_t>>& keys,
+                         base::OnceClosure done_cb) {
+        crypto_.OnTrustedVaultKeyAccepted();
+        std::move(done_cb).Run();
+      });
+  MimicKeyRetrievalByUser();
+  ASSERT_TRUE(trusted_vault_client_.CompleteFetchKeysRequest());
+  ASSERT_TRUE(crypto_.IsTrustedVaultKeyRequiredStateKnown());
+  ASSERT_FALSE(crypto_.IsTrustedVaultKeyRequired());
 
+  // The recoverability state should be exposed.
   EXPECT_TRUE(crypto_.IsTrustedVaultRecoverabilityDegraded());
 }
 

components/sync/driver/trusted_vault_client.h

@@ -74,6 +74,21 @@ class TrustedVaultClient {
       const CoreAccountInfo& account_info,
       base::OnceCallback<void(bool)> cb) = 0;
 
+  // Registers an observer-like callback that will be invoked when the
+  // recoverability of the keys has changed. The subscription must not outlive
+  // |*this|.
+  // TODO(crbug.com/1081649): Unify with AddKeysChangedObserver() by introducing
+  // an observer interface instead.
+  virtual std::unique_ptr<Subscription> AddRecoverabilityObserver(
+      const base::RepeatingClosure& cb) = 0;
+
+  // Registers a new trusted recovery method that can be used to retrieve keys,
+  // usually for the purpose of resolving a recoverability-degraded case
+  // surfaced by GetIsRecoverabilityDegraded().
+  virtual void AddTrustedRecoveryMethod(const std::string& gaia_id,
+                                        const std::vector<uint8_t>& public_key,
+                                        base::OnceClosure cb) = 0;
+
  private:
   DISALLOW_COPY_AND_ASSIGN(TrustedVaultClient);
 };

components/sync/engine/sync_engine_switches.cc

@@ -20,10 +20,6 @@ const base::Feature kSyncUseScryptForNewCustomPassphrases{
 const base::Feature kSyncSupportTrustedVaultPassphrase{
     "SyncSupportTrustedVaultPassphrase", base::FEATURE_ENABLED_BY_DEFAULT};
 
-const base::Feature kSyncSupportTrustedVaultPassphraseRecovery{
-    "SyncSupportTrustedVaultPassphraseRecovery",
-    base::FEATURE_DISABLED_BY_DEFAULT};
-
 const base::Feature kSyncTriggerFullKeystoreMigration{
     "SyncTriggerFullKeystoreMigration", base::FEATURE_DISABLED_BY_DEFAULT};
 

components/sync/engine/sync_engine_switches.h

@@ -12,7 +12,6 @@ namespace switches {
 extern const base::Feature kSyncResetPollIntervalOnStart;
 extern const base::Feature kSyncUseScryptForNewCustomPassphrases;
 extern const base::Feature kSyncSupportTrustedVaultPassphrase;
-extern const base::Feature kSyncSupportTrustedVaultPassphraseRecovery;
 extern const base::Feature kSyncTriggerFullKeystoreMigration;
 
 }  // namespace switches

components/sync/trusted_vault/standalone_trusted_vault_backend.cc

@@ -50,8 +50,11 @@ void WriteToDisk(const sync_pb::LocalTrustedVault& data,
 
 StandaloneTrustedVaultBackend::StandaloneTrustedVaultBackend(
     const base::FilePath& file_path,
+    std::unique_ptr<Delegate> delegate,
     std::unique_ptr<TrustedVaultConnection> connection)
-    : file_path_(file_path), connection_(std::move(connection)) {}
+    : file_path_(file_path),
+      delegate_(std::move(delegate)),
+      connection_(std::move(connection)) {}
 
 StandaloneTrustedVaultBackend::~StandaloneTrustedVaultBackend() = default;
 
@@ -182,6 +185,28 @@ bool StandaloneTrustedVaultBackend::MarkKeysAsStale(
   return true;
 }
 
+void StandaloneTrustedVaultBackend::GetIsRecoverabilityDegraded(
+    const CoreAccountInfo& account_info,
+    base::OnceCallback<void(bool)> cb) {
+  // TODO(crbug.com/1081649): Implement logic.
+  NOTIMPLEMENTED();
+  std::move(cb).Run(is_recoverability_degraded_for_testing_);
+}
+
+void StandaloneTrustedVaultBackend::AddTrustedRecoveryMethod(
+    const std::string& gaia_id,
+    const std::vector<uint8_t>& public_key,
+    base::OnceClosure cb) {
+  if (primary_account_->gaia == gaia_id) {
+    // TODO(crbug.com/1081649): Implement logic.
+    NOTIMPLEMENTED();
+    is_recoverability_degraded_for_testing_ = false;
+    delegate_->NotifyRecoverabilityDegradedChanged();
+  }
+
+  std::move(cb).Run();
+}
+
 base::Optional<CoreAccountInfo>
 StandaloneTrustedVaultBackend::GetPrimaryAccountForTesting() const {
   return primary_account_;
@@ -197,6 +222,16 @@ StandaloneTrustedVaultBackend::GetDeviceRegistrationInfoForTesting(
   return per_user_vault->local_device_registration_info();
 }
 
+void StandaloneTrustedVaultBackend::SetRecoverabilityDegradedForTesting() {
+  is_recoverability_degraded_for_testing_ = true;
+  delegate_->NotifyRecoverabilityDegradedChanged();
+}
+
+void StandaloneTrustedVaultBackend::ResolveRecoverabilityDegradedForTesting() {
+  is_recoverability_degraded_for_testing_ = false;
+  delegate_->NotifyRecoverabilityDegradedChanged();
+}
+
 void StandaloneTrustedVaultBackend::MaybeRegisterDevice(
     const std::string& gaia_id) {
   if (!base::FeatureList::IsEnabled(switches::kFollowTrustedVaultKeyRotation)) {

components/sync/trusted_vault/standalone_trusted_vault_backend.h

@@ -31,8 +31,20 @@ class StandaloneTrustedVaultBackend
   using FetchKeysCallback = base::OnceCallback<void(
       const std::vector<std::vector<uint8_t>>& vault_keys)>;
 
+  class Delegate {
+   public:
+    Delegate() = default;
+    Delegate(const Delegate&) = delete;
+    virtual ~Delegate() = default;
+
+    Delegate& operator=(const Delegate&) = delete;
+
+    virtual void NotifyRecoverabilityDegradedChanged() = 0;
+  };
+
   StandaloneTrustedVaultBackend(
       const base::FilePath& file_path,
+      std::unique_ptr<Delegate> delegate,
       std::unique_ptr<TrustedVaultConnection> connection);
   StandaloneTrustedVaultBackend(const StandaloneTrustedVaultBackend& other) =
       delete;
@@ -68,11 +80,24 @@ class StandaloneTrustedVaultBackend
   void SetPrimaryAccount(
       const base::Optional<CoreAccountInfo>& primary_account);
 
+  // Returns whether recoverability of the keys is degraded and user action is
+  // required to add a new method.
+  void GetIsRecoverabilityDegraded(const CoreAccountInfo& account_info,
+                                   base::OnceCallback<void(bool)> cb);
+
+  // Registers a new trusted recovery method that can be used to retrieve keys.
+  void AddTrustedRecoveryMethod(const std::string& gaia_id,
+                                const std::vector<uint8_t>& public_key,
+                                base::OnceClosure cb);
+
   base::Optional<CoreAccountInfo> GetPrimaryAccountForTesting() const;
 
   sync_pb::LocalDeviceRegistrationInfo GetDeviceRegistrationInfoForTesting(
       const std::string& gaia_id);
 
+  void SetRecoverabilityDegradedForTesting();
+  void ResolveRecoverabilityDegradedForTesting();
+
  private:
   friend class base::RefCountedThreadSafe<StandaloneTrustedVaultBackend>;
 
@@ -102,21 +127,25 @@ class StandaloneTrustedVaultBackend
 
   const base::FilePath file_path_;
 
+  const std::unique_ptr<Delegate> delegate_;
+
+  // Used for communication with trusted vault server.
+  const std::unique_ptr<TrustedVaultConnection> connection_;
+
   sync_pb::LocalTrustedVault data_;
 
   // Only current |primary_account_| can be used for communication with trusted
   // vault server.
   base::Optional<CoreAccountInfo> primary_account_;
 
-  // Used for communication with trusted vault server.
-  std::unique_ptr<TrustedVaultConnection> connection_;
-
   // Used to plumb FetchKeys() result to the caller.
   FetchKeysCallback ongoing_fetch_keys_callback_;
 
   // Account used in last FetchKeys() call.
   base::Optional<std::string> ongoing_fetch_keys_gaia_id_;
 
+  bool is_recoverability_degraded_for_testing_ = false;
+
   // Used for cancellation of callbacks passed to |connection_|.
   base::WeakPtrFactory<StandaloneTrustedVaultBackend>
       weak_factory_for_connection_{this};

components/sync/trusted_vault/standalone_trusted_vault_backend_unittest.cc

@@ -45,6 +45,14 @@ base::FilePath CreateUniqueTempDir(base::ScopedTempDir* temp_dir) {
   return temp_dir->GetPath();
 }
 
+class MockDelegate : public StandaloneTrustedVaultBackend::Delegate {
+ public:
+  MockDelegate() = default;
+  ~MockDelegate() override = default;
+
+  MOCK_METHOD0(NotifyRecoverabilityDegradedChanged, void());
+};
+
 class MockTrustedVaultConnection : public TrustedVaultConnection {
  public:
   MockTrustedVaultConnection() = default;
@@ -72,11 +80,16 @@ class StandaloneTrustedVaultBackendTest : public testing::Test {
                 .Append(base::FilePath(FILE_PATH_LITERAL("some_file")))) {
     override_features.InitAndEnableFeature(
         switches::kFollowTrustedVaultKeyRotation);
+
+    auto delegate = std::make_unique<testing::NiceMock<MockDelegate>>();
+    delegate_ = delegate.get();
+
     auto connection =
         std::make_unique<testing::NiceMock<MockTrustedVaultConnection>>();
     connection_ = connection.get();
+
     backend_ = base::MakeRefCounted<StandaloneTrustedVaultBackend>(
-        file_path_, std::move(connection));
+        file_path_, std::move(delegate), std::move(connection));
   }
 
   ~StandaloneTrustedVaultBackendTest() override = default;
@@ -135,6 +148,7 @@ class StandaloneTrustedVaultBackendTest : public testing::Test {
 
   base::ScopedTempDir temp_dir_;
   const base::FilePath file_path_;
+  testing::NiceMock<MockDelegate>* delegate_;
   testing::NiceMock<MockTrustedVaultConnection>* connection_;
   scoped_refptr<StandaloneTrustedVaultBackend> backend_;
 };
@@ -230,7 +244,7 @@ TEST_F(StandaloneTrustedVaultBackendTest, ShouldFetchPreviouslyStoredKeys) {
 
   // Instantiate a second backend to read the file.
   auto other_backend = base::MakeRefCounted<StandaloneTrustedVaultBackend>(
-      file_path(),
+      file_path(), std::make_unique<testing::NiceMock<MockDelegate>>(),
       std::make_unique<testing::NiceMock<MockTrustedVaultConnection>>());
   other_backend->ReadDataFromDisk();
 

components/sync/trusted_vault/standalone_trusted_vault_client.cc

@@ -111,6 +111,25 @@ void PrimaryAccountObserver::UpdatePrimaryAccountIfNeeded() {
                      backend_, optional_primary_account));
 }
 
+// Backend delegate that dispatches delegate notifications to custom callbacks,
+// used to post notifications from the backend sequence to the UI thread.
+class BackendDelegate : public StandaloneTrustedVaultBackend::Delegate {
+ public:
+  explicit BackendDelegate(
+      const base::RepeatingClosure& notify_recoverability_degraded_cb)
+      : notify_recoverability_degraded_cb_(notify_recoverability_degraded_cb) {}
+
+  ~BackendDelegate() override = default;
+
+  // StandaloneTrustedVaultBackend::Delegate implementation.
+  void NotifyRecoverabilityDegradedChanged() override {
+    notify_recoverability_degraded_cb_.Run();
+  }
+
+ private:
+  const base::RepeatingClosure notify_recoverability_degraded_cb_;
+};
+
 }  // namespace
 
 StandaloneTrustedVaultClient::StandaloneTrustedVaultClient(
@@ -128,10 +147,15 @@ StandaloneTrustedVaultClient::StandaloneTrustedVaultClient(
   // TODO(crbug.com/1102340): allow setting custom TrustedVaultConnection for
   // testing.
   backend_ = base::MakeRefCounted<StandaloneTrustedVaultBackend>(
-      file_path, std::make_unique<TrustedVaultConnectionImpl>(
-                     /*url_loader_factory=*/nullptr,
-                     std::make_unique<TrustedVaultAccessTokenFetcherImpl>(
-                         access_token_fetcher_frontend_.GetWeakPtr())));
+      file_path,
+      std::make_unique<
+          BackendDelegate>(BindToCurrentSequence(base::BindRepeating(
+          &StandaloneTrustedVaultClient::NotifyRecoverabilityDegradedChanged,
+          weak_ptr_factory_.GetWeakPtr()))),
+      std::make_unique<TrustedVaultConnectionImpl>(
+          /*url_loader_factory=*/nullptr,
+          std::make_unique<TrustedVaultAccessTokenFetcherImpl>(
+              access_token_fetcher_frontend_.GetWeakPtr())));
   backend_task_runner_->PostTask(
       FROM_HERE,
       base::BindOnce(&StandaloneTrustedVaultBackend::ReadDataFromDisk,
@@ -145,12 +169,14 @@ StandaloneTrustedVaultClient::~StandaloneTrustedVaultClient() = default;
 std::unique_ptr<StandaloneTrustedVaultClient::Subscription>
 StandaloneTrustedVaultClient::AddKeysChangedObserver(
     const base::RepeatingClosure& cb) {
-  return observer_list_.Add(cb);
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+  return keys_observer_list_.Add(cb);
 }
 
 void StandaloneTrustedVaultClient::FetchKeys(
     const CoreAccountInfo& account_info,
     base::OnceCallback<void(const std::vector<std::vector<uint8_t>>&)> cb) {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
   DCHECK(backend_);
   backend_task_runner_->PostTask(
       FROM_HERE,
@@ -162,25 +188,28 @@ void StandaloneTrustedVaultClient::StoreKeys(
     const std::string& gaia_id,
     const std::vector<std::vector<uint8_t>>& keys,
     int last_key_version) {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
   DCHECK(backend_);
   backend_task_runner_->PostTask(
       FROM_HERE, base::BindOnce(&StandaloneTrustedVaultBackend::StoreKeys,
                                 backend_, gaia_id, keys, last_key_version));
-  observer_list_.Notify();
+  keys_observer_list_.Notify();
 }
 
 void StandaloneTrustedVaultClient::RemoveAllStoredKeys() {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
   DCHECK(backend_);
   backend_task_runner_->PostTask(
       FROM_HERE,
       base::BindOnce(&StandaloneTrustedVaultBackend::RemoveAllStoredKeys,
                      backend_));
-  observer_list_.Notify();
+  keys_observer_list_.Notify();
 }
 
 void StandaloneTrustedVaultClient::MarkKeysAsStale(
     const CoreAccountInfo& account_info,
     base::OnceCallback<void(bool)> cb) {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
   DCHECK(backend_);
   base::PostTaskAndReplyWithResult(
       backend_task_runner_.get(), FROM_HERE,
@@ -192,19 +221,45 @@ void StandaloneTrustedVaultClient::MarkKeysAsStale(
 void StandaloneTrustedVaultClient::GetIsRecoverabilityDegraded(
     const CoreAccountInfo& account_info,
     base::OnceCallback<void(bool)> cb) {
-  // TODO(crbug.com/1081649): Implement logic.
-  NOTIMPLEMENTED();
-  std::move(cb).Run(is_recoverability_degraded_for_testing_);
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+  DCHECK(backend_);
+  backend_task_runner_->PostTask(
+      FROM_HERE,
+      base::BindOnce(
+          &StandaloneTrustedVaultBackend::GetIsRecoverabilityDegraded, backend_,
+          account_info, BindToCurrentSequence(std::move(cb))));
+}
+
+std::unique_ptr<StandaloneTrustedVaultClient::Subscription>
+StandaloneTrustedVaultClient::AddRecoverabilityObserver(
+    const base::RepeatingClosure& cb) {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+  return recoverability_observer_list_.Add(cb);
+}
+
+void StandaloneTrustedVaultClient::AddTrustedRecoveryMethod(
+    const std::string& gaia_id,
+    const std::vector<uint8_t>& public_key,
+    base::OnceClosure cb) {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+  DCHECK(backend_);
+  backend_task_runner_->PostTask(
+      FROM_HERE,
+      base::BindOnce(&StandaloneTrustedVaultBackend::AddTrustedRecoveryMethod,
+                     backend_, gaia_id, public_key,
+                     BindToCurrentSequence(std::move(cb))));
 }
 
 void StandaloneTrustedVaultClient::WaitForFlushForTesting(
     base::OnceClosure cb) const {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
   backend_task_runner_->PostTaskAndReply(FROM_HERE, base::DoNothing(),
                                          std::move(cb));
 }
 
 void StandaloneTrustedVaultClient::FetchBackendPrimaryAccountForTesting(
     base::OnceCallback<void(const base::Optional<CoreAccountInfo>&)> cb) const {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
   DCHECK(backend_);
   base::PostTaskAndReplyWithResult(
       backend_task_runner_.get(), FROM_HERE,
@@ -215,7 +270,27 @@ void StandaloneTrustedVaultClient::FetchBackendPrimaryAccountForTesting(
 }
 
 void StandaloneTrustedVaultClient::SetRecoverabilityDegradedForTesting() {
-  is_recoverability_degraded_for_testing_ = true;
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+  DCHECK(backend_);
+  backend_task_runner_->PostTask(
+      FROM_HERE,
+      base::BindOnce(
+          &StandaloneTrustedVaultBackend::SetRecoverabilityDegradedForTesting,
+          backend_));
+}
+
+void StandaloneTrustedVaultClient::ResolveRecoverabilityDegradedForTesting() {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+  DCHECK(backend_);
+  backend_task_runner_->PostTask(
+      FROM_HERE, base::BindOnce(&StandaloneTrustedVaultBackend::
+                                    ResolveRecoverabilityDegradedForTesting,
+                                backend_));
+}
+
+void StandaloneTrustedVaultClient::NotifyRecoverabilityDegradedChanged() {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+  recoverability_observer_list_.Notify();
 }
 
 }  // namespace syncer

components/sync/trusted_vault/standalone_trusted_vault_client.h

@@ -12,6 +12,8 @@
 #include "base/callback.h"
 #include "base/files/file_path.h"
 #include "base/memory/scoped_refptr.h"
+#include "base/memory/weak_ptr.h"
+#include "base/sequence_checker.h"
 #include "base/sequenced_task_runner.h"
 #include "components/signin/public/identity_manager/identity_manager.h"
 #include "components/sync/driver/trusted_vault_client.h"
@@ -54,6 +56,11 @@ class StandaloneTrustedVaultClient : public TrustedVaultClient {
                        base::OnceCallback<void(bool)> cb) override;
   void GetIsRecoverabilityDegraded(const CoreAccountInfo& account_info,
                                    base::OnceCallback<void(bool)> cb) override;
+  std::unique_ptr<Subscription> AddRecoverabilityObserver(
+      const base::RepeatingClosure& cb) override;
+  void AddTrustedRecoveryMethod(const std::string& gaia_id,
+                                const std::vector<uint8_t>& public_key,
+                                base::OnceClosure cb) override;
 
   // Runs |cb| when all requests have completed.
   void WaitForFlushForTesting(base::OnceClosure cb) const;
@@ -61,11 +68,17 @@ class StandaloneTrustedVaultClient : public TrustedVaultClient {
       base::OnceCallback<void(const base::Optional<CoreAccountInfo>&)> callback)
       const;
   void SetRecoverabilityDegradedForTesting();
+  void ResolveRecoverabilityDegradedForTesting();
 
  private:
+  void NotifyRecoverabilityDegradedChanged();
+
   const scoped_refptr<base::SequencedTaskRunner> backend_task_runner_;
 
-  CallbackList observer_list_;
+  SEQUENCE_CHECKER(sequence_checker_);
+
+  CallbackList keys_observer_list_;
+  CallbackList recoverability_observer_list_;
 
   // Allows access token fetching for primary account on the ui thread. Passed
   // as WeakPtr to TrustedVaultAccessTokenFetcherImpl.
@@ -79,7 +92,7 @@ class StandaloneTrustedVaultClient : public TrustedVaultClient {
   // Holds references to |backend_| and |backend_task_runner_|.
   std::unique_ptr<signin::IdentityManager::Observer> primary_account_observer_;
 
-  bool is_recoverability_degraded_for_testing_ = false;
+  base::WeakPtrFactory<StandaloneTrustedVaultClient> weak_ptr_factory_{this};
 };
 
 }  // namespace syncer

ios/chrome/browser/sync/ios_trusted_vault_client.h

@@ -30,6 +30,11 @@ class IOSTrustedVaultClient : public syncer::TrustedVaultClient {
   void GetIsRecoverabilityDegraded(
       const CoreAccountInfo& account_info,
       base::OnceCallback<void(bool)> callback) override;
+  std::unique_ptr<Subscription> AddRecoverabilityObserver(
+      const base::RepeatingClosure& callback) override;
+  void AddTrustedRecoveryMethod(const std::string& gaia_id,
+                                const std::vector<uint8_t>& public_key,
+                                base::OnceClosure callback) override;
 
   // Not copyable or movable
   IOSTrustedVaultClient(const IOSTrustedVaultClient&) = delete;

ios/chrome/browser/sync/ios_trusted_vault_client.mm

@@ -72,3 +72,18 @@ void IOSTrustedVaultClient::GetIsRecoverabilityDegraded(
   // TODO(crbug.com/1100278): Needs implementation.
   std::move(callback).Run(false);
 }
+
+std::unique_ptr<IOSTrustedVaultClient::Subscription>
+IOSTrustedVaultClient::AddRecoverabilityObserver(
+    const base::RepeatingClosure& callback) {
+  // TODO(crbug.com/1100278): Needs implementation.
+  return nullptr;
+}
+
+void IOSTrustedVaultClient::AddTrustedRecoveryMethod(
+    const std::string& gaia_id,
+    const std::vector<uint8_t>& public_key,
+    base::OnceClosure callback) {
+  // TODO(crbug.com/1100278): Needs implementation.
+  std::move(callback).Run();
+}