From d578c18be6220adb21d8e1261c9196cc27c4a879 Mon Sep 17 00:00:00 2001 From: fgorski Date: Wed, 24 Sep 2014 16:40:17 -0700 Subject: [PATCH] Loading the account mappings from store to the driver. This is to make sure the starting list of account mapping properly reflects the mappings present in the previous Chrome session. BUG=374969 Review URL: https://codereview.chromium.org/600053002 Cr-Commit-Position: refs/heads/master@{#296567} --- components/gcm_driver/BUILD.gn | 4 ++ components/gcm_driver/fake_gcm_client.cc | 3 +- components/gcm_driver/gcm_client.h | 5 ++- components/gcm_driver/gcm_client_impl.cc | 14 +++++-- components/gcm_driver/gcm_client_impl.h | 2 +- .../gcm_driver/gcm_client_impl_unittest.cc | 8 +++- components/gcm_driver/gcm_driver_desktop.cc | 12 ++++-- components/gcm_driver/gcm_driver_desktop.h | 3 +- google_apis/gcm/engine/gcm_store.h | 6 +-- google_apis/gcm/engine/gcm_store_impl.cc | 6 +-- .../gcm/engine/gcm_store_impl_unittest.cc | 41 +++++++++---------- 11 files changed, 62 insertions(+), 42 deletions(-) diff --git a/components/gcm_driver/BUILD.gn b/components/gcm_driver/BUILD.gn index 9e4f3d7e1747e4..92330f8007508c 100644 --- a/components/gcm_driver/BUILD.gn +++ b/components/gcm_driver/BUILD.gn @@ -11,6 +11,8 @@ static_library("gcm_driver") { "default_gcm_app_handler.h", "gcm_activity.cc", "gcm_activity.h", + "gcm_account_mapper.cc", + "gcm_account_mapper.h", "gcm_app_handler.cc", "gcm_app_handler.h", "gcm_backoff_policy.cc", @@ -51,6 +53,8 @@ static_library("gcm_driver") { if (is_android) { sources -= [ + "gcm_account_mapper.cc", + "gcm_account_mapper.h", "gcm_channel_status_request.cc", "gcm_channel_status_request.h", "gcm_channel_status_syncer.cc", diff --git a/components/gcm_driver/fake_gcm_client.cc b/components/gcm_driver/fake_gcm_client.cc index 9110d1b1d6088b..91d0ecb79269df 100644 --- a/components/gcm_driver/fake_gcm_client.cc +++ b/components/gcm_driver/fake_gcm_client.cc @@ -11,6 +11,7 @@ #include "base/sys_byteorder.h" #include "base/time/time.h" #include "google_apis/gcm/base/encryptor.h" +#include "google_apis/gcm/engine/account_mapping.h" #include "net/base/ip_endpoint.h" namespace gcm { @@ -179,7 +180,7 @@ std::string FakeGCMClient::GetRegistrationIdFromSenderIds( } void FakeGCMClient::CheckinFinished() { - delegate_->OnGCMReady(); + delegate_->OnGCMReady(std::vector()); delegate_->OnConnected(net::IPEndPoint()); } diff --git a/components/gcm_driver/gcm_client.h b/components/gcm_driver/gcm_client.h index abf2fd5e9bfb80..740a3c75f35ba2 100644 --- a/components/gcm_driver/gcm_client.h +++ b/components/gcm_driver/gcm_client.h @@ -201,7 +201,10 @@ class GCMClient { // Called when the GCM becomes ready. To get to this state, GCMClient // finished loading from the GCM store and retrieved the device check-in // from the server if it hadn't yet. - virtual void OnGCMReady() = 0; + // |account_mappings|: a persisted list of accounts mapped to this GCM + // client. + virtual void OnGCMReady( + const std::vector& account_mappings) = 0; // Called when activities are being recorded and a new activity has just // been recorded. diff --git a/components/gcm_driver/gcm_client_impl.cc b/components/gcm_driver/gcm_client_impl.cc index d1fc7a08851675..57a665a8ed5b51 100644 --- a/components/gcm_driver/gcm_client_impl.cc +++ b/components/gcm_driver/gcm_client_impl.cc @@ -321,11 +321,16 @@ void GCMClientImpl::OnLoadCompleted(scoped_ptr result) { device_checkin_info_.accounts_set = true; last_checkin_time_ = result->last_checkin_time; gservices_settings_.UpdateFromLoadResult(*result); + // Taking over the value of account_mappings before passing the ownership of + // load result to InitializeMCSClient. + std::vector account_mappings; + account_mappings.swap(result->account_mappings); + InitializeMCSClient(result.Pass()); if (device_checkin_info_.IsValid()) { SchedulePeriodicCheckin(); - OnReady(); + OnReady(account_mappings); return; } @@ -379,14 +384,15 @@ void GCMClientImpl::OnFirstTimeDeviceCheckinCompleted( base::Bind(&GCMClientImpl::SetDeviceCredentialsCallback, weak_ptr_factory_.GetWeakPtr())); - OnReady(); + OnReady(std::vector()); } -void GCMClientImpl::OnReady() { +void GCMClientImpl::OnReady( + const std::vector& account_mappings) { state_ = READY; StartMCSLogin(); - delegate_->OnGCMReady(); + delegate_->OnGCMReady(account_mappings); } void GCMClientImpl::StartMCSLogin() { diff --git a/components/gcm_driver/gcm_client_impl.h b/components/gcm_driver/gcm_client_impl.h index 48bfa6a0284400..8e43f6d3743896 100644 --- a/components/gcm_driver/gcm_client_impl.h +++ b/components/gcm_driver/gcm_client_impl.h @@ -198,7 +198,7 @@ class GCMClientImpl void ResetState(); // Sets state to ready. This will initiate the MCS login and notify the // delegates. - void OnReady(); + void OnReady(const std::vector& account_mappings); // Starts a first time device checkin. void StartCheckin(); diff --git a/components/gcm_driver/gcm_client_impl_unittest.cc b/components/gcm_driver/gcm_client_impl_unittest.cc index c0d3e822934184..62416c986f6860 100644 --- a/components/gcm_driver/gcm_client_impl_unittest.cc +++ b/components/gcm_driver/gcm_client_impl_unittest.cc @@ -258,7 +258,8 @@ class GCMClientImplTest : public testing::Test, const gcm::GCMClient::SendErrorDetails& send_error_details) OVERRIDE; virtual void OnSendAcknowledged(const std::string& app_id, const std::string& message_id) OVERRIDE; - virtual void OnGCMReady() OVERRIDE; + virtual void OnGCMReady( + const std::vector& account_mappings) OVERRIDE; virtual void OnActivityRecorded() OVERRIDE {} virtual void OnConnected(const net::IPEndPoint& ip_endpoint) OVERRIDE {} virtual void OnDisconnected() OVERRIDE {} @@ -490,9 +491,12 @@ void GCMClientImplTest::ReceiveOnMessageSentToMCS( gcm_client_->OnMessageSentToMCS(0LL, app_id, message_id, status); } -void GCMClientImplTest::OnGCMReady() { +void GCMClientImplTest::OnGCMReady( + const std::vector& account_mappings) { last_event_ = LOADING_COMPLETED; QuitLoop(); + // TODO(fgorski): Add scenario verifying contents of account_mappings, when + // the list is not empty. } void GCMClientImplTest::OnMessageReceived( diff --git a/components/gcm_driver/gcm_driver_desktop.cc b/components/gcm_driver/gcm_driver_desktop.cc index 82ca4b34320e1c..b2d2e558c32c20 100644 --- a/components/gcm_driver/gcm_driver_desktop.cc +++ b/components/gcm_driver/gcm_driver_desktop.cc @@ -50,7 +50,8 @@ class GCMDriverDesktop::IOWorker : public GCMClient::Delegate { const GCMClient::SendErrorDetails& send_error_details) OVERRIDE; virtual void OnSendAcknowledged(const std::string& app_id, const std::string& message_id) OVERRIDE; - virtual void OnGCMReady() OVERRIDE; + virtual void OnGCMReady( + const std::vector& account_mappings) OVERRIDE; virtual void OnActivityRecorded() OVERRIDE; virtual void OnConnected(const net::IPEndPoint& ip_endpoint) OVERRIDE; virtual void OnDisconnected() OVERRIDE; @@ -201,10 +202,12 @@ void GCMDriverDesktop::IOWorker::OnSendAcknowledged( &GCMDriverDesktop::SendAcknowledged, service_, app_id, message_id)); } -void GCMDriverDesktop::IOWorker::OnGCMReady() { +void GCMDriverDesktop::IOWorker::OnGCMReady( + const std::vector& account_mappings) { ui_thread_->PostTask( FROM_HERE, - base::Bind(&GCMDriverDesktop::GCMClientReady, service_)); + base::Bind( + &GCMDriverDesktop::GCMClientReady, service_, account_mappings)); } void GCMDriverDesktop::IOWorker::OnActivityRecorded() { @@ -714,7 +717,8 @@ void GCMDriverDesktop::SendAcknowledged(const std::string& app_id, GetAppHandler(app_id)->OnSendAcknowledged(app_id, message_id); } -void GCMDriverDesktop::GCMClientReady() { +void GCMDriverDesktop::GCMClientReady( + const std::vector& account_mappings) { DCHECK(ui_thread_->RunsTasksOnCurrentThread()); delayed_task_controller_->SetReady(); diff --git a/components/gcm_driver/gcm_driver_desktop.h b/components/gcm_driver/gcm_driver_desktop.h index 0ef949546fc0f6..c1900ede8d455f 100644 --- a/components/gcm_driver/gcm_driver_desktop.h +++ b/components/gcm_driver/gcm_driver_desktop.h @@ -129,7 +129,8 @@ class GCMDriverDesktop : public GCMDriver { const GCMClient::SendErrorDetails& send_error_details); void SendAcknowledged(const std::string& app_id, const std::string& message_id); - void GCMClientReady(); + void GCMClientReady( + const std::vector& account_mappings); void OnConnected(const net::IPEndPoint& ip_endpoint); void OnDisconnected(); diff --git a/google_apis/gcm/engine/gcm_store.h b/google_apis/gcm/engine/gcm_store.h index 2b35549d92f6ee..9a1d8d87cb7ef6 100644 --- a/google_apis/gcm/engine/gcm_store.h +++ b/google_apis/gcm/engine/gcm_store.h @@ -34,8 +34,8 @@ class GCM_EXPORT GCMStore { typedef std::map > OutgoingMessageMap; - // Map of account id to account info for account mappings. - typedef std::map AccountMappingMap; + // List of account mappings. + typedef std::vector AccountMappings; // Container for Load(..) results. struct GCM_EXPORT LoadResult { @@ -54,7 +54,7 @@ class GCM_EXPORT GCMStore { std::string gservices_digest; base::Time last_checkin_time; std::set last_checkin_accounts; - AccountMappingMap account_mappings; + AccountMappings account_mappings; }; typedef std::vector PersistentIdList; diff --git a/google_apis/gcm/engine/gcm_store_impl.cc b/google_apis/gcm/engine/gcm_store_impl.cc index fe761ce6a4b41f..64683ed86c1bdf 100644 --- a/google_apis/gcm/engine/gcm_store_impl.cc +++ b/google_apis/gcm/engine/gcm_store_impl.cc @@ -179,7 +179,7 @@ class GCMStoreImpl::Backend std::set* accounts); bool LoadGServicesSettings(std::map* settings, std::string* digest); - bool LoadAccountMappingInfo(AccountMappingMap* account_mappings); + bool LoadAccountMappingInfo(AccountMappings* account_mappings); const base::FilePath path_; scoped_refptr foreground_task_runner_; @@ -792,7 +792,7 @@ bool GCMStoreImpl::Backend::LoadGServicesSettings( } bool GCMStoreImpl::Backend::LoadAccountMappingInfo( - AccountMappingMap* account_mappings) { + AccountMappings* account_mappings) { leveldb::ReadOptions read_options; read_options.verify_checksums = true; @@ -808,7 +808,7 @@ bool GCMStoreImpl::Backend::LoadAccountMappingInfo( return false; } DVLOG(1) << "Found account mapping with ID: " << account_mapping.account_id; - (*account_mappings)[account_mapping.account_id] = account_mapping; + account_mappings->push_back(account_mapping); } return true; diff --git a/google_apis/gcm/engine/gcm_store_impl_unittest.cc b/google_apis/gcm/engine/gcm_store_impl_unittest.cc index 81b5bbfe8290fb..939ef80c9ec647 100644 --- a/google_apis/gcm/engine/gcm_store_impl_unittest.cc +++ b/google_apis/gcm/engine/gcm_store_impl_unittest.cc @@ -558,25 +558,23 @@ TEST_F(GCMStoreImplTest, AccountMapping) { PumpLoop(); EXPECT_EQ(2UL, load_result->account_mappings.size()); - GCMStore::AccountMappingMap::iterator iter = + GCMStore::AccountMappings::iterator iter = load_result->account_mappings.begin(); - EXPECT_EQ("account_id_1", iter->first); - EXPECT_EQ(account_mapping1.account_id, iter->second.account_id); - EXPECT_EQ(account_mapping1.email, iter->second.email); - EXPECT_TRUE(iter->second.access_token.empty()); - EXPECT_EQ(AccountMapping::ADDING, iter->second.status); + EXPECT_EQ(account_mapping1.account_id, iter->account_id); + EXPECT_EQ(account_mapping1.email, iter->email); + EXPECT_TRUE(iter->access_token.empty()); + EXPECT_EQ(AccountMapping::ADDING, iter->status); EXPECT_EQ(account_mapping1.status_change_timestamp, - iter->second.status_change_timestamp); - EXPECT_EQ(account_mapping1.last_message_id, iter->second.last_message_id); + iter->status_change_timestamp); + EXPECT_EQ(account_mapping1.last_message_id, iter->last_message_id); ++iter; - EXPECT_EQ("account_id_2", iter->first); - EXPECT_EQ(account_mapping2.account_id, iter->second.account_id); - EXPECT_EQ(account_mapping2.email, iter->second.email); - EXPECT_TRUE(iter->second.access_token.empty()); - EXPECT_EQ(AccountMapping::REMOVING, iter->second.status); + EXPECT_EQ(account_mapping2.account_id, iter->account_id); + EXPECT_EQ(account_mapping2.email, iter->email); + EXPECT_TRUE(iter->access_token.empty()); + EXPECT_EQ(AccountMapping::REMOVING, iter->status); EXPECT_EQ(account_mapping2.status_change_timestamp, - iter->second.status_change_timestamp); - EXPECT_EQ(account_mapping2.last_message_id, iter->second.last_message_id); + iter->status_change_timestamp); + EXPECT_EQ(account_mapping2.last_message_id, iter->last_message_id); gcm_store->RemoveAccountMapping( account_mapping1.account_id, @@ -590,14 +588,13 @@ TEST_F(GCMStoreImplTest, AccountMapping) { EXPECT_EQ(1UL, load_result->account_mappings.size()); iter = load_result->account_mappings.begin(); - EXPECT_EQ("account_id_2", iter->first); - EXPECT_EQ(account_mapping2.account_id, iter->second.account_id); - EXPECT_EQ(account_mapping2.email, iter->second.email); - EXPECT_TRUE(iter->second.access_token.empty()); - EXPECT_EQ(AccountMapping::REMOVING, iter->second.status); + EXPECT_EQ(account_mapping2.account_id, iter->account_id); + EXPECT_EQ(account_mapping2.email, iter->email); + EXPECT_TRUE(iter->access_token.empty()); + EXPECT_EQ(AccountMapping::REMOVING, iter->status); EXPECT_EQ(account_mapping2.status_change_timestamp, - iter->second.status_change_timestamp); - EXPECT_EQ(account_mapping2.last_message_id, iter->second.last_message_id); + iter->status_change_timestamp); + EXPECT_EQ(account_mapping2.last_message_id, iter->last_message_id); } // When the database is destroyed, all database updates should fail. At the