Refactor CWVSyncController for butter sync

- Replace SigninErrorController with the errors wrapped by SyncService
- Call opt ins for transport only data
- Rely on SyncService state change for delegate callbacks
- Introduce some new userInfo keys in the NSError object
- Redo unit test to use more fakes

Change-Id: Ibb954e142c254627380658dc9af1e624b96edf98
Bug: 1056416
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2103555
Commit-Queue: John Wu <jzw@chromium.org>
Reviewed-by: Hiroshi Ichikawa <ichikawa@chromium.org>
Cr-Commit-Position: refs/heads/master@{#759132}

ios/web_view/BUILD.gn

@@ -188,8 +188,6 @@ source_set("web_view_sources") {
     "internal/signin/web_view_identity_manager_factory.mm",
     "internal/signin/web_view_signin_client_factory.h",
     "internal/signin/web_view_signin_client_factory.mm",
-    "internal/signin/web_view_signin_error_controller_factory.h",
-    "internal/signin/web_view_signin_error_controller_factory.mm",
     "internal/sync/cwv_sync_controller.mm",
     "internal/sync/cwv_sync_controller_internal.h",
     "internal/sync/web_view_device_info_sync_service_factory.h",

ios/web_view/internal/cwv_web_view_configuration.mm

@@ -19,7 +19,6 @@
 #import "ios/web_view/internal/cwv_web_view_internal.h"
 #import "ios/web_view/internal/passwords/web_view_account_password_store_factory.h"
 #include "ios/web_view/internal/signin/web_view_identity_manager_factory.h"
-#include "ios/web_view/internal/signin/web_view_signin_error_controller_factory.h"
 #import "ios/web_view/internal/sync/cwv_sync_controller_internal.h"
 #import "ios/web_view/internal/sync/web_view_profile_sync_service_factory.h"
 #include "ios/web_view/internal/web_view_browser_state.h"
@@ -147,14 +146,10 @@ CWVWebViewConfiguration* gIncognitoConfiguration = nil;
     signin::IdentityManager* identityManager =
         ios_web_view::WebViewIdentityManagerFactory::GetForBrowserState(
             self.browserState);
-    SigninErrorController* signinErrorController =
-        ios_web_view::WebViewSigninErrorControllerFactory::GetForBrowserState(
-            self.browserState);
-
-    _syncController =
-        [[CWVSyncController alloc] initWithSyncService:syncService
+    _syncController = [[CWVSyncController alloc]
+        initWithSyncService:syncService
             identityManager:identityManager
-                                 signinErrorController:signinErrorController];
+                prefService:_browserState->GetPrefs()];
   }
   return _syncController;
 }
@@ -179,7 +174,6 @@ CWVWebViewConfiguration* gIncognitoConfiguration = nil;
   for (CWVWebView* webView in _webViews) {
     [webView shutDown];
   }
-  [_syncController shutDown];
   _browserState.reset();
 }
 

ios/web_view/internal/signin/web_view_signin_error_controller_factory.h

-// Copyright 2017 The Chromium Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file.
-
-#ifndef IOS_WEB_VIEW_INTERNAL_SIGNIN_WEB_VIEW_SIGNIN_ERROR_CONTROLLER_FACTORY_H_
-#define IOS_WEB_VIEW_INTERNAL_SIGNIN_WEB_VIEW_SIGNIN_ERROR_CONTROLLER_FACTORY_H_
-
-#include <memory>
-
-#include "base/macros.h"
-#include "base/no_destructor.h"
-#include "components/keyed_service/ios/browser_state_keyed_service_factory.h"
-
-class SigninErrorController;
-
-namespace ios_web_view {
-
-class WebViewBrowserState;
-
-// Singleton that owns all SigninErrorControllers and associates them with
-// a browser state.
-class WebViewSigninErrorControllerFactory
-    : public BrowserStateKeyedServiceFactory {
- public:
-  static SigninErrorController* GetForBrowserState(
-      ios_web_view::WebViewBrowserState* browser_state);
-  static WebViewSigninErrorControllerFactory* GetInstance();
-
- private:
-  friend class base::NoDestructor<WebViewSigninErrorControllerFactory>;
-
-  WebViewSigninErrorControllerFactory();
-  ~WebViewSigninErrorControllerFactory() override = default;
-
-  // BrowserStateKeyedServiceFactory implementation.
-  std::unique_ptr<KeyedService> BuildServiceInstanceFor(
-      web::BrowserState* context) const override;
-
-  DISALLOW_COPY_AND_ASSIGN(WebViewSigninErrorControllerFactory);
-};
-
-}  // namespace ios_web_view
-
-#endif  // IOS_WEB_VIEW_INTERNAL_SIGNIN_WEB_VIEW_SIGNIN_ERROR_CONTROLLER_FACTORY_H_

ios/web_view/internal/signin/web_view_signin_error_controller_factory.mm

