diff options
author | pavely <pavely@chromium.org> | 2015-12-22 22:47:19 -0800 |
---|---|---|
committer | Commit bot <commit-bot@chromium.org> | 2015-12-23 06:48:40 +0000 |
commit | 7623e624861d45f61632aaba650b4182c5d022ff (patch) | |
tree | 10321308c997c3260154245c7a268667e85ddbc0 | |
parent | e7259757073db5b53542ce4896d65ea208c8060d (diff) | |
download | chromium_src-7623e624861d45f61632aaba650b4182c5d022ff.zip chromium_src-7623e624861d45f61632aaba650b4182c5d022ff.tar.gz chromium_src-7623e624861d45f61632aaba650b4182c5d022ff.tar.bz2 |
[Sync] Use account_id instead of username in a few places in sync
Currently sync gets username from SigninManagerBase::GetAuthenticatedAccountInfo().
In some cases it returns empty result which prevents sync backend from starting.
Username is used in following places:
- passed to sync directory database to be stored as share id
- passed to server in ClientToServerMessage.share
- passed to attachment uploader/downloader to be used as account_id
when requesting access tokens
The change is to switch to more stable SigninManagerBase::GetAuthenticatedAccountId().
account_id will be passed to directory and attachment uploader/downloader.
Username is still used in communications with server but it is not
enforced to be nonempty.
BUG=554551
R=zea@chromium.org
Review URL: https://codereview.chromium.org/1544893002
Cr-Commit-Position: refs/heads/master@{#366721}
-rw-r--r-- | components/browser_sync/browser/profile_sync_components_factory_impl.cc | 6 | ||||
-rw-r--r-- | components/browser_sync/browser/profile_sync_service.cc | 3 | ||||
-rw-r--r-- | components/sync_driver/glue/sync_backend_host_impl_unittest.cc | 5 | ||||
-rw-r--r-- | components/sync_driver/startup_controller.cc | 7 | ||||
-rw-r--r-- | components/sync_driver/startup_controller_unittest.cc | 32 | ||||
-rw-r--r-- | sync/engine/sync_scheduler_impl.cc | 1 | ||||
-rw-r--r-- | sync/internal_api/public/sync_manager.h | 3 | ||||
-rw-r--r-- | sync/internal_api/sync_manager_impl.cc | 14 | ||||
-rw-r--r-- | sync/internal_api/sync_manager_impl_unittest.cc | 1 | ||||
-rw-r--r-- | sync/sessions/sync_session_context.h | 1 | ||||
-rw-r--r-- | sync/tools/sync_client.cc | 1 |
11 files changed, 42 insertions, 32 deletions
diff --git a/components/browser_sync/browser/profile_sync_components_factory_impl.cc b/components/browser_sync/browser/profile_sync_components_factory_impl.cc index 2ed6c9d..a67bd90 100644 --- a/components/browser_sync/browser/profile_sync_components_factory_impl.cc +++ b/components/browser_sync/browser/profile_sync_components_factory_impl.cc @@ -318,7 +318,7 @@ ProfileSyncComponentsFactoryImpl::CreateAttachmentService( // Only construct an AttachmentUploader and AttachmentDownload if we have sync // credentials. We may not have sync credentials because there may not be a // signed in sync user (e.g. sync is running in "backup" mode). - if (!user_share.sync_credentials.email.empty() && + if (!user_share.sync_credentials.account_id.empty() && !user_share.sync_credentials.scope_set.empty()) { scoped_refptr<OAuth2TokenServiceRequest::TokenServiceProvider> token_service_provider( @@ -328,7 +328,7 @@ ProfileSyncComponentsFactoryImpl::CreateAttachmentService( // per AttachmentService (bug 369536). attachment_uploader.reset(new syncer::AttachmentUploaderImpl( sync_service_url_, url_request_context_getter_, - user_share.sync_credentials.email, + user_share.sync_credentials.account_id, user_share.sync_credentials.scope_set, token_service_provider, store_birthday, model_type)); @@ -336,7 +336,7 @@ ProfileSyncComponentsFactoryImpl::CreateAttachmentService( new TokenServiceProvider(ui_thread_, token_service_); attachment_downloader = syncer::AttachmentDownloader::Create( sync_service_url_, url_request_context_getter_, - user_share.sync_credentials.email, + user_share.sync_credentials.account_id, user_share.sync_credentials.scope_set, token_service_provider, store_birthday, model_type); } diff --git a/components/browser_sync/browser/profile_sync_service.cc b/components/browser_sync/browser/profile_sync_service.cc index 55b74d5..5316c4f 100644 --- a/components/browser_sync/browser/profile_sync_service.cc +++ b/components/browser_sync/browser/profile_sync_service.cc @@ -499,8 +499,9 @@ void ProfileSyncService::OnSessionRestoreComplete() { SyncCredentials ProfileSyncService::GetCredentials() { SyncCredentials credentials; if (backend_mode_ == SYNC) { + credentials.account_id = signin_->GetAccountIdToUse(); + DCHECK(!credentials.account_id.empty()); credentials.email = signin_->GetEffectiveUsername(); - DCHECK(!credentials.email.empty()); credentials.sync_token = access_token_; if (credentials.sync_token.empty()) diff --git a/components/sync_driver/glue/sync_backend_host_impl_unittest.cc b/components/sync_driver/glue/sync_backend_host_impl_unittest.cc index 4c60358..f34cf97 100644 --- a/components/sync_driver/glue/sync_backend_host_impl_unittest.cc +++ b/components/sync_driver/glue/sync_backend_host_impl_unittest.cc @@ -5,6 +5,7 @@ #include "components/sync_driver/glue/sync_backend_host_impl.h" #include <cstddef> +#include <map> #include "base/files/file_util.h" #include "base/files/scoped_temp_dir.h" @@ -168,8 +169,7 @@ class BackendSyncClient : public sync_driver::FakeSyncClient { class SyncBackendHostTest : public testing::Test { protected: SyncBackendHostTest() - : fake_manager_(NULL) { - } + : fake_manager_(NULL) {} ~SyncBackendHostTest() override {} @@ -185,6 +185,7 @@ class SyncBackendHostTest : public testing::Test { nullptr, sync_prefs_->AsWeakPtr(), temp_dir_.path().Append(base::FilePath(kTestSyncDir)))); + credentials_.account_id = "user@example.com"; credentials_.email = "user@example.com"; credentials_.sync_token = "sync_token"; credentials_.scope_set.insert(GaiaConstants::kChromeSyncOAuth2Scope); diff --git a/components/sync_driver/startup_controller.cc b/components/sync_driver/startup_controller.cc index bc8926e..ec4424d 100644 --- a/components/sync_driver/startup_controller.cc +++ b/components/sync_driver/startup_controller.cc @@ -4,6 +4,8 @@ #include "components/sync_driver/startup_controller.h" +#include <string> + #include "base/command_line.h" #include "base/location.h" #include "base/metrics/histogram.h" @@ -124,14 +126,13 @@ bool StartupController::TryStart() { if (!sync_prefs_->IsSyncRequested()) return false; - if (signin_->GetEffectiveUsername().empty()) + if (signin_->GetAccountIdToUse().empty()) return false; if (!token_service_) return false; - if (!token_service_->RefreshTokenIsAvailable( - signin_->GetAccountIdToUse())) { + if (!token_service_->RefreshTokenIsAvailable(signin_->GetAccountIdToUse())) { return false; } diff --git a/components/sync_driver/startup_controller_unittest.cc b/components/sync_driver/startup_controller_unittest.cc index 0999a96..9447640 100644 --- a/components/sync_driver/startup_controller_unittest.cc +++ b/components/sync_driver/startup_controller_unittest.cc @@ -4,6 +4,8 @@ #include "components/sync_driver/startup_controller.h" +#include <string> + #include "base/command_line.h" #include "base/message_loop/message_loop.h" #include "base/run_loop.h" @@ -31,14 +33,16 @@ static const char kStateStringNotStarted[] = "Not started"; class FakeSigninManagerWrapper : public SigninManagerWrapper { public: FakeSigninManagerWrapper() : SigninManagerWrapper(NULL) {} - std::string GetEffectiveUsername() const override { return account_; } + std::string GetEffectiveUsername() const override { return std::string(); } - std::string GetAccountIdToUse() const override { return account_; } + std::string GetAccountIdToUse() const override { return account_id_; } - void set_account(const std::string& account) { account_ = account; } + void set_account_id(const std::string& account_id) { + account_id_ = account_id; + } private: - std::string account_; + std::string account_id_; }; class StartupControllerTest : public testing::Test { @@ -105,7 +109,7 @@ TEST_F(StartupControllerTest, Basic) { sync_prefs()->SetSyncSetupCompleted(); controller()->TryStart(); EXPECT_FALSE(started()); - signin()->set_account(kTestUser); + signin()->set_account_id(kTestUser); controller()->TryStart(); EXPECT_FALSE(started()); token_service()->UpdateCredentials(kTestUser, kTestToken); @@ -124,7 +128,7 @@ TEST_F(StartupControllerTest, Basic) { TEST_F(StartupControllerTest, NotRequested) { sync_prefs()->SetSyncSetupCompleted(); sync_prefs()->SetSyncRequested(false); - signin()->set_account(kTestUser); + signin()->set_account_id(kTestUser); token_service()->UpdateCredentials(kTestUser, kTestToken); controller()->TryStart(); EXPECT_FALSE(started()); @@ -136,7 +140,7 @@ TEST_F(StartupControllerTest, NotRequested) { TEST_F(StartupControllerTest, Managed) { sync_prefs()->SetSyncSetupCompleted(); sync_prefs()->SetManagedForTest(true); - signin()->set_account(kTestUser); + signin()->set_account_id(kTestUser); token_service()->UpdateCredentials(kTestUser, kTestToken); controller()->TryStart(); EXPECT_FALSE(started()); @@ -148,7 +152,7 @@ TEST_F(StartupControllerTest, Managed) { // data type triggers sync startup. TEST_F(StartupControllerTest, DataTypeTriggered) { sync_prefs()->SetSyncSetupCompleted(); - signin()->set_account(kTestUser); + signin()->set_account_id(kTestUser); token_service()->UpdateCredentials(kTestUser, kTestToken); controller()->TryStart(); EXPECT_FALSE(started()); @@ -170,7 +174,7 @@ TEST_F(StartupControllerTest, DataTypeTriggered) { // conditions are met and no data type requests sync. TEST_F(StartupControllerTest, FallbackTimer) { sync_prefs()->SetSyncSetupCompleted(); - signin()->set_account(kTestUser); + signin()->set_account_id(kTestUser); token_service()->UpdateCredentials(kTestUser, kTestToken); controller()->TryStart(); EXPECT_FALSE(started()); @@ -190,7 +194,7 @@ TEST_F(StartupControllerTest, NoDeferralWithoutSessionsSync) { sync_prefs()->SetPreferredDataTypes(syncer::UserTypes(), types); controller()->Reset(syncer::UserTypes()); sync_prefs()->SetSyncSetupCompleted(); - signin()->set_account(kTestUser); + signin()->set_account_id(kTestUser); token_service()->UpdateCredentials(kTestUser, kTestToken); controller()->TryStart(); EXPECT_TRUE(started()); @@ -208,7 +212,7 @@ TEST_F(StartupControllerTest, FallbackTimerWaits) { // Test that sync starts without the user having to explicitly ask for // setup when AUTO_START is the startup behavior requested. TEST_F(StartupControllerTest, FirstSetupWithAutoStart) { - signin()->set_account(kTestUser); + signin()->set_account_id(kTestUser); token_service()->UpdateCredentials(kTestUser, kTestToken); controller()->TryStart(); EXPECT_TRUE(started()); @@ -217,7 +221,7 @@ TEST_F(StartupControllerTest, FirstSetupWithAutoStart) { // Test that sync starts only after user explicitly asks for setup when // MANUAL_START is the startup behavior requested. TEST_F(StartupControllerTest, FirstSetupWithManualStart) { - signin()->set_account(kTestUser); + signin()->set_account_id(kTestUser); token_service()->UpdateCredentials(kTestUser, kTestToken); SetUpController(MANUAL_START); controller()->TryStart(); @@ -229,7 +233,7 @@ TEST_F(StartupControllerTest, FirstSetupWithManualStart) { TEST_F(StartupControllerTest, Reset) { sync_prefs()->SetSyncSetupCompleted(); - signin()->set_account(kTestUser); + signin()->set_account_id(kTestUser); token_service()->UpdateCredentials(kTestUser, kTestToken); controller()->TryStart(); const bool deferred_start = @@ -248,7 +252,7 @@ TEST_F(StartupControllerTest, Reset) { // Test that setup-in-progress tracking is persistent across a Reset. TEST_F(StartupControllerTest, ResetDuringSetup) { - signin()->set_account(kTestUser); + signin()->set_account_id(kTestUser); token_service()->UpdateCredentials(kTestUser, kTestToken); // Simulate UI telling us setup is in progress. diff --git a/sync/engine/sync_scheduler_impl.cc b/sync/engine/sync_scheduler_impl.cc index 4353be0..20cdfc1 100644 --- a/sync/engine/sync_scheduler_impl.cc +++ b/sync/engine/sync_scheduler_impl.cc @@ -233,7 +233,6 @@ void SyncSchedulerImpl::Start(Mode mode, base::Time last_poll_time) { SendInitialSnapshot(); } - DCHECK(!session_context_->account_name().empty()); DCHECK(syncer_.get()); if (mode == CLEAR_SERVER_DATA_MODE) { diff --git a/sync/internal_api/public/sync_manager.h b/sync/internal_api/public/sync_manager.h index 7e75983..e1cfc29 100644 --- a/sync/internal_api/public/sync_manager.h +++ b/sync/internal_api/public/sync_manager.h @@ -68,6 +68,9 @@ struct SYNC_EXPORT SyncCredentials { SyncCredentials(); ~SyncCredentials(); + // Account_id of signed in account. + std::string account_id; + // The email associated with this account. std::string email; diff --git a/sync/internal_api/sync_manager_impl.cc b/sync/internal_api/sync_manager_impl.cc index 046ee3d..13e4244 100644 --- a/sync/internal_api/sync_manager_impl.cc +++ b/sync/internal_api/sync_manager_impl.cc @@ -228,7 +228,7 @@ void SyncManagerImpl::Init(InitArgs* args) { CHECK(!initialized_); DCHECK(thread_checker_.CalledOnValidThread()); DCHECK(args->post_factory.get()); - DCHECK(!args->credentials.email.empty()); + DCHECK(!args->credentials.account_id.empty()); DCHECK(!args->credentials.sync_token.empty()); DCHECK(!args->credentials.scope_set.empty()); DCHECK(args->cancelation_signal); @@ -263,7 +263,7 @@ void SyncManagerImpl::Init(InitArgs* args) { scoped_ptr<syncable::DirectoryBackingStore> backing_store = args->internal_components_factory->BuildDirectoryBackingStore( InternalComponentsFactory::STORAGE_ON_DISK, - args->credentials.email, absolute_db_path).Pass(); + args->credentials.account_id, absolute_db_path).Pass(); DCHECK(backing_store.get()); share_.directory.reset( @@ -279,9 +279,9 @@ void SyncManagerImpl::Init(InitArgs* args) { // sync token so clear sync_token from the UserShare. share_.sync_credentials.sync_token = ""; - const std::string& username = args->credentials.email; - DVLOG(1) << "Username: " << username; - if (!OpenDirectory(username)) { + DVLOG(1) << "Username: " << args->credentials.email; + DVLOG(1) << "AccountId: " << args->credentials.account_id; + if (!OpenDirectory(args->credentials.account_id)) { NotifyInitializationFailure(); LOG(ERROR) << "Sync manager initialization failed!"; return; @@ -337,7 +337,6 @@ void SyncManagerImpl::Init(InitArgs* args) { model_type_registry_.get(), args->invalidator_client_id) .Pass(); - session_context_->set_account_name(args->credentials.email); scheduler_ = args->internal_components_factory->BuildScheduler( name_, session_context_.get(), args->cancelation_signal).Pass(); @@ -506,9 +505,10 @@ bool SyncManagerImpl::PurgeDisabledTypes( void SyncManagerImpl::UpdateCredentials(const SyncCredentials& credentials) { DCHECK(thread_checker_.CalledOnValidThread()); DCHECK(initialized_); - DCHECK(!credentials.email.empty()); + DCHECK(!credentials.account_id.empty()); DCHECK(!credentials.sync_token.empty()); DCHECK(!credentials.scope_set.empty()); + session_context_->set_account_name(credentials.email); observing_network_connectivity_changes_ = true; if (!connection_manager_->SetAuthToken(credentials.sync_token)) diff --git a/sync/internal_api/sync_manager_impl_unittest.cc b/sync/internal_api/sync_manager_impl_unittest.cc index 4f08cb6..f231757 100644 --- a/sync/internal_api/sync_manager_impl_unittest.cc +++ b/sync/internal_api/sync_manager_impl_unittest.cc @@ -882,6 +882,7 @@ class SyncManagerTest : public testing::Test, extensions_activity_ = new ExtensionsActivity(); SyncCredentials credentials; + credentials.account_id = "foo@bar.com"; credentials.email = "foo@bar.com"; credentials.sync_token = "sometoken"; OAuth2TokenService::ScopeSet scope_set; diff --git a/sync/sessions/sync_session_context.h b/sync/sessions/sync_session_context.h index 008e51a..194ce46 100644 --- a/sync/sessions/sync_session_context.h +++ b/sync/sessions/sync_session_context.h @@ -84,7 +84,6 @@ class SYNC_EXPORT SyncSessionContext { // Account name, set once a directory has been opened. void set_account_name(const std::string& name) { - DCHECK(account_name_.empty()); account_name_ = name; } const std::string& account_name() const { return account_name_; } diff --git a/sync/tools/sync_client.cc b/sync/tools/sync_client.cc index 1c60222..3781899 100644 --- a/sync/tools/sync_client.cc +++ b/sync/tools/sync_client.cc @@ -310,6 +310,7 @@ int SyncClientMain(int argc, char* argv[]) { const base::CommandLine& command_line = *base::CommandLine::ForCurrentProcess(); SyncCredentials credentials; + credentials.account_id = command_line.GetSwitchValueASCII(kEmailSwitch); credentials.email = command_line.GetSwitchValueASCII(kEmailSwitch); credentials.sync_token = command_line.GetSwitchValueASCII(kTokenSwitch); // TODO(akalin): Write a wrapper script that gets a token for an |