Commit 671afa99 authored by James Cook's avatar James Cook Committed by Commit Bot

Migrate the Chrome OS device_sync service off of the mojo pref service

DeviceSyncService runs in the browser process on the UI thread. It can
directly use the Profile's PrefService*. This simplifies the code, and
will reduce the amount of work rockot@ has to do for Service Manager
cleanup.

Delete the DeviceSyncService mojo interface and implementation object.
They primarily existed to support wiring up the mojo pref service.
Instead, directly connect the DeviceSyncClientImpl to the backing
DeviceSyncImpl.

Explicitly initialize the DeviceSyncImpl when the user's profile is
ready. The old code had startup timing dependencies that assumed that
DeviceSyncImpl was created after IdentityManager had information about
the primary profile. The new code instantiates the DeviceSyncImpl
object earlier, and IdentityManager may not be ready yet.

Bug: 977637, 1012941
Test: bots
Change-Id: Ia32b687404e5b68980cb3aecd3003e6f7e48ffcd
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1849217
Commit-Queue: James Cook <jamescook@chromium.org>
Reviewed-by: default avatarGreg Kerr <kerrnel@chromium.org>
Reviewed-by: default avatarJosh Nohle <nohle@chromium.org>
Reviewed-by: default avatarScott Violet <sky@chromium.org>
Reviewed-by: default avatarKen Rockot <rockot@google.com>
Cr-Commit-Position: refs/heads/master@{#706181}
parent 873ef4bc
......@@ -5,6 +5,7 @@
#include "chrome/browser/chromeos/device_sync/device_sync_client_factory.h"
#include "base/macros.h"
#include "base/timer/timer.h"
#include "chrome/browser/chromeos/cryptauth/client_app_metadata_provider_service.h"
#include "chrome/browser/chromeos/cryptauth/client_app_metadata_provider_service_factory.h"
#include "chrome/browser/chromeos/cryptauth/gcm_device_info_provider_impl.h"
......@@ -12,9 +13,7 @@
#include "chrome/browser/gcm/gcm_profile_service_factory.h"
#include "chrome/browser/profiles/profile.h"
#include "chrome/browser/signin/identity_manager_factory.h"
#include "chrome/common/pref_names.h"
#include "chromeos/constants/chromeos_features.h"
#include "chromeos/services/device_sync/device_sync_service.h"
#include "chromeos/services/device_sync/device_sync_impl.h"
#include "chromeos/services/device_sync/public/cpp/device_sync_client.h"
#include "chromeos/services/device_sync/public/cpp/device_sync_client_impl.h"
#include "chromeos/services/multidevice_setup/public/cpp/prefs.h"
......@@ -22,12 +21,7 @@
#include "components/keyed_service/content/browser_context_dependency_manager.h"
#include "components/keyed_service/core/keyed_service.h"
#include "components/prefs/pref_service.h"
#include "content/public/browser/browser_context.h"
#include "mojo/public/cpp/bindings/pending_remote.h"
#include "mojo/public/cpp/bindings/remote.h"
#include "services/network/public/cpp/shared_url_loader_factory.h"
#include "services/preferences/public/mojom/preferences.mojom.h"
#include "services/service_manager/public/cpp/connector.h"
namespace chromeos {
......@@ -43,24 +37,14 @@ bool IsEnrollmentAllowedByPolicy(content::BrowserContext* context) {
Profile::FromBrowserContext(context)->GetPrefs());
}
std::unique_ptr<DeviceSyncService> CreateServiceInstanceForProfile(
Profile* profile,
mojo::Remote<chromeos::device_sync::mojom::DeviceSyncService>* remote) {
mojo::Remote<chromeos::device_sync::mojom::DeviceSyncServiceInitializer>
initializer;
auto service = std::make_unique<DeviceSyncService>(
std::unique_ptr<DeviceSyncBase> CreateDeviceSyncImplForProfile(
Profile* profile) {
return DeviceSyncImpl::Factory::Get()->BuildInstance(
IdentityManagerFactory::GetForProfile(profile),
gcm::GCMProfileServiceFactory::GetForProfile(profile)->driver(),
chromeos::GcmDeviceInfoProviderImpl::GetInstance(),
profile->GetPrefs(), chromeos::GcmDeviceInfoProviderImpl::GetInstance(),
chromeos::ClientAppMetadataProviderServiceFactory::GetForProfile(profile),
profile->GetURLLoaderFactory(), initializer.BindNewPipeAndPassReceiver());
mojo::PendingRemote<prefs::mojom::PrefStoreConnector> pref_store_connector;
content::BrowserContext::GetConnectorFor(profile)->Connect(
prefs::mojom::kServiceName,
pref_store_connector.InitWithNewPipeAndPassReceiver());
initializer->Initialize(remote->BindNewPipeAndPassReceiver(),
std::move(pref_store_connector));
return service;
profile->GetURLLoaderFactory(), std::make_unique<base::OneShotTimer>());
}
} // namespace
......@@ -69,27 +53,29 @@ std::unique_ptr<DeviceSyncService> CreateServiceInstanceForProfile(
class DeviceSyncClientHolder : public KeyedService {
public:
explicit DeviceSyncClientHolder(content::BrowserContext* context)
: service_(CreateServiceInstanceForProfile(
Profile::FromBrowserContext(context),
&remote_service_)),
device_sync_client_(DeviceSyncClientImpl::Factory::Get()->BuildInstance(
remote_service_.get())) {}
: device_sync_(CreateDeviceSyncImplForProfile(
Profile::FromBrowserContext(context))),
device_sync_client_(
DeviceSyncClientImpl::Factory::Get()->BuildInstance()) {
// Connect the client's mojo interface pointer to the implementation.
device_sync_->BindRequest(
mojo::MakeRequest(device_sync_client_->GetDeviceSyncPtr()));
// Finish client initialization.
device_sync_client_->Initialize(base::ThreadTaskRunnerHandle::Get());
}
DeviceSyncBase* device_sync() { return device_sync_.get(); }
DeviceSyncClient* device_sync_client() { return device_sync_client_.get(); }
private:
// KeyedService:
void Shutdown() override {
device_sync_client_.reset();
service_.reset();
device_sync_->CloseAllBindings();
device_sync_.reset();
}
mojo::Remote<chromeos::device_sync::mojom::DeviceSyncService> remote_service_;
// The in-process service instance. Never exposed publicly except through the
// DeviceSyncClient, which is isolated from the service by Mojo interfaces.
std::unique_ptr<chromeos::device_sync::DeviceSyncService> service_;
std::unique_ptr<DeviceSyncBase> device_sync_;
std::unique_ptr<DeviceSyncClient> device_sync_client_;
DISALLOW_COPY_AND_ASSIGN(DeviceSyncClientHolder);
......@@ -118,6 +104,15 @@ DeviceSyncClientFactory* DeviceSyncClientFactory::GetInstance() {
return base::Singleton<DeviceSyncClientFactory>::get();
}
// static
void DeviceSyncClientFactory::OnProfileInitialized(Profile* profile) {
DeviceSyncClientHolder* holder = static_cast<DeviceSyncClientHolder*>(
GetInstance()->GetServiceForBrowserContext(profile, true));
if (holder)
holder->device_sync()->OnProfileInitialized();
}
KeyedService* DeviceSyncClientFactory::BuildServiceInstanceFor(
content::BrowserContext* context) const {
// TODO(crbug.com/848347): Check prohibited by policy in services that depend
......
......@@ -24,6 +24,9 @@ class DeviceSyncClientFactory : public BrowserContextKeyedServiceFactory {
static DeviceSyncClientFactory* GetInstance();
// Notifies the DeviceSync subsystem that the profile is ready.
static void OnProfileInitialized(Profile* profile);
private:
friend struct base::DefaultSingletonTraits<DeviceSyncClientFactory>;
......
......@@ -47,6 +47,7 @@
#include "chrome/browser/chromeos/child_accounts/consumer_status_reporting_service_factory.h"
#include "chrome/browser/chromeos/child_accounts/screen_time_controller_factory.h"
#include "chrome/browser/chromeos/crostini/crostini_manager.h"
#include "chrome/browser/chromeos/device_sync/device_sync_client_factory.h"
#include "chrome/browser/chromeos/first_run/first_run.h"
#include "chrome/browser/chromeos/first_run/goodies_displayer.h"
#include "chrome/browser/chromeos/lock_screen_apps/state_controller.h"
......@@ -1464,6 +1465,9 @@ void UserSessionManager::InitProfilePreferences(
user_manager::known_user::UpdateGaiaID(user_context.GetAccountId(),
gaia_id);
}
// DeviceSync initialization must occur after primary profile is available.
device_sync::DeviceSyncClientFactory::OnProfileInitialized(profile);
}
}
......
......@@ -266,8 +266,8 @@ class FakeDeviceSyncClientImplFactory
~FakeDeviceSyncClientImplFactory() override = default;
// chromeos::device_sync::DeviceSyncClientImpl::Factory:
std::unique_ptr<chromeos::device_sync::DeviceSyncClient> BuildInstance(
chromeos::device_sync::mojom::DeviceSyncService* service) override {
std::unique_ptr<chromeos::device_sync::DeviceSyncClient> BuildInstance()
override {
auto fake_device_sync_client =
std::make_unique<chromeos::device_sync::FakeDeviceSyncClient>();
fake_device_sync_client->NotifyReady();
......
......@@ -308,6 +308,7 @@
#include "chromeos/network/fast_transition_observer.h"
#include "chromeos/network/proxy/proxy_config_handler.h"
#include "chromeos/services/assistant/public/cpp/assistant_prefs.h"
#include "chromeos/services/device_sync/device_sync_impl.h"
#include "chromeos/services/multidevice_setup/multidevice_setup_service.h"
#include "chromeos/timezone/timezone_resolver.h"
#include "components/arc/arc_prefs.h"
......@@ -913,6 +914,7 @@ void RegisterProfilePrefs(user_prefs::PrefRegistrySyncable* registry,
chromeos::AccountManager::RegisterPrefs(registry);
chromeos::assistant::prefs::RegisterProfilePrefsForBrowser(registry);
chromeos::CupsPrintersManager::RegisterProfilePrefs(registry);
chromeos::device_sync::DeviceSyncImpl::RegisterProfilePrefs(registry);
chromeos::first_run::RegisterProfilePrefs(registry);
chromeos::file_system_provider::RegisterProfilePrefs(registry);
chromeos::KerberosCredentialsManager::RegisterProfilePrefs(registry);
......
......@@ -126,8 +126,7 @@ class FakeDeviceSyncImplFactory
std::unique_ptr<chromeos::device_sync::DeviceSyncBase> BuildInstance(
signin::IdentityManager* identity_manager,
gcm::GCMDriver* gcm_driver,
mojo::PendingRemote<prefs::mojom::PrefStoreConnector>
pref_store_connector,
PrefService* profile_prefs,
const chromeos::device_sync::GcmDeviceInfoProvider*
gcm_device_info_provider,
chromeos::device_sync::ClientAppMetadataProvider*
......
......@@ -106,8 +106,6 @@ static_library("device_sync") {
"device_sync_base.h",
"device_sync_impl.cc",
"device_sync_impl.h",
"device_sync_service.cc",
"device_sync_service.h",
"device_sync_type_converters.cc",
"device_sync_type_converters.h",
"network_request_error.cc",
......@@ -152,8 +150,8 @@ static_library("device_sync") {
"//chromeos/services/device_sync/public/cpp",
"//chromeos/services/device_sync/public/mojom",
"//components/gcm_driver",
"//components/prefs",
"//net",
"//services/preferences/public/cpp",
]
visibility = [
......
......@@ -8,6 +8,5 @@ include_rules = [
"+mojo/public/cpp",
"+services/network/public",
"+services/network/test",
"+services/preferences/public",
"+components/cryptauth"
]
......@@ -2,11 +2,12 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
#include "chromeos/services/device_sync/device_sync_base.h"
#include <utility>
#include "base/bind.h"
#include "base/callback.h"
#include "chromeos/services/device_sync/device_sync_base.h"
namespace chromeos {
......
......@@ -21,6 +21,9 @@ class DeviceSyncBase : public mojom::DeviceSync {
public:
~DeviceSyncBase() override;
// Informs the implementation that accounts and preferences are ready to use.
virtual void OnProfileInitialized() {}
// mojom::DeviceSync:
void AddObserver(mojom::DeviceSyncObserverPtr observer,
AddObserverCallback callback) override;
......
......@@ -11,7 +11,7 @@
#include "base/no_destructor.h"
#include "base/optional.h"
#include "base/time/default_clock.h"
#include "base/token.h"
#include "base/unguessable_token.h"
#include "chromeos/components/multidevice/logging/logging.h"
#include "chromeos/components/multidevice/secure_message_delegate_impl.h"
#include "chromeos/constants/chromeos_features.h"
......@@ -41,19 +41,6 @@ namespace device_sync {
namespace {
void RegisterDeviceSyncPrefs(PrefRegistrySimple* registry) {
CryptAuthGCMManager::RegisterPrefs(registry);
CryptAuthDeviceManager::RegisterPrefs(registry);
if (base::FeatureList::IsEnabled(
chromeos::features::kCryptAuthV2Enrollment)) {
CryptAuthV2EnrollmentManagerImpl::RegisterPrefs(registry);
CryptAuthKeyRegistryImpl::RegisterPrefs(registry);
CryptAuthSchedulerImpl::RegisterPrefs(registry);
} else {
CryptAuthEnrollmentManagerImpl::RegisterPrefs(registry);
}
}
constexpr base::TimeDelta kSetFeatureEnabledTimeout =
base::TimeDelta::FromSeconds(5);
......@@ -220,32 +207,15 @@ DeviceSyncImpl::Factory::~Factory() = default;
std::unique_ptr<DeviceSyncBase> DeviceSyncImpl::Factory::BuildInstance(
signin::IdentityManager* identity_manager,
gcm::GCMDriver* gcm_driver,
mojo::PendingRemote<prefs::mojom::PrefStoreConnector> pref_store_connector,
PrefService* profile_prefs,
const GcmDeviceInfoProvider* gcm_device_info_provider,
ClientAppMetadataProvider* client_app_metadata_provider,
scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory,
std::unique_ptr<base::OneShotTimer> timer) {
return base::WrapUnique(new DeviceSyncImpl(
identity_manager, gcm_driver, std::move(pref_store_connector),
gcm_device_info_provider, client_app_metadata_provider,
std::move(url_loader_factory), base::DefaultClock::GetInstance(),
std::make_unique<PrefConnectionDelegate>(), std::move(timer)));
}
DeviceSyncImpl::PrefConnectionDelegate::~PrefConnectionDelegate() = default;
scoped_refptr<PrefRegistrySimple>
DeviceSyncImpl::PrefConnectionDelegate::CreatePrefRegistry() {
return base::MakeRefCounted<PrefRegistrySimple>();
}
void DeviceSyncImpl::PrefConnectionDelegate::ConnectToPrefService(
mojo::PendingRemote<prefs::mojom::PrefStoreConnector> pref_store_connector,
scoped_refptr<PrefRegistrySimple> pref_registry,
prefs::ConnectCallback callback) {
prefs::ConnectToPrefService(std::move(pref_store_connector),
std::move(pref_registry),
base::Token::CreateRandom(), std::move(callback));
identity_manager, gcm_driver, profile_prefs, gcm_device_info_provider,
client_app_metadata_provider, std::move(url_loader_factory),
base::DefaultClock::GetInstance(), std::move(timer)));
}
DeviceSyncImpl::PendingSetSoftwareFeatureRequest::
......@@ -299,29 +269,42 @@ void DeviceSyncImpl::PendingSetSoftwareFeatureRequest::InvokeCallback(
std::move(callback_).Run(result);
}
// static
void DeviceSyncImpl::RegisterProfilePrefs(PrefRegistrySimple* registry) {
CryptAuthGCMManager::RegisterPrefs(registry);
CryptAuthDeviceManager::RegisterPrefs(registry);
if (base::FeatureList::IsEnabled(
chromeos::features::kCryptAuthV2Enrollment)) {
CryptAuthV2EnrollmentManagerImpl::RegisterPrefs(registry);
CryptAuthKeyRegistryImpl::RegisterPrefs(registry);
CryptAuthSchedulerImpl::RegisterPrefs(registry);
} else {
CryptAuthEnrollmentManagerImpl::RegisterPrefs(registry);
}
}
DeviceSyncImpl::DeviceSyncImpl(
signin::IdentityManager* identity_manager,
gcm::GCMDriver* gcm_driver,
mojo::PendingRemote<prefs::mojom::PrefStoreConnector> pref_store_connector,
PrefService* profile_prefs,
const GcmDeviceInfoProvider* gcm_device_info_provider,
ClientAppMetadataProvider* client_app_metadata_provider,
scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory,
base::Clock* clock,
std::unique_ptr<PrefConnectionDelegate> pref_connection_delegate,
std::unique_ptr<base::OneShotTimer> timer)
: DeviceSyncBase(),
identity_manager_(identity_manager),
gcm_driver_(gcm_driver),
pref_store_connector_(std::move(pref_store_connector)),
profile_prefs_(profile_prefs),
gcm_device_info_provider_(gcm_device_info_provider),
client_app_metadata_provider_(client_app_metadata_provider),
url_loader_factory_(std::move(url_loader_factory)),
clock_(clock),
pref_connection_delegate_(std::move(pref_connection_delegate)),
set_software_feature_timer_(std::move(timer)),
status_(Status::FETCHING_ACCOUNT_INFO) {
status_(Status::WAITING_FOR_PROFILE_INIT) {
DCHECK(profile_prefs_);
PA_LOG(VERBOSE) << "DeviceSyncImpl: Initializing.";
ProcessPrimaryAccountInfo(identity_manager_->GetPrimaryAccountInfo());
// Initialization will complete when the profile is fully loaded.
}
DeviceSyncImpl::~DeviceSyncImpl() {
......@@ -551,6 +534,11 @@ void DeviceSyncImpl::OnSyncDeviceListChanged() {
}
}
void DeviceSyncImpl::OnProfileInitialized() {
PA_LOG(VERBOSE) << "DeviceSyncImpl: OnProfileInitialized";
ProcessPrimaryAccountInfo(identity_manager_->GetPrimaryAccountInfo());
}
void DeviceSyncImpl::Shutdown() {
software_feature_manager_.reset();
remote_device_provider_.reset();
......@@ -560,10 +548,10 @@ void DeviceSyncImpl::Shutdown() {
cryptauth_key_registry_.reset();
cryptauth_client_factory_.reset();
cryptauth_gcm_manager_.reset();
pref_connection_delegate_.reset();
identity_manager_ = nullptr;
gcm_driver_ = nullptr;
profile_prefs_ = nullptr;
gcm_device_info_provider_ = nullptr;
client_app_metadata_provider_ = nullptr;
url_loader_factory_ = nullptr;
......@@ -582,31 +570,12 @@ void DeviceSyncImpl::ProcessPrimaryAccountInfo(
}
primary_account_info_ = primary_account_info;
ConnectToPrefStore();
}
void DeviceSyncImpl::ConnectToPrefStore() {
DCHECK(status_ == Status::FETCHING_ACCOUNT_INFO);
status_ = Status::CONNECTING_TO_USER_PREFS;
auto pref_registry = pref_connection_delegate_->CreatePrefRegistry();
RegisterDeviceSyncPrefs(pref_registry.get());
PA_LOG(VERBOSE) << "DeviceSyncImpl: Connecting to pref service.";
pref_connection_delegate_->ConnectToPrefService(
std::move(pref_store_connector_), std::move(pref_registry),
base::Bind(&DeviceSyncImpl::OnConnectedToPrefService,
weak_ptr_factory_.GetWeakPtr()));
}
void DeviceSyncImpl::OnConnectedToPrefService(
std::unique_ptr<PrefService> pref_service) {
DCHECK(status_ == Status::CONNECTING_TO_USER_PREFS);
DCHECK(status_ == Status::WAITING_FOR_PROFILE_INIT);
status_ = Status::WAITING_FOR_ENROLLMENT;
PA_LOG(VERBOSE) << "DeviceSyncImpl: Connected to pref service; initializing "
PA_LOG(VERBOSE) << "DeviceSyncImpl: Profile initialized; initializing "
<< "CryptAuth managers.";
pref_service_ = std::move(pref_service);
InitializeCryptAuthManagementObjects();
// If enrollment has not yet completed successfully, initialization cannot
......@@ -626,7 +595,7 @@ void DeviceSyncImpl::InitializeCryptAuthManagementObjects() {
// Initialize |cryptauth_gcm_manager_| and have it start listening for GCM
// tickles.
cryptauth_gcm_manager_ = CryptAuthGCMManagerImpl::Factory::NewInstance(
gcm_driver_, pref_service_.get());
gcm_driver_, profile_prefs_);
cryptauth_gcm_manager_->StartListening();
cryptauth_client_factory_ = std::make_unique<CryptAuthClientFactoryImpl>(
......@@ -637,25 +606,23 @@ void DeviceSyncImpl::InitializeCryptAuthManagementObjects() {
// called yet since the device has not completed enrollment.
cryptauth_device_manager_ = CryptAuthDeviceManagerImpl::Factory::NewInstance(
clock_, cryptauth_client_factory_.get(), cryptauth_gcm_manager_.get(),
pref_service_.get());
profile_prefs_);
// Initialize |cryptauth_enrollment_manager_| and start observing, then call
// Start() immediately to schedule enrollment.
if (base::FeatureList::IsEnabled(
chromeos::features::kCryptAuthV2Enrollment)) {
cryptauth_key_registry_ =
CryptAuthKeyRegistryImpl::Factory::Get()->BuildInstance(
pref_service_.get());
CryptAuthKeyRegistryImpl::Factory::Get()->BuildInstance(profile_prefs_);
cryptauth_scheduler_ =
CryptAuthSchedulerImpl::Factory::Get()->BuildInstance(
pref_service_.get());
CryptAuthSchedulerImpl::Factory::Get()->BuildInstance(profile_prefs_);
cryptauth_enrollment_manager_ =
CryptAuthV2EnrollmentManagerImpl::Factory::Get()->BuildInstance(
client_app_metadata_provider_, cryptauth_key_registry_.get(),
cryptauth_client_factory_.get(), cryptauth_gcm_manager_.get(),
cryptauth_scheduler_.get(), pref_service_.get(), clock_);
cryptauth_scheduler_.get(), profile_prefs_, clock_);
} else {
cryptauth_enrollment_manager_ =
CryptAuthEnrollmentManagerImpl::Factory::NewInstance(
......@@ -664,7 +631,7 @@ void DeviceSyncImpl::InitializeCryptAuthManagementObjects() {
cryptauth_client_factory_.get()),
multidevice::SecureMessageDelegateImpl::Factory::NewInstance(),
gcm_device_info_provider_->GetGcmDeviceInfo(),
cryptauth_gcm_manager_.get(), pref_service_.get());
cryptauth_gcm_manager_.get(), profile_prefs_);
}
cryptauth_enrollment_manager_->AddObserver(this);
......@@ -827,11 +794,6 @@ void DeviceSyncImpl::OnSetSoftwareFeatureTimerFired() {
}
}
void DeviceSyncImpl::SetPrefConnectionDelegateForTesting(
std::unique_ptr<PrefConnectionDelegate> pref_connection_delegate) {
pref_connection_delegate_ = std::move(pref_connection_delegate);
}
} // namespace device_sync
} // namespace chromeos
......@@ -18,11 +18,8 @@
#include "chromeos/services/device_sync/public/mojom/device_sync.mojom.h"
#include "chromeos/services/device_sync/remote_device_provider.h"
#include "components/signin/public/identity_manager/account_info.h"
#include "mojo/public/cpp/bindings/pending_remote.h"
#include "mojo/public/cpp/bindings/remote.h"
#include "services/preferences/public/cpp/pref_service_factory.h"
#include "services/preferences/public/mojom/preferences.mojom.h"
class PrefRegistrySimple;
class PrefService;
namespace base {
......@@ -77,8 +74,7 @@ class DeviceSyncImpl : public DeviceSyncBase,
virtual std::unique_ptr<DeviceSyncBase> BuildInstance(
signin::IdentityManager* identity_manager,
gcm::GCMDriver* gcm_driver,
mojo::PendingRemote<prefs::mojom::PrefStoreConnector>
pref_store_conector,
PrefService* profile_prefs,
const GcmDeviceInfoProvider* gcm_device_info_provider,
ClientAppMetadataProvider* client_app_metadata_provider,
scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory,
......@@ -88,6 +84,8 @@ class DeviceSyncImpl : public DeviceSyncBase,
static Factory* test_factory_instance_;
};
static void RegisterProfilePrefs(PrefRegistrySimple* registry);
~DeviceSyncImpl() override;
protected:
......@@ -117,27 +115,7 @@ class DeviceSyncImpl : public DeviceSyncBase,
private:
friend class DeviceSyncServiceTest;
enum class Status {
FETCHING_ACCOUNT_INFO,
CONNECTING_TO_USER_PREFS,
WAITING_FOR_ENROLLMENT,
READY
};
// Wrapper around preferences code. This class is necessary so that tests can
// override this functionality to use a fake PrefService rather than a real
// connection to the Preferences service.
class PrefConnectionDelegate {
public:
virtual ~PrefConnectionDelegate();
virtual scoped_refptr<PrefRegistrySimple> CreatePrefRegistry();
virtual void ConnectToPrefService(
mojo::PendingRemote<prefs::mojom::PrefStoreConnector>
pref_store_connector,
scoped_refptr<PrefRegistrySimple> pref_registry,
prefs::ConnectCallback callback);
};
enum class Status { WAITING_FOR_PROFILE_INIT, WAITING_FOR_ENROLLMENT, READY };
class PendingSetSoftwareFeatureRequest {
public:
......@@ -172,21 +150,18 @@ class DeviceSyncImpl : public DeviceSyncBase,
DeviceSyncImpl(
signin::IdentityManager* identity_manager,
gcm::GCMDriver* gcm_driver,
mojo::PendingRemote<prefs::mojom::PrefStoreConnector>
pref_store_connector,
PrefService* profile_prefs,
const GcmDeviceInfoProvider* gcm_device_info_provider,
ClientAppMetadataProvider* client_app_metadata_provider,
scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory,
base::Clock* clock,
std::unique_ptr<PrefConnectionDelegate> pref_connection_delegate,
std::unique_ptr<base::OneShotTimer> timer);
// DeviceSyncBase:
void OnProfileInitialized() override;
void Shutdown() override;
void ProcessPrimaryAccountInfo(const CoreAccountInfo& primary_account_info);
void ConnectToPrefStore();
void OnConnectedToPrefService(std::unique_ptr<PrefService> pref_service);
void InitializeCryptAuthManagementObjects();
void CompleteInitializationAfterSuccessfulEnrollment();
......@@ -221,22 +196,17 @@ class DeviceSyncImpl : public DeviceSyncBase,
void StartSetSoftwareFeatureTimer();
void OnSetSoftwareFeatureTimerFired();
void SetPrefConnectionDelegateForTesting(
std::unique_ptr<PrefConnectionDelegate> pref_connection_delegate);
signin::IdentityManager* identity_manager_;
gcm::GCMDriver* gcm_driver_;
mojo::PendingRemote<prefs::mojom::PrefStoreConnector> pref_store_connector_;
PrefService* profile_prefs_;
const GcmDeviceInfoProvider* gcm_device_info_provider_;
ClientAppMetadataProvider* client_app_metadata_provider_;
scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory_;
base::Clock* clock_;
std::unique_ptr<PrefConnectionDelegate> pref_connection_delegate_;
std::unique_ptr<base::OneShotTimer> set_software_feature_timer_;
Status status_;
CoreAccountInfo primary_account_info_;
std::unique_ptr<PrefService> pref_service_;
base::flat_map<base::UnguessableToken,
std::unique_ptr<PendingSetSoftwareFeatureRequest>>
id_to_pending_set_software_feature_request_map_;
......
// Copyright 2018 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 "chromeos/services/device_sync/device_sync_service.h"
#include "base/bind.h"
#include "base/timer/timer.h"
#include "chromeos/components/multidevice/logging/logging.h"
#include "chromeos/services/device_sync/device_sync_base.h"
#include "chromeos/services/device_sync/device_sync_impl.h"
#include "services/network/public/cpp/shared_url_loader_factory.h"
namespace chromeos {
namespace device_sync {
DeviceSyncService::DeviceSyncService(
signin::IdentityManager* identity_manager,
gcm::GCMDriver* gcm_driver,
const GcmDeviceInfoProvider* gcm_device_info_provider,
ClientAppMetadataProvider* client_app_metadata_provider,
scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory,
mojo::PendingReceiver<mojom::DeviceSyncServiceInitializer> init_receiver)
: init_receiver_(this, std::move(init_receiver)),
identity_manager_(identity_manager),
gcm_driver_(gcm_driver),
gcm_device_info_provider_(gcm_device_info_provider),
client_app_metadata_provider_(client_app_metadata_provider),
url_loader_factory_(std::move(url_loader_factory)) {}
DeviceSyncService::~DeviceSyncService() {
// Subclasses may hold onto message response callbacks. It's important that
// all bindings are closed by the time those callbacks are destroyed, or they
// will DCHECK.
if (device_sync_)
device_sync_->CloseAllBindings();
}
void DeviceSyncService::Initialize(
mojo::PendingReceiver<mojom::DeviceSyncService> receiver,
mojo::PendingRemote<prefs::mojom::PrefStoreConnector>
pref_store_connector) {
PA_LOG(VERBOSE) << "DeviceSyncService::Init()";
receiver_.Bind(std::move(receiver));
device_sync_ = DeviceSyncImpl::Factory::Get()->BuildInstance(
identity_manager_, gcm_driver_, std::move(pref_store_connector),
gcm_device_info_provider_, client_app_metadata_provider_,
url_loader_factory_, std::make_unique<base::OneShotTimer>());
}
void DeviceSyncService::BindDeviceSync(
mojo::PendingReceiver<mojom::DeviceSync> receiver) {
PA_LOG(VERBOSE) << "DeviceSyncService::BindDeviceSync()";
device_sync_->BindRequest(std::move(receiver));
}
} // namespace device_sync
} // namespace chromeos
// Copyright 2018 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 CHROMEOS_SERVICES_DEVICE_SYNC_DEVICE_SYNC_SERVICE_H_
#define CHROMEOS_SERVICES_DEVICE_SYNC_DEVICE_SYNC_SERVICE_H_
#include <memory>
#include "base/memory/ref_counted.h"
#include "chromeos/services/device_sync/public/mojom/device_sync.mojom.h"
#include "mojo/public/cpp/bindings/pending_receiver.h"
#include "mojo/public/cpp/bindings/receiver.h"
namespace gcm {
class GCMDriver;
} // namespace gcm
namespace signin {
class IdentityManager;
} // namespace signin
namespace network {
class SharedURLLoaderFactory;
} // namespace network
namespace chromeos {
namespace device_sync {
class ClientAppMetadataProvider;
class DeviceSyncBase;
class GcmDeviceInfoProvider;
// Service which provides an implementation for DeviceSync. This service creates
// one implementation and shares it among all connection requests.
class DeviceSyncService : public mojom::DeviceSyncServiceInitializer,
public mojom::DeviceSyncService {
public:
DeviceSyncService(
signin::IdentityManager* identity_manager,
gcm::GCMDriver* gcm_driver,
const GcmDeviceInfoProvider* gcm_device_info_provider,
ClientAppMetadataProvider* client_app_metadata_provider,
scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory,
mojo::PendingReceiver<mojom::DeviceSyncServiceInitializer> init_receiver);
~DeviceSyncService() override;
private:
// mojom::DeviceSyncServiceInitializer:
void Initialize(mojo::PendingReceiver<mojom::DeviceSyncService> receiver,
mojo::PendingRemote<prefs::mojom::PrefStoreConnector>
pref_store_connector) override;
// mojom::DeviceSyncService:
void BindDeviceSync(
mojo::PendingReceiver<mojom::DeviceSync> receiver) override;
mojo::Receiver<mojom::DeviceSyncServiceInitializer> init_receiver_;
mojo::Receiver<mojom::DeviceSyncService> receiver_{this};
signin::IdentityManager* identity_manager_;
gcm::GCMDriver* gcm_driver_;
const GcmDeviceInfoProvider* gcm_device_info_provider_;
ClientAppMetadataProvider* client_app_metadata_provider_;
scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory_;
std::unique_ptr<DeviceSyncBase> device_sync_;
DISALLOW_COPY_AND_ASSIGN(DeviceSyncService);
};
} // namespace device_sync
} // namespace chromeos
#endif // CHROMEOS_SERVICES_DEVICE_SYNC_DEVICE_SYNC_SERVICE_H_
......@@ -69,6 +69,7 @@ source_set("unit_tests") {
"//chromeos/services/device_sync",
"//chromeos/services/device_sync:test_support",
"//components/gcm_driver:test_support",
"//components/prefs:test_support",
"//components/signin/public/identity_manager:test_support",
"//net",
"//services/network:test_support",
......
......@@ -12,6 +12,10 @@ DeviceSyncClient::DeviceSyncClient() = default;
DeviceSyncClient::~DeviceSyncClient() = default;
mojom::DeviceSyncPtr* DeviceSyncClient::GetDeviceSyncPtr() {
return nullptr;
}
void DeviceSyncClient::AddObserver(Observer* observer) {
observer_list_.AddObserver(observer);
}
......
......@@ -10,12 +10,17 @@
#include "base/callback.h"
#include "base/macros.h"
#include "base/memory/scoped_refptr.h"
#include "base/observer_list.h"
#include "base/optional.h"
#include "chromeos/components/multidevice/remote_device_ref.h"
#include "chromeos/components/multidevice/software_feature.h"
#include "chromeos/services/device_sync/public/mojom/device_sync.mojom.h"
namespace base {
class TaskRunner;
} // namespace base
namespace chromeos {
namespace device_sync {
......@@ -47,6 +52,13 @@ class DeviceSyncClient {
DeviceSyncClient();
virtual ~DeviceSyncClient();
// Completes initialization. Must be called after connecting the DeviceSync
// mojo interface pointer to the implementation.
virtual void Initialize(scoped_refptr<base::TaskRunner> task_runner) {}
// Returns the DeviceSync mojo interface pointer.
virtual mojom::DeviceSyncPtr* GetDeviceSyncPtr();
void AddObserver(Observer* observer);
void RemoveObserver(Observer* observer);
......
......@@ -40,24 +40,23 @@ void DeviceSyncClientImpl::Factory::SetInstanceForTesting(
DeviceSyncClientImpl::Factory::~Factory() = default;
std::unique_ptr<DeviceSyncClient> DeviceSyncClientImpl::Factory::BuildInstance(
mojom::DeviceSyncService* service) {
return base::WrapUnique(new DeviceSyncClientImpl(service));
std::unique_ptr<DeviceSyncClient>
DeviceSyncClientImpl::Factory::BuildInstance() {
return std::make_unique<DeviceSyncClientImpl>();
}
DeviceSyncClientImpl::DeviceSyncClientImpl(mojom::DeviceSyncService* service)
: DeviceSyncClientImpl(service, base::ThreadTaskRunnerHandle::Get()) {}
DeviceSyncClientImpl::DeviceSyncClientImpl(
mojom::DeviceSyncService* service,
scoped_refptr<base::TaskRunner> task_runner)
: binding_(this),
DeviceSyncClientImpl::DeviceSyncClientImpl()
: observer_binding_(this),
expiring_device_cache_(
std::make_unique<multidevice::ExpiringRemoteDeviceCache>()) {
service->BindDeviceSync(mojo::MakeRequest(&device_sync_ptr_));
std::make_unique<multidevice::ExpiringRemoteDeviceCache>()) {}
DeviceSyncClientImpl::~DeviceSyncClientImpl() = default;
void DeviceSyncClientImpl::Initialize(
scoped_refptr<base::TaskRunner> task_runner) {
device_sync_ptr_->AddObserver(GenerateInterfacePtr(), base::OnceClosure());
// Delay calling these until after the constructor finishes.
// Delay calling these until after initialization finishes.
task_runner->PostTask(
FROM_HERE, base::BindOnce(&DeviceSyncClientImpl::LoadLocalDeviceMetadata,
weak_ptr_factory_.GetWeakPtr()));
......@@ -66,7 +65,9 @@ DeviceSyncClientImpl::DeviceSyncClientImpl(
weak_ptr_factory_.GetWeakPtr()));
}
DeviceSyncClientImpl::~DeviceSyncClientImpl() = default;
mojom::DeviceSyncPtr* DeviceSyncClientImpl::GetDeviceSyncPtr() {
return &device_sync_ptr_;
}
void DeviceSyncClientImpl::OnEnrollmentFinished() {
// Before notifying observers that enrollment has finished, sync down the
......@@ -239,7 +240,7 @@ void DeviceSyncClientImpl::OnFindEligibleDevicesCompleted(
mojom::DeviceSyncObserverPtr DeviceSyncClientImpl::GenerateInterfacePtr() {
mojom::DeviceSyncObserverPtr interface_ptr;
binding_.Bind(mojo::MakeRequest(&interface_ptr));
observer_binding_.Bind(mojo::MakeRequest(&interface_ptr));
return interface_ptr;
}
......
......@@ -41,16 +41,18 @@ class DeviceSyncClientImpl : public DeviceSyncClient,
static Factory* Get();
static void SetInstanceForTesting(Factory* test_factory);
virtual ~Factory();
virtual std::unique_ptr<DeviceSyncClient> BuildInstance(
mojom::DeviceSyncService* service);
virtual std::unique_ptr<DeviceSyncClient> BuildInstance();
private:
static Factory* test_factory_;
};
explicit DeviceSyncClientImpl(mojom::DeviceSyncService* service);
DeviceSyncClientImpl();
~DeviceSyncClientImpl() override;
void Initialize(scoped_refptr<base::TaskRunner> task_runner) override;
mojom::DeviceSyncPtr* GetDeviceSyncPtr() override;
// DeviceSyncClient:
void ForceEnrollmentNow(
mojom::DeviceSync::ForceEnrollmentNowCallback callback) override;
......@@ -75,9 +77,6 @@ class DeviceSyncClientImpl : public DeviceSyncClient,
private:
friend class DeviceSyncClientImplTest;
DeviceSyncClientImpl(mojom::DeviceSyncService* service,
scoped_refptr<base::TaskRunner> task_runner);
void AttemptToBecomeReady();
void LoadSyncedDevices();
......@@ -98,7 +97,7 @@ class DeviceSyncClientImpl : public DeviceSyncClient,
void FlushForTesting();
mojom::DeviceSyncPtr device_sync_ptr_;
mojo::Binding<mojom::DeviceSyncObserver> binding_;
mojo::Binding<mojom::DeviceSyncObserver> observer_binding_;
std::unique_ptr<multidevice::ExpiringRemoteDeviceCache>
expiring_device_cache_;
......
......@@ -19,12 +19,12 @@
#include "base/test/test_simple_task_runner.h"
#include "chromeos/components/multidevice/remote_device_test_util.h"
#include "chromeos/services/device_sync/device_sync_impl.h"
#include "chromeos/services/device_sync/device_sync_service.h"
#include "chromeos/services/device_sync/fake_device_sync.h"
#include "chromeos/services/device_sync/public/cpp/fake_client_app_metadata_provider.h"
#include "chromeos/services/device_sync/public/cpp/fake_gcm_device_info_provider.h"
#include "chromeos/services/device_sync/public/mojom/device_sync.mojom.h"
#include "components/gcm_driver/fake_gcm_driver.h"
#include "components/prefs/testing_pref_service.h"
#include "components/signin/public/identity_manager/identity_test_environment.h"
#include "services/network/public/cpp/weak_wrapper_shared_url_loader_factory.h"
#include "testing/gtest/include/gtest/gtest.h"
......@@ -61,8 +61,7 @@ class FakeDeviceSyncImplFactory : public DeviceSyncImpl::Factory {
std::unique_ptr<DeviceSyncBase> BuildInstance(
signin::IdentityManager* identity_manager,
gcm::GCMDriver* gcm_driver,
mojo::PendingRemote<prefs::mojom::PrefStoreConnector>
pref_store_connector,
PrefService* profile_prefs,
const GcmDeviceInfoProvider* gcm_device_info_provider,
ClientAppMetadataProvider* client_app_metadata_provider,
scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory,
......@@ -169,33 +168,26 @@ class DeviceSyncClientImplTest : public testing::Test {
return nullptr;
}));
mojo::Remote<mojom::DeviceSyncServiceInitializer> initializer;
service_ = std::make_unique<DeviceSyncService>(
test_pref_service_ = std::make_unique<TestingPrefServiceSimple>();
device_sync_ = DeviceSyncImpl::Factory::Get()->BuildInstance(
identity_test_environment_->identity_manager(), fake_gcm_driver_.get(),
fake_gcm_device_info_provider_.get(),
test_pref_service_.get(), fake_gcm_device_info_provider_.get(),
fake_client_app_metadata_provider_.get(), shared_url_loader_factory,
initializer.BindNewPipeAndPassReceiver());
mojo::PendingRemote<prefs::mojom::PrefStoreConnector> pref_store_connector;
ignore_result(pref_store_connector.InitWithNewPipeAndPassReceiver());
initializer->Initialize(remote_service_.BindNewPipeAndPassReceiver(),
std::move(pref_store_connector));
std::make_unique<base::OneShotTimer>());
test_observer_ = std::make_unique<TestDeviceSyncClientObserver>();
CreateClient();
}
void CreateClient() {
// DeviceSyncClient's constructor posts two tasks to the TaskRunner. Idle
// DeviceSyncClient initialization posts two tasks to the TaskRunner. Idle
// the TaskRunner so that the tasks can be run via a RunLoop later on.
auto test_task_runner = base::MakeRefCounted<base::TestSimpleTaskRunner>();
client_ = base::WrapUnique(
new DeviceSyncClientImpl(remote_service_.get(), test_task_runner));
client_ = std::make_unique<DeviceSyncClientImpl>();
device_sync_->BindRequest(mojo::MakeRequest(client_->GetDeviceSyncPtr()));
client_->Initialize(test_task_runner);
test_task_runner->RunUntilIdle();
}
void InitializeClient(bool complete_enrollment_before_sync = true) {
void SetupClient(bool complete_enrollment_before_sync = true) {
client_->AddObserver(test_observer_.get());
SendPendingMojoMessages();
......@@ -435,8 +427,8 @@ class DeviceSyncClientImplTest : public testing::Test {
fake_client_app_metadata_provider_;
FakeDeviceSync* fake_device_sync_;
std::unique_ptr<FakeDeviceSyncImplFactory> fake_device_sync_impl_factory_;
std::unique_ptr<DeviceSyncService> service_;
mojo::Remote<mojom::DeviceSyncService> remote_service_;
std::unique_ptr<TestingPrefServiceSimple> test_pref_service_;
std::unique_ptr<DeviceSyncBase> device_sync_;
std::unique_ptr<TestDeviceSyncClientObserver> test_observer_;
std::unique_ptr<DeviceSyncClientImpl> client_;
......@@ -494,7 +486,7 @@ class DeviceSyncClientImplTest : public testing::Test {
TEST_F(DeviceSyncClientImplTest,
TestCompleteInitialSyncBeforeInitialEnrollment) {
InitializeClient(false /* complete_enrollment_before_sync */);
SetupClient(false /* complete_enrollment_before_sync */);
}
TEST_F(
......@@ -549,7 +541,7 @@ TEST_F(
TEST_F(DeviceSyncClientImplTest, TestOnEnrollmentFinished) {
EXPECT_EQ(0u, test_observer_->enrollment_finished_count());
InitializeClient();
SetupClient();
EXPECT_EQ(test_remote_device_list_[0].public_key,
client_->GetLocalDeviceMetadata()->public_key());
......@@ -586,7 +578,7 @@ TEST_F(DeviceSyncClientImplTest, TestOnEnrollmentFinished) {
TEST_F(DeviceSyncClientImplTest, TestOnNewDevicesSynced) {
EXPECT_EQ(0u, test_observer_->new_devices_synced_count());
InitializeClient();
SetupClient();
VerifyRemoteDeviceRefListAndRemoteDeviceListAreEqual(
client_->GetSyncedDevices(), test_remote_device_list_);
......@@ -615,31 +607,31 @@ TEST_F(DeviceSyncClientImplTest, TestOnNewDevicesSynced) {
}
TEST_F(DeviceSyncClientImplTest, TestForceEnrollmentNow_ExpectSuccess) {
InitializeClient();
SetupClient();
CallForceEnrollmentNow(true /* expected_success */);
}
TEST_F(DeviceSyncClientImplTest, TestForceEnrollmentNow_ExpectFailure) {
InitializeClient();
SetupClient();
CallForceEnrollmentNow(false /* expected_success */);
}
TEST_F(DeviceSyncClientImplTest, TestSyncNow_ExpectSuccess) {
InitializeClient();
SetupClient();
CallSyncNow(true /* expected_success */);
}
TEST_F(DeviceSyncClientImplTest, TestSyncNow_ExpectFailure) {
InitializeClient();
SetupClient();
CallSyncNow(false /* expected_success */);
}
TEST_F(DeviceSyncClientImplTest, TestGetSyncedDevices_DeviceRemovedFromCache) {
InitializeClient();
SetupClient();
VerifyRemoteDeviceRefListAndRemoteDeviceListAreEqual(
client_->GetSyncedDevices(), test_remote_device_list_);
......@@ -662,13 +654,13 @@ TEST_F(DeviceSyncClientImplTest, TestGetSyncedDevices_DeviceRemovedFromCache) {
}
TEST_F(DeviceSyncClientImplTest, TestSetSoftwareFeatureState) {
InitializeClient();
SetupClient();
CallSetSoftwareFeatureState(mojom::NetworkRequestResult::kSuccess);
}
TEST_F(DeviceSyncClientImplTest, TestFindEligibleDevices_NoErrorCode) {
InitializeClient();
SetupClient();
multidevice::RemoteDeviceList expected_eligible_devices(
{test_remote_device_list_[0], test_remote_device_list_[1]});
......@@ -682,7 +674,7 @@ TEST_F(DeviceSyncClientImplTest, TestFindEligibleDevices_NoErrorCode) {
}
TEST_F(DeviceSyncClientImplTest, TestFindEligibleDevices_ErrorCode) {
InitializeClient();
SetupClient();
CallFindEligibleDevices(mojom::NetworkRequestResult::kEndpointNotFound,
multidevice::RemoteDeviceList(),
......@@ -690,7 +682,7 @@ TEST_F(DeviceSyncClientImplTest, TestFindEligibleDevices_ErrorCode) {
}
TEST_F(DeviceSyncClientImplTest, TestGetDebugInfo) {
InitializeClient();
SetupClient();
CallGetDebugInfo();
}
......
......@@ -12,7 +12,6 @@ mojom("mojom") {
public_deps = [
"//chromeos/components/multidevice/mojom",
"//mojo/public/mojom/base",
"//services/preferences/public/mojom",
]
}
......
......@@ -6,7 +6,6 @@ module chromeos.device_sync.mojom;
import "chromeos/components/multidevice/mojom/multidevice_types.mojom";
import "mojo/public/mojom/base/time.mojom";
import "services/preferences/public/mojom/preferences.mojom";
enum NetworkRequestResult {
// Successful network request.
......@@ -160,8 +159,8 @@ interface DeviceSync {
// On success, this function returns a null error_code with the activity
// statuses to the callback; on error, it returns a valid error_code string
// indicating the reason for failure along with a null activity statuses.
//
// This method is expected to be used by multidevice_setup service running
//
// This method is expected to be used by multidevice_setup service running
// in the browser process.
GetDevicesActivityStatus() =>
(NetworkRequestResult result_code,
......@@ -174,19 +173,3 @@ interface DeviceSync {
GetDebugInfo() => (DebugInfo? debug_info);
};
// The main interface to the Device Sync service, used to initialized a new
// instance of the service.
interface DeviceSyncServiceInitializer {
// Initializes the service instance, binding a DeviceSyncService endpoint
// with access to a given PrefStoreConnector.
Initialize(
pending_receiver<DeviceSyncService> receiver,
pending_remote<prefs.mojom.PrefStoreConnector> pref_store_connector);
};
// The main interface to the Device Sync service.
interface DeviceSyncService {
// Binds a new DeviceSync endpoint.
BindDeviceSync(pending_receiver<DeviceSync> receiver);
};
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