Skip to content

Commit

Permalink
[GCM] Move registration info persistence from extension state store t…
Browse files Browse the repository at this point in the history 
…o GCM store

This is the effort decouple the extension specific logic from
GCMProfileService.

Also remove the code to persist user serial number mappings since
it is not needed any more.

BUG=343268
TEST=tests updated

Review URL: https://codereview.chromium.org/207443002

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@258706 0039d316-1c4b-4281-b951-d872f2087c98
  • Loading branch information
jianli@chromium.org committed Mar 21, 2014
1 parent 1be9fa0 commit 3a20a4d
Show file tree
Hide file tree
Showing 17 changed files with 539 additions and 857 deletions.
369 changes: 27 additions & 342 deletions chrome/browser/services/gcm/gcm_profile_service.cc
View file

Large diffs are not rendered by default.

32 changes: 1 addition & 31 deletions chrome/browser/services/gcm/gcm_profile_service.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -135,15 +135,6 @@ class GCMProfileService : public KeyedService,
class DelayedTaskController;
class IOWorker;

struct RegistrationInfo {
RegistrationInfo();
~RegistrationInfo();
bool IsValid() const;

std::vector<std::string> sender_ids;
std::string registration_id;
};

typedef std::map<std::string, GCMAppHandler*> GCMAppHandlerMap;

// Overridden from content::NotificationObserver:
Expand All @@ -160,9 +151,8 @@ class GCMProfileService : public KeyedService,
// the profile was signed in.
void EnsureLoaded();

// Remove cached or persisted data when GCM service is stopped.
// Remove cached data when GCM service is stopped.
void RemoveCachedData();
void RemovePersistedData();

// Checks out of GCM when the profile has been signed out. This will erase
// all the cached and persisted data.
Expand Down Expand Up @@ -204,24 +194,8 @@ class GCMProfileService : public KeyedService,
// Returns the handler for the given app.
GCMAppHandler* GetAppHandler(const std::string& app_id);

// Used to persist the IDs of registered apps.
void ReadRegisteredAppIDs();
void WriteRegisteredAppIDs();

// Used to persist registration info into the app's state store.
void DeleteRegistrationInfo(const std::string& app_id);
void WriteRegistrationInfo(const std::string& app_id);
void ReadRegistrationInfo(const std::string& app_id);
void ReadRegistrationInfoFinished(const std::string& app_id,
scoped_ptr<base::Value> value);
bool ParsePersistedRegistrationInfo(scoped_ptr<base::Value> value,
RegistrationInfo* registration_info);
void RequestGCMStatisticsFinished(GCMClient::GCMStatistics stats);

// Returns the key used to identify the registration info saved into the
// app's state store. Used for testing purpose.
static const char* GetPersistentRegisterKeyForTesting();

// The profile which owns this object.
Profile* profile_;

Expand Down Expand Up @@ -257,10 +231,6 @@ class GCMProfileService : public KeyedService,
// Callback for RequestGCMStatistics.
RequestGCMStatisticsCallback request_gcm_statistics_callback_;

// Map from app_id to registration info (sender ids & registration ID).
typedef std::map<std::string, RegistrationInfo> RegistrationInfoMap;
RegistrationInfoMap registration_info_map_;

// Used to pass a weak pointer to the IO worker.
base::WeakPtrFactory<GCMProfileService> weak_ptr_factory_;

Expand Down
150 changes: 3 additions & 147 deletions chrome/browser/services/gcm/gcm_profile_service_unittest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -395,26 +395,6 @@ class GCMProfileServiceTestConsumer {
waiter_->SignalCompleted();
}

bool HasPersistedRegistrationInfo(const std::string& app_id) {
StateStore* storage = ExtensionSystem::Get(profile())->state_store();
if (!storage)
return false;
has_persisted_registration_info_ = false;
storage->GetExtensionValue(
app_id,
GCMProfileService::GetPersistentRegisterKeyForTesting(),
base::Bind(
&GCMProfileServiceTestConsumer::ReadRegistrationInfoFinished,
base::Unretained(this)));
waiter_->WaitUntilCompleted();
return has_persisted_registration_info_;
}

void ReadRegistrationInfoFinished(scoped_ptr<base::Value> value) {
has_persisted_registration_info_ = value.get() != NULL;
waiter_->SignalCompleted();
}

void Send(const std::string& app_id,
const std::string& receiver_id,
const GCMClient::OutgoingMessage& message) {
Expand Down Expand Up @@ -461,10 +441,6 @@ class GCMProfileServiceTestConsumer {
return GetGCMProfileService()->gcm_client_ready_;
}

bool ExistsCachedRegistrationInfo() const {
return !GetGCMProfileService()->registration_info_map_.empty();
}

bool HasAppHandlers() const {
return !GetGCMProfileService()->app_handlers_.empty();
}
Expand Down Expand Up @@ -872,12 +848,13 @@ TEST_F(GCMProfileServiceSingleProfileTest, RegisterAgainWithSameSenderIDs) {
consumer()->clear_registration_result();

// Calling register 2nd time with the same set of sender IDs but different
// ordering will get back the same registration ID. There is no need to wait
// since register simply returns the cached registration ID.
// ordering will get back the same registration ID.
std::vector<std::string> another_sender_ids;
another_sender_ids.push_back("sender2");
another_sender_ids.push_back("sender1");
consumer()->Register(kTestingAppId, another_sender_ids);

WaitUntilCompleted();
EXPECT_EQ(expected_registration_id, consumer()->registration_id());
EXPECT_EQ(GCMClient::SUCCESS, consumer()->registration_result());
}
Expand Down Expand Up @@ -907,40 +884,6 @@ TEST_F(GCMProfileServiceSingleProfileTest,
EXPECT_EQ(GCMClient::SUCCESS, consumer()->registration_result());
}

TEST_F(GCMProfileServiceSingleProfileTest, ReadRegistrationFromStateStore) {
scoped_refptr<Extension> extension(consumer()->CreateExtension());

std::vector<std::string> sender_ids;
sender_ids.push_back("sender1");
consumer()->Register(extension->id(), sender_ids);

WaitUntilCompleted();
EXPECT_FALSE(consumer()->registration_id().empty());
EXPECT_EQ(GCMClient::SUCCESS, consumer()->registration_result());
std::string old_registration_id = consumer()->registration_id();

// Clears the results that would be set by the Register callback in
// preparation to call register 2nd time.
consumer()->clear_registration_result();

// Register should not reach the server. Forcing GCMClient server error should
// help catch this.
consumer()->set_gcm_client_error_simulation(GCMClientMock::FORCE_ERROR);

// Simulate start-up by recreating GCMProfileService.
consumer()->CreateGCMProfileServiceInstance();

// Simulate start-up by reloading extension.
consumer()->ReloadExtension(extension);

// This should read the registration info from the extension's state store.
consumer()->Register(extension->id(), sender_ids);
PumpIOLoop();
PumpUILoop();
EXPECT_EQ(old_registration_id, consumer()->registration_id());
EXPECT_EQ(GCMClient::SUCCESS, consumer()->registration_result());
}

TEST_F(GCMProfileServiceSingleProfileTest,
GCMClientReadyAfterReadingRegistration) {
scoped_refptr<Extension> extension(consumer()->CreateExtension());
Expand Down Expand Up @@ -979,25 +922,6 @@ TEST_F(GCMProfileServiceSingleProfileTest,
EXPECT_EQ(GCMClient::SUCCESS, consumer()->registration_result());
}

TEST_F(GCMProfileServiceSingleProfileTest,
PersistedRegistrationInfoRemoveAfterSignOut) {
std::vector<std::string> sender_ids;
sender_ids.push_back("sender1");
consumer()->Register(kTestingAppId, sender_ids);
WaitUntilCompleted();

// The app id and registration info should be persisted.
EXPECT_TRUE(profile()->GetPrefs()->HasPrefPath(prefs::kGCMRegisteredAppIDs));
EXPECT_TRUE(consumer()->HasPersistedRegistrationInfo(kTestingAppId));

consumer()->SignOut();
PumpUILoop();

// The app id and persisted registration info should be removed.
EXPECT_FALSE(profile()->GetPrefs()->HasPrefPath(prefs::kGCMRegisteredAppIDs));
EXPECT_FALSE(consumer()->HasPersistedRegistrationInfo(kTestingAppId));
}

TEST_F(GCMProfileServiceSingleProfileTest, RegisterAfterSignOut) {
// This will trigger check-out.
consumer()->SignOut();
Expand All @@ -1021,21 +945,9 @@ TEST_F(GCMProfileServiceSingleProfileTest, UnregisterImplicitly) {
EXPECT_FALSE(consumer()->registration_id().empty());
EXPECT_EQ(GCMClient::SUCCESS, consumer()->registration_result());

// The registration info should be cached.
EXPECT_TRUE(consumer()->ExistsCachedRegistrationInfo());

// The registration info should be persisted.
EXPECT_TRUE(consumer()->HasPersistedRegistrationInfo(extension->id()));

// Uninstall the extension.
consumer()->UninstallExtension(extension);
base::MessageLoop::current()->RunUntilIdle();

// The cached registration info should be removed.
EXPECT_FALSE(consumer()->ExistsCachedRegistrationInfo());

// The persisted registration info should be removed.
EXPECT_FALSE(consumer()->HasPersistedRegistrationInfo(extension->id()));
}

TEST_F(GCMProfileServiceSingleProfileTest, UnregisterExplicitly) {
Expand All @@ -1047,22 +959,10 @@ TEST_F(GCMProfileServiceSingleProfileTest, UnregisterExplicitly) {
EXPECT_FALSE(consumer()->registration_id().empty());
EXPECT_EQ(GCMClient::SUCCESS, consumer()->registration_result());

// The registration info should be cached.
EXPECT_TRUE(consumer()->ExistsCachedRegistrationInfo());

// The registration info should be persisted.
EXPECT_TRUE(consumer()->HasPersistedRegistrationInfo(kTestingAppId));

consumer()->Unregister(kTestingAppId);

WaitUntilCompleted();
EXPECT_EQ(GCMClient::SUCCESS, consumer()->unregistration_result());

// The cached registration info should be removed.
EXPECT_FALSE(consumer()->ExistsCachedRegistrationInfo());

// The persisted registration info should be removed.
EXPECT_FALSE(consumer()->HasPersistedRegistrationInfo(kTestingAppId));
}

TEST_F(GCMProfileServiceSingleProfileTest,
Expand Down Expand Up @@ -1206,50 +1106,6 @@ TEST_F(GCMProfileServiceSingleProfileTest, MessageReceived) {
consumer()->gcm_app_handler()->message().sender_id);
}

// Flaky on all platforms: http://crbug.com/354803
TEST_F(GCMProfileServiceSingleProfileTest,
DISABLED_MessageNotReceivedFromNotRegisteredSender) {
// Explicitly not registering the sender2 here, so that message gets dropped.
consumer()->Register(kTestingAppId, ToSenderList("sender1"));
WaitUntilCompleted();
GCMClient::IncomingMessage message;
message.data["key1"] = "value1";
message.data["key2"] = "value2";
message.sender_id = "sender2";
consumer()->GetGCMClient()->ReceiveMessage(kTestingAppId, message);
PumpUILoop();
EXPECT_EQ(FakeGCMAppHandler::NO_EVENT,
consumer()->gcm_app_handler()->received_event());
consumer()->gcm_app_handler()->clear_results();

// Register for sender2 and try to receive the message again, which should
// work with no problems.
consumer()->Register(kTestingAppId, ToSenderList("sender1,sender2"));
WaitUntilCompleted();
consumer()->GetGCMClient()->ReceiveMessage(kTestingAppId, message);
WaitUntilCompleted();
EXPECT_EQ(FakeGCMAppHandler::MESSAGE_EVENT,
consumer()->gcm_app_handler()->received_event());
consumer()->gcm_app_handler()->clear_results();

// Making sure that sender1 can receive the message as well.
message.sender_id = "sender1";
consumer()->GetGCMClient()->ReceiveMessage(kTestingAppId, message);
WaitUntilCompleted();
EXPECT_EQ(FakeGCMAppHandler::MESSAGE_EVENT,
consumer()->gcm_app_handler()->received_event());
consumer()->gcm_app_handler()->clear_results();

// Register for sender1 only and make sure it is not possible to receive the
// message again from from sender1.
consumer()->Register(kTestingAppId, ToSenderList("sender2"));
WaitUntilCompleted();
consumer()->GetGCMClient()->ReceiveMessage(kTestingAppId, message);
PumpUILoop();
EXPECT_EQ(FakeGCMAppHandler::NO_EVENT,
consumer()->gcm_app_handler()->received_event());
}

TEST_F(GCMProfileServiceSingleProfileTest, MessageWithCollapseKeyReceived) {
consumer()->Register(kTestingAppId, ToSenderList("sender"));
WaitUntilCompleted();
Expand Down
3 changes: 0 additions & 3 deletions chrome/common/pref_names.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1309,9 +1309,6 @@ const char kProfileResetPromptMemento[] = "profile.reset_prompt_memento";
// The GCM channel's enabled state.
const char kGCMChannelEnabled[] = "gcm.channel_enabled";

// Registered GCM application ids.
const char kGCMRegisteredAppIDs[] = "gcm.register_app_ids";

// Whether Easy Unlock is enabled.
extern const char kEasyUnlockEnabled[] = "easy_unlock.enabled";

Expand Down
1 change: 0 additions & 1 deletion chrome/common/pref_names.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -410,7 +410,6 @@ extern const char kPreferenceResetTime[];
extern const char kProfileResetPromptMemento[];

extern const char kGCMChannelEnabled[];
extern const char kGCMRegisteredAppIDs[];

extern const char kEasyUnlockEnabled[];
extern const char kEasyUnlockShowTutorial[];
Expand Down
7 changes: 0 additions & 7 deletions google_apis/gcm/engine/gcm_store.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,6 @@

namespace gcm {

GCMStore::SerialNumberMappings::SerialNumberMappings()
: next_serial_number(1LL) {
}

GCMStore::SerialNumberMappings::~SerialNumberMappings() {
}

GCMStore::LoadResult::LoadResult()
: success(false),
device_android_id(0),
Expand Down
28 changes: 9 additions & 19 deletions google_apis/gcm/engine/gcm_store.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
#include "base/memory/ref_counted.h"
#include "base/memory/scoped_ptr.h"
#include "google_apis/gcm/base/gcm_export.h"
#include "google_apis/gcm/engine/registration_info.h"
#include "google_apis/gcm/protocol/mcs.pb.h"

namespace google {
Expand All @@ -35,15 +36,6 @@ class GCM_EXPORT GCMStore {
typedef std::map<std::string, linked_ptr<google::protobuf::MessageLite> >
OutgoingMessageMap;

// Part of load results storing user serial number mapping related values.
struct GCM_EXPORT SerialNumberMappings {
SerialNumberMappings();
~SerialNumberMappings();

int64 next_serial_number;
std::map<std::string, int64> user_serial_numbers;
};

// Container for Load(..) results.
struct GCM_EXPORT LoadResult {
LoadResult();
Expand All @@ -52,9 +44,9 @@ class GCM_EXPORT GCMStore {
bool success;
uint64 device_android_id;
uint64 device_security_token;
RegistrationInfoMap registrations;
std::vector<std::string> incoming_messages;
OutgoingMessageMap outgoing_messages;
SerialNumberMappings serial_number_mappings;
};

typedef std::vector<std::string> PersistentIdList;
Expand All @@ -79,6 +71,13 @@ class GCM_EXPORT GCMStore {
uint64 device_security_token,
const UpdateCallback& callback) = 0;

// Registration info.
virtual void AddRegistration(const std::string& app_id,
const linked_ptr<RegistrationInfo>& registration,
const UpdateCallback& callback) = 0;
virtual void RemoveRegistration(const std::string& app_id,
const UpdateCallback& callback) = 0;

// Unacknowledged incoming message handling.
virtual void AddIncomingMessage(const std::string& persistent_id,
const UpdateCallback& callback) = 0;
Expand All @@ -102,15 +101,6 @@ class GCM_EXPORT GCMStore {
virtual void RemoveOutgoingMessages(const PersistentIdList& persistent_ids,
const UpdateCallback& callback) = 0;

// User serial number handling.
virtual void SetNextSerialNumber(int64 next_serial_number,
const UpdateCallback& callback) = 0;
virtual void AddUserSerialNumber(const std::string& username,
int64 serial_number,
const UpdateCallback& callback) = 0;
virtual void RemoveUserSerialNumber(const std::string& username,
const UpdateCallback& callback) = 0;

private:
DISALLOW_COPY_AND_ASSIGN(GCMStore);
};
Expand Down
Loading

0 comments on commit 3a20a4d

Please sign in to comment.