Skip to content

Commit

Permalink
Loading the account mappings from store to the driver.
Browse files Browse the repository at this point in the history
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}
  • Loading branch information
fgorski authored and Commit bot committed Sep 24, 2014
1 parent 9e2bd7a commit d578c18
Show file tree
Hide file tree
Showing 11 changed files with 62 additions and 42 deletions.
4 changes: 4 additions & 0 deletions components/gcm_driver/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -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",
Expand Down
3 changes: 2 additions & 1 deletion components/gcm_driver/fake_gcm_client.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -179,7 +180,7 @@ std::string FakeGCMClient::GetRegistrationIdFromSenderIds(
}

void FakeGCMClient::CheckinFinished() {
delegate_->OnGCMReady();
delegate_->OnGCMReady(std::vector<AccountMapping>());
delegate_->OnConnected(net::IPEndPoint());
}

Expand Down
5 changes: 4 additions & 1 deletion components/gcm_driver/gcm_client.h
Original file line number Diff line number Diff line change
Expand Up @@ -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<AccountMapping>& account_mappings) = 0;

// Called when activities are being recorded and a new activity has just
// been recorded.
Expand Down
14 changes: 10 additions & 4 deletions components/gcm_driver/gcm_client_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -321,11 +321,16 @@ void GCMClientImpl::OnLoadCompleted(scoped_ptr<GCMStore::LoadResult> 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<AccountMapping> account_mappings;
account_mappings.swap(result->account_mappings);

InitializeMCSClient(result.Pass());

if (device_checkin_info_.IsValid()) {
SchedulePeriodicCheckin();
OnReady();
OnReady(account_mappings);
return;
}

Expand Down Expand Up @@ -379,14 +384,15 @@ void GCMClientImpl::OnFirstTimeDeviceCheckinCompleted(
base::Bind(&GCMClientImpl::SetDeviceCredentialsCallback,
weak_ptr_factory_.GetWeakPtr()));

OnReady();
OnReady(std::vector<AccountMapping>());
}

void GCMClientImpl::OnReady() {
void GCMClientImpl::OnReady(
const std::vector<AccountMapping>& account_mappings) {
state_ = READY;
StartMCSLogin();

delegate_->OnGCMReady();
delegate_->OnGCMReady(account_mappings);
}

void GCMClientImpl::StartMCSLogin() {
Expand Down
2 changes: 1 addition & 1 deletion components/gcm_driver/gcm_client_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -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<AccountMapping>& account_mappings);

// Starts a first time device checkin.
void StartCheckin();
Expand Down
8 changes: 6 additions & 2 deletions components/gcm_driver/gcm_client_impl_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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<AccountMapping>& account_mappings) OVERRIDE;
virtual void OnActivityRecorded() OVERRIDE {}
virtual void OnConnected(const net::IPEndPoint& ip_endpoint) OVERRIDE {}
virtual void OnDisconnected() OVERRIDE {}
Expand Down Expand Up @@ -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<AccountMapping>& 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(
Expand Down
12 changes: 8 additions & 4 deletions components/gcm_driver/gcm_driver_desktop.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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<AccountMapping>& account_mappings) OVERRIDE;
virtual void OnActivityRecorded() OVERRIDE;
virtual void OnConnected(const net::IPEndPoint& ip_endpoint) OVERRIDE;
virtual void OnDisconnected() OVERRIDE;
Expand Down Expand Up @@ -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<AccountMapping>& account_mappings) {
ui_thread_->PostTask(
FROM_HERE,
base::Bind(&GCMDriverDesktop::GCMClientReady, service_));
base::Bind(
&GCMDriverDesktop::GCMClientReady, service_, account_mappings));
}

void GCMDriverDesktop::IOWorker::OnActivityRecorded() {
Expand Down Expand Up @@ -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<AccountMapping>& account_mappings) {
DCHECK(ui_thread_->RunsTasksOnCurrentThread());

delayed_task_controller_->SetReady();
Expand Down
3 changes: 2 additions & 1 deletion components/gcm_driver/gcm_driver_desktop.h
Original file line number Diff line number Diff line change
Expand Up @@ -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<AccountMapping>& account_mappings);
void OnConnected(const net::IPEndPoint& ip_endpoint);
void OnDisconnected();

Expand Down
6 changes: 3 additions & 3 deletions google_apis/gcm/engine/gcm_store.h
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,8 @@ class GCM_EXPORT GCMStore {
typedef std::map<std::string, linked_ptr<google::protobuf::MessageLite> >
OutgoingMessageMap;

// Map of account id to account info for account mappings.
typedef std::map<std::string, AccountMapping> AccountMappingMap;
// List of account mappings.
typedef std::vector<AccountMapping> AccountMappings;

// Container for Load(..) results.
struct GCM_EXPORT LoadResult {
Expand All @@ -54,7 +54,7 @@ class GCM_EXPORT GCMStore {
std::string gservices_digest;
base::Time last_checkin_time;
std::set<std::string> last_checkin_accounts;
AccountMappingMap account_mappings;
AccountMappings account_mappings;
};

typedef std::vector<std::string> PersistentIdList;
Expand Down
6 changes: 3 additions & 3 deletions google_apis/gcm/engine/gcm_store_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,7 @@ class GCMStoreImpl::Backend
std::set<std::string>* accounts);
bool LoadGServicesSettings(std::map<std::string, std::string>* settings,
std::string* digest);
bool LoadAccountMappingInfo(AccountMappingMap* account_mappings);
bool LoadAccountMappingInfo(AccountMappings* account_mappings);

const base::FilePath path_;
scoped_refptr<base::SequencedTaskRunner> foreground_task_runner_;
Expand Down Expand Up @@ -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;

Expand All @@ -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;
Expand Down
41 changes: 19 additions & 22 deletions google_apis/gcm/engine/gcm_store_impl_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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
Expand Down

0 comments on commit d578c18

Please sign in to comment.