-// Copyright 2017 The Chromium Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file.
-
-#include "ios/web_view/internal/signin/web_view_signin_error_controller_factory.h"
-
-#include <utility>
-
-#include "base/no_destructor.h"
-#include "components/keyed_service/core/service_access_type.h"
-#include "components/keyed_service/ios/browser_state_dependency_manager.h"
-#include "components/signin/core/browser/signin_error_controller.h"
-#include "ios/web_view/internal/signin/web_view_identity_manager_factory.h"
-#include "ios/web_view/internal/web_view_browser_state.h"
-
-#if !defined(__has_feature) || !__has_feature(objc_arc)
-#error "This file requires ARC support."
-#endif
-
-namespace ios_web_view {
-
-// static
-SigninErrorController* WebViewSigninErrorControllerFactory::GetForBrowserState(
-    ios_web_view::WebViewBrowserState* browser_state) {
-  return static_cast<SigninErrorController*>(
-      GetInstance()->GetServiceForBrowserState(browser_state, true));
-}
-
-// static
-WebViewSigninErrorControllerFactory*
-WebViewSigninErrorControllerFactory::GetInstance() {
-  static base::NoDestructor<WebViewSigninErrorControllerFactory> instance;
-  return instance.get();
-}
-
-WebViewSigninErrorControllerFactory::WebViewSigninErrorControllerFactory()
-    : BrowserStateKeyedServiceFactory(
-          "SigninErrorController",
-          BrowserStateDependencyManager::GetInstance()) {
-  DependsOn(WebViewIdentityManagerFactory::GetInstance());
-}
-
-std::unique_ptr<KeyedService>
-WebViewSigninErrorControllerFactory::BuildServiceInstanceFor(
-    web::BrowserState* context) const {
-  WebViewBrowserState* browser_state =
-      WebViewBrowserState::FromBrowserState(context);
-  return std::make_unique<SigninErrorController>(
-      SigninErrorController::AccountMode::ANY_ACCOUNT,
-      WebViewIdentityManagerFactory::GetForBrowserState(browser_state));
-}
-
-}  // namespace ios_web_view

ios/web_view/internal/sync/cwv_sync_controller.mm

@@ -7,19 +7,15 @@
 #import <UIKit/UIKit.h>
 #include <memory>
 
-#include "base/bind.h"
-#include "base/bind_helpers.h"
-#include "base/callback_helpers.h"
 #include "base/strings/sys_string_conversions.h"
-#include "base/time/time.h"
-#include "components/signin/core/browser/signin_error_controller.h"
+#include "components/autofill/core/common/autofill_prefs.h"
+#include "components/password_manager/core/browser/password_manager_util.h"
 #include "components/signin/public/identity_manager/account_info.h"
 #include "components/signin/public/identity_manager/device_accounts_synchronizer.h"
 #include "components/signin/public/identity_manager/identity_manager.h"
 #include "components/signin/public/identity_manager/primary_account_mutator.h"
 #include "components/sync/driver/sync_service.h"
 #include "components/sync/driver/sync_user_settings.h"
-#include "ios/web/public/thread/web_thread.h"
 #import "ios/web_view/public/cwv_identity.h"
 #import "ios/web_view/public/cwv_sync_controller_data_source.h"
 #import "ios/web_view/public/cwv_sync_controller_delegate.h"
@@ -30,6 +26,12 @@
 
 NSErrorDomain const CWVSyncErrorDomain =
     @"org.chromium.chromewebview.SyncErrorDomain";
+NSErrorUserInfoKey const CWVSyncErrorDescriptionKey =
+    @"org.chromium.chromewebview.SyncErrorDescriptionKey";
+NSErrorUserInfoKey const CWVSyncErrorMessageKey =
+    @"org.chromium.chromewebview.SyncErrorMessageKey";
+NSErrorUserInfoKey const CWVSyncErrorIsTransientKey =
+    @"org.chromium.chromewebview.SyncErrorIsTransientKey";
 
 namespace {
 CWVSyncError CWVConvertGoogleServiceAuthErrorStateToCWVSyncError(
@@ -60,18 +62,13 @@ CWVSyncError CWVConvertGoogleServiceAuthErrorStateToCWVSyncError(
 
 @interface CWVSyncController ()
 
-// Called by WebViewSyncControllerObserverBridge's
-// |OnSyncConfigurationCompleted|.
-- (void)didCompleteSyncConfiguration;
 // Called by WebViewSyncControllerObserverBridge's |OnSyncShutdown|.
 - (void)didShutdownSync;
-// Called by WebViewSyncControllerObserverBridge's |OnErrorChanged|.
-- (void)didUpdateAuthError;
-// Called by WebViewSyncControllerObserverBridge's |OnPrimaryAccountCleared|.
-- (void)didClearPrimaryAccount;
+// Called by WebViewSyncControllerObserverBridge's |OnStateChanged|.
+- (void)syncStateDidChange;
 
-// Call to refresh access tokens for |currentIdentity|.
-- (void)reloadCredentials;
+// Call to reload accounts from the |dataSource|.
+- (void)reloadAccounts;
 
 @end
 
@@ -79,33 +76,20 @@ namespace ios_web_view {
 
 // Bridge that observes syncer::SyncService and calls analagous
 // methods on CWVSyncController.
-class WebViewSyncControllerObserverBridge
-    : public syncer::SyncServiceObserver,
-      public signin::IdentityManager::Observer,
-      public SigninErrorController::Observer {
+class WebViewSyncControllerObserverBridge : public syncer::SyncServiceObserver {
  public:
   explicit WebViewSyncControllerObserverBridge(
       CWVSyncController* sync_controller)
       : sync_controller_(sync_controller) {}
 
-  // syncer::SyncServiceObserver:
-  void OnSyncConfigurationCompleted(syncer::SyncService* sync) override {
-    [sync_controller_ didCompleteSyncConfiguration];
+  void OnStateChanged(syncer::SyncService* sync) override {
+    [sync_controller_ syncStateDidChange];
   }
 
   void OnSyncShutdown(syncer::SyncService* sync) override {
     [sync_controller_ didShutdownSync];
   }
 
-  // signin::IdentityManager::Observer
-  void OnPrimaryAccountCleared(
-      const CoreAccountInfo& previous_primary_account_info) override {
-    [sync_controller_ didClearPrimaryAccount];
-  }
-
-  // SigninErrorController::Observer:
-  void OnErrorChanged() override { [sync_controller_ didUpdateAuthError]; }
-
  private:
   __weak CWVSyncController* sync_controller_;
 };
@@ -115,13 +99,12 @@ class WebViewSyncControllerObserverBridge
 @implementation CWVSyncController {
   syncer::SyncService* _syncService;
   signin::IdentityManager* _identityManager;
-  SigninErrorController* _signinErrorController;
   std::unique_ptr<ios_web_view::WebViewSyncControllerObserverBridge> _observer;
+  PrefService* _prefService;
+  syncer::SyncService::TransportState _lastTransportState;
+  GoogleServiceAuthError _lastAuthError;
 }
 
-@synthesize currentIdentity = _currentIdentity;
-@synthesize delegate = _delegate;
-
 namespace {
 // Data source that can provide access tokens.
 __weak id<CWVSyncControllerDataSource> gSyncDataSource;
@@ -137,32 +120,21 @@ __weak id<CWVSyncControllerDataSource> gSyncDataSource;
 
 - (instancetype)initWithSyncService:(syncer::SyncService*)syncService
                     identityManager:(signin::IdentityManager*)identityManager
-              signinErrorController:
-                  (SigninErrorController*)signinErrorController {
+                        prefService:(PrefService*)prefService {
   self = [super init];
   if (self) {
     _syncService = syncService;
     _identityManager = identityManager;
-    _signinErrorController = signinErrorController;
+    _prefService = prefService;
     _observer =
         std::make_unique<ios_web_view::WebViewSyncControllerObserverBridge>(
             self);
     _syncService->AddObserver(_observer.get());
-    _identityManager->AddObserver(_observer.get());
-    _signinErrorController->AddObserver(_observer.get());
-
-    if (_identityManager->HasPrimaryAccount()) {
-      CoreAccountInfo accountInfo = _identityManager->GetPrimaryAccountInfo();
-      _currentIdentity = [[CWVIdentity alloc]
-          initWithEmail:base::SysUTF8ToNSString(accountInfo.email)
-               fullName:nil
-                 gaiaID:base::SysUTF8ToNSString(accountInfo.gaia)];
-    }
 
     // Refresh access tokens on foreground to extend expiration dates.
     [[NSNotificationCenter defaultCenter]
         addObserver:self
-           selector:@selector(reloadCredentials)
+           selector:@selector(reloadAccounts)
                name:UIApplicationWillEnterForegroundNotification
              object:nil];
   }
@@ -171,23 +143,31 @@ __weak id<CWVSyncControllerDataSource> gSyncDataSource;
 
 - (void)dealloc {
   _syncService->RemoveObserver(_observer.get());
-  _identityManager->RemoveObserver(_observer.get());
-  _signinErrorController->RemoveObserver(_observer.get());
 }
 
 #pragma mark - Public Methods
 
+- (CWVIdentity*)currentIdentity {
+  if (_identityManager->HasPrimaryAccount()) {
+    CoreAccountInfo accountInfo = _identityManager->GetPrimaryAccountInfo();
+    return [[CWVIdentity alloc]
+        initWithEmail:base::SysUTF8ToNSString(accountInfo.email)
+             fullName:nil
+               gaiaID:base::SysUTF8ToNSString(accountInfo.gaia)];
+  }
+
+  return nil;
+}
+
 - (BOOL)isPassphraseNeeded {
   return _syncService->GetUserSettings()
       ->IsPassphraseRequiredForPreferredDataTypes();
 }
 
 - (void)startSyncWithIdentity:(CWVIdentity*)identity {
-  DCHECK(!_currentIdentity)
+  DCHECK(!self.currentIdentity)
       << "Already syncing! Call -stopSyncAndClearIdentity first.";
 
-  _currentIdentity = identity;
-
   const CoreAccountId accountId = _identityManager->PickAccountIdForAccount(
       base::SysNSStringToUTF8(identity.gaiaID),
       base::SysNSStringToUTF8(identity.email));
@@ -198,6 +178,13 @@ __weak id<CWVSyncControllerDataSource> gSyncDataSource;
 
   _identityManager->GetPrimaryAccountMutator()->SetPrimaryAccount(accountId);
   CHECK_EQ(_identityManager->GetPrimaryAccountId(), accountId);
+
+  autofill::prefs::SetUserOptedInWalletSyncTransport(_prefService, accountId,
+                                                     /*opted_in=*/true);
+  password_manager_util::SetDefaultPasswordStore(
+      _prefService, _syncService, autofill::PasswordForm::Store::kAccountStore);
+  password_manager_util::SetAccountStorageOptIn(_prefService, _syncService,
+                                                /*opt_in=*/true);
 }
 
 - (void)stopSyncAndClearIdentity {
@@ -215,58 +202,57 @@ __weak id<CWVSyncControllerDataSource> gSyncDataSource;
 
 #pragma mark - Private Methods
 
-- (void)didCompleteSyncConfiguration {
-  if ([_delegate respondsToSelector:@selector(syncControllerDidStartSync:)]) {
-    [_delegate syncControllerDidStartSync:self];
-  }
-}
-
 - (void)didShutdownSync {
   _syncService->RemoveObserver(_observer.get());
-  _signinErrorController->RemoveObserver(_observer.get());
-}
-
-- (void)reloadCredentials {
-  _identityManager->GetDeviceAccountsSynchronizer()
-      ->ReloadAllAccountsFromSystem();
 }
 
-#pragma mark - Internal Methods
+- (void)syncStateDidChange {
+  if (_lastTransportState != _syncService->GetTransportState()) {
+    _lastTransportState = _syncService->GetTransportState();
 
-- (void)shutDown {
-  _syncService->RemoveObserver(_observer.get());
-  _identityManager->RemoveObserver(_observer.get());
-  _signinErrorController->RemoveObserver(_observer.get());
-}
-
-- (void)didClearPrimaryAccount {
-  _currentIdentity = nil;
-  if ([_delegate respondsToSelector:@selector(syncControllerDidStopSync:)]) {
+    if (_lastTransportState == syncer::SyncService::TransportState::ACTIVE) {
+      if ([_delegate
+              respondsToSelector:@selector(syncControllerDidStartSync:)]) {
+        [_delegate syncControllerDidStartSync:self];
+      }
+    } else if (_lastTransportState ==
+               syncer::SyncService::TransportState::DISABLED) {
+      if ([_delegate
+              respondsToSelector:@selector(syncControllerDidStopSync:)]) {
         [_delegate syncControllerDidStopSync:self];
       }
-}
+    }
+  }
+
+  if (_lastAuthError.state() != _syncService->GetAuthError().state()) {
+    _lastAuthError = _syncService->GetAuthError();
 
-- (void)didUpdateAuthError {
-  GoogleServiceAuthError authError = _signinErrorController->auth_error();
-  CWVSyncError code =
-      CWVConvertGoogleServiceAuthErrorStateToCWVSyncError(authError.state());
-  if (code != CWVSyncErrorNone) {
+    CWVSyncError code = CWVConvertGoogleServiceAuthErrorStateToCWVSyncError(
+        _lastAuthError.state());
+    if (code != CWVSyncErrorNone &&
+        [_delegate respondsToSelector:@selector(syncController:
+                                              didFailWithError:)]) {
+      NSString* description =
+          base::SysUTF8ToNSString(_lastAuthError.ToString());
+      NSString* message =
+          base::SysUTF8ToNSString(_lastAuthError.error_message());
+      BOOL isTransient = _lastAuthError.IsTransientError();
       NSError* error =
           [NSError errorWithDomain:CWVSyncErrorDomain
                               code:code
                           userInfo:@{
-                          NSLocalizedDescriptionKey :
-                              base::SysUTF8ToNSString(authError.ToString())
+                            CWVSyncErrorDescriptionKey : description,
+                            CWVSyncErrorMessageKey : message,
+                            CWVSyncErrorIsTransientKey : @(isTransient),
                           }];
-    [self invokeDelegateDidFailWithError:error];
+      [_delegate syncController:self didFailWithError:error];
+    }
   }
 }
 
-- (void)invokeDelegateDidFailWithError:(NSError*)error {
-  if ([_delegate respondsToSelector:@selector(syncController:
-                                            didFailWithError:)]) {
-    [_delegate syncController:self didFailWithError:error];
-  }
+- (void)reloadAccounts {
+  _identityManager->GetDeviceAccountsSynchronizer()
+      ->ReloadAllAccountsFromSystem();
 }
 
 @end

ios/web_view/internal/sync/cwv_sync_controller_internal.h

@@ -17,21 +17,16 @@ namespace signin {
 class IdentityManager;
 }  // namespace signin
 
-class SigninErrorController;
+class PrefService;
 
 @interface CWVSyncController ()
 
 // All dependencies must out live this class.
-- (instancetype)
-       initWithSyncService:(syncer::SyncService*)syncService
+- (instancetype)initWithSyncService:(syncer::SyncService*)syncService
                     identityManager:(signin::IdentityManager*)identityManager
-     signinErrorController:(SigninErrorController*)signinErrorController
+                        prefService:(PrefService*)prefService
     NS_DESIGNATED_INITIALIZER;
 
-// Called by the associated CWVWebViewConfiguration in order to shut
-// down cleanly. See CWVWebViewConfiguration's |shutDown| method for more info.
-- (void)shutDown;
-
 @end
 
 NS_ASSUME_NONNULL_END

ios/web_view/internal/sync/cwv_sync_controller_unittest.mm

@@ -10,34 +10,31 @@
 #include "base/bind_helpers.h"
 #include "base/callback.h"
 #include "base/files/file_path.h"
+#include "base/strings/sys_string_conversions.h"
 #include "base/test/bind_test_util.h"
-#import "base/test/ios/wait_util.h"
-#include "components/signin/core/browser/signin_error_controller.h"
+#include "base/test/scoped_feature_list.h"
+#include "base/test/task_environment.h"
+#include "components/autofill/core/common/autofill_prefs.h"
+#include "components/image_fetcher/ios/ios_image_decoder_impl.h"
+#include "components/password_manager/core/browser/password_manager_util.h"
+#include "components/password_manager/core/common/password_manager_features.h"
+#include "components/password_manager/core/common/password_manager_pref_names.h"
+#include "components/prefs/pref_registry_simple.h"
+#include "components/prefs/testing_pref_service.h"
 #include "components/signin/public/base/account_consistency_method.h"
 #include "components/signin/public/base/signin_pref_names.h"
-#include "components/signin/public/identity_manager/identity_test_utils.h"
+#include "components/signin/public/base/test_signin_client.h"
+#include "components/signin/public/identity_manager/device_accounts_synchronizer.h"
+#include "components/signin/public/identity_manager/identity_manager_builder.h"
+#include "components/signin/public/identity_manager/identity_test_environment.h"
 #include "components/signin/public/identity_manager/primary_account_mutator.h"
-#include "components/sync/driver/mock_sync_service.h"
 #include "components/sync/driver/sync_service_observer.h"
+#include "components/sync/driver/test_sync_service.h"
 #include "google_apis/gaia/google_service_auth_error.h"
-#import "ios/web/public/test/fakes/test_web_state.h"
-#include "ios/web/public/test/scoped_testing_web_client.h"
-#include "ios/web/public/test/web_task_environment.h"
-#include "ios/web/public/web_client.h"
-#include "ios/web_view/internal/app/application_context.h"
-#include "ios/web_view/internal/signin/ios_web_view_signin_client.h"
 #include "ios/web_view/internal/signin/web_view_device_accounts_provider_impl.h"
-#include "ios/web_view/internal/signin/web_view_identity_manager_factory.h"
-#include "ios/web_view/internal/signin/web_view_signin_client_factory.h"
-#include "ios/web_view/internal/signin/web_view_signin_error_controller_factory.h"
-#include "ios/web_view/internal/sync/web_view_profile_sync_service_factory.h"
-#include "ios/web_view/internal/web_view_browser_state.h"
 #import "ios/web_view/public/cwv_identity.h"
 #import "ios/web_view/public/cwv_sync_controller_data_source.h"
 #import "ios/web_view/public/cwv_sync_controller_delegate.h"
-#include "ios/web_view/test/test_with_locale_and_resources.h"
-#include "services/network/public/cpp/shared_url_loader_factory.h"
-#include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #import "testing/gtest_mac.h"
 #include "testing/platform_test.h"
@@ -50,175 +47,141 @@
 namespace ios_web_view {
 namespace {
 
-using testing::_;
-using testing::Invoke;
-using testing::Return;
-
-const char kTestGaiaId[] = "1337";
 const char kTestEmail[] = "johndoe@chromium.org";
-const char kTestFullName[] = "John Doe";
-const char kTestPassphrase[] = "dummy-passphrase";
-const char kTestScope1[] = "scope1.chromium.org";
-const char kTestScope2[] = "scope2.chromium.org";
-
-using base::test::ios::kWaitForActionTimeout;
-using base::test::ios::WaitUntilConditionOrTimeout;
-
-std::unique_ptr<KeyedService> BuildMockSyncService(web::BrowserState* context) {
-  return std::make_unique<syncer::MockSyncService>();
-}
 
 }  // namespace
 
-class CWVSyncControllerTest : public TestWithLocaleAndResources {
+class CWVSyncControllerTest : public PlatformTest {
  protected:
-  CWVSyncControllerTest()
-      : web_client_(std::make_unique<web::WebClient>()),
-        browser_state_(/*off_the_record=*/false) {
-    // Clear account info before each test.
-    PrefService* pref_service = browser_state_.GetPrefs();
-    pref_service->ClearPref(prefs::kGoogleServicesAccountId);
-    pref_service->ClearPref(prefs::kGoogleServicesConsentedToSync);
-    pref_service->ClearPref(prefs::kAccountInfo);
-
-    WebViewProfileSyncServiceFactory::GetInstance()->SetTestingFactory(
-        &browser_state_, base::BindRepeating(&BuildMockSyncService));
-
-    EXPECT_CALL(*mock_sync_service(), AddObserver(_))
-        .WillOnce(Invoke(this, &CWVSyncControllerTest::AddObserver));
-
-    sync_controller_ = [[CWVSyncController alloc]
-          initWithSyncService:mock_sync_service()
-              identityManager:identity_manager()
-        signinErrorController:signin_error_controller()];
+  CWVSyncControllerTest() {
+    scoped_feature_.InitAndEnableFeature(
+        password_manager::features::kEnablePasswordsAccountStorage);
+
+    pref_service_.registry()->RegisterDictionaryPref(
+        autofill::prefs::kAutofillSyncTransportOptIn);
+    pref_service_.registry()->RegisterDictionaryPref(
+        password_manager::prefs::kAccountStoragePerAccountSettings);
+
+    // Change the default transport state to be disabled.
+    sync_service_.SetTransportState(
+        syncer::SyncService::TransportState::DISABLED);
   }
 
-  ~CWVSyncControllerTest() override {
-    EXPECT_CALL(*mock_sync_service(), RemoveObserver(_));
-    EXPECT_CALL(*mock_sync_service(), Shutdown());
-  }
-
-  void AddObserver(syncer::SyncServiceObserver* observer) {
-    sync_service_observer_ = observer;
-  }
-
-  signin::IdentityManager* identity_manager() {
-    return WebViewIdentityManagerFactory::GetForBrowserState(&browser_state_);
-  }
-
-  syncer::MockSyncService* mock_sync_service() {
-    return static_cast<syncer::MockSyncService*>(
-        WebViewProfileSyncServiceFactory::GetForBrowserState(&browser_state_));
-  }
-
-  SigninErrorController* signin_error_controller() {
-    return WebViewSigninErrorControllerFactory::GetForBrowserState(
-        &browser_state_);
-  }
-
-  web::WebTaskEnvironment task_environment_;
-  web::ScopedTestingWebClient web_client_;
-  ios_web_view::WebViewBrowserState browser_state_;
-  CWVSyncController* sync_controller_ = nil;
-  syncer::SyncServiceObserver* sync_service_observer_ = nullptr;
+  base::test::ScopedFeatureList scoped_feature_;
+  base::test::TaskEnvironment task_environment_;
+  signin::IdentityTestEnvironment identity_test_environment_;
+  syncer::TestSyncService sync_service_;
+  TestingPrefServiceSimple local_state_;
+  TestingPrefServiceSimple pref_service_;
 };
 
-// Verifies CWVSyncControllerDataSource methods are invoked with the correct
-// parameters.
-TEST_F(CWVSyncControllerTest, DataSourceCallbacks) {
-  // [data_source expect] returns an autoreleased object, but it must be
-  // destroyed before this test exits to avoid holding on to |sync_controller_|.
-  @autoreleasepool {
-    id data_source = OCMProtocolMock(@protocol(CWVSyncControllerDataSource));
-    CWVSyncController.dataSource = data_source;
-    [[data_source expect]
-        fetchAccessTokenForIdentity:[OCMArg checkWithBlock:^BOOL(
-                                                CWVIdentity* identity) {
-          return [identity.gaiaID isEqualToString:@(kTestGaiaId)];
-        }]
-                             scopes:[OCMArg checkWithBlock:^BOOL(
-                                                NSArray* scopes) {
-                               return [scopes containsObject:@(kTestScope1)] &&
-                                      [scopes containsObject:@(kTestScope2)];
-                             }]
-                  completionHandler:[OCMArg any]];
-
-    WebViewDeviceAccountsProviderImpl accounts_provider;
-    std::set<std::string> scopes = {kTestScope1, kTestScope2};
-    accounts_provider.GetAccessToken(kTestGaiaId, "dummy-client-id", scopes,
-                                     base::DoNothing());
-
-    [data_source verify];
-  }
+TEST_F(CWVSyncControllerTest, StartSyncWithIdentity) {
+  CoreAccountInfo account_info =
+      identity_test_environment_.MakeAccountAvailable(kTestEmail);
+
+  CWVIdentity* identity = [[CWVIdentity alloc]
+      initWithEmail:@(kTestEmail)
+           fullName:nil
+             gaiaID:base::SysUTF8ToNSString(account_info.gaia)];
+
+  // Preconfigure TestSyncService as if it was enabled in transport mode.
+  sync_service_.SetFirstSetupComplete(false);
+  sync_service_.SetTransportState(syncer::SyncService::TransportState::ACTIVE);
+  sync_service_.SetIsUsingSecondaryPassphrase(false);
+  sync_service_.SetAuthenticatedAccountInfo(account_info);
+
+  CWVSyncController* sync_controller = [[CWVSyncController alloc]
+      initWithSyncService:&sync_service_
+          identityManager:identity_test_environment_.identity_manager()
+              prefService:&pref_service_];
+  [sync_controller startSyncWithIdentity:identity];
+  EXPECT_NSEQ(sync_controller.currentIdentity.gaiaID, identity.gaiaID);
+
+  CoreAccountInfo primary_account_info =
+      identity_test_environment_.identity_manager()->GetPrimaryAccountInfo();
+  EXPECT_EQ(primary_account_info, account_info);
+
+  // Ensure opt-ins for transport only sync data is flipped to true.
+  EXPECT_TRUE(autofill::prefs::IsUserOptedInWalletSyncTransport(
+      &pref_service_, primary_account_info.account_id));
+  EXPECT_EQ(password_manager_util::GetDefaultPasswordStore(&pref_service_,
+                                                           &sync_service_),
+            autofill::PasswordForm::Store::kAccountStore);
+  EXPECT_TRUE(password_manager_util::IsOptedInForAccountStorage(
+      &pref_service_, &sync_service_));
 }
 
-// Verifies CWVSyncControllerDelegate methods are invoked with the correct
-// parameters.
-TEST_F(CWVSyncControllerTest, DelegateCallbacks) {
-  // [delegate expect] returns an autoreleased object, but it must be destroyed
-  // before this test exits to avoid holding on to |sync_controller_|.
-  @autoreleasepool {
-    CWVIdentity* identity = [[CWVIdentity alloc] initWithEmail:@(kTestEmail)
-                                                      fullName:@(kTestFullName)
-                                                        gaiaID:@(kTestGaiaId)];
-    id data_source = OCMProtocolMock(@protocol(CWVSyncControllerDataSource));
-    [[[data_source stub] andReturn:@[ identity ]] allKnownIdentities];
-    CWVSyncController.dataSource = data_source;
-    id delegate = OCMProtocolMock(@protocol(CWVSyncControllerDelegate));
-    sync_controller_.delegate = delegate;
-    [sync_controller_ startSyncWithIdentity:identity];
-
-    [[delegate expect] syncControllerDidStartSync:sync_controller_];
-    sync_service_observer_->OnSyncConfigurationCompleted(mock_sync_service());
-
-    [[delegate expect]
-          syncController:sync_controller_
-        didFailWithError:[OCMArg checkWithBlock:^BOOL(NSError* error) {
-          return error.code == CWVSyncErrorInvalidGAIACredentials;
-        }]];
-    GoogleServiceAuthError auth_error(
-        GoogleServiceAuthError::INVALID_GAIA_CREDENTIALS);
-    signin::UpdatePersistentErrorOfRefreshTokenForAccount(
-        identity_manager(), identity_manager()->GetPrimaryAccountId(),
-        auth_error);
+TEST_F(CWVSyncControllerTest, StopSyncAndClearIdentity) {
+  CoreAccountInfo account_info =
+      identity_test_environment_.MakePrimaryAccountAvailable(kTestEmail);
+
+  CWVSyncController* sync_controller = [[CWVSyncController alloc]
+      initWithSyncService:&sync_service_
+          identityManager:identity_test_environment_.identity_manager()
+              prefService:&pref_service_];
+  CWVIdentity* current_identity = sync_controller.currentIdentity;
+  ASSERT_TRUE(current_identity);
+  EXPECT_NSEQ(current_identity.gaiaID,
+              base::SysUTF8ToNSString(account_info.gaia));
+  EXPECT_NSEQ(current_identity.email, base::SysUTF8ToNSString(kTestEmail));
+
+  [sync_controller stopSyncAndClearIdentity];
+  EXPECT_FALSE(sync_controller.currentIdentity);
+}
+
+TEST_F(CWVSyncControllerTest, PassphraseNeeded) {
+  CWVSyncController* sync_controller = [[CWVSyncController alloc]
+      initWithSyncService:&sync_service_
+          identityManager:identity_test_environment_.identity_manager()
+              prefService:&pref_service_];
+  sync_service_.SetPassphraseRequiredForPreferredDataTypes(false);
+  EXPECT_FALSE(sync_controller.passphraseNeeded);
+  sync_service_.SetPassphraseRequiredForPreferredDataTypes(true);
+  EXPECT_TRUE(sync_controller.passphraseNeeded);
+}
 
-    [[delegate expect] syncControllerDidStopSync:sync_controller_];
-    [sync_controller_ stopSyncAndClearIdentity];
+TEST_F(CWVSyncControllerTest, DelegateDidStartAndStopSync) {
+  CWVSyncController* sync_controller = [[CWVSyncController alloc]
+      initWithSyncService:&sync_service_
+          identityManager:identity_test_environment_.identity_manager()
+              prefService:&pref_service_];
+
+  id delegate = OCMStrictProtocolMock(@protocol(CWVSyncControllerDelegate));
+  sync_controller.delegate = delegate;
+
+  // TestSyncService's transport state has to actually change before a callback
+  // will be fired, so we have to start it before we can stop it.
+  OCMExpect([delegate syncControllerDidStartSync:sync_controller]);
+  OCMExpect([delegate syncControllerDidStopSync:sync_controller]);
+  sync_service_.SetTransportState(syncer::SyncService::TransportState::ACTIVE);
+  sync_service_.FireStateChanged();
+  sync_service_.SetTransportState(
+      syncer::SyncService::TransportState::DISABLED);
+  sync_service_.FireStateChanged();
 
   [delegate verify];
-  }
 }
 
-// Verifies CWVSyncController properly maintains the current syncing user.
-TEST_F(CWVSyncControllerTest, CurrentIdentity) {
-  CWVIdentity* identity = [[CWVIdentity alloc] initWithEmail:@(kTestEmail)
-                                                    fullName:@(kTestFullName)
-                                                      gaiaID:@(kTestGaiaId)];
-  id data_source = OCMProtocolMock(@protocol(CWVSyncControllerDataSource));
-  [[[data_source stub] andReturn:@[ identity ]] allKnownIdentities];
-  CWVSyncController.dataSource = data_source;
-  [sync_controller_ startSyncWithIdentity:identity];
-  CWVIdentity* currentIdentity = sync_controller_.currentIdentity;
-  EXPECT_TRUE(currentIdentity);
-  EXPECT_NSEQ(identity.email, currentIdentity.email);
-  EXPECT_NSEQ(identity.fullName, currentIdentity.fullName);
-  EXPECT_NSEQ(identity.gaiaID, currentIdentity.gaiaID);
-
-  [sync_controller_ stopSyncAndClearIdentity];
-
-  EXPECT_FALSE(sync_controller_.currentIdentity);
-}
+TEST_F(CWVSyncControllerTest, DelegateDidFailWithError) {
+  CWVSyncController* sync_controller = [[CWVSyncController alloc]
+      initWithSyncService:&sync_service_
+          identityManager:identity_test_environment_.identity_manager()
+              prefService:&pref_service_];
+
+  id delegate = OCMStrictProtocolMock(@protocol(CWVSyncControllerDelegate));
+  sync_controller.delegate = delegate;
 
-// Verifies CWVSyncController's passphrase API.
-TEST_F(CWVSyncControllerTest, Passphrase) {
-  EXPECT_CALL(*mock_sync_service()->GetMockUserSettings(),
-              IsPassphraseRequiredForPreferredDataTypes())
-      .WillOnce(Return(true));
-  EXPECT_TRUE(sync_controller_.passphraseNeeded);
-  EXPECT_CALL(*mock_sync_service()->GetMockUserSettings(),
-              SetDecryptionPassphrase(kTestPassphrase))
-      .WillOnce(Return(true));
-  EXPECT_TRUE([sync_controller_ unlockWithPassphrase:@(kTestPassphrase)]);
+  OCMExpect([delegate
+        syncController:sync_controller
+      didFailWithError:[OCMArg checkWithBlock:^BOOL(NSError* error) {
+        return error.code == CWVSyncErrorConnectionFailed &&
+               error.domain == CWVSyncErrorDomain &&
+               [error.userInfo[CWVSyncErrorIsTransientKey] boolValue];
+      }]]);
+  sync_service_.SetAuthError(GoogleServiceAuthError::FromConnectionError(0));
+  sync_service_.FireStateChanged();
+
+  [delegate verify];
 }
 
 }  // namespace ios_web_view

ios/web_view/internal/web_view_browser_state.mm

@@ -42,7 +42,6 @@
 #include "ios/web_view/internal/passwords/web_view_password_store_factory.h"
 #include "ios/web_view/internal/signin/web_view_identity_manager_factory.h"
 #include "ios/web_view/internal/signin/web_view_signin_client_factory.h"
-#include "ios/web_view/internal/signin/web_view_signin_error_controller_factory.h"
 #import "ios/web_view/internal/sync/web_view_gcm_profile_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"
@@ -186,7 +185,6 @@ void WebViewBrowserState::RegisterPrefs(
   WebViewAccountPasswordStoreFactory::GetInstance();
   WebViewPasswordStoreFactory::GetInstance();
   WebViewSigninClientFactory::GetInstance();
-  WebViewSigninErrorControllerFactory::GetInstance();
   WebViewIdentityManagerFactory::GetInstance();
   WebViewGCMProfileServiceFactory::GetInstance();
   WebViewProfileInvalidationProviderFactory::GetInstance();

ios/web_view/public/cwv_sync_controller.h

@@ -17,6 +17,14 @@ NS_ASSUME_NONNULL_BEGIN
 
 // The error domain for sync errors.
 FOUNDATION_EXPORT CWV_EXPORT NSErrorDomain const CWVSyncErrorDomain;
+// NSString description for the type of error.
+FOUNDATION_EXPORT CWV_EXPORT
+    NSErrorUserInfoKey const CWVSyncErrorDescriptionKey;
+// NSString message describing the error in more detail.
+FOUNDATION_EXPORT CWV_EXPORT NSErrorUserInfoKey const CWVSyncErrorMessageKey;
+// NSValue wrapped BOOL indicating if the error is transient.
+FOUNDATION_EXPORT CWV_EXPORT
+    NSErrorUserInfoKey const CWVSyncErrorIsTransientKey;
 
 // Possible error codes during syncing.
 typedef NS_ENUM(NSInteger, CWVSyncError) {