Clear account-storage opt-in when signin cookies are cleared

When signin cookies are cleared, then related data like the opt-in for
using the account-scoped password storage should be cleared too.
This CL implements that by adding a function
ClearAccountStorageSettingsForAllUsers() and calling it at the
appropriate times (i.e.
IdentityManager::Observer::OnAccountsCookieDeletedByUserAction).
(This is analogous to autofill::prefs::ClearSyncTransportOptIns().)

Bug: 1052005
Change-Id: I9a1c628900b8d64573ff0eec206f151a4a128abb
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2062278
Commit-Queue: Marc Treib <treib@chromium.org>
Reviewed-by: Vadym Doroshenko <dvadym@chromium.org>
Reviewed-by: Mohamed Amir Yosef <mamir@chromium.org>
Cr-Commit-Position: refs/heads/master@{#742737}

chrome/browser/sync/chrome_sync_client.cc

@@ -30,6 +30,7 @@
 #include "chrome/browser/sharing/sharing_message_bridge.h"
 #include "chrome/browser/sharing/sharing_message_bridge_factory.h"
 #include "chrome/browser/sharing/sharing_message_model_type_controller.h"
+#include "chrome/browser/signin/identity_manager_factory.h"
 #include "chrome/browser/sync/bookmark_sync_service_factory.h"
 #include "chrome/browser/sync/device_info_sync_service_factory.h"
 #include "chrome/browser/sync/model_type_store_service_factory.h"
@@ -236,6 +237,11 @@ PrefService* ChromeSyncClient::GetPrefService() {
   return profile_->GetPrefs();
 }
 
+signin::IdentityManager* ChromeSyncClient::GetIdentityManager() {
+  DCHECK_CURRENTLY_ON(BrowserThread::UI);
+  return IdentityManagerFactory::GetForProfile(profile_);
+}
+
 base::FilePath ChromeSyncClient::GetLocalSyncBackendFolder() {
   base::FilePath local_sync_backend_folder =
       GetPrefService()->GetFilePath(syncer::prefs::kLocalSyncBackendDir);

chrome/browser/sync/chrome_sync_client.h

@@ -41,6 +41,7 @@ class ChromeSyncClient : public browser_sync::BrowserSyncClient {
 
   // BrowserSyncClient implementation.
   PrefService* GetPrefService() override;
+  signin::IdentityManager* GetIdentityManager() override;
   base::FilePath GetLocalSyncBackendFolder() override;
   syncer::ModelTypeStoreService* GetModelTypeStoreService() override;
   syncer::DeviceInfoSyncService* GetDeviceInfoSyncService() override;

components/browser_sync/profile_sync_components_factory_impl.cc

@@ -289,7 +289,8 @@ ProfileSyncComponentsFactoryImpl::CreateCommonDataTypeControllers(
                 account_password_store_
                     ? account_password_store_->CreateSyncControllerDelegate()
                     : nullptr,
-                sync_client_->GetPrefService(), sync_service,
+                sync_client_->GetPrefService(),
+                sync_client_->GetIdentityManager(), sync_service,
                 sync_client_->GetPasswordStateChangedCallback()));
       }
     } else {

components/password_manager/core/browser/password_manager_util.cc

@@ -536,4 +536,10 @@ void SetDefaultPasswordStore(PrefService* pref_service,
       .SetDefaultStore(default_store);
 }
 
+void ClearAccountStorageSettingsForAllUsers(PrefService* pref_service) {
+  DCHECK(pref_service);
+  pref_service->ClearPref(
+      password_manager::prefs::kAccountStoragePerAccountSettings);
+}
+
 }  // namespace password_manager_util

components/password_manager/core/browser/password_manager_util.h

@@ -178,16 +178,26 @@ bool ShouldShowPasswordStorePicker(const PrefService* pref_service,
 // Returns the default storage location for signed-in but non-syncing users
 // (i.e. will new passwords be saved to locally or to the account by default).
 // Always returns an actual value, never kNotSet.
+// |pref_service| must not be null.
+// |sync_service| may be null (commonly the case in incognito mode), in which
+// case this will return kProfileStore.
 autofill::PasswordForm::Store GetDefaultPasswordStore(
     const PrefService* pref_service,
     const syncer::SyncService* sync_service);
 
 // Sets the default storage location for signed-in but non-syncing users (i.e.
 // will new passwords be saved to locally or to the account by default).
+// |pref_service| and |sync_service| must not be null.
 void SetDefaultPasswordStore(PrefService* pref_service,
                              const syncer::SyncService* sync_service,
                              autofill::PasswordForm::Store default_store);
 
+// Clears all account-storage-related settings for all users. Most notably, this
+// includes the opt-in, but also all other related settings like the default
+// password store. Meant to be called when account cookies were cleared.
+// |pref_service| must not be null.
+void ClearAccountStorageSettingsForAllUsers(PrefService* pref_service);
+
 }  // namespace password_manager_util
 
 #endif  // COMPONENTS_PASSWORD_MANAGER_CORE_BROWSER_PASSWORD_MANAGER_UTIL_H_

components/password_manager/core/browser/sync/password_model_type_controller.cc

@@ -22,23 +22,31 @@ PasswordModelTypeController::PasswordModelTypeController(
     std::unique_ptr<syncer::ModelTypeControllerDelegate>
         delegate_for_transport_mode,
     PrefService* pref_service,
+    signin::IdentityManager* identity_manager,
     syncer::SyncService* sync_service,
     const base::RepeatingClosure& state_changed_callback)
     : ModelTypeController(syncer::PASSWORDS,
                           std::move(delegate_for_full_sync_mode),
                           std::move(delegate_for_transport_mode)),
       pref_service_(pref_service),
+      identity_manager_(identity_manager),
       sync_service_(sync_service),
       state_changed_callback_(state_changed_callback) {
+  // TODO(crbug.com/1024332): Use PasswordAccountStorageOptInWatcher instead of
+  // observing the pref directly.
   pref_registrar_.Init(pref_service_);
   pref_registrar_.Add(
       prefs::kAccountStoragePerAccountSettings,
       base::BindRepeating(
           &PasswordModelTypeController::OnOptInStateMaybeChanged,
           base::Unretained(this)));
+
+  identity_manager_->AddObserver(this);
 }
 
-PasswordModelTypeController::~PasswordModelTypeController() = default;
+PasswordModelTypeController::~PasswordModelTypeController() {
+  identity_manager_->RemoveObserver(this);
+}
 
 void PasswordModelTypeController::LoadModels(
     const syncer::ConfigureContext& configure_context,
@@ -89,6 +97,10 @@ void PasswordModelTypeController::OnStateChanged(syncer::SyncService* sync) {
   state_changed_callback_.Run();
 }
 
+void PasswordModelTypeController::OnAccountsCookieDeletedByUserAction() {
+  password_manager_util::ClearAccountStorageSettingsForAllUsers(pref_service_);
+}
+
 void PasswordModelTypeController::OnOptInStateMaybeChanged() {
   // Note: This method gets called in many other situations as well, not just
   // when the opt-in state changes, but DataTypePreconditionChanged() is cheap

components/password_manager/core/browser/sync/password_model_type_controller.h

@@ -10,6 +10,7 @@
 #include "base/callback.h"
 #include "base/macros.h"
 #include "components/prefs/pref_change_registrar.h"
+#include "components/signin/public/identity_manager/identity_manager.h"
 #include "components/sync/driver/model_type_controller.h"
 #include "components/sync/driver/sync_service_observer.h"
 
@@ -24,7 +25,8 @@ namespace password_manager {
 
 // A class that manages the startup and shutdown of password sync.
 class PasswordModelTypeController : public syncer::ModelTypeController,
-                                    public syncer::SyncServiceObserver {
+                                    public syncer::SyncServiceObserver,
+                                    public signin::IdentityManager::Observer {
  public:
   PasswordModelTypeController(
       std::unique_ptr<syncer::ModelTypeControllerDelegate>
@@ -32,6 +34,7 @@ class PasswordModelTypeController : public syncer::ModelTypeController,
       std::unique_ptr<syncer::ModelTypeControllerDelegate>
           delegate_for_transport_mode,
       PrefService* pref_service,
+      signin::IdentityManager* identity_manager,
       syncer::SyncService* sync_service,
       const base::RepeatingClosure& state_changed_callback);
   ~PasswordModelTypeController() override;
@@ -46,10 +49,14 @@ class PasswordModelTypeController : public syncer::ModelTypeController,
   // SyncServiceObserver overrides.
   void OnStateChanged(syncer::SyncService* sync) override;
 
+  // IdentityManager::Observer overrides.
+  void OnAccountsCookieDeletedByUserAction() override;
+
  private:
   void OnOptInStateMaybeChanged();
 
   PrefService* const pref_service_;
+  signin::IdentityManager* const identity_manager_;
   syncer::SyncService* const sync_service_;
   const base::RepeatingClosure state_changed_callback_;
 

components/sync/driver/profile_sync_service.h

@@ -88,6 +88,8 @@ class ProfileSyncService : public SyncService,
     ~InitParams();
 
     std::unique_ptr<SyncClient> sync_client;
+    // TODO(treib): Remove this and instead retrieve it via
+    // SyncClient::GetIdentityManager (but mind LocalSync).
     signin::IdentityManager* identity_manager = nullptr;
     // TODO(crbug.com/1029481): This vector only ever has one entry; replace by
     // just a pointer.

components/sync/driver/sync_client.h

@@ -21,6 +21,10 @@ namespace invalidation {
 class InvalidationService;
 }  // namespace invalidation
 
+namespace signin {
+class IdentityManager;
+}
+
 namespace syncer {
 
 class SyncApiComponentFactory;
@@ -43,6 +47,8 @@ class SyncClient {
   // Returns the current profile's preference service.
   virtual PrefService* GetPrefService() = 0;
 
+  virtual signin::IdentityManager* GetIdentityManager() = 0;
+
   virtual base::FilePath GetSyncDataPath() = 0;
 
   // Returns the path to the folder used for storing the local sync database.

components/sync/driver/sync_client_mock.h

@@ -17,6 +17,7 @@ class SyncClientMock : public SyncClient {
   ~SyncClientMock() override;
 
   MOCK_METHOD0(GetPrefService, PrefService*());
+  MOCK_METHOD0(GetIdentityManager, signin::IdentityManager*());
   MOCK_METHOD0(GetSyncDataPath, base::FilePath());
   MOCK_METHOD0(GetLocalSyncBackendFolder, base::FilePath());
   MOCK_METHOD1(CreateDataTypeControllers,

ios/chrome/browser/sync/ios_chrome_sync_client.h

@@ -34,6 +34,7 @@ class IOSChromeSyncClient : public browser_sync::BrowserSyncClient {
 
   // BrowserSyncClient implementation.
   PrefService* GetPrefService() override;
+  signin::IdentityManager* GetIdentityManager() override;
   base::FilePath GetLocalSyncBackendFolder() override;
   syncer::ModelTypeStoreService* GetModelTypeStoreService() override;
   syncer::DeviceInfoSyncService* GetDeviceInfoSyncService() override;

ios/chrome/browser/sync/ios_chrome_sync_client.mm

@@ -53,6 +53,7 @@
 #include "ios/chrome/browser/passwords/ios_chrome_password_store_factory.h"
 #include "ios/chrome/browser/pref_names.h"
 #include "ios/chrome/browser/reading_list/reading_list_model_factory.h"
+#include "ios/chrome/browser/signin/identity_manager_factory.h"
 #include "ios/chrome/browser/sync/consent_auditor_factory.h"
 #include "ios/chrome/browser/sync/device_info_sync_service_factory.h"
 #include "ios/chrome/browser/sync/ios_user_event_service_factory.h"
@@ -127,6 +128,11 @@ PrefService* IOSChromeSyncClient::GetPrefService() {
   return browser_state_->GetPrefs();
 }
 
+signin::IdentityManager* IOSChromeSyncClient::GetIdentityManager() {
+  DCHECK_CURRENTLY_ON(web::WebThread::UI);
+  return IdentityManagerFactory::GetForBrowserState(browser_state_);
+}
+
 base::FilePath IOSChromeSyncClient::GetLocalSyncBackendFolder() {
   return base::FilePath();
 }

ios/web_view/internal/sync/web_view_sync_client.h

@@ -39,6 +39,7 @@ class WebViewSyncClient : public browser_sync::BrowserSyncClient {
 
   // BrowserSyncClient implementation.
   PrefService* GetPrefService() override;
+  signin::IdentityManager* GetIdentityManager() override;
   base::FilePath GetLocalSyncBackendFolder() override;
   syncer::ModelTypeStoreService* GetModelTypeStoreService() override;
   syncer::DeviceInfoSyncService* GetDeviceInfoSyncService() override;

ios/web_view/internal/sync/web_view_sync_client.mm

@@ -32,6 +32,7 @@
 #include "ios/web/public/thread/web_thread.h"
 #include "ios/web_view/internal/passwords/web_view_password_store_factory.h"
 #include "ios/web_view/internal/pref_names.h"
+#include "ios/web_view/internal/signin/web_view_identity_manager_factory.h"
 #import "ios/web_view/internal/sync/web_view_device_info_sync_service_factory.h"
 #import "ios/web_view/internal/sync/web_view_model_type_store_service_factory.h"
 #import "ios/web_view/internal/sync/web_view_profile_invalidation_provider_factory.h"
@@ -92,6 +93,11 @@ PrefService* WebViewSyncClient::GetPrefService() {
   return browser_state_->GetPrefs();
 }
 
+signin::IdentityManager* WebViewSyncClient::GetIdentityManager() {
+  DCHECK_CURRENTLY_ON(web::WebThread::UI);
+  return WebViewIdentityManagerFactory::GetForBrowserState(browser_state_);
+}
+
 base::FilePath WebViewSyncClient::GetLocalSyncBackendFolder() {
   return base::FilePath();
 }