Commit dd88196c authored by Mario Sanchez Prada's avatar Mario Sanchez Prada Committed by Commit Bot

Migrate the SyncEngine drive backend to the IdentityManager

Remove all trace of SigninManager from SyncEngine, which also
required some changes in clients and unit tests due to the removal
of one parameter from the SyncEngine's constructor.

Bug: 890798
Change-Id: I10596e61c46f7b86b8afb37c2dbfc401f5f651a8
Reviewed-on: https://chromium-review.googlesource.com/c/1297981
Commit-Queue: Mario Sanchez Prada <mario@igalia.com>
Reviewed-by: default avatarTaiju Tsuiki <tzik@chromium.org>
Reviewed-by: default avatarColin Blundell <blundell@chromium.org>
Cr-Commit-Position: refs/heads/master@{#606096}
parent a16ca17f
...@@ -12,7 +12,6 @@ ...@@ -12,7 +12,6 @@
#include "base/threading/thread_task_runner_handle.h" #include "base/threading/thread_task_runner_handle.h"
#include "chrome/browser/apps/platform_apps/app_browsertest_util.h" #include "chrome/browser/apps/platform_apps/app_browsertest_util.h"
#include "chrome/browser/extensions/extension_service.h" #include "chrome/browser/extensions/extension_service.h"
#include "chrome/browser/signin/fake_signin_manager_builder.h"
#include "chrome/browser/sync_file_system/drive_backend/sync_engine.h" #include "chrome/browser/sync_file_system/drive_backend/sync_engine.h"
#include "chrome/browser/sync_file_system/local/local_file_sync_service.h" #include "chrome/browser/sync_file_system/local/local_file_sync_service.h"
#include "chrome/browser/sync_file_system/sync_file_system_service.h" #include "chrome/browser/sync_file_system/sync_file_system_service.h"
...@@ -22,6 +21,7 @@ ...@@ -22,6 +21,7 @@
#include "extensions/test/extension_test_message_listener.h" #include "extensions/test/extension_test_message_listener.h"
#include "extensions/test/result_catcher.h" #include "extensions/test/result_catcher.h"
#include "services/identity/public/cpp/identity_manager.h" #include "services/identity/public/cpp/identity_manager.h"
#include "services/identity/public/cpp/identity_test_environment.h"
#include "storage/browser/quota/quota_manager.h" #include "storage/browser/quota/quota_manager.h"
#include "third_party/leveldatabase/leveldb_chrome.h" #include "third_party/leveldatabase/leveldb_chrome.h"
...@@ -83,8 +83,7 @@ class SyncFileSystemTest : public extensions::PlatformAppBrowserTest, ...@@ -83,8 +83,7 @@ class SyncFileSystemTest : public extensions::PlatformAppBrowserTest,
std::unique_ptr<drive_backend::SyncEngine::DriveServiceFactory> std::unique_ptr<drive_backend::SyncEngine::DriveServiceFactory>
drive_service_factory(new FakeDriveServiceFactory(this)); drive_service_factory(new FakeDriveServiceFactory(this));
fake_signin_manager_.reset( identity_test_env_.reset(new identity::IdentityTestEnvironment);
new FakeSigninManagerForTesting(browser()->profile()));
remote_service_ = new drive_backend::SyncEngine( remote_service_ = new drive_backend::SyncEngine(
base::ThreadTaskRunnerHandle::Get(), // ui_task_runner base::ThreadTaskRunnerHandle::Get(), // ui_task_runner
...@@ -93,9 +92,8 @@ class SyncFileSystemTest : public extensions::PlatformAppBrowserTest, ...@@ -93,9 +92,8 @@ class SyncFileSystemTest : public extensions::PlatformAppBrowserTest,
nullptr, // task_logger nullptr, // task_logger
nullptr, // notification_manager nullptr, // notification_manager
extension_service, extension_service,
fake_signin_manager_.get(), // signin_manager identity_test_env_->identity_manager(), // identity_manager
nullptr, // identity_manager nullptr, // url_loader_factory
nullptr, // url_loader_factory
std::move(drive_service_factory), in_memory_env_.get()); std::move(drive_service_factory), in_memory_env_.get());
remote_service_->SetSyncEnabled(true); remote_service_->SetSyncEnabled(true);
factory->set_mock_remote_file_service( factory->set_mock_remote_file_service(
...@@ -121,9 +119,12 @@ class SyncFileSystemTest : public extensions::PlatformAppBrowserTest, ...@@ -121,9 +119,12 @@ class SyncFileSystemTest : public extensions::PlatformAppBrowserTest,
} }
void SignIn() { void SignIn() {
fake_signin_manager_->SetAuthenticatedAccountInfo(kGaiaId, kEmail); identity_test_env_->SetPrimaryAccount(kEmail);
// It's necessary to invoke this method manually as the observer callback is
// not triggered on ChromeOS.
sync_engine()->OnPrimaryAccountSet( sync_engine()->OnPrimaryAccountSet(
fake_signin_manager_->GetAuthenticatedAccountInfo()); identity_test_env_->identity_manager()->GetPrimaryAccountInfo());
} }
void SetSyncEnabled(bool enabled) { void SetSyncEnabled(bool enabled) {
...@@ -140,7 +141,7 @@ class SyncFileSystemTest : public extensions::PlatformAppBrowserTest, ...@@ -140,7 +141,7 @@ class SyncFileSystemTest : public extensions::PlatformAppBrowserTest,
base::ScopedTempDir base_dir_; base::ScopedTempDir base_dir_;
std::unique_ptr<leveldb::Env> in_memory_env_; std::unique_ptr<leveldb::Env> in_memory_env_;
std::unique_ptr<FakeSigninManagerForTesting> fake_signin_manager_; std::unique_ptr<identity::IdentityTestEnvironment> identity_test_env_;
drive_backend::SyncEngine* remote_service_; drive_backend::SyncEngine* remote_service_;
......
...@@ -121,8 +121,7 @@ class DriveBackendSyncTest : public testing::Test, ...@@ -121,8 +121,7 @@ class DriveBackendSyncTest : public testing::Test,
nullptr, // task_logger nullptr, // task_logger
nullptr, // notification_manager nullptr, // notification_manager
nullptr, // extension_service nullptr, // extension_service
nullptr, // signin_manager nullptr, // identity_manager
nullptr, // token_service
nullptr, // url_loader_factory nullptr, // url_loader_factory
nullptr, // drive_service nullptr, // drive_service
in_memory_env_.get())); in_memory_env_.get()));
......
...@@ -18,7 +18,6 @@ ...@@ -18,7 +18,6 @@
#include "chrome/browser/extensions/extension_service.h" #include "chrome/browser/extensions/extension_service.h"
#include "chrome/browser/profiles/profile.h" #include "chrome/browser/profiles/profile.h"
#include "chrome/browser/signin/identity_manager_factory.h" #include "chrome/browser/signin/identity_manager_factory.h"
#include "chrome/browser/signin/signin_manager_factory.h"
#include "chrome/browser/sync_file_system/drive_backend/callback_helper.h" #include "chrome/browser/sync_file_system/drive_backend/callback_helper.h"
#include "chrome/browser/sync_file_system/drive_backend/conflict_resolver.h" #include "chrome/browser/sync_file_system/drive_backend/conflict_resolver.h"
#include "chrome/browser/sync_file_system/drive_backend/drive_backend_constants.h" #include "chrome/browser/sync_file_system/drive_backend/drive_backend_constants.h"
...@@ -46,7 +45,6 @@ ...@@ -46,7 +45,6 @@
#include "components/drive/drive_uploader.h" #include "components/drive/drive_uploader.h"
#include "components/drive/service/drive_api_service.h" #include "components/drive/service/drive_api_service.h"
#include "components/drive/service/drive_service_interface.h" #include "components/drive/service/drive_service_interface.h"
#include "components/signin/core/browser/signin_manager.h"
#include "content/public/browser/browser_thread.h" #include "content/public/browser/browser_thread.h"
#include "content/public/browser/network_service_instance.h" #include "content/public/browser/network_service_instance.h"
#include "content/public/browser/storage_partition.h" #include "content/public/browser/storage_partition.h"
...@@ -211,8 +209,6 @@ std::unique_ptr<SyncEngine> SyncEngine::CreateForBrowserContext( ...@@ -211,8 +209,6 @@ std::unique_ptr<SyncEngine> SyncEngine::CreateForBrowserContext(
drive::DriveNotificationManagerFactory::GetForBrowserContext(context); drive::DriveNotificationManagerFactory::GetForBrowserContext(context);
extensions::ExtensionService* extension_service = extensions::ExtensionService* extension_service =
extensions::ExtensionSystem::Get(context)->extension_service(); extensions::ExtensionSystem::Get(context)->extension_service();
SigninManagerBase* signin_manager =
SigninManagerFactory::GetForProfile(profile);
identity::IdentityManager* identity_manager = identity::IdentityManager* identity_manager =
IdentityManagerFactory::GetForProfile(profile); IdentityManagerFactory::GetForProfile(profile);
scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory = scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory =
...@@ -222,7 +218,7 @@ std::unique_ptr<SyncEngine> SyncEngine::CreateForBrowserContext( ...@@ -222,7 +218,7 @@ std::unique_ptr<SyncEngine> SyncEngine::CreateForBrowserContext(
std::unique_ptr<drive_backend::SyncEngine> sync_engine(new SyncEngine( std::unique_ptr<drive_backend::SyncEngine> sync_engine(new SyncEngine(
ui_task_runner.get(), worker_task_runner.get(), drive_task_runner.get(), ui_task_runner.get(), worker_task_runner.get(), drive_task_runner.get(),
GetSyncFileSystemDir(context->GetPath()), task_logger, GetSyncFileSystemDir(context->GetPath()), task_logger,
notification_manager, extension_service, signin_manager, identity_manager, notification_manager, extension_service, identity_manager,
url_loader_factory, std::make_unique<DriveServiceFactory>(), url_loader_factory, std::make_unique<DriveServiceFactory>(),
nullptr /* env_override */)); nullptr /* env_override */));
...@@ -234,7 +230,6 @@ void SyncEngine::AppendDependsOnFactories( ...@@ -234,7 +230,6 @@ void SyncEngine::AppendDependsOnFactories(
std::set<BrowserContextKeyedServiceFactory*>* factories) { std::set<BrowserContextKeyedServiceFactory*>* factories) {
DCHECK(factories); DCHECK(factories);
factories->insert(drive::DriveNotificationManagerFactory::GetInstance()); factories->insert(drive::DriveNotificationManagerFactory::GetInstance());
factories->insert(SigninManagerFactory::GetInstance());
factories->insert( factories->insert(
extensions::ExtensionsBrowserClient::Get()->GetExtensionSystemFactory()); extensions::ExtensionsBrowserClient::Get()->GetExtensionSystemFactory());
factories->insert(IdentityManagerFactory::GetInstance()); factories->insert(IdentityManagerFactory::GetInstance());
...@@ -271,7 +266,7 @@ void SyncEngine::Initialize() { ...@@ -271,7 +266,7 @@ void SyncEngine::Initialize() {
DCHECK_CURRENTLY_ON(content::BrowserThread::UI); DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
Reset(); Reset();
if (!signin_manager_ || !signin_manager_->IsAuthenticated()) if (!identity_manager_ || !identity_manager_->HasPrimaryAccount())
return; return;
DCHECK(drive_service_factory_); DCHECK(drive_service_factory_);
...@@ -311,8 +306,9 @@ void SyncEngine::InitializeInternal( ...@@ -311,8 +306,9 @@ void SyncEngine::InitializeInternal(
drive_service_wrapper_.reset(new DriveServiceWrapper(drive_service_.get())); drive_service_wrapper_.reset(new DriveServiceWrapper(drive_service_.get()));
std::string account_id; std::string account_id;
if (signin_manager_)
account_id = signin_manager_->GetAuthenticatedAccountId(); if (identity_manager_)
account_id = identity_manager_->GetPrimaryAccountId();
drive_service_->Initialize(account_id); drive_service_->Initialize(account_id);
drive_uploader_ = std::move(drive_uploader); drive_uploader_ = std::move(drive_uploader);
...@@ -386,7 +382,7 @@ void SyncEngine::RegisterOrigin(const GURL& origin, ...@@ -386,7 +382,7 @@ void SyncEngine::RegisterOrigin(const GURL& origin,
if (!sync_worker_) { if (!sync_worker_) {
// TODO(tzik): Record |origin| and retry the registration after late // TODO(tzik): Record |origin| and retry the registration after late
// sign-in. Then, return SYNC_STATUS_OK. // sign-in. Then, return SYNC_STATUS_OK.
if (!signin_manager_ || !signin_manager_->IsAuthenticated()) if (!identity_manager_ || !identity_manager_->HasPrimaryAccount())
callback.Run(SYNC_STATUS_AUTHENTICATION_FAILED); callback.Run(SYNC_STATUS_AUTHENTICATION_FAILED);
else else
callback.Run(SYNC_STATUS_ABORT); callback.Run(SYNC_STATUS_ABORT);
...@@ -732,7 +728,6 @@ SyncEngine::SyncEngine( ...@@ -732,7 +728,6 @@ SyncEngine::SyncEngine(
TaskLogger* task_logger, TaskLogger* task_logger,
drive::DriveNotificationManager* notification_manager, drive::DriveNotificationManager* notification_manager,
extensions::ExtensionServiceInterface* extension_service, extensions::ExtensionServiceInterface* extension_service,
SigninManagerBase* signin_manager,
identity::IdentityManager* identity_manager, identity::IdentityManager* identity_manager,
scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory, scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory,
std::unique_ptr<DriveServiceFactory> drive_service_factory, std::unique_ptr<DriveServiceFactory> drive_service_factory,
...@@ -744,7 +739,6 @@ SyncEngine::SyncEngine( ...@@ -744,7 +739,6 @@ SyncEngine::SyncEngine(
task_logger_(task_logger), task_logger_(task_logger),
notification_manager_(notification_manager), notification_manager_(notification_manager),
extension_service_(extension_service), extension_service_(extension_service),
signin_manager_(signin_manager),
identity_manager_(identity_manager), identity_manager_(identity_manager),
url_loader_factory_(url_loader_factory), url_loader_factory_(url_loader_factory),
drive_service_factory_(std::move(drive_service_factory)), drive_service_factory_(std::move(drive_service_factory)),
......
...@@ -21,7 +21,6 @@ ...@@ -21,7 +21,6 @@
#include "components/drive/drive_notification_observer.h" #include "components/drive/drive_notification_observer.h"
#include "components/drive/service/drive_service_interface.h" #include "components/drive/service/drive_service_interface.h"
#include "components/signin/core/browser/account_info.h" #include "components/signin/core/browser/account_info.h"
#include "components/signin/core/browser/signin_manager_base.h"
#include "services/identity/public/cpp/identity_manager.h" #include "services/identity/public/cpp/identity_manager.h"
#include "services/network/public/cpp/network_connection_tracker.h" #include "services/network/public/cpp/network_connection_tracker.h"
#include "services/network/public/cpp/shared_url_loader_factory.h" #include "services/network/public/cpp/shared_url_loader_factory.h"
...@@ -168,7 +167,6 @@ class SyncEngine ...@@ -168,7 +167,6 @@ class SyncEngine
TaskLogger* task_logger, TaskLogger* task_logger,
drive::DriveNotificationManager* notification_manager, drive::DriveNotificationManager* notification_manager,
extensions::ExtensionServiceInterface* extension_service, extensions::ExtensionServiceInterface* extension_service,
SigninManagerBase* signin_manager,
identity::IdentityManager* identity_manager, identity::IdentityManager* identity_manager,
scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory, scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory,
std::unique_ptr<DriveServiceFactory> drive_service_factory, std::unique_ptr<DriveServiceFactory> drive_service_factory,
...@@ -199,7 +197,6 @@ class SyncEngine ...@@ -199,7 +197,6 @@ class SyncEngine
// KeyedService::DependsOn(). // KeyedService::DependsOn().
drive::DriveNotificationManager* notification_manager_; drive::DriveNotificationManager* notification_manager_;
extensions::ExtensionServiceInterface* extension_service_; extensions::ExtensionServiceInterface* extension_service_;
SigninManagerBase* signin_manager_;
identity::IdentityManager* identity_manager_; identity::IdentityManager* identity_manager_;
scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory_; scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory_;
......
...@@ -54,8 +54,7 @@ class SyncEngineTest : public testing::Test, ...@@ -54,8 +54,7 @@ class SyncEngineTest : public testing::Test,
nullptr, // task_logger nullptr, // task_logger
nullptr, // notification_manager nullptr, // notification_manager
nullptr, // extension_service nullptr, // extension_service
nullptr, // signin_manager nullptr, // identity_manager
nullptr, // token_service
nullptr, // url_loader_factory nullptr, // url_loader_factory
nullptr, // drive_service_factory nullptr, // drive_service_factory
nullptr)); // in_memory_env nullptr)); // in_memory_env
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment