From 3a20a4de02c1500fd3880050a638f49c928b2881 Mon Sep 17 00:00:00 2001 From: "jianli@chromium.org" Date: Fri, 21 Mar 2014 22:54:21 +0000 Subject: [PATCH] [GCM] Move registration info persistence from extension state store to 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 --- chrome/browser/services/gcm/gcm_profile_service.cc | 369 ++------------------- chrome/browser/services/gcm/gcm_profile_service.h | 32 +- .../services/gcm/gcm_profile_service_unittest.cc | 150 +-------- chrome/common/pref_names.cc | 3 - chrome/common/pref_names.h | 1 - google_apis/gcm/engine/gcm_store.cc | 7 - google_apis/gcm/engine/gcm_store.h | 28 +- google_apis/gcm/engine/gcm_store_impl.cc | 308 +++++++---------- google_apis/gcm/engine/gcm_store_impl.h | 16 +- google_apis/gcm/engine/gcm_store_impl_unittest.cc | 118 +++---- google_apis/gcm/engine/registration_info.cc | 62 ++++ google_apis/gcm/engine/registration_info.h | 35 ++ google_apis/gcm/gcm.gyp | 2 + google_apis/gcm/gcm_client_impl.cc | 98 ++++-- google_apis/gcm/gcm_client_impl.h | 25 +- google_apis/gcm/gcm_client_impl_unittest.cc | 132 +++++++- tools/metrics/histograms/histograms.xml | 10 + 17 files changed, 539 insertions(+), 857 deletions(-) create mode 100644 google_apis/gcm/engine/registration_info.cc create mode 100644 google_apis/gcm/engine/registration_info.h diff --git a/chrome/browser/services/gcm/gcm_profile_service.cc b/chrome/browser/services/gcm/gcm_profile_service.cc index 2f8fcc08187f..d27c2314e312 100644 --- a/chrome/browser/services/gcm/gcm_profile_service.cc +++ b/chrome/browser/services/gcm/gcm_profile_service.cc @@ -16,7 +16,6 @@ #include "base/strings/string_number_conversions.h" #include "base/threading/sequenced_worker_pool.h" #include "chrome/browser/chrome_notification_types.h" -#include "chrome/browser/extensions/state_store.h" #include "chrome/browser/services/gcm/gcm_app_handler.h" #include "chrome/browser/services/gcm/gcm_client_factory.h" #include "chrome/browser/signin/profile_oauth2_token_service_factory.h" @@ -31,7 +30,6 @@ #include "content/public/browser/browser_thread.h" #include "content/public/browser/notification_details.h" #include "content/public/browser/notification_source.h" -#include "extensions/browser/extension_system.h" #include "google_apis/gcm/protocol/android_checkin.pb.h" #include "net/url_request/url_request_context_getter.h" @@ -39,10 +37,6 @@ namespace gcm { namespace { -const char kRegistrationKey[] = "gcm.registration"; -const char kSendersKey[] = "senders"; -const char kRegistrationIDKey[] = "reg_id"; - checkin_proto::ChromeBuildProto_Platform GetPlatform() { #if defined(OS_WIN) return checkin_proto::ChromeBuildProto_Platform_PLATFORM_WIN; @@ -90,139 +84,51 @@ class GCMProfileService::DelayedTaskController { DelayedTaskController(); ~DelayedTaskController(); - // Adds an app to the tracking list. It will be first marked as not ready. - // Tasks will be queued for delay execution until the app is marked as ready. - void AddApp(const std::string& app_id); - - // Removes the app from the tracking list. - void RemoveApp(const std::string& app_id); - // Adds a task that will be invoked once we're ready. - void AddTask(const std::string& app_id, base::Closure task); + void AddTask(base::Closure task); - // Sets GCM ready status. GCM is ready only when check-in is completed and - // the GCMClient is fully initialized. If it is set to ready for the first - // time, all the pending tasks for any ready apps will be run. - void SetGCMReady(); + // Sets ready status. It is ready only when check-in is completed and + // the GCMClient is fully initialized. + void SetReady(); - // Sets ready status for the app. If GCM is already ready, all the pending - // tasks for this app will be run. - void SetAppReady(const std::string& app_id); - - // Returns true if it is ready to perform operations for an app. - bool CanRunTaskWithoutDelay(const std::string& app_id) const; - - // Returns true if the app has been tracked for readiness. - bool IsAppTracked(const std::string& app_id) const; + // Returns true if it is ready to perform tasks. + bool CanRunTaskWithoutDelay() const; private: - struct AppTaskQueue { - AppTaskQueue(); - ~AppTaskQueue(); - - // The flag that indicates if GCMClient is ready. - bool ready; + void RunTasks(); - // Tasks to be invoked upon ready. - std::vector tasks; - }; - - void RunTasks(AppTaskQueue* task_queue); - - // Flag that indicates that GCM is done. - bool gcm_ready_; + // Flag that indicates that GCM is ready. + bool ready_; - // Map from app_id to AppTaskQueue storing the tasks that will be invoked - // when both GCM and the app get ready. - typedef std::map DelayedTaskMap; - DelayedTaskMap delayed_task_map_; + std::vector delayed_tasks_; }; -GCMProfileService::DelayedTaskController::AppTaskQueue::AppTaskQueue() - : ready(false) { -} - -GCMProfileService::DelayedTaskController::AppTaskQueue::~AppTaskQueue() { -} - GCMProfileService::DelayedTaskController::DelayedTaskController() - : gcm_ready_(false) { + : ready_(false) { } GCMProfileService::DelayedTaskController::~DelayedTaskController() { - for (DelayedTaskMap::const_iterator iter = delayed_task_map_.begin(); - iter != delayed_task_map_.end(); ++iter) { - delete iter->second; - } -} - -void GCMProfileService::DelayedTaskController::AddApp( - const std::string& app_id) { - DCHECK(delayed_task_map_.find(app_id) == delayed_task_map_.end()); - delayed_task_map_[app_id] = new AppTaskQueue; } -void GCMProfileService::DelayedTaskController::RemoveApp( - const std::string& app_id) { - DelayedTaskMap::iterator iter = delayed_task_map_.find(app_id); - if (iter == delayed_task_map_.end()) - return; - delete iter->second; - delayed_task_map_.erase(iter); +void GCMProfileService::DelayedTaskController::AddTask(base::Closure task) { + delayed_tasks_.push_back(task); } -void GCMProfileService::DelayedTaskController::AddTask( - const std::string& app_id, base::Closure task) { - DelayedTaskMap::const_iterator iter = delayed_task_map_.find(app_id); - DCHECK(iter != delayed_task_map_.end()); - iter->second->tasks.push_back(task); +void GCMProfileService::DelayedTaskController::SetReady() { + ready_ = true; + RunTasks(); } -void GCMProfileService::DelayedTaskController::SetGCMReady() { - gcm_ready_ = true; - - for (DelayedTaskMap::iterator iter = delayed_task_map_.begin(); - iter != delayed_task_map_.end(); ++iter) { - if (iter->second->ready) - RunTasks(iter->second); - } +bool GCMProfileService::DelayedTaskController::CanRunTaskWithoutDelay() const { + return ready_; } -void GCMProfileService::DelayedTaskController::SetAppReady( - const std::string& app_id) { - DelayedTaskMap::iterator iter = delayed_task_map_.find(app_id); - DCHECK(iter != delayed_task_map_.end()); - - AppTaskQueue* task_queue = iter->second; - DCHECK(task_queue); - task_queue->ready = true; - - if (gcm_ready_) - RunTasks(task_queue); -} +void GCMProfileService::DelayedTaskController::RunTasks() { + DCHECK(ready_); -bool GCMProfileService::DelayedTaskController::CanRunTaskWithoutDelay( - const std::string& app_id) const { - if (!gcm_ready_) - return false; - DelayedTaskMap::const_iterator iter = delayed_task_map_.find(app_id); - if (iter == delayed_task_map_.end()) - return true; - return iter->second->ready; -} - -bool GCMProfileService::DelayedTaskController::IsAppTracked( - const std::string& app_id) const { - return delayed_task_map_.find(app_id) != delayed_task_map_.end(); -} - -void GCMProfileService::DelayedTaskController::RunTasks( - AppTaskQueue* task_queue) { - DCHECK(gcm_ready_ && task_queue->ready); - - for (size_t i = 0; i < task_queue->tasks.size(); ++i) - task_queue->tasks[i].Run(); - task_queue->tasks.clear(); + for (size_t i = 0; i < delayed_tasks_.size(); ++i) + delayed_tasks_[i].Run(); + delayed_tasks_.clear(); } class GCMProfileService::IOWorker @@ -494,16 +400,6 @@ std::string GCMProfileService::GetGCMEnabledStateString(GCMEnabledState state) { } } -GCMProfileService::RegistrationInfo::RegistrationInfo() { -} - -GCMProfileService::RegistrationInfo::~RegistrationInfo() { -} - -bool GCMProfileService::RegistrationInfo::IsValid() const { - return !sender_ids.empty() && !registration_id.empty(); -} - // static GCMProfileService::GCMEnabledState GCMProfileService::GetGCMEnabledState( Profile* profile) { @@ -534,9 +430,6 @@ void GCMProfileService::RegisterProfilePrefs( prefs::kGCMChannelEnabled, on_by_default, user_prefs::PrefRegistrySyncable::UNSYNCABLE_PREF); - registry->RegisterListPref( - prefs::kGCMRegisteredAppIDs, - user_prefs::PrefRegistrySyncable::UNSYNCABLE_PREF); } GCMProfileService::GCMProfileService(Profile* profile) @@ -652,9 +545,8 @@ void GCMProfileService::Register(const std::string& app_id, register_callbacks_[app_id] = callback; // Delay the register operation until GCMClient is ready. - if (!delayed_task_controller_->CanRunTaskWithoutDelay(app_id)) { + if (!delayed_task_controller_->CanRunTaskWithoutDelay()) { delayed_task_controller_->AddTask( - app_id, base::Bind(&GCMProfileService::DoRegister, weak_ptr_factory_.GetWeakPtr(), app_id, @@ -678,29 +570,6 @@ void GCMProfileService::DoRegister(const std::string& app_id, std::vector normalized_sender_ids = sender_ids; std::sort(normalized_sender_ids.begin(), normalized_sender_ids.end()); - // If the same sender ids is provided, return the cached registration ID - // directly. - RegistrationInfoMap::const_iterator registration_info_iter = - registration_info_map_.find(app_id); - if (registration_info_iter != registration_info_map_.end() && - registration_info_iter->second.sender_ids == normalized_sender_ids) { - RegisterCallback callback = callback_iter->second; - register_callbacks_.erase(callback_iter); - callback.Run(registration_info_iter->second.registration_id, - GCMClient::SUCCESS); - return; - } - - // Cache the sender IDs. The registration ID will be filled when the - // registration completes. - RegistrationInfo registration_info; - registration_info.sender_ids = normalized_sender_ids; - registration_info_map_[app_id] = registration_info; - - // Save the IDs of all registered apps such that we know what to remove from - // the the app's state store when the profile is signed out. - WriteRegisteredAppIDs(); - content::BrowserThread::PostTask( content::BrowserThread::IO, FROM_HERE, @@ -730,9 +599,8 @@ void GCMProfileService::Unregister(const std::string& app_id, unregister_callbacks_[app_id] = callback; // Delay the unregister operation until GCMClient is ready. - if (!delayed_task_controller_->CanRunTaskWithoutDelay(app_id)) { + if (!delayed_task_controller_->CanRunTaskWithoutDelay()) { delayed_task_controller_->AddTask( - app_id, base::Bind(&GCMProfileService::DoUnregister, weak_ptr_factory_.GetWeakPtr(), app_id)); @@ -745,23 +613,6 @@ void GCMProfileService::Unregister(const std::string& app_id, void GCMProfileService::DoUnregister(const std::string& app_id) { DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI)); - // Unregister might be triggered in response to a message that is missing - // recipient and in that case there will not be an entry in the map. - RegistrationInfoMap::iterator registration_info_iter = - registration_info_map_.find(app_id); - if (registration_info_iter != registration_info_map_.end()) { - registration_info_map_.erase(registration_info_iter); - - // Update the persisted IDs of registered apps. - WriteRegisteredAppIDs(); - - // Remove the persisted registration info. - DeleteRegistrationInfo(app_id); - - // No need to track the app any more. - delayed_task_controller_->RemoveApp(app_id); - } - // Ask the server to unregister it. There could be a small chance that the // unregister request fails. If this occurs, it does not bring any harm since // we simply reject the messages/events received from the server. @@ -796,9 +647,8 @@ void GCMProfileService::Send(const std::string& app_id, send_callbacks_[key] = callback; // Delay the send operation until all GCMClient is ready. - if (!delayed_task_controller_->CanRunTaskWithoutDelay(app_id)) { + if (!delayed_task_controller_->CanRunTaskWithoutDelay()) { delayed_task_controller_->AddTask( - app_id, base::Bind(&GCMProfileService::DoSend, weak_ptr_factory_.GetWeakPtr(), app_id, @@ -890,9 +740,6 @@ void GCMProfileService::EnsureLoaded() { DCHECK(!delayed_task_controller_); delayed_task_controller_.reset(new DelayedTaskController); - // Load all the registered apps. - ReadRegisteredAppIDs(); - // This will load the data from the gcm store and trigger the check-in if // the persisted check-in info is not found. // Note that we need to pass weak pointer again since the existing weak @@ -915,19 +762,6 @@ void GCMProfileService::RemoveCachedData() { delayed_task_controller_.reset(); register_callbacks_.clear(); send_callbacks_.clear(); - registration_info_map_.clear(); -} - -void GCMProfileService::RemovePersistedData() { - // Remove persisted data from app's state store. - for (RegistrationInfoMap::const_iterator iter = - registration_info_map_.begin(); - iter != registration_info_map_.end(); ++iter) { - DeleteRegistrationInfo(iter->first); - } - - // Remove persisted data from prefs store. - profile_->GetPrefs()->ClearPref(prefs::kGCMRegisteredAppIDs); } void GCMProfileService::CheckOut() { @@ -937,10 +771,6 @@ void GCMProfileService::CheckOut() { // initiated in the current session. This will make sure that all the // persisted data written previously will get purged. - // This has to be done before removing the cached data since we need to do - // the lookup based on the cached data. - RemovePersistedData(); - RemoveCachedData(); content::BrowserThread::PostTask( @@ -964,10 +794,6 @@ GCMClient::Result GCMProfileService::EnsureAppReady(const std::string& app_id) { if (username_.empty()) return GCMClient::NOT_SIGNED_IN; - // Ensure that app registration information was read. - if (!delayed_task_controller_->IsAppTracked(app_id)) - ReadRegistrationInfo(app_id); - return GCMClient::SUCCESS; } @@ -989,20 +815,6 @@ void GCMProfileService::RegisterFinished(const std::string& app_id, return; } - // Cache the registration ID if the registration succeeds. Otherwise, - // removed the cached info. - RegistrationInfoMap::iterator registration_info_iter = - registration_info_map_.find(app_id); - // This is unlikely to happen because the app will not be uninstalled before - // the asynchronous extension function completes. - DCHECK(registration_info_iter != registration_info_map_.end()); - if (result == GCMClient::SUCCESS) { - registration_info_iter->second.registration_id = registration_id; - WriteRegistrationInfo(app_id); - } else { - registration_info_map_.erase(registration_info_iter); - } - RegisterCallback callback = callback_iter->second; register_callbacks_.erase(callback_iter); callback.Run(registration_id, result); @@ -1048,16 +860,6 @@ void GCMProfileService::MessageReceived(const std::string& app_id, if (username_.empty()) return; - // Dropping the message when application does not have a registration entry - // or the app is not registered for the sender of the message. - RegistrationInfoMap::iterator iter = registration_info_map_.find(app_id); - if (iter == registration_info_map_.end() || - std::find(iter->second.sender_ids.begin(), - iter->second.sender_ids.end(), - message.sender_id) == iter->second.sender_ids.end()) { - return; - } - GetAppHandler(app_id)->OnMessage(app_id, message); } @@ -1090,7 +892,7 @@ void GCMProfileService::GCMClientReady() { return; gcm_client_ready_ = true; - delayed_task_controller_->SetGCMReady(); + delayed_task_controller_->SetReady(); } GCMAppHandler* GCMProfileService::GetAppHandler(const std::string& app_id) { @@ -1099,118 +901,6 @@ GCMAppHandler* GCMProfileService::GetAppHandler(const std::string& app_id) { return iter == app_handlers_.end() ? &default_app_handler_ : iter->second; } -void GCMProfileService::ReadRegisteredAppIDs() { - const base::ListValue* app_id_list = - profile_->GetPrefs()->GetList(prefs::kGCMRegisteredAppIDs); - for (size_t i = 0; i < app_id_list->GetSize(); ++i) { - std::string app_id; - if (!app_id_list->GetString(i, &app_id)) - continue; - ReadRegistrationInfo(app_id); - } -} - -void GCMProfileService::WriteRegisteredAppIDs() { - base::ListValue apps; - for (RegistrationInfoMap::const_iterator iter = - registration_info_map_.begin(); - iter != registration_info_map_.end(); ++iter) { - apps.Append(new base::StringValue(iter->first)); - } - profile_->GetPrefs()->Set(prefs::kGCMRegisteredAppIDs, apps); -} - -void GCMProfileService::DeleteRegistrationInfo(const std::string& app_id) { - extensions::StateStore* storage = - extensions::ExtensionSystem::Get(profile_)->state_store(); - DCHECK(storage); - - storage->RemoveExtensionValue(app_id, kRegistrationKey); -} - -void GCMProfileService::WriteRegistrationInfo(const std::string& app_id) { - extensions::StateStore* storage = - extensions::ExtensionSystem::Get(profile_)->state_store(); - DCHECK(storage); - - RegistrationInfoMap::const_iterator registration_info_iter = - registration_info_map_.find(app_id); - if (registration_info_iter == registration_info_map_.end()) - return; - const RegistrationInfo& registration_info = registration_info_iter->second; - - scoped_ptr senders_list(new base::ListValue()); - for (std::vector::const_iterator senders_iter = - registration_info.sender_ids.begin(); - senders_iter != registration_info.sender_ids.end(); - ++senders_iter) { - senders_list->AppendString(*senders_iter); - } - - scoped_ptr registration_info_dict( - new base::DictionaryValue()); - registration_info_dict->Set(kSendersKey, senders_list.release()); - registration_info_dict->SetString(kRegistrationIDKey, - registration_info.registration_id); - - storage->SetExtensionValue( - app_id, kRegistrationKey, registration_info_dict.PassAs()); -} - -void GCMProfileService::ReadRegistrationInfo(const std::string& app_id) { - delayed_task_controller_->AddApp(app_id); - - extensions::StateStore* storage = - extensions::ExtensionSystem::Get(profile_)->state_store(); - DCHECK(storage); - storage->GetExtensionValue( - app_id, - kRegistrationKey, - base::Bind( - &GCMProfileService::ReadRegistrationInfoFinished, - weak_ptr_factory_.GetWeakPtr(), - app_id)); -} - -void GCMProfileService::ReadRegistrationInfoFinished( - const std::string& app_id, - scoped_ptr value) { - RegistrationInfo registration_info; - if (value && - !ParsePersistedRegistrationInfo(value.Pass(), ®istration_info)) { - // Delete the persisted data if it is corrupted. - DeleteRegistrationInfo(app_id); - } - - if (registration_info.IsValid()) - registration_info_map_[app_id] = registration_info; - - delayed_task_controller_->SetAppReady(app_id); -} - -bool GCMProfileService::ParsePersistedRegistrationInfo( - scoped_ptr value, - RegistrationInfo* registration_info) { - base::DictionaryValue* dict = NULL; - if (!value.get() || !value->GetAsDictionary(&dict)) - return false; - - if (!dict->GetString(kRegistrationIDKey, ®istration_info->registration_id)) - return false; - - const base::ListValue* senders_list = NULL; - if (!dict->GetList(kSendersKey, &senders_list) || !senders_list->GetSize()) - return false; - for (size_t i = 0; i < senders_list->GetSize(); ++i) { - std::string sender; - if (!senders_list->GetString(i, &sender)) - return false; - registration_info->sender_ids.push_back(sender); - } - - return true; -} - void GCMProfileService::RequestGCMStatisticsFinished( GCMClient::GCMStatistics stats) { DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI)); @@ -1218,9 +908,4 @@ void GCMProfileService::RequestGCMStatisticsFinished( request_gcm_statistics_callback_.Run(stats); } -// static -const char* GCMProfileService::GetPersistentRegisterKeyForTesting() { - return kRegistrationKey; -} - } // namespace gcm diff --git a/chrome/browser/services/gcm/gcm_profile_service.h b/chrome/browser/services/gcm/gcm_profile_service.h index 672fccc5e4c2..75eefe659049 100644 --- a/chrome/browser/services/gcm/gcm_profile_service.h +++ b/chrome/browser/services/gcm/gcm_profile_service.h @@ -135,15 +135,6 @@ class GCMProfileService : public KeyedService, class DelayedTaskController; class IOWorker; - struct RegistrationInfo { - RegistrationInfo(); - ~RegistrationInfo(); - bool IsValid() const; - - std::vector sender_ids; - std::string registration_id; - }; - typedef std::map GCMAppHandlerMap; // Overridden from content::NotificationObserver: @@ -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. @@ -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 value); - bool ParsePersistedRegistrationInfo(scoped_ptr 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_; @@ -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 RegistrationInfoMap; - RegistrationInfoMap registration_info_map_; - // Used to pass a weak pointer to the IO worker. base::WeakPtrFactory weak_ptr_factory_; diff --git a/chrome/browser/services/gcm/gcm_profile_service_unittest.cc b/chrome/browser/services/gcm/gcm_profile_service_unittest.cc index 827368ca928d..39bbaf0552d6 100644 --- a/chrome/browser/services/gcm/gcm_profile_service_unittest.cc +++ b/chrome/browser/services/gcm/gcm_profile_service_unittest.cc @@ -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 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) { @@ -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(); } @@ -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 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()); } @@ -907,40 +884,6 @@ TEST_F(GCMProfileServiceSingleProfileTest, EXPECT_EQ(GCMClient::SUCCESS, consumer()->registration_result()); } -TEST_F(GCMProfileServiceSingleProfileTest, ReadRegistrationFromStateStore) { - scoped_refptr extension(consumer()->CreateExtension()); - - std::vector 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(consumer()->CreateExtension()); @@ -979,25 +922,6 @@ TEST_F(GCMProfileServiceSingleProfileTest, EXPECT_EQ(GCMClient::SUCCESS, consumer()->registration_result()); } -TEST_F(GCMProfileServiceSingleProfileTest, - PersistedRegistrationInfoRemoveAfterSignOut) { - std::vector 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(); @@ -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) { @@ -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, @@ -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(); diff --git a/chrome/common/pref_names.cc b/chrome/common/pref_names.cc index 6953a8adc2e2..4e62c2a9c6a1 100644 --- a/chrome/common/pref_names.cc +++ b/chrome/common/pref_names.cc @@ -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"; diff --git a/chrome/common/pref_names.h b/chrome/common/pref_names.h index 545d5c032681..bf6d2d623fea 100644 --- a/chrome/common/pref_names.h +++ b/chrome/common/pref_names.h @@ -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[]; diff --git a/google_apis/gcm/engine/gcm_store.cc b/google_apis/gcm/engine/gcm_store.cc index f6c81dda8958..91ad60fe8524 100644 --- a/google_apis/gcm/engine/gcm_store.cc +++ b/google_apis/gcm/engine/gcm_store.cc @@ -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), diff --git a/google_apis/gcm/engine/gcm_store.h b/google_apis/gcm/engine/gcm_store.h index 4387042a8447..659fc7f9d1ea 100644 --- a/google_apis/gcm/engine/gcm_store.h +++ b/google_apis/gcm/engine/gcm_store.h @@ -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 { @@ -35,15 +36,6 @@ class GCM_EXPORT GCMStore { typedef std::map > OutgoingMessageMap; - // Part of load results storing user serial number mapping related values. - struct GCM_EXPORT SerialNumberMappings { - SerialNumberMappings(); - ~SerialNumberMappings(); - - int64 next_serial_number; - std::map user_serial_numbers; - }; - // Container for Load(..) results. struct GCM_EXPORT LoadResult { LoadResult(); @@ -52,9 +44,9 @@ class GCM_EXPORT GCMStore { bool success; uint64 device_android_id; uint64 device_security_token; + RegistrationInfoMap registrations; std::vector incoming_messages; OutgoingMessageMap outgoing_messages; - SerialNumberMappings serial_number_mappings; }; typedef std::vector PersistentIdList; @@ -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& 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; @@ -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); }; diff --git a/google_apis/gcm/engine/gcm_store_impl.cc b/google_apis/gcm/engine/gcm_store_impl.cc index 100cd5730452..7f272870d808 100644 --- a/google_apis/gcm/engine/gcm_store_impl.cc +++ b/google_apis/gcm/engine/gcm_store_impl.cc @@ -35,29 +35,32 @@ const int kMessagesPerAppLimit = 20; const char kDeviceAIDKey[] = "device_aid_key"; // Key for this device's android security token. const char kDeviceTokenKey[] = "device_token_key"; +// Lowest lexicographically ordered app ids. +// Used for prefixing app id. +const char kRegistrationKeyStart[] = "reg1-"; +// Key guaranteed to be higher than all app ids. +// Used for limiting iteration. +const char kRegistrationKeyEnd[] = "reg2-"; // Lowest lexicographically ordered incoming message key. // Used for prefixing messages. const char kIncomingMsgKeyStart[] = "incoming1-"; // Key guaranteed to be higher than all incoming message keys. // Used for limiting iteration. const char kIncomingMsgKeyEnd[] = "incoming2-"; -// Key for next serial number assigned to the user. -const char kNextSerialNumberKey[] = "next_serial_number_key"; // Lowest lexicographically ordered outgoing message key. // Used for prefixing outgoing messages. const char kOutgoingMsgKeyStart[] = "outgoing1-"; // Key guaranteed to be higher than all outgoing message keys. // Used for limiting iteration. const char kOutgoingMsgKeyEnd[] = "outgoing2-"; -// Lowest lexicographically ordered username. -// Used for prefixing username to serial number mappings. -const char kUserSerialNumberKeyStart[] = "user1-"; -// Key guaranteed to be higher than all usernames. -// Used for limiting iteration. -const char kUserSerialNumberKeyEnd[] = "user2-"; -// Value indicating that serial number was not assigned. -const int64 kSerialNumberMissing = -1LL; +std::string MakeRegistrationKey(const std::string& app_id) { + return kRegistrationKeyStart + app_id; +} + +std::string ParseRegistrationKey(const std::string& key) { + return key.substr(arraysize(kRegistrationKeyStart) - 1); +} std::string MakeIncomingKey(const std::string& persistent_id) { return kIncomingMsgKeyStart + persistent_id; @@ -67,18 +70,10 @@ std::string MakeOutgoingKey(const std::string& persistent_id) { return kOutgoingMsgKeyStart + persistent_id; } -std::string MakeUserSerialNumberKey(const std::string& username) { - return kUserSerialNumberKeyStart + username; -} - std::string ParseOutgoingKey(const std::string& key) { return key.substr(arraysize(kOutgoingMsgKeyStart) - 1); } -std::string ParseUsername(const std::string& key) { - return key.substr(arraysize(kUserSerialNumberKeyStart) - 1); -} - // Note: leveldb::Slice keeps a pointer to the data in |s|, which must therefore // outlive the slice. // For example: MakeSlice(MakeOutgoingKey(x)) is invalid. @@ -101,6 +96,11 @@ class GCMStoreImpl::Backend void SetDeviceCredentials(uint64 device_android_id, uint64 device_security_token, const UpdateCallback& callback); + void AddRegistration(const std::string& app_id, + const linked_ptr& registration, + const UpdateCallback& callback); + void RemoveRegistration(const std::string& app_id, + const UpdateCallback& callback); void AddIncomingMessage(const std::string& persistent_id, const UpdateCallback& callback); void RemoveIncomingMessages(const PersistentIdList& persistent_ids, @@ -124,11 +124,9 @@ class GCMStoreImpl::Backend ~Backend(); bool LoadDeviceCredentials(uint64* android_id, uint64* security_token); + bool LoadRegistrations(RegistrationInfoMap* registrations); bool LoadIncomingMessages(std::vector* incoming_messages); bool LoadOutgoingMessages(OutgoingMessageMap* outgoing_messages); - bool LoadNextSerialNumber(int64* next_serial_number); - bool LoadUserSerialNumberMap( - std::map* user_serial_number_map); const base::FilePath path_; scoped_refptr foreground_task_runner_; @@ -171,14 +169,12 @@ void GCMStoreImpl::Backend::Load(const LoadCallback& callback) { if (!LoadDeviceCredentials(&result->device_android_id, &result->device_security_token) || + !LoadRegistrations(&result->registrations) || !LoadIncomingMessages(&result->incoming_messages) || - !LoadOutgoingMessages(&result->outgoing_messages) || - !LoadNextSerialNumber( - &result->serial_number_mappings.next_serial_number) || - !LoadUserSerialNumberMap( - &result->serial_number_mappings.user_serial_numbers)) { + !LoadOutgoingMessages(&result->outgoing_messages)) { result->device_android_id = 0; result->device_security_token = 0; + result->registrations.clear(); result->incoming_messages.clear(); result->outgoing_messages.clear(); foreground_task_runner_->PostTask(FROM_HERE, @@ -194,16 +190,17 @@ void GCMStoreImpl::Backend::Load(const LoadCallback& callback) { UMA_HISTOGRAM_COUNTS("GCM.StoreSizeKB", static_cast(file_size / 1024)); } + UMA_HISTOGRAM_COUNTS("GCM.RestoredRegistrations", + result->registrations.size()); UMA_HISTOGRAM_COUNTS("GCM.RestoredOutgoingMessages", result->outgoing_messages.size()); UMA_HISTOGRAM_COUNTS("GCM.RestoredIncomingMessages", result->incoming_messages.size()); - UMA_HISTOGRAM_COUNTS( - "GCM.NumUsers", - result->serial_number_mappings.user_serial_numbers.size()); } - DVLOG(1) << "Succeeded in loading " << result->incoming_messages.size() + DVLOG(1) << "Succeeded in loading " << result->registrations.size() + << " registrations, " + << result->incoming_messages.size() << " unacknowledged incoming messages and " << result->outgoing_messages.size() << " unacknowledged outgoing messages."; @@ -266,6 +263,51 @@ void GCMStoreImpl::Backend::SetDeviceCredentials( foreground_task_runner_->PostTask(FROM_HERE, base::Bind(callback, false)); } +void GCMStoreImpl::Backend::AddRegistration( + const std::string& app_id, + const linked_ptr& registration, + const UpdateCallback& callback) { + DVLOG(1) << "Saving registration info for app: " << app_id; + if (!db_.get()) { + LOG(ERROR) << "GCMStore db doesn't exist."; + foreground_task_runner_->PostTask(FROM_HERE, base::Bind(callback, false)); + return; + } + leveldb::WriteOptions write_options; + write_options.sync = true; + + std::string key = MakeRegistrationKey(app_id); + std::string value = registration->SerializeAsString(); + const leveldb::Status status = db_->Put(write_options, + MakeSlice(key), + MakeSlice(value)); + if (status.ok()) { + foreground_task_runner_->PostTask(FROM_HERE, base::Bind(callback, true)); + return; + } + LOG(ERROR) << "LevelDB put failed: " << status.ToString(); + foreground_task_runner_->PostTask(FROM_HERE, base::Bind(callback, false)); +} + +void GCMStoreImpl::Backend::RemoveRegistration(const std::string& app_id, + const UpdateCallback& callback) { + if (!db_.get()) { + LOG(ERROR) << "GCMStore db doesn't exist."; + foreground_task_runner_->PostTask(FROM_HERE, base::Bind(callback, false)); + return; + } + leveldb::WriteOptions write_options; + write_options.sync = true; + + leveldb::Status status = db_->Delete(write_options, MakeSlice(app_id)); + if (status.ok()) { + foreground_task_runner_->PostTask(FROM_HERE, base::Bind(callback, true)); + return; + } + LOG(ERROR) << "LevelDB remove failed: " << status.ToString(); + foreground_task_runner_->PostTask(FROM_HERE, base::Bind(callback, false)); +} + void GCMStoreImpl::Backend::AddIncomingMessage(const std::string& persistent_id, const UpdateCallback& callback) { DVLOG(1) << "Saving incoming message with id " << persistent_id; @@ -403,79 +445,6 @@ void GCMStoreImpl::Backend::RemoveOutgoingMessages( AppIdToMessageCountMap())); } -void GCMStoreImpl::Backend::AddUserSerialNumber( - const std::string& username, - int64 serial_number, - const UpdateCallback& callback) { - DVLOG(1) << "Saving username to serial number mapping for user: " << username; - if (!db_.get()) { - LOG(ERROR) << "GCMStore db doesn't exist."; - foreground_task_runner_->PostTask(FROM_HERE, base::Bind(callback, false)); - return; - } - leveldb::WriteOptions write_options; - write_options.sync = true; - - std::string key = MakeUserSerialNumberKey(username); - std::string serial_number_str = base::Int64ToString(serial_number); - const leveldb::Status status = - db_->Put(write_options, - MakeSlice(key), - MakeSlice(serial_number_str)); - if (status.ok()) { - foreground_task_runner_->PostTask(FROM_HERE, base::Bind(callback, true)); - return; - } - LOG(ERROR) << "LevelDB put failed: " << status.ToString(); - foreground_task_runner_->PostTask(FROM_HERE, base::Bind(callback, false)); -} - -void GCMStoreImpl::Backend::RemoveUserSerialNumber( - const std::string& username, - const UpdateCallback& callback) { - if (!db_.get()) { - LOG(ERROR) << "GCMStore db doesn't exist."; - foreground_task_runner_->PostTask(FROM_HERE, base::Bind(callback, false)); - return; - } - leveldb::WriteOptions write_options; - write_options.sync = true; - - leveldb::Status status = db_->Delete(write_options, MakeSlice(username)); - if (status.ok()) { - foreground_task_runner_->PostTask(FROM_HERE, base::Bind(callback, true)); - return; - } - LOG(ERROR) << "LevelDB remove failed: " << status.ToString(); - foreground_task_runner_->PostTask(FROM_HERE, base::Bind(callback, false)); -} - -void GCMStoreImpl::Backend::SetNextSerialNumber( - int64 next_serial_number, - const UpdateCallback& callback) { - DVLOG(1) << "Updating the value of next user serial number to: " - << next_serial_number; - if (!db_.get()) { - LOG(ERROR) << "GCMStore db doesn't exist."; - foreground_task_runner_->PostTask(FROM_HERE, base::Bind(callback, false)); - return; - } - leveldb::WriteOptions write_options; - write_options.sync = true; - - std::string serial_number_str = base::Int64ToString(next_serial_number); - const leveldb::Status status = - db_->Put(write_options, - MakeSlice(kNextSerialNumberKey), - MakeSlice(serial_number_str)); - if (status.ok()) { - foreground_task_runner_->PostTask(FROM_HERE, base::Bind(callback, true)); - return; - } - LOG(ERROR) << "LevelDB put failed: " << status.ToString(); - foreground_task_runner_->PostTask(FROM_HERE, base::Bind(callback, false)); -} - bool GCMStoreImpl::Backend::LoadDeviceCredentials(uint64* android_id, uint64* security_token) { leveldb::ReadOptions read_options; @@ -510,6 +479,33 @@ bool GCMStoreImpl::Backend::LoadDeviceCredentials(uint64* android_id, return false; } +bool GCMStoreImpl::Backend::LoadRegistrations( + RegistrationInfoMap* registrations) { + leveldb::ReadOptions read_options; + read_options.verify_checksums = true; + + scoped_ptr iter(db_->NewIterator(read_options)); + for (iter->Seek(MakeSlice(kRegistrationKeyStart)); + iter->Valid() && iter->key().ToString() < kRegistrationKeyEnd; + iter->Next()) { + leveldb::Slice s = iter->value(); + if (s.size() <= 1) { + LOG(ERROR) << "Error reading registration with key " << s.ToString(); + return false; + } + std::string app_id = ParseRegistrationKey(iter->key().ToString()); + linked_ptr registration(new RegistrationInfo); + if (!registration->ParseFromString(iter->value().ToString())) { + LOG(ERROR) << "Failed to parse registration with app id " << app_id; + return false; + } + DVLOG(1) << "Found registration with app id " << app_id; + (*registrations)[app_id] = registration; + } + + return true; +} + bool GCMStoreImpl::Backend::LoadIncomingMessages( std::vector* incoming_messages) { leveldb::ReadOptions read_options; @@ -564,57 +560,6 @@ bool GCMStoreImpl::Backend::LoadOutgoingMessages( return true; } -bool GCMStoreImpl::Backend::LoadNextSerialNumber(int64* next_serial_number) { - leveldb::ReadOptions read_options; - read_options.verify_checksums = true; - - std::string result; - leveldb::Status status = - db_->Get(read_options, MakeSlice(kNextSerialNumberKey), &result); - if (status.ok()) { - if (!base::StringToInt64(result, next_serial_number)) { - LOG(ERROR) << "Failed to restore the next serial number."; - return false; - } - return true; - } - - if (status.IsNotFound()) { - DVLOG(1) << "No next serial number found."; - return true; - } - - LOG(ERROR) << "Error when reading the next serial number."; - return false; -} - -bool GCMStoreImpl::Backend::LoadUserSerialNumberMap( - std::map* user_serial_number_map) { - leveldb::ReadOptions read_options; - read_options.verify_checksums = true; - - scoped_ptr iter(db_->NewIterator(read_options)); - for (iter->Seek(MakeSlice(kUserSerialNumberKeyStart)); - iter->Valid() && iter->key().ToString() < kUserSerialNumberKeyEnd; - iter->Next()) { - std::string username = ParseUsername(iter->key().ToString()); - if (username.empty()) { - LOG(ERROR) << "Error reading username. It should not be empty."; - return false; - } - std::string serial_number_string = iter->value().ToString(); - int64 serial_number = kSerialNumberMissing; - if (!base::StringToInt64(serial_number_string, &serial_number)) { - LOG(ERROR) << "Error reading user serial number for user: " << username; - return false; - } - - (*user_serial_number_map)[username] = serial_number; - } - - return true; -} - GCMStoreImpl::GCMStoreImpl( bool use_mock_keychain, const base::FilePath& path, @@ -664,6 +609,29 @@ void GCMStoreImpl::SetDeviceCredentials(uint64 device_android_id, callback)); } +void GCMStoreImpl::AddRegistration( + const std::string& app_id, + const linked_ptr& registration, + const UpdateCallback& callback) { + blocking_task_runner_->PostTask( + FROM_HERE, + base::Bind(&GCMStoreImpl::Backend::AddRegistration, + backend_, + app_id, + registration, + callback)); +} + +void GCMStoreImpl::RemoveRegistration(const std::string& app_id, + const UpdateCallback& callback) { + blocking_task_runner_->PostTask( + FROM_HERE, + base::Bind(&GCMStoreImpl::Backend::RemoveRegistration, + backend_, + app_id, + callback)); +} + void GCMStoreImpl::AddIncomingMessage(const std::string& persistent_id, const UpdateCallback& callback) { blocking_task_runner_->PostTask( @@ -766,38 +734,6 @@ void GCMStoreImpl::RemoveOutgoingMessages( callback))); } -void GCMStoreImpl::SetNextSerialNumber(int64 next_serial_number, - const UpdateCallback& callback) { - blocking_task_runner_->PostTask( - FROM_HERE, - base::Bind(&GCMStoreImpl::Backend::SetNextSerialNumber, - backend_, - next_serial_number, - callback)); -} - -void GCMStoreImpl::AddUserSerialNumber(const std::string& username, - int64 serial_number, - const UpdateCallback& callback) { - blocking_task_runner_->PostTask( - FROM_HERE, - base::Bind(&GCMStoreImpl::Backend::AddUserSerialNumber, - backend_, - username, - serial_number, - callback)); -} - -void GCMStoreImpl::RemoveUserSerialNumber(const std::string& username, - const UpdateCallback& callback) { - blocking_task_runner_->PostTask( - FROM_HERE, - base::Bind(&GCMStoreImpl::Backend::RemoveUserSerialNumber, - backend_, - username, - callback)); -} - void GCMStoreImpl::LoadContinuation(const LoadCallback& callback, scoped_ptr result) { if (!result->success) { diff --git a/google_apis/gcm/engine/gcm_store_impl.h b/google_apis/gcm/engine/gcm_store_impl.h index 88ed164c7da7..971367a8bf01 100644 --- a/google_apis/gcm/engine/gcm_store_impl.h +++ b/google_apis/gcm/engine/gcm_store_impl.h @@ -46,6 +46,13 @@ class GCM_EXPORT GCMStoreImpl : public GCMStore { uint64 device_security_token, const UpdateCallback& callback) OVERRIDE; + // Registration info. + virtual void AddRegistration(const std::string& app_id, + const linked_ptr& registration, + const UpdateCallback& callback) OVERRIDE; + virtual void RemoveRegistration(const std::string& app_id, + const UpdateCallback& callback) OVERRIDE; + // Unacknowledged incoming message handling. virtual void AddIncomingMessage(const std::string& persistent_id, const UpdateCallback& callback) OVERRIDE; @@ -67,15 +74,6 @@ class GCM_EXPORT GCMStoreImpl : public GCMStore { virtual void RemoveOutgoingMessages(const PersistentIdList& persistent_ids, const UpdateCallback& callback) OVERRIDE; - // User serial number handling. - virtual void SetNextSerialNumber(int64 next_serial_number, - const UpdateCallback& callback) OVERRIDE; - virtual void AddUserSerialNumber(const std::string& username, - int64 serial_number, - const UpdateCallback& callback) OVERRIDE; - virtual void RemoveUserSerialNumber(const std::string& username, - const UpdateCallback& callback) OVERRIDE; - private: typedef std::map AppIdToMessageCountMap; diff --git a/google_apis/gcm/engine/gcm_store_impl_unittest.cc b/google_apis/gcm/engine/gcm_store_impl_unittest.cc index 0a34307f7618..8cc39cc18393 100644 --- a/google_apis/gcm/engine/gcm_store_impl_unittest.cc +++ b/google_apis/gcm/engine/gcm_store_impl_unittest.cc @@ -108,8 +108,6 @@ TEST_F(GCMStoreImplTest, LoadNew) { EXPECT_EQ(0U, load_result->device_security_token); EXPECT_TRUE(load_result->incoming_messages.empty()); EXPECT_TRUE(load_result->outgoing_messages.empty()); - EXPECT_EQ(1LL, load_result->serial_number_mappings.next_serial_number); - EXPECT_TRUE(load_result->serial_number_mappings.user_serial_numbers.empty()); } TEST_F(GCMStoreImplTest, DeviceCredentials) { @@ -134,6 +132,58 @@ TEST_F(GCMStoreImplTest, DeviceCredentials) { ASSERT_EQ(kDeviceToken, load_result->device_security_token); } +TEST_F(GCMStoreImplTest, Registrations) { + scoped_ptr gcm_store(BuildGCMStore()); + scoped_ptr load_result; + gcm_store->Load(base::Bind( + &GCMStoreImplTest::LoadCallback, base::Unretained(this), &load_result)); + PumpLoop(); + + // Add one registration with one sender. + linked_ptr registration1(new RegistrationInfo); + registration1->sender_ids.push_back("sender1"); + registration1->registration_id = "registration1"; + gcm_store->AddRegistration( + "app1", + registration1, + base::Bind(&GCMStoreImplTest::UpdateCallback, base::Unretained(this))); + PumpLoop(); + + // Add one registration with multiple senders. + linked_ptr registration2(new RegistrationInfo); + registration2->sender_ids.push_back("sender2_1"); + registration2->sender_ids.push_back("sender2_2"); + registration2->registration_id = "registration2"; + gcm_store->AddRegistration( + "app2", + registration2, + base::Bind(&GCMStoreImplTest::UpdateCallback, base::Unretained(this))); + PumpLoop(); + + gcm_store = BuildGCMStore().Pass(); + gcm_store->Load(base::Bind( + &GCMStoreImplTest::LoadCallback, base::Unretained(this), &load_result)); + PumpLoop(); + + ASSERT_EQ(2, load_result->registrations.size()); + ASSERT_TRUE(load_result->registrations.find("app1") != + load_result->registrations.end()); + EXPECT_EQ(registration1->registration_id, + load_result->registrations["app1"]->registration_id); + ASSERT_EQ(1, load_result->registrations["app1"]->sender_ids.size()); + EXPECT_EQ(registration1->sender_ids[0], + load_result->registrations["app1"]->sender_ids[0]); + ASSERT_TRUE(load_result->registrations.find("app2") != + load_result->registrations.end()); + EXPECT_EQ(registration2->registration_id, + load_result->registrations["app2"]->registration_id); + ASSERT_EQ(2, load_result->registrations["app2"]->sender_ids.size()); + EXPECT_EQ(registration2->sender_ids[0], + load_result->registrations["app2"]->sender_ids[0]); + EXPECT_EQ(registration2->sender_ids[1], + load_result->registrations["app2"]->sender_ids[1]); +} + // Verify saving some incoming messages, reopening the directory, and then // removing those incoming messages. TEST_F(GCMStoreImplTest, IncomingMessages) { @@ -294,70 +344,6 @@ TEST_F(GCMStoreImplTest, IncomingAndOutgoingMessages) { ASSERT_TRUE(load_result->outgoing_messages.empty()); } -// Verify that the next serial number of persisted properly. -TEST_F(GCMStoreImplTest, NextSerialNumber) { - const int64 kNextSerialNumber = 77LL; - scoped_ptr gcm_store(BuildGCMStore()); - scoped_ptr load_result; - gcm_store->Load(base::Bind( - &GCMStoreImplTest::LoadCallback, base::Unretained(this), &load_result)); - PumpLoop(); - - gcm_store->SetNextSerialNumber( - kNextSerialNumber, - base::Bind(&GCMStoreImplTest::UpdateCallback, base::Unretained(this))); - PumpLoop(); - - gcm_store = BuildGCMStore().Pass(); - gcm_store->Load(base::Bind( - &GCMStoreImplTest::LoadCallback, base::Unretained(this), &load_result)); - PumpLoop(); - - EXPECT_EQ(kNextSerialNumber, - load_result->serial_number_mappings.next_serial_number); -} - -// Verify that user serial number mappings are persisted properly. -TEST_F(GCMStoreImplTest, UserSerialNumberMappings) { - scoped_ptr gcm_store(BuildGCMStore()); - scoped_ptr load_result; - gcm_store->Load(base::Bind( - &GCMStoreImplTest::LoadCallback, base::Unretained(this), &load_result)); - PumpLoop(); - - std::string username1 = "username1"; - int64 serial_number1 = 34LL; - gcm_store->AddUserSerialNumber( - username1, - serial_number1, - base::Bind(&GCMStoreImplTest::UpdateCallback, base::Unretained(this))); - - std::string username2 = "username2"; - int64 serial_number2 = 56LL; - gcm_store->AddUserSerialNumber( - username2, - serial_number2, - base::Bind(&GCMStoreImplTest::UpdateCallback, base::Unretained(this))); - PumpLoop(); - - gcm_store = BuildGCMStore().Pass(); - gcm_store->Load(base::Bind( - &GCMStoreImplTest::LoadCallback, base::Unretained(this), &load_result)); - PumpLoop(); - - ASSERT_EQ(2u, load_result->serial_number_mappings.user_serial_numbers.size()); - ASSERT_NE( - load_result->serial_number_mappings.user_serial_numbers.end(), - load_result->serial_number_mappings.user_serial_numbers.find(username1)); - EXPECT_EQ(serial_number1, - load_result->serial_number_mappings.user_serial_numbers[username1]); - ASSERT_NE( - load_result->serial_number_mappings.user_serial_numbers.end(), - load_result->serial_number_mappings.user_serial_numbers.find(username2)); - EXPECT_EQ(serial_number2, - load_result->serial_number_mappings.user_serial_numbers[username2]); -} - // Test that per-app message limits are enforced, persisted across restarts, // and updated as messages are removed. TEST_F(GCMStoreImplTest, PerAppMessageLimits) { diff --git a/google_apis/gcm/engine/registration_info.cc b/google_apis/gcm/engine/registration_info.cc new file mode 100644 index 000000000000..6a41c76e00e2 --- /dev/null +++ b/google_apis/gcm/engine/registration_info.cc @@ -0,0 +1,62 @@ +// Copyright 2014 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 "google_apis/gcm/engine/registration_info.h" + +#include "base/strings/string_util.h" + +namespace gcm { + +RegistrationInfo::RegistrationInfo() { +} + +RegistrationInfo::~RegistrationInfo() { +} + +std::string RegistrationInfo::SerializeAsString() const { + if (sender_ids.empty() || registration_id.empty()) + return std::string(); + + // Serialize as: + // sender1,sender2,...=reg_id + std::string value; + for (std::vector::const_iterator iter = sender_ids.begin(); + iter != sender_ids.end(); ++iter) { + DCHECK(!iter->empty() && + iter->find(',') == std::string::npos && + iter->find('=') == std::string::npos); + if (!value.empty()) + value += ","; + value += *iter; + } + + DCHECK(registration_id.find('=') == std::string::npos); + value += '='; + value += registration_id; + return value; +} + +bool RegistrationInfo::ParseFromString(const std::string& value) { + if (value.empty()) + return true; + + size_t pos = value.find('='); + if (pos == std::string::npos) + return false; + + std::string senders = value.substr(0, pos); + registration_id = value.substr(pos + 1); + + Tokenize(senders, ",", &sender_ids); + + if (sender_ids.empty() || registration_id.empty()) { + sender_ids.clear(); + registration_id.clear(); + return false; + } + + return true; +} + +} // namespace gcm diff --git a/google_apis/gcm/engine/registration_info.h b/google_apis/gcm/engine/registration_info.h new file mode 100644 index 000000000000..c06a6aa4c560 --- /dev/null +++ b/google_apis/gcm/engine/registration_info.h @@ -0,0 +1,35 @@ +// Copyright 2014 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 GOOGLE_APIS_GCM_ENGINE_REGISTRATION_INFO_H_ +#define GOOGLE_APIS_GCM_ENGINE_REGISTRATION_INFO_H_ + +#include +#include +#include + +#include "base/basictypes.h" +#include "base/memory/linked_ptr.h" +#include "google_apis/gcm/base/gcm_export.h" + +namespace gcm { + +struct GCM_EXPORT RegistrationInfo { + RegistrationInfo(); + ~RegistrationInfo(); + + std::string SerializeAsString() const; + bool ParseFromString(const std::string& value); + + std::vector sender_ids; + std::string registration_id; +}; + +// Map of app id to registration info. +typedef std::map > +RegistrationInfoMap; + +} // namespace gcm + +#endif // GOOGLE_APIS_GCM_ENGINE_REGISTRATION_INFO_H_ diff --git a/google_apis/gcm/gcm.gyp b/google_apis/gcm/gcm.gyp index c5dfa6dd6326..98437763d72e 100644 --- a/google_apis/gcm/gcm.gyp +++ b/google_apis/gcm/gcm.gyp @@ -62,6 +62,8 @@ 'engine/heartbeat_manager.h', 'engine/mcs_client.cc', 'engine/mcs_client.h', + 'engine/registration_info.cc', + 'engine/registration_info.h', 'engine/registration_request.cc', 'engine/registration_request.h', 'engine/unregistration_request.cc', diff --git a/google_apis/gcm/gcm_client_impl.cc b/google_apis/gcm/gcm_client_impl.cc index 5663eae1aead..ad02a4d80d36 100644 --- a/google_apis/gcm/gcm_client_impl.cc +++ b/google_apis/gcm/gcm_client_impl.cc @@ -115,8 +115,9 @@ GCMClientImpl::GCMClientImpl() : state_(UNINITIALIZED), clock_(new base::DefaultClock()), url_request_context_getter_(NULL), - pending_registrations_deleter_(&pending_registrations_), - pending_unregistrations_deleter_(&pending_unregistrations_), + pending_registration_requests_deleter_(&pending_registration_requests_), + pending_unregistration_requests_deleter_( + &pending_unregistration_requests_), weak_ptr_factory_(this) { } @@ -163,6 +164,8 @@ void GCMClientImpl::OnLoadCompleted(scoped_ptr result) { return; } + registrations_ = result->registrations; + device_checkin_info_.android_id = result->device_android_id; device_checkin_info_.secret = result->device_security_token; InitializeMCSClient(result.Pass()); @@ -284,11 +287,16 @@ void GCMClientImpl::SetDeviceCredentialsCallback(bool success) { DCHECK(success); } +void GCMClientImpl::UpdateRegistrationCallback(bool success) { + // TODO(fgorski): This is one of the signals that store needs a rebuild. + DCHECK(success); +} + void GCMClientImpl::Stop() { device_checkin_info_.Reset(); mcs_client_.reset(); checkin_request_.reset(); - pending_registrations_.clear(); + pending_registration_requests_.clear(); state_ = INITIALIZED; gcm_store_->Close(); } @@ -302,33 +310,49 @@ void GCMClientImpl::CheckOut() { void GCMClientImpl::Register(const std::string& app_id, const std::vector& sender_ids) { DCHECK_EQ(state_, READY); + + // If the same sender ids is provided, return the cached registration ID + // directly. + RegistrationInfoMap::const_iterator registrations_iter = + registrations_.find(app_id); + if (registrations_iter != registrations_.end() && + registrations_iter->second->sender_ids == sender_ids) { + delegate_->OnRegisterFinished( + app_id, registrations_iter->second->registration_id, SUCCESS); + return; + } + RegistrationRequest::RequestInfo request_info( device_checkin_info_.android_id, device_checkin_info_.secret, app_id, sender_ids); - DCHECK_EQ(0u, pending_registrations_.count(app_id)); + DCHECK_EQ(0u, pending_registration_requests_.count(app_id)); RegistrationRequest* registration_request = new RegistrationRequest(request_info, kDefaultBackoffPolicy, base::Bind(&GCMClientImpl::OnRegisterCompleted, weak_ptr_factory_.GetWeakPtr(), - app_id), + app_id, + sender_ids), kMaxRegistrationRetries, url_request_context_getter_); - pending_registrations_[app_id] = registration_request; + pending_registration_requests_[app_id] = registration_request; registration_request->Start(); } -void GCMClientImpl::OnRegisterCompleted(const std::string& app_id, - RegistrationRequest::Status status, - const std::string& registration_id) { +void GCMClientImpl::OnRegisterCompleted( + const std::string& app_id, + const std::vector& sender_ids, + RegistrationRequest::Status status, + const std::string& registration_id) { DCHECK(delegate_); Result result; - PendingRegistrations::iterator iter = pending_registrations_.find(app_id); - if (iter == pending_registrations_.end()) + PendingRegistrationRequests::iterator iter = + pending_registration_requests_.find(app_id); + if (iter == pending_registration_requests_.end()) result = UNKNOWN_ERROR; else if (status == RegistrationRequest::INVALID_SENDER) result = INVALID_PARAMETER; @@ -337,20 +361,42 @@ void GCMClientImpl::OnRegisterCompleted(const std::string& app_id, else result = SUCCESS; + if (result == SUCCESS) { + // Cache it. + linked_ptr registration(new RegistrationInfo); + registration->sender_ids = sender_ids; + registration->registration_id = registration_id; + registrations_[app_id] = registration; + + // Save it in the persistent store. + gcm_store_->AddRegistration( + app_id, + registration, + base::Bind(&GCMClientImpl::UpdateRegistrationCallback, + weak_ptr_factory_.GetWeakPtr())); + } + delegate_->OnRegisterFinished( app_id, result == SUCCESS ? registration_id : std::string(), result); - if (iter != pending_registrations_.end()) { + if (iter != pending_registration_requests_.end()) { delete iter->second; - pending_registrations_.erase(iter); + pending_registration_requests_.erase(iter); } } void GCMClientImpl::Unregister(const std::string& app_id) { DCHECK_EQ(state_, READY); - if (pending_unregistrations_.count(app_id) == 1) + if (pending_unregistration_requests_.count(app_id) == 1) return; + // Remove from the cache and persistent store. + registrations_.erase(app_id); + gcm_store_->RemoveRegistration( + app_id, + base::Bind(&GCMClientImpl::UpdateRegistrationCallback, + weak_ptr_factory_.GetWeakPtr())); + UnregistrationRequest::RequestInfo request_info( device_checkin_info_.android_id, device_checkin_info_.secret, @@ -364,7 +410,7 @@ void GCMClientImpl::Unregister(const std::string& app_id) { weak_ptr_factory_.GetWeakPtr(), app_id), url_request_context_getter_); - pending_unregistrations_[app_id] = unregistration_request; + pending_unregistration_requests_[app_id] = unregistration_request; unregistration_request->Start(); } @@ -377,12 +423,13 @@ void GCMClientImpl::OnUnregisterCompleted( app_id, status == UnregistrationRequest::SUCCESS ? SUCCESS : SERVER_ERROR); - PendingUnregistrations::iterator iter = pending_unregistrations_.find(app_id); - if (iter == pending_unregistrations_.end()) + PendingUnregistrationRequests::iterator iter = + pending_unregistration_requests_.find(app_id); + if (iter == pending_unregistration_requests_.end()) return; delete iter->second; - pending_unregistrations_.erase(iter); + pending_unregistration_requests_.erase(iter); } void GCMClientImpl::OnGCMStoreDestroyed(bool success) { @@ -539,13 +586,24 @@ void GCMClientImpl::HandleIncomingMessage(const gcm::MCSMessage& message) { void GCMClientImpl::HandleIncomingDataMessage( const mcs_proto::DataMessageStanza& data_message_stanza, MessageData& message_data) { + std::string app_id = data_message_stanza.category(); + + // Drop the message when the app is not registered for the sender of the + // message. + RegistrationInfoMap::iterator iter = registrations_.find(app_id); + if (iter == registrations_.end() || + std::find(iter->second->sender_ids.begin(), + iter->second->sender_ids.end(), + data_message_stanza.from()) == iter->second->sender_ids.end()) { + return; + } + IncomingMessage incoming_message; incoming_message.sender_id = data_message_stanza.from(); if (data_message_stanza.has_token()) incoming_message.collapse_key = data_message_stanza.token(); incoming_message.data = message_data; - delegate_->OnMessageReceived(data_message_stanza.category(), - incoming_message); + delegate_->OnMessageReceived(app_id, incoming_message); } void GCMClientImpl::HandleIncomingSendError( diff --git a/google_apis/gcm/gcm_client_impl.h b/google_apis/gcm/gcm_client_impl.h index f91a820f8703..e969ec018093 100644 --- a/google_apis/gcm/gcm_client_impl.h +++ b/google_apis/gcm/gcm_client_impl.h @@ -100,13 +100,13 @@ class GCM_EXPORT GCMClientImpl : public GCMClient { // are pending registration requests to obtain a registration ID for // requesting application. typedef std::map - PendingRegistrations; + PendingRegistrationRequests; // Collection of pending unregistration requests. Keys are app IDs, while // values are pending unregistration requests to disable the registration ID // currently assigned to the application. typedef std::map - PendingUnregistrations; + PendingUnregistrationRequests; friend class GCMClientImplTest; @@ -150,8 +150,12 @@ class GCM_EXPORT GCMClientImpl : public GCMClient { // Callback for persisting device credentials in the |gcm_store_|. void SetDeviceCredentialsCallback(bool success); + // Callback for persisting registration info in the |gcm_store_|. + void UpdateRegistrationCallback(bool success); + // Completes the registration request. void OnRegisterCompleted(const std::string& app_id, + const std::vector& sender_ids, RegistrationRequest::Status status, const std::string& registration_id); @@ -214,15 +218,20 @@ class GCM_EXPORT GCMClientImpl : public GCMClient { scoped_ptr checkin_request_; std::vector account_ids_; - // Currently pending registrations. GCMClientImpl owns the + // Cached registration info. + RegistrationInfoMap registrations_; + + // Currently pending registration requests. GCMClientImpl owns the // RegistrationRequests. - PendingRegistrations pending_registrations_; - STLValueDeleter pending_registrations_deleter_; + PendingRegistrationRequests pending_registration_requests_; + STLValueDeleter + pending_registration_requests_deleter_; - // Currently pending unregistrations. GCMClientImpl owns the + // Currently pending unregistration requests. GCMClientImpl owns the // UnregistrationRequests. - PendingUnregistrations pending_unregistrations_; - STLValueDeleter pending_unregistrations_deleter_; + PendingUnregistrationRequests pending_unregistration_requests_; + STLValueDeleter + pending_unregistration_requests_deleter_; // Factory for creating references in callbacks. base::WeakPtrFactory weak_ptr_factory_; diff --git a/google_apis/gcm/gcm_client_impl_unittest.cc b/google_apis/gcm/gcm_client_impl_unittest.cc index f8129b4de396..867990374658 100644 --- a/google_apis/gcm/gcm_client_impl_unittest.cc +++ b/google_apis/gcm/gcm_client_impl_unittest.cc @@ -37,6 +37,10 @@ enum LastEvent { const uint64 kDeviceAndroidId = 54321; const uint64 kDeviceSecurityToken = 12345; +const char kAppId[] = "app_id"; +const char kSender[] = "project_id"; +const char kSender2[] = "project_id2"; +const char kSender3[] = "project_id3"; const char kRegistrationResponsePrefix[] = "token="; const char kUnregistrationResponsePrefix[] = "deleted="; @@ -127,6 +131,11 @@ class GCMClientImplTest : public testing::Test, void CompleteRegistration(const std::string& registration_id); void CompleteUnregistration(const std::string& app_id); + bool ExistsRegistration(const std::string& app_id) const; + void AddRegistration(const std::string& app_id, + const std::vector& sender_ids, + const std::string& registration_id); + // GCMClient::Delegate overrides (for verification). virtual void OnRegisterFinished(const std::string& app_id, const std::string& registration_id, @@ -150,6 +159,14 @@ class GCMClientImplTest : public testing::Test, return reinterpret_cast(gcm_client_->mcs_client_.get()); } + void reset_last_event() { + last_event_ = NONE; + last_app_id_.clear(); + last_registration_id_.clear(); + last_message_id_.clear(); + last_result_ = GCMClient::UNKNOWN_ERROR; + } + LastEvent last_event() const { return last_event_; } const std::string& last_app_id() const { return last_app_id_; } const std::string& last_registration_id() const { @@ -211,6 +228,7 @@ void GCMClientImplTest::SetUp() { run_loop_.reset(new base::RunLoop); BuildGCMClient(); InitializeGCMClient(); + CompleteCheckin(kDeviceAndroidId, kDeviceSecurityToken); } void GCMClientImplTest::PumpLoop() { @@ -259,6 +277,7 @@ void GCMClientImplTest::CompleteRegistration( fetcher->set_response_code(net::HTTP_OK); fetcher->SetResponseString(response); fetcher->delegate()->OnURLFetchComplete(fetcher); + url_fetcher_factory_.RemoveFetcherFromMap(0); } void GCMClientImplTest::CompleteUnregistration( @@ -270,6 +289,21 @@ void GCMClientImplTest::CompleteUnregistration( fetcher->set_response_code(net::HTTP_OK); fetcher->SetResponseString(response); fetcher->delegate()->OnURLFetchComplete(fetcher); + url_fetcher_factory_.RemoveFetcherFromMap(0); +} + +bool GCMClientImplTest::ExistsRegistration(const std::string& app_id) const { + return gcm_client_->registrations_.count(app_id) > 0; +} + +void GCMClientImplTest::AddRegistration( + const std::string& app_id, + const std::vector& sender_ids, + const std::string& registration_id) { + linked_ptr registration(new RegistrationInfo); + registration->sender_ids = sender_ids; + registration->registration_id = registration_id; + gcm_client_->registrations_[app_id] = registration; } void GCMClientImplTest::InitializeGCMClient() { @@ -300,7 +334,6 @@ void GCMClientImplTest::InitializeGCMClient() { // Ensuring that mcs_client is using the same gcm_store as gcm_client. mcs_client()->set_gcm_store(gcm_client_->gcm_store_.get()); PumpLoopUntilIdle(); - CompleteCheckin(kDeviceAndroidId, kDeviceSecurityToken); } void GCMClientImplTest::ReceiveMessageFromMCS(const MCSMessage& message) { @@ -367,42 +400,105 @@ TEST_F(GCMClientImplTest, CheckOut) { } TEST_F(GCMClientImplTest, RegisterApp) { + EXPECT_FALSE(ExistsRegistration(kAppId)); + std::vector senders; senders.push_back("sender"); - gcm_client()->Register("app_id", senders); + gcm_client()->Register(kAppId, senders); CompleteRegistration("reg_id"); EXPECT_EQ(REGISTRATION_COMPLETED, last_event()); - EXPECT_EQ("app_id", last_app_id()); + EXPECT_EQ(kAppId, last_app_id()); + EXPECT_EQ("reg_id", last_registration_id()); + EXPECT_EQ(GCMClient::SUCCESS, last_result()); + EXPECT_TRUE(ExistsRegistration(kAppId)); +} + +TEST_F(GCMClientImplTest, RegisterAppFromCache) { + EXPECT_FALSE(ExistsRegistration(kAppId)); + + std::vector senders; + senders.push_back("sender"); + gcm_client()->Register(kAppId, senders); + CompleteRegistration("reg_id"); + EXPECT_TRUE(ExistsRegistration(kAppId)); + + EXPECT_EQ(kAppId, last_app_id()); EXPECT_EQ("reg_id", last_registration_id()); EXPECT_EQ(GCMClient::SUCCESS, last_result()); + EXPECT_EQ(REGISTRATION_COMPLETED, last_event()); + + // Recreate GCMClient in order to load from the persistent store. + BuildGCMClient(); + InitializeGCMClient(); + + EXPECT_TRUE(ExistsRegistration(kAppId)); } TEST_F(GCMClientImplTest, UnregisterApp) { - gcm_client()->Unregister("app_id"); - CompleteUnregistration("app_id"); + EXPECT_FALSE(ExistsRegistration(kAppId)); + + std::vector senders; + senders.push_back("sender"); + gcm_client()->Register(kAppId, senders); + CompleteRegistration("reg_id"); + EXPECT_TRUE(ExistsRegistration(kAppId)); + + gcm_client()->Unregister(kAppId); + CompleteUnregistration(kAppId); EXPECT_EQ(UNREGISTRATION_COMPLETED, last_event()); - EXPECT_EQ("app_id", last_app_id()); + EXPECT_EQ(kAppId, last_app_id()); EXPECT_EQ(GCMClient::SUCCESS, last_result()); + EXPECT_FALSE(ExistsRegistration(kAppId)); } TEST_F(GCMClientImplTest, DispatchDownstreamMessage) { + // Register to receive messages from kSender and kSender2 only. + std::vector senders; + senders.push_back(kSender); + senders.push_back(kSender2); + AddRegistration(kAppId, senders, "reg_id"); + std::map expected_data; expected_data["message_type"] = "gcm"; expected_data["key"] = "value"; expected_data["key2"] = "value2"; - MCSMessage message(BuildDownstreamMessage( - "project_id", "app_id", expected_data)); + + // Message for kSender will be received. + MCSMessage message(BuildDownstreamMessage(kSender, kAppId, expected_data)); EXPECT_TRUE(message.IsValid()); ReceiveMessageFromMCS(message); expected_data.erase(expected_data.find("message_type")); EXPECT_EQ(MESSAGE_RECEIVED, last_event()); - EXPECT_EQ("app_id", last_app_id()); + EXPECT_EQ(kAppId, last_app_id()); EXPECT_EQ(expected_data.size(), last_message().data.size()); EXPECT_EQ(expected_data, last_message().data); - EXPECT_EQ("project_id", last_message().sender_id); + EXPECT_EQ(kSender, last_message().sender_id); + + reset_last_event(); + + // Message for kSender2 will be received. + MCSMessage message2(BuildDownstreamMessage(kSender2, kAppId, expected_data)); + EXPECT_TRUE(message2.IsValid()); + ReceiveMessageFromMCS(message2); + + EXPECT_EQ(MESSAGE_RECEIVED, last_event()); + EXPECT_EQ(kAppId, last_app_id()); + EXPECT_EQ(expected_data.size(), last_message().data.size()); + EXPECT_EQ(expected_data, last_message().data); + EXPECT_EQ(kSender2, last_message().sender_id); + + reset_last_event(); + + // Message from kSender3 will be dropped. + MCSMessage message3(BuildDownstreamMessage(kSender3, kAppId, expected_data)); + EXPECT_TRUE(message3.IsValid()); + ReceiveMessageFromMCS(message3); + + EXPECT_NE(MESSAGE_RECEIVED, last_event()); + EXPECT_NE(kAppId, last_app_id()); } TEST_F(GCMClientImplTest, DispatchDownstreamMessageSendError) { @@ -411,12 +507,12 @@ TEST_F(GCMClientImplTest, DispatchDownstreamMessageSendError) { expected_data["google.message_id"] = "007"; expected_data["error_details"] = "some details"; MCSMessage message(BuildDownstreamMessage( - "project_id", "app_id", expected_data)); + kSender, kAppId, expected_data)); EXPECT_TRUE(message.IsValid()); ReceiveMessageFromMCS(message); EXPECT_EQ(MESSAGE_SEND_ERROR, last_event()); - EXPECT_EQ("app_id", last_app_id()); + EXPECT_EQ(kAppId, last_app_id()); EXPECT_EQ("007", last_error_details().message_id); EXPECT_EQ(1UL, last_error_details().additional_data.size()); GCMClient::MessageData::const_iterator iter = @@ -429,12 +525,12 @@ TEST_F(GCMClientImplTest, DispatchDownstreamMessgaesDeleted) { std::map expected_data; expected_data["message_type"] = "deleted_messages"; MCSMessage message(BuildDownstreamMessage( - "project_id", "app_id", expected_data)); + kSender, kAppId, expected_data)); EXPECT_TRUE(message.IsValid()); ReceiveMessageFromMCS(message); EXPECT_EQ(MESSAGES_DELETED, last_event()); - EXPECT_EQ("app_id", last_app_id()); + EXPECT_EQ(kAppId, last_app_id()); } TEST_F(GCMClientImplTest, SendMessage) { @@ -445,16 +541,16 @@ TEST_F(GCMClientImplTest, SendMessage) { message.id = "007"; message.time_to_live = 500; message.data["key"] = "value"; - gcm_client()->Send("app_id", "project_id", message); + gcm_client()->Send(kAppId, kSender, message); EXPECT_EQ(kDataMessageStanzaTag, mcs_client()->last_message_tag()); - EXPECT_EQ("app_id", mcs_client()->last_data_message_stanza().category()); - EXPECT_EQ("project_id", mcs_client()->last_data_message_stanza().to()); + EXPECT_EQ(kAppId, mcs_client()->last_data_message_stanza().category()); + EXPECT_EQ(kSender, mcs_client()->last_data_message_stanza().to()); EXPECT_EQ(500, mcs_client()->last_data_message_stanza().ttl()); EXPECT_EQ(CurrentTime(), mcs_client()->last_data_message_stanza().sent()); EXPECT_EQ("007", mcs_client()->last_data_message_stanza().id()); EXPECT_EQ("gcm@chrome.com", mcs_client()->last_data_message_stanza().from()); - EXPECT_EQ("project_id", mcs_client()->last_data_message_stanza().to()); + EXPECT_EQ(kSender, mcs_client()->last_data_message_stanza().to()); EXPECT_EQ("key", mcs_client()->last_data_message_stanza().app_data(0).key()); EXPECT_EQ("value", mcs_client()->last_data_message_stanza().app_data(0).value()); diff --git a/tools/metrics/histograms/histograms.xml b/tools/metrics/histograms/histograms.xml index 176148fe9b29..9a9cc3b37e48 100644 --- a/tools/metrics/histograms/histograms.xml +++ b/tools/metrics/histograms/histograms.xml @@ -6601,6 +6601,9 @@ other types of suffix sets. + + Deprecated as of 3/2014. + zea@chromium.org Number of GCM users associated with this client at startup time. @@ -6629,6 +6632,13 @@ other types of suffix sets. + + jianli@chromium.org + + Number of registrations restored from the persistent store at startup. + + + zea@chromium.org -- 2.11.4.GIT