From f3e52af4a756df548982d4eb9e118ea66563f6ed Mon Sep 17 00:00:00 2001 From: jianli Date: Wed, 21 Jan 2015 15:18:47 -0800 Subject: [PATCH] Delay start the GCM when it is actually used GCM should not be started when installed GCM apps do not invoke GCM operations yet. BUG=448925 TEST=test updated and new tests added TBR=kalman@chromium.org Review URL: https://codereview.chromium.org/852083003 Cr-Commit-Position: refs/heads/master@{#312492} --- .../extension_gcm_app_handler_unittest.cc | 1 - .../services/gcm/gcm_profile_service_unittest.cc | 1 - components/gcm_driver/fake_gcm_client.cc | 31 ++-- components/gcm_driver/fake_gcm_client.h | 30 ++-- components/gcm_driver/fake_gcm_client_factory.cc | 7 +- components/gcm_driver/fake_gcm_client_factory.h | 2 - components/gcm_driver/fake_gcm_driver.cc | 3 +- components/gcm_driver/fake_gcm_driver.h | 3 +- components/gcm_driver/gcm_client.h | 20 ++- components/gcm_driver/gcm_client_impl.cc | 56 ++++++- components/gcm_driver/gcm_client_impl.h | 52 ++++--- components/gcm_driver/gcm_client_impl_unittest.cc | 85 +++++++++-- components/gcm_driver/gcm_driver.cc | 6 +- components/gcm_driver/gcm_driver.h | 2 +- components/gcm_driver/gcm_driver_android.cc | 3 +- components/gcm_driver/gcm_driver_android.h | 3 +- components/gcm_driver/gcm_driver_desktop.cc | 53 +++---- components/gcm_driver/gcm_driver_desktop.h | 2 +- .../gcm_driver/gcm_driver_desktop_unittest.cc | 170 +++++++++++---------- 19 files changed, 339 insertions(+), 191 deletions(-) diff --git a/chrome/browser/extensions/extension_gcm_app_handler_unittest.cc b/chrome/browser/extensions/extension_gcm_app_handler_unittest.cc index cde5081ce7b1..ecf8310f6f4e 100644 --- a/chrome/browser/extensions/extension_gcm_app_handler_unittest.cc +++ b/chrome/browser/extensions/extension_gcm_app_handler_unittest.cc @@ -182,7 +182,6 @@ class ExtensionGCMAppHandlerTest : public testing::Test { return new gcm::GCMProfileService( Profile::FromBrowserContext(context), scoped_ptr(new gcm::FakeGCMClientFactory( - gcm::FakeGCMClient::NO_DELAY_START, content::BrowserThread::GetMessageLoopProxyForThread( content::BrowserThread::UI), content::BrowserThread::GetMessageLoopProxyForThread( diff --git a/chrome/browser/services/gcm/gcm_profile_service_unittest.cc b/chrome/browser/services/gcm/gcm_profile_service_unittest.cc index 7347acf25359..2b1499a63681 100644 --- a/chrome/browser/services/gcm/gcm_profile_service_unittest.cc +++ b/chrome/browser/services/gcm/gcm_profile_service_unittest.cc @@ -39,7 +39,6 @@ KeyedService* BuildGCMProfileService(content::BrowserContext* context) { return new GCMProfileService( Profile::FromBrowserContext(context), scoped_ptr(new FakeGCMClientFactory( - FakeGCMClient::NO_DELAY_START, content::BrowserThread::GetMessageLoopProxyForThread( content::BrowserThread::UI), content::BrowserThread::GetMessageLoopProxyForThread( diff --git a/components/gcm_driver/fake_gcm_client.cc b/components/gcm_driver/fake_gcm_client.cc index 4e1444cc8af5..17952888e98c 100644 --- a/components/gcm_driver/fake_gcm_client.cc +++ b/components/gcm_driver/fake_gcm_client.cc @@ -19,13 +19,12 @@ namespace gcm { FakeGCMClient::FakeGCMClient( - StartMode start_mode, const scoped_refptr& ui_thread, const scoped_refptr& io_thread) : delegate_(NULL), - sequence_id_(0), started_(false), - start_mode_(start_mode), + start_mode_(DELAYED_START), + start_mode_overridding_(RESPECT_START_MODE), ui_thread_(ui_thread), io_thread_(io_thread), weak_ptr_factory_(this) { @@ -45,20 +44,27 @@ void FakeGCMClient::Initialize( delegate_ = delegate; } -void FakeGCMClient::Start() { +void FakeGCMClient::Start(StartMode start_mode) { DCHECK(io_thread_->RunsTasksOnCurrentThread()); - DCHECK(!started_); - if (start_mode_ == DELAY_START) + if (started_) return; - DoLoading(); + + if (start_mode == IMMEDIATE_START) + start_mode_ = IMMEDIATE_START; + if (start_mode_ == DELAYED_START || + start_mode_overridding_ == FORCE_TO_ALWAYS_DELAY_START_GCM) { + return; + } + + DoStart(); } -void FakeGCMClient::DoLoading() { +void FakeGCMClient::DoStart() { started_ = true; base::MessageLoop::current()->PostTask( FROM_HERE, - base::Bind(&FakeGCMClient::CheckinFinished, + base::Bind(&FakeGCMClient::Started, weak_ptr_factory_.GetWeakPtr())); } @@ -131,12 +137,12 @@ void FakeGCMClient::SetLastTokenFetchTime(const base::Time& time) { void FakeGCMClient::UpdateHeartbeatTimer(scoped_ptr timer) { } -void FakeGCMClient::PerformDelayedLoading() { +void FakeGCMClient::PerformDelayedStart() { DCHECK(ui_thread_->RunsTasksOnCurrentThread()); io_thread_->PostTask( FROM_HERE, - base::Bind(&FakeGCMClient::DoLoading, weak_ptr_factory_.GetWeakPtr())); + base::Bind(&FakeGCMClient::DoStart, weak_ptr_factory_.GetWeakPtr())); } void FakeGCMClient::ReceiveMessage(const std::string& app_id, @@ -178,12 +184,11 @@ std::string FakeGCMClient::GetRegistrationIdFromSenderIds( registration_id += ","; registration_id += normalized_sender_ids[i]; } - registration_id += base::IntToString(sequence_id_); } return registration_id; } -void FakeGCMClient::CheckinFinished() { +void FakeGCMClient::Started() { delegate_->OnGCMReady(std::vector(), base::Time()); delegate_->OnConnected(net::IPEndPoint()); } diff --git a/components/gcm_driver/fake_gcm_client.h b/components/gcm_driver/fake_gcm_client.h index c0a02d0fdb02..21d696d70edb 100644 --- a/components/gcm_driver/fake_gcm_client.h +++ b/components/gcm_driver/fake_gcm_client.h @@ -17,13 +17,15 @@ namespace gcm { class FakeGCMClient : public GCMClient { public: - enum StartMode { - NO_DELAY_START, - DELAY_START, + // For testing purpose. + enum StartModeOverridding { + // No change to how delay start is handled. + RESPECT_START_MODE, + // Force to delay start GCM until PerformDelayedStart is called. + FORCE_TO_ALWAYS_DELAY_START_GCM, }; - FakeGCMClient(StartMode start_mode, - const scoped_refptr& ui_thread, + FakeGCMClient(const scoped_refptr& ui_thread, const scoped_refptr& io_thread); ~FakeGCMClient() override; @@ -37,7 +39,7 @@ class FakeGCMClient : public GCMClient { url_request_context_getter, scoped_ptr encryptor, Delegate* delegate) override; - void Start() override; + void Start(StartMode start_mode) override; void Stop() override; void Register(const std::string& app_id, const std::vector& sender_ids) override; @@ -55,9 +57,9 @@ class FakeGCMClient : public GCMClient { void SetLastTokenFetchTime(const base::Time& time) override; void UpdateHeartbeatTimer(scoped_ptr timer) override; - // Initiate the loading that has been delayed. + // Initiate the start that has been delayed. // Called on UI thread. - void PerformDelayedLoading(); + void PerformDelayedStart(); // Simulate receiving something from the server. // Called on UI thread. @@ -68,10 +70,14 @@ class FakeGCMClient : public GCMClient { std::string GetRegistrationIdFromSenderIds( const std::vector& sender_ids) const; + void set_start_mode_overridding(StartModeOverridding overridding) { + start_mode_overridding_ = overridding; + } + private: // Called on IO thread. - void DoLoading(); - void CheckinFinished(); + void DoStart(); + void Started(); void RegisterFinished(const std::string& app_id, const std::string& registrion_id); void UnregisterFinished(const std::string& app_id); @@ -85,11 +91,9 @@ class FakeGCMClient : public GCMClient { const std::string& message_id); Delegate* delegate_; - // Increased at checkout in order to produce a different registration ID - // after checkout and re-checkin. - int sequence_id_; bool started_; StartMode start_mode_; + StartModeOverridding start_mode_overridding_; scoped_refptr ui_thread_; scoped_refptr io_thread_; base::WeakPtrFactory weak_ptr_factory_; diff --git a/components/gcm_driver/fake_gcm_client_factory.cc b/components/gcm_driver/fake_gcm_client_factory.cc index e592bec1db16..d14fc846acb5 100644 --- a/components/gcm_driver/fake_gcm_client_factory.cc +++ b/components/gcm_driver/fake_gcm_client_factory.cc @@ -11,11 +11,9 @@ namespace gcm { FakeGCMClientFactory::FakeGCMClientFactory( - FakeGCMClient::StartMode gcm_client_start_mode, const scoped_refptr& ui_thread, const scoped_refptr& io_thread) - : gcm_client_start_mode_(gcm_client_start_mode), - ui_thread_(ui_thread), + : ui_thread_(ui_thread), io_thread_(io_thread) { } @@ -23,8 +21,7 @@ FakeGCMClientFactory::~FakeGCMClientFactory() { } scoped_ptr FakeGCMClientFactory::BuildInstance() { - return scoped_ptr(new FakeGCMClient( - gcm_client_start_mode_, ui_thread_, io_thread_)); + return scoped_ptr(new FakeGCMClient(ui_thread_, io_thread_)); } } // namespace gcm diff --git a/components/gcm_driver/fake_gcm_client_factory.h b/components/gcm_driver/fake_gcm_client_factory.h index 949c225702d8..e70e48aa4cce 100644 --- a/components/gcm_driver/fake_gcm_client_factory.h +++ b/components/gcm_driver/fake_gcm_client_factory.h @@ -21,7 +21,6 @@ class GCMClient; class FakeGCMClientFactory : public GCMClientFactory { public: FakeGCMClientFactory( - FakeGCMClient::StartMode gcm_client_start_mode, const scoped_refptr& ui_thread, const scoped_refptr& io_thread); ~FakeGCMClientFactory() override; @@ -30,7 +29,6 @@ class FakeGCMClientFactory : public GCMClientFactory { scoped_ptr BuildInstance() override; private: - FakeGCMClient::StartMode gcm_client_start_mode_; scoped_refptr ui_thread_; scoped_refptr io_thread_; diff --git a/components/gcm_driver/fake_gcm_driver.cc b/components/gcm_driver/fake_gcm_driver.cc index d44e1693cc70..45791f60899a 100644 --- a/components/gcm_driver/fake_gcm_driver.cc +++ b/components/gcm_driver/fake_gcm_driver.cc @@ -60,7 +60,8 @@ void FakeGCMDriver::SetGCMRecording(const GetGCMStatisticsCallback& callback, bool recording) { } -GCMClient::Result FakeGCMDriver::EnsureStarted() { +GCMClient::Result FakeGCMDriver::EnsureStarted( + GCMClient::StartMode start_mode) { return GCMClient::SUCCESS; } diff --git a/components/gcm_driver/fake_gcm_driver.h b/components/gcm_driver/fake_gcm_driver.h index 25576ccc60c9..9b57af2ee21f 100644 --- a/components/gcm_driver/fake_gcm_driver.h +++ b/components/gcm_driver/fake_gcm_driver.h @@ -44,7 +44,8 @@ class FakeGCMDriver : public GCMDriver { protected: // GCMDriver implementation: - GCMClient::Result EnsureStarted() override; + GCMClient::Result EnsureStarted( + GCMClient::StartMode start_mode) override; void RegisterImpl(const std::string& app_id, const std::vector& sender_ids) override; void UnregisterImpl(const std::string& app_id) override; diff --git a/components/gcm_driver/gcm_client.h b/components/gcm_driver/gcm_client.h index 0f10674294e5..2aa224a369ab 100644 --- a/components/gcm_driver/gcm_client.h +++ b/components/gcm_driver/gcm_client.h @@ -37,6 +37,17 @@ struct AccountMapping; // Messaging server. This interface is not supposed to be thread-safe. class GCMClient { public: + // Controls how GCM is being started. At first, GCMClient will be initialized + // and GCM store will be loaded. Then GCM connection may or may not be + // initiated depending on this enum value. + enum StartMode { + // GCM should be started only when it is being actually used. If no + // registration record is found, GCM will not kick off. + DELAYED_START, + // GCM should be started immediately. + IMMEDIATE_START + }; + enum Result { // Successful operation. SUCCESS, @@ -238,10 +249,11 @@ class GCMClient { scoped_ptr encryptor, Delegate* delegate) = 0; - // Starts the GCM service by first loading the data from the persistent store. - // This will then kick off the check-in if the check-in info is not found in - // the store. - virtual void Start() = 0; + // This will initiate the GCM connection only if |start_mode| means to start + // the GCM immediately or the GCM registration records are found in the store. + // Note that it is OK to call Start multiple times and the implementation + // should handle it gracefully. + virtual void Start(StartMode start_mode) = 0; // Stops using the GCM service. This will not erase the persisted data. virtual void Stop() = 0; diff --git a/components/gcm_driver/gcm_client_impl.cc b/components/gcm_driver/gcm_client_impl.cc index 281395dd51f0..6b1c980f00ff 100644 --- a/components/gcm_driver/gcm_client_impl.cc +++ b/components/gcm_driver/gcm_client_impl.cc @@ -16,6 +16,7 @@ #include "base/strings/stringprintf.h" #include "base/time/default_clock.h" #include "base/timer/timer.h" +#include "components/gcm_driver/gcm_account_mapper.h" #include "components/gcm_driver/gcm_backoff_policy.h" #include "google_apis/gcm/base/encryptor.h" #include "google_apis/gcm/base/mcs_message.h" @@ -242,6 +243,7 @@ GCMClientImpl::GCMClientImpl(scoped_ptr internals_builder) : internals_builder_(internals_builder.Pass()), state_(UNINITIALIZED), delegate_(NULL), + start_mode_(DELAYED_START), clock_(internals_builder_->BuildClock()), url_request_context_getter_(NULL), pending_registration_requests_deleter_(&pending_registration_requests_), @@ -285,8 +287,24 @@ void GCMClientImpl::Initialize( state_ = INITIALIZED; } -void GCMClientImpl::Start() { - DCHECK_EQ(INITIALIZED, state_); +void GCMClientImpl::Start(StartMode start_mode) { + DCHECK_NE(UNINITIALIZED, state_); + + if (state_ == LOADED) { + // Start the GCM if not yet. + if (start_mode == IMMEDIATE_START) + StartGCM(); + return; + } + + // The delay start behavior will be abandoned when Start has been called + // once with IMMEDIATE_START behavior. + if (start_mode == IMMEDIATE_START) + start_mode_ = IMMEDIATE_START; + + // Bail out if the loading is not started or completed. + if (state_ != INITIALIZED) + return; // Once the loading is completed, the check-in will be initiated. gcm_store_->Load(base::Bind(&GCMClientImpl::OnLoadCompleted, @@ -315,13 +333,25 @@ void GCMClientImpl::OnLoadCompleted(scoped_ptr result) { device_checkin_info_.accounts_set = true; last_checkin_time_ = result->last_checkin_time; gservices_settings_.UpdateFromLoadResult(*result); + load_result_ = result.Pass(); + state_ = LOADED; + + // Don't initiate the GCM connection when GCM is in delayed start mode and + // not any standalone app has registered GCM yet. + if (start_mode_ == DELAYED_START && !HasStandaloneRegisteredApp()) + return; + + StartGCM(); +} + +void GCMClientImpl::StartGCM() { // Taking over the value of account_mappings before passing the ownership of // load result to InitializeMCSClient. std::vector account_mappings; - account_mappings.swap(result->account_mappings); - base::Time last_token_fetch_time = result->last_token_fetch_time; + account_mappings.swap(load_result_->account_mappings); + base::Time last_token_fetch_time = load_result_->last_token_fetch_time; - InitializeMCSClient(result.Pass()); + InitializeMCSClient(); if (device_checkin_info_.IsValid()) { SchedulePeriodicCheckin(); @@ -334,8 +364,7 @@ void GCMClientImpl::OnLoadCompleted(scoped_ptr result) { StartCheckin(); } -void GCMClientImpl::InitializeMCSClient( - scoped_ptr result) { +void GCMClientImpl::InitializeMCSClient() { std::vector endpoints; endpoints.push_back(gservices_settings_.GetMCSMainEndpoint()); endpoints.push_back(gservices_settings_.GetMCSFallbackEndpoint()); @@ -362,7 +391,7 @@ void GCMClientImpl::InitializeMCSClient( weak_ptr_factory_.GetWeakPtr()), base::Bind(&GCMClientImpl::OnMessageSentToMCS, weak_ptr_factory_.GetWeakPtr()), - result.Pass()); + load_result_.Pass()); } void GCMClientImpl::OnFirstTimeDeviceCheckinCompleted( @@ -786,6 +815,8 @@ std::string GCMClientImpl::GetStateString() const { return "UNINITIALIZED"; case GCMClientImpl::LOADING: return "LOADING"; + case GCMClientImpl::LOADED: + return "LOADED"; case GCMClientImpl::INITIAL_DEVICE_CHECKIN: return "INITIAL_DEVICE_CHECKIN"; case GCMClientImpl::READY: @@ -986,4 +1017,13 @@ void GCMClientImpl::HandleIncomingSendError( send_error_details); } +bool GCMClientImpl::HasStandaloneRegisteredApp() const { + if (registrations_.empty()) + return false; + // Note that account mapper is not counted as a standalone app since it is + // automatically started when other app uses GCM. + return registrations_.size() > 1 || + !registrations_.count(kGCMAccountMapperAppId); +} + } // namespace gcm diff --git a/components/gcm_driver/gcm_client_impl.h b/components/gcm_driver/gcm_client_impl.h index 9299a0bfd63e..cf1b33b940d2 100644 --- a/components/gcm_driver/gcm_client_impl.h +++ b/components/gcm_driver/gcm_client_impl.h @@ -79,6 +79,24 @@ class GCMClientImpl : public GCMClient, public GCMStatsRecorder::Delegate, public ConnectionFactory::ConnectionListener { public: + // State representation of the GCMClient. + // Any change made to this enum should have corresponding change in the + // GetStateString(...) function. + enum State { + // Uninitialized. + UNINITIALIZED, + // Initialized, + INITIALIZED, + // GCM store loading is in progress. + LOADING, + // GCM store is loaded. + LOADED, + // Initial device checkin is in progress. + INITIAL_DEVICE_CHECKIN, + // Ready to accept requests. + READY, + }; + explicit GCMClientImpl(scoped_ptr internals_builder); ~GCMClientImpl() override; @@ -91,7 +109,7 @@ class GCMClientImpl url_request_context_getter, scoped_ptr encryptor, GCMClient::Delegate* delegate) override; - void Start() override; + void Start(StartMode start_mode) override; void Stop() override; void Register(const std::string& app_id, const std::vector& sender_ids) override; @@ -118,22 +136,6 @@ class GCMClientImpl void OnDisconnected() override; private: - // State representation of the GCMClient. - // Any change made to this enum should have corresponding change in the - // GetStateString(...) function. - enum State { - // Uninitialized. - UNINITIALIZED, - // Initialized, - INITIALIZED, - // GCM store loading is in progress. - LOADING, - // Initial device checkin is in progress. - INITIAL_DEVICE_CHECKIN, - // Ready to accept requests. - READY, - }; - // The check-in info for the device. // TODO(fgorski): Convert to a class with explicit getters/setters. struct CheckinInfo { @@ -188,8 +190,10 @@ class GCMClientImpl // Runs after GCM Store load is done to trigger continuation of the // initialization. void OnLoadCompleted(scoped_ptr result); + // Starts the GCM. + void StartGCM(); // Initializes mcs_client_, which handles the connection to MCS. - void InitializeMCSClient(scoped_ptr result); + void InitializeMCSClient(); // Complets the first time device checkin. void OnFirstTimeDeviceCheckinCompleted(const CheckinInfo& checkin_info); // Starts a login on mcs_client_. @@ -261,6 +265,9 @@ class GCMClientImpl const mcs_proto::DataMessageStanza& data_message_stanza, MessageData& message_data); + // Is there any standalone app being registered for GCM? + bool HasStandaloneRegisteredApp() const; + // Builder for the GCM internals (mcs client, etc.). scoped_ptr internals_builder_; @@ -272,6 +279,12 @@ class GCMClientImpl GCMClient::Delegate* delegate_; + // Flag to indicate if the GCM should be delay started until it is actually + // used in either of the following cases: + // 1) The GCM store contains the registration records. + // 2) GCM functionailities are explicitly called. + StartMode start_mode_; + // Device checkin info (android ID and security token used by device). CheckinInfo device_checkin_info_; @@ -287,6 +300,9 @@ class GCMClientImpl // serial number mappings. scoped_ptr gcm_store_; + // Data loaded from the GCM store. + scoped_ptr load_result_; + scoped_refptr network_session_; net::BoundNetLog net_log_; scoped_ptr connection_factory_; diff --git a/components/gcm_driver/gcm_client_impl_unittest.cc b/components/gcm_driver/gcm_client_impl_unittest.cc index 7ae42b2a7519..f9a7b5391564 100644 --- a/components/gcm_driver/gcm_client_impl_unittest.cc +++ b/components/gcm_driver/gcm_client_impl_unittest.cc @@ -286,6 +286,9 @@ class GCMClientImplTest : public testing::Test, void OnDisconnected() override {} GCMClientImpl* gcm_client() const { return gcm_client_.get(); } + GCMClientImpl::State gcm_client_state() const { + return gcm_client_->state_; + } FakeMCSClient* mcs_client() const { return reinterpret_cast(gcm_client_->mcs_client_.get()); } @@ -511,7 +514,7 @@ void GCMClientImplTest::InitializeGCMClient() { void GCMClientImplTest::StartGCMClient() { // Start loading and check-in. - gcm_client_->Start(); + gcm_client_->Start(GCMClient::IMMEDIATE_START); PumpLoopUntilIdle(); } @@ -1037,41 +1040,101 @@ void GCMClientImplStartAndStopTest::DefaultCompleteCheckin() { } TEST_F(GCMClientImplStartAndStopTest, StartStopAndRestart) { - // Start the GCM and wait until it is ready. - gcm_client()->Start(); + // GCMClientImpl should be in INITIALIZED state at first. + EXPECT_EQ(GCMClientImpl::INITIALIZED, gcm_client_state()); + + // Delay start the GCM. + gcm_client()->Start(GCMClient::DELAYED_START); PumpLoopUntilIdle(); + EXPECT_EQ(GCMClientImpl::LOADED, gcm_client_state()); // Stop the GCM. gcm_client()->Stop(); PumpLoopUntilIdle(); + EXPECT_EQ(GCMClientImpl::INITIALIZED, gcm_client_state()); - // Restart the GCM. - gcm_client()->Start(); + // Restart the GCM without delay. + gcm_client()->Start(GCMClient::IMMEDIATE_START); PumpLoopUntilIdle(); + EXPECT_EQ(GCMClientImpl::INITIAL_DEVICE_CHECKIN, gcm_client_state()); } TEST_F(GCMClientImplStartAndStopTest, StartAndStopImmediately) { - // Start the GCM and then stop it immediately. - gcm_client()->Start(); + // GCMClientImpl should be in INITIALIZED state at first. + EXPECT_EQ(GCMClientImpl::INITIALIZED, gcm_client_state()); + + // Delay start the GCM and then stop it immediately. + gcm_client()->Start(GCMClient::DELAYED_START); gcm_client()->Stop(); + PumpLoopUntilIdle(); + EXPECT_EQ(GCMClientImpl::INITIALIZED, gcm_client_state()); + // Start the GCM and then stop it immediately. + gcm_client()->Start(GCMClient::IMMEDIATE_START); + gcm_client()->Stop(); PumpLoopUntilIdle(); + EXPECT_EQ(GCMClientImpl::INITIALIZED, gcm_client_state()); } TEST_F(GCMClientImplStartAndStopTest, StartStopAndRestartImmediately) { + // GCMClientImpl should be in INITIALIZED state at first. + EXPECT_EQ(GCMClientImpl::INITIALIZED, gcm_client_state()); + + // Delay start the GCM and then stop and restart it immediately. + gcm_client()->Start(GCMClient::DELAYED_START); + gcm_client()->Stop(); + gcm_client()->Start(GCMClient::DELAYED_START); + PumpLoopUntilIdle(); + EXPECT_EQ(GCMClientImpl::LOADED, gcm_client_state()); + // Start the GCM and then stop and restart it immediately. - gcm_client()->Start(); + gcm_client()->Start(GCMClient::IMMEDIATE_START); + gcm_client()->Stop(); + gcm_client()->Start(GCMClient::IMMEDIATE_START); + PumpLoopUntilIdle(); + EXPECT_EQ(GCMClientImpl::INITIAL_DEVICE_CHECKIN, gcm_client_state()); +} + +TEST_F(GCMClientImplStartAndStopTest, DelayStart) { + // GCMClientImpl should be in INITIALIZED state at first. + EXPECT_EQ(GCMClientImpl::INITIALIZED, gcm_client_state()); + + // Delay start the GCM. + gcm_client()->Start(GCMClient::DELAYED_START); + PumpLoopUntilIdle(); + EXPECT_EQ(GCMClientImpl::LOADED, gcm_client_state()); + + // Start the GCM immediately and complete the checkin. + gcm_client()->Start(GCMClient::IMMEDIATE_START); + PumpLoopUntilIdle(); + EXPECT_EQ(GCMClientImpl::INITIAL_DEVICE_CHECKIN, gcm_client_state()); + DefaultCompleteCheckin(); + EXPECT_EQ(GCMClientImpl::READY, gcm_client_state()); + + // Registration. + std::vector senders; + senders.push_back("sender"); + gcm_client()->Register(kAppId, senders); + CompleteRegistration("reg_id"); + EXPECT_EQ(GCMClientImpl::READY, gcm_client_state()); + + // Stop the GCM. gcm_client()->Stop(); - gcm_client()->Start(); + PumpLoopUntilIdle(); + EXPECT_EQ(GCMClientImpl::INITIALIZED, gcm_client_state()); + // Delay start the GCM. GCM is indeed started without delay because the + // registration record has been found. + gcm_client()->Start(GCMClient::DELAYED_START); PumpLoopUntilIdle(); + EXPECT_EQ(GCMClientImpl::READY, gcm_client_state()); } // Test for known account mappings and last token fetching time being passed // to OnGCMReady. TEST_F(GCMClientImplStartAndStopTest, OnGCMReadyAccountsAndTokenFetchingTime) { // Start the GCM and wait until it is ready. - gcm_client()->Start(); + gcm_client()->Start(GCMClient::IMMEDIATE_START); PumpLoopUntilIdle(); DefaultCompleteCheckin(); @@ -1090,7 +1153,7 @@ TEST_F(GCMClientImplStartAndStopTest, OnGCMReadyAccountsAndTokenFetchingTime) { PumpLoopUntilIdle(); // Restart the GCM. - gcm_client()->Start(); + gcm_client()->Start(GCMClient::IMMEDIATE_START); PumpLoopUntilIdle(); EXPECT_EQ(LOADING_COMPLETED, last_event()); diff --git a/components/gcm_driver/gcm_driver.cc b/components/gcm_driver/gcm_driver.cc index c3aeb917c170..a82e7b1bcff9 100644 --- a/components/gcm_driver/gcm_driver.cc +++ b/components/gcm_driver/gcm_driver.cc @@ -25,7 +25,7 @@ void GCMDriver::Register(const std::string& app_id, DCHECK(!sender_ids.empty()); DCHECK(!callback.is_null()); - GCMClient::Result result = EnsureStarted(); + GCMClient::Result result = EnsureStarted(GCMClient::IMMEDIATE_START); if (result != GCMClient::SUCCESS) { callback.Run(std::string(), result); return; @@ -73,7 +73,7 @@ void GCMDriver::Unregister(const std::string& app_id, DCHECK(!app_id.empty()); DCHECK(!callback.is_null()); - GCMClient::Result result = EnsureStarted(); + GCMClient::Result result = EnsureStarted(GCMClient::IMMEDIATE_START); if (result != GCMClient::SUCCESS) { callback.Run(result); return; @@ -99,7 +99,7 @@ void GCMDriver::Send(const std::string& app_id, DCHECK(!receiver_id.empty()); DCHECK(!callback.is_null()); - GCMClient::Result result = EnsureStarted(); + GCMClient::Result result = EnsureStarted(GCMClient::IMMEDIATE_START); if (result != GCMClient::SUCCESS) { callback.Run(std::string(), result); return; diff --git a/components/gcm_driver/gcm_driver.h b/components/gcm_driver/gcm_driver.h index 8d2b0d87a137..04fb53660dbd 100644 --- a/components/gcm_driver/gcm_driver.h +++ b/components/gcm_driver/gcm_driver.h @@ -137,7 +137,7 @@ class GCMDriver { protected: // Ensures that the GCM service starts (if necessary conditions are met). - virtual GCMClient::Result EnsureStarted() = 0; + virtual GCMClient::Result EnsureStarted(GCMClient::StartMode start_mode) = 0; // Platform-specific implementation of Register. virtual void RegisterImpl(const std::string& app_id, diff --git a/components/gcm_driver/gcm_driver_android.cc b/components/gcm_driver/gcm_driver_android.cc index a8e69a91c2ef..93b2f919ba32 100644 --- a/components/gcm_driver/gcm_driver_android.cc +++ b/components/gcm_driver/gcm_driver_android.cc @@ -162,7 +162,8 @@ void GCMDriverAndroid::SetLastTokenFetchTime(const base::Time& time) { void GCMDriverAndroid::WakeFromSuspendForHeartbeat(bool wake) { } -GCMClient::Result GCMDriverAndroid::EnsureStarted() { +GCMClient::Result GCMDriverAndroid::EnsureStarted( + GCMClient::StartMode start_mode) { // TODO(johnme): Maybe we should check if GMS is available? return GCMClient::SUCCESS; } diff --git a/components/gcm_driver/gcm_driver_android.h b/components/gcm_driver/gcm_driver_android.h index 7bb1cbdb2ecc..566c91cbc29a 100644 --- a/components/gcm_driver/gcm_driver_android.h +++ b/components/gcm_driver/gcm_driver_android.h @@ -69,7 +69,8 @@ class GCMDriverAndroid : public GCMDriver { protected: // GCMDriver implementation: - virtual GCMClient::Result EnsureStarted() override; + virtual GCMClient::Result EnsureStarted( + GCMClient::StartMode start_mode) override; virtual void RegisterImpl( const std::string& app_id, const std::vector& sender_ids) override; diff --git a/components/gcm_driver/gcm_driver_desktop.cc b/components/gcm_driver/gcm_driver_desktop.cc index 2c15ef28d142..6a9153986819 100644 --- a/components/gcm_driver/gcm_driver_desktop.cc +++ b/components/gcm_driver/gcm_driver_desktop.cc @@ -68,7 +68,8 @@ class GCMDriverDesktop::IOWorker : public GCMClient::Delegate { const base::FilePath& store_path, const scoped_refptr& request_context, const scoped_refptr blocking_task_runner); - void Start(const base::WeakPtr& service); + void Start(GCMClient::StartMode start_mode, + const base::WeakPtr& service); void Stop(); void Register(const std::string& app_id, const std::vector& sender_ids); @@ -239,11 +240,12 @@ void GCMDriverDesktop::IOWorker::OnDisconnected() { } void GCMDriverDesktop::IOWorker::Start( + GCMClient::StartMode start_mode, const base::WeakPtr& service) { DCHECK(io_thread_->RunsTasksOnCurrentThread()); service_ = service; - gcm_client_->Start(); + gcm_client_->Start(start_mode); } void GCMDriverDesktop::IOWorker::Stop() { @@ -427,7 +429,7 @@ void GCMDriverDesktop::AddAppHandler(const std::string& app_id, GCMDriver::AddAppHandler(app_id, handler); // Ensures that the GCM service is started when there is an interest. - EnsureStarted(); + EnsureStarted(GCMClient::DELAYED_START); } void GCMDriverDesktop::RemoveAppHandler(const std::string& app_id) { @@ -458,7 +460,7 @@ void GCMDriverDesktop::Enable() { return; gcm_enabled_ = true; - EnsureStarted(); + EnsureStarted(GCMClient::DELAYED_START); } void GCMDriverDesktop::Disable() { @@ -656,11 +658,13 @@ void GCMDriverDesktop::WakeFromSuspendForHeartbeat(bool wake) { DCHECK(ui_thread_->RunsTasksOnCurrentThread()); wake_from_suspend_enabled_ = wake; - if (!IsStarted()) + + // The GCM service has not been initialized. + if (!delayed_task_controller_) return; if (!delayed_task_controller_->CanRunTaskWithoutDelay()) { - // The GCM service has started but the client is not ready yet. + // The GCM service was initialized but has not started yet. delayed_task_controller_->AddTask( base::Bind(&GCMDriverDesktop::WakeFromSuspendForHeartbeat, weak_ptr_factory_.GetWeakPtr(), @@ -690,45 +694,36 @@ void GCMDriverDesktop::SetAccountTokens( account_tokens)); } -GCMClient::Result GCMDriverDesktop::EnsureStarted() { +GCMClient::Result GCMDriverDesktop::EnsureStarted( + GCMClient::StartMode start_mode) { DCHECK(ui_thread_->RunsTasksOnCurrentThread()); if (gcm_started_) return GCMClient::SUCCESS; - if (!gcm_enabled_) { - // Poll for channel status in order to find out when it is re-enabled when - // GCM is currently disabled. - gcm_channel_status_syncer_->EnsureStarted(); - - return GCMClient::GCM_DISABLED; - } - // Have any app requested the service? if (app_handlers().empty()) return GCMClient::UNKNOWN_ERROR; - DCHECK(!delayed_task_controller_); - delayed_task_controller_.reset(new GCMDelayedTaskController); - - // Polling for channel status is only needed when GCM is supported for all - // users. + // Polling for channel status should be invoked when GCM is being requested, + // no matter whether GCM is enabled or nor. gcm_channel_status_syncer_->EnsureStarted(); - UMA_HISTOGRAM_BOOLEAN("GCM.UserSignedIn", signed_in_); + if (!gcm_enabled_) + return GCMClient::GCM_DISABLED; + + if (!delayed_task_controller_) + delayed_task_controller_.reset(new GCMDelayedTaskController); // Note that we need to pass weak pointer again since the existing weak - // pointer in IOWorker might have been invalidated when check-out occurs. + // pointer in IOWorker might have been invalidated when GCM is stopped. io_thread_->PostTask( FROM_HERE, base::Bind(&GCMDriverDesktop::IOWorker::Start, base::Unretained(io_worker_.get()), + start_mode, weak_ptr_factory_.GetWeakPtr())); - gcm_started_ = true; - if (wake_from_suspend_enabled_) - WakeFromSuspendForHeartbeat(wake_from_suspend_enabled_); - return GCMClient::SUCCESS; } @@ -793,6 +788,12 @@ void GCMDriverDesktop::GCMClientReady( const base::Time& last_token_fetch_time) { DCHECK(ui_thread_->RunsTasksOnCurrentThread()); + UMA_HISTOGRAM_BOOLEAN("GCM.UserSignedIn", signed_in_); + + gcm_started_ = true; + if (wake_from_suspend_enabled_) + WakeFromSuspendForHeartbeat(wake_from_suspend_enabled_); + last_token_fetch_time_ = last_token_fetch_time; GCMDriver::AddAppHandler(kGCMAccountMapperAppId, account_mapper_.get()); diff --git a/components/gcm_driver/gcm_driver_desktop.h b/components/gcm_driver/gcm_driver_desktop.h index 8cc048186b0b..6ea70d618128 100644 --- a/components/gcm_driver/gcm_driver_desktop.h +++ b/components/gcm_driver/gcm_driver_desktop.h @@ -92,7 +92,7 @@ class GCMDriverDesktop : public GCMDriver { protected: // GCMDriver implementation: - GCMClient::Result EnsureStarted() override; + GCMClient::Result EnsureStarted(GCMClient::StartMode start_mode) override; void RegisterImpl(const std::string& app_id, const std::vector& sender_ids) override; void UnregisterImpl(const std::string& app_id) override; diff --git a/components/gcm_driver/gcm_driver_desktop_unittest.cc b/components/gcm_driver/gcm_driver_desktop_unittest.cc index d4f22041b8cf..e20081efd3c1 100644 --- a/components/gcm_driver/gcm_driver_desktop_unittest.cc +++ b/components/gcm_driver/gcm_driver_desktop_unittest.cc @@ -123,7 +123,7 @@ class GCMDriverTest : public testing::Test { bool HasAppHandlers() const; FakeGCMClient* GetGCMClient(); - void CreateDriver(FakeGCMClient::StartMode gcm_client_start_mode); + void CreateDriver(); void ShutdownDriver(); void AddAppHandlers(); void RemoveAppHandlers(); @@ -222,15 +222,13 @@ FakeGCMClient* GCMDriverTest::GetGCMClient() { return static_cast(driver_->GetGCMClientForTesting()); } -void GCMDriverTest::CreateDriver( - FakeGCMClient::StartMode gcm_client_start_mode) { +void GCMDriverTest::CreateDriver() { scoped_refptr request_context = new net::TestURLRequestContextGetter(io_thread_.message_loop_proxy()); // TODO(johnme): Need equivalent test coverage of GCMDriverAndroid. driver_.reset(new GCMDriverDesktop( scoped_ptr( - new FakeGCMClientFactory(gcm_client_start_mode, - base::MessageLoopProxy::current(), + new FakeGCMClientFactory(base::MessageLoopProxy::current(), io_thread_.message_loop_proxy())).Pass(), GCMClient::ChromeBuildInfo(), "http://channel.status.request.url", @@ -332,20 +330,27 @@ void GCMDriverTest::UnregisterCompleted(GCMClient::Result result) { } TEST_F(GCMDriverTest, Create) { - // Create GCMDriver first. GCM is not started. - CreateDriver(FakeGCMClient::NO_DELAY_START); + // Create GCMDriver first. By default GCM is set to delay start. + CreateDriver(); EXPECT_FALSE(driver()->IsStarted()); - // GCM will be started after an app handler is added. + // Adding an app handler will not start GCM. AddAppHandlers(); - EXPECT_TRUE(driver()->IsStarted()); PumpIOLoop(); + PumpUILoop(); + EXPECT_FALSE(driver()->IsStarted()); + EXPECT_FALSE(driver()->IsConnected()); + EXPECT_FALSE(gcm_connection_observer()->connected()); + + // The GCM registration will kick off the GCM. + Register(kTestAppID1, ToSenderList("sender"), GCMDriverTest::WAIT); + EXPECT_TRUE(driver()->IsStarted()); EXPECT_TRUE(driver()->IsConnected()); EXPECT_TRUE(gcm_connection_observer()->connected()); } TEST_F(GCMDriverTest, Shutdown) { - CreateDriver(FakeGCMClient::NO_DELAY_START); + CreateDriver(); EXPECT_FALSE(HasAppHandlers()); AddAppHandlers(); @@ -358,40 +363,43 @@ TEST_F(GCMDriverTest, Shutdown) { } TEST_F(GCMDriverTest, DisableAndReenableGCM) { - CreateDriver(FakeGCMClient::NO_DELAY_START); + CreateDriver(); AddAppHandlers(); PumpIOLoop(); PumpUILoop(); + EXPECT_FALSE(driver()->IsStarted()); - // GCMClient should be started. + // The GCM registration will kick off the GCM. + Register(kTestAppID1, ToSenderList("sender"), GCMDriverTest::WAIT); EXPECT_TRUE(driver()->IsStarted()); - // Disables the GCM. + // Disables the GCM. GCM will be stopped. driver()->Disable(); PumpIOLoop(); PumpUILoop(); - - // GCMClient should be stopped. EXPECT_FALSE(driver()->IsStarted()); - // Enables the GCM. + // Enables the GCM. GCM will be started. driver()->Enable(); PumpIOLoop(); PumpUILoop(); - - // GCMClient should be started. EXPECT_TRUE(driver()->IsStarted()); } TEST_F(GCMDriverTest, StartOrStopGCMOnDemand) { - CreateDriver(FakeGCMClient::NO_DELAY_START); + CreateDriver(); PumpIOLoop(); PumpUILoop(); + EXPECT_FALSE(driver()->IsStarted()); - // GCMClient is started after an app handler has been added. + // Adding an app handler will not start GCM. driver()->AddAppHandler(kTestAppID1, gcm_app_handler()); PumpIOLoop(); PumpUILoop(); + EXPECT_FALSE(driver()->IsStarted()); + + // The GCM registration will kick off the GCM. + Register(kTestAppID1, ToSenderList("sender"), GCMDriverTest::WAIT); EXPECT_TRUE(driver()->IsStarted()); // Add another app handler. @@ -423,39 +431,37 @@ TEST_F(GCMDriverTest, RegisterFailed) { std::vector sender_ids; sender_ids.push_back("sender1"); - CreateDriver(FakeGCMClient::NO_DELAY_START); + CreateDriver(); - // Registration fails when GCM is disabled. - driver()->Disable(); + // Registration fails when the no app handler is added. Register(kTestAppID1, sender_ids, GCMDriverTest::WAIT); EXPECT_TRUE(registration_id().empty()); - EXPECT_EQ(GCMClient::GCM_DISABLED, registration_result()); + EXPECT_EQ(GCMClient::UNKNOWN_ERROR, registration_result()); ClearResults(); - // Registration fails when the no app handler is added. - driver()->Enable(); - RemoveAppHandlers(); + // Registration fails when GCM is disabled. + AddAppHandlers(); + driver()->Disable(); Register(kTestAppID1, sender_ids, GCMDriverTest::WAIT); EXPECT_TRUE(registration_id().empty()); - EXPECT_EQ(GCMClient::UNKNOWN_ERROR, registration_result()); + EXPECT_EQ(GCMClient::GCM_DISABLED, registration_result()); } TEST_F(GCMDriverTest, UnregisterFailed) { - CreateDriver(FakeGCMClient::NO_DELAY_START); + CreateDriver(); - // Unregistration fails when GCM is disabled. - driver()->Disable(); + // Unregistration fails when the no app handler is added. Unregister(kTestAppID1, GCMDriverTest::WAIT); - EXPECT_EQ(GCMClient::GCM_DISABLED, unregistration_result()); + EXPECT_EQ(GCMClient::UNKNOWN_ERROR, unregistration_result()); ClearResults(); - // Unregistration fails when the no app handler is added. - driver()->Enable(); - RemoveAppHandlers(); + // Unregistration fails when GCM is disabled. + AddAppHandlers(); + driver()->Disable(); Unregister(kTestAppID1, GCMDriverTest::WAIT); - EXPECT_EQ(GCMClient::UNKNOWN_ERROR, unregistration_result()); + EXPECT_EQ(GCMClient::GCM_DISABLED, unregistration_result()); } TEST_F(GCMDriverTest, SendFailed) { @@ -463,27 +469,32 @@ TEST_F(GCMDriverTest, SendFailed) { message.id = "1"; message.data["key1"] = "value1"; - CreateDriver(FakeGCMClient::NO_DELAY_START); + CreateDriver(); - // Sending fails when GCM is disabled. - driver()->Disable(); + // Sending fails when the no app handler is added. Send(kTestAppID1, kUserID1, message, GCMDriverTest::WAIT); EXPECT_TRUE(send_message_id().empty()); - EXPECT_EQ(GCMClient::GCM_DISABLED, send_result()); + EXPECT_EQ(GCMClient::UNKNOWN_ERROR, send_result()); ClearResults(); - // Sending fails when the no app handler is added. - driver()->Enable(); - RemoveAppHandlers(); + // Sending fails when GCM is disabled. + AddAppHandlers(); + driver()->Disable(); Send(kTestAppID1, kUserID1, message, GCMDriverTest::WAIT); EXPECT_TRUE(send_message_id().empty()); - EXPECT_EQ(GCMClient::UNKNOWN_ERROR, send_result()); + EXPECT_EQ(GCMClient::GCM_DISABLED, send_result()); } TEST_F(GCMDriverTest, GCMClientNotReadyBeforeRegistration) { - // Make GCMClient not ready initially. - CreateDriver(FakeGCMClient::DELAY_START); + CreateDriver(); + PumpIOLoop(); + PumpUILoop(); + + // Make GCMClient not ready until PerformDelayedStart is called. + GetGCMClient()->set_start_mode_overridding( + FakeGCMClient::FORCE_TO_ALWAYS_DELAY_START_GCM); + AddAppHandlers(); // The registration is on hold until GCMClient is ready. @@ -498,15 +509,21 @@ TEST_F(GCMDriverTest, GCMClientNotReadyBeforeRegistration) { EXPECT_EQ(GCMClient::UNKNOWN_ERROR, registration_result()); // Register operation will be invoked after GCMClient becomes ready. - GetGCMClient()->PerformDelayedLoading(); + GetGCMClient()->PerformDelayedStart(); WaitForAsyncOperation(); EXPECT_FALSE(registration_id().empty()); EXPECT_EQ(GCMClient::SUCCESS, registration_result()); } TEST_F(GCMDriverTest, GCMClientNotReadyBeforeSending) { - // Make GCMClient not ready initially. - CreateDriver(FakeGCMClient::DELAY_START); + CreateDriver(); + PumpIOLoop(); + PumpUILoop(); + + // Make GCMClient not ready until PerformDelayedStart is called. + GetGCMClient()->set_start_mode_overridding( + FakeGCMClient::FORCE_TO_ALWAYS_DELAY_START_GCM); + AddAppHandlers(); // The sending is on hold until GCMClient is ready. @@ -522,7 +539,7 @@ TEST_F(GCMDriverTest, GCMClientNotReadyBeforeSending) { EXPECT_EQ(GCMClient::UNKNOWN_ERROR, send_result()); // Send operation will be invoked after GCMClient becomes ready. - GetGCMClient()->PerformDelayedLoading(); + GetGCMClient()->PerformDelayedStart(); WaitForAsyncOperation(); EXPECT_EQ(message.id, send_message_id()); EXPECT_EQ(GCMClient::SUCCESS, send_result()); @@ -550,7 +567,7 @@ GCMDriverFunctionalTest::~GCMDriverFunctionalTest() { void GCMDriverFunctionalTest::SetUp() { GCMDriverTest::SetUp(); - CreateDriver(FakeGCMClient::NO_DELAY_START); + CreateDriver(); AddAppHandlers(); PumpIOLoop(); PumpUILoop(); @@ -755,7 +772,9 @@ TEST_F(GCMDriverFunctionalTest, SendError) { } TEST_F(GCMDriverFunctionalTest, MessageReceived) { + // GCM registration has to be performed otherwise GCM will not be started. Register(kTestAppID1, ToSenderList("sender"), GCMDriverTest::WAIT); + GCMClient::IncomingMessage message; message.data["key1"] = "value1"; message.data["key2"] = "value2"; @@ -771,7 +790,9 @@ TEST_F(GCMDriverFunctionalTest, MessageReceived) { } TEST_F(GCMDriverFunctionalTest, MessageWithCollapseKeyReceived) { + // GCM registration has to be performed otherwise GCM will not be started. Register(kTestAppID1, ToSenderList("sender"), GCMDriverTest::WAIT); + GCMClient::IncomingMessage message; message.data["key1"] = "value1"; message.collapse_key = "collapse_key_value"; @@ -787,6 +808,9 @@ TEST_F(GCMDriverFunctionalTest, MessageWithCollapseKeyReceived) { } TEST_F(GCMDriverFunctionalTest, MessagesDeleted) { + // GCM registration has to be performed otherwise GCM will not be started. + Register(kTestAppID1, ToSenderList("sender"), GCMDriverTest::WAIT); + GetGCMClient()->DeleteMessages(kTestAppID1); gcm_app_handler()->WaitForNotification(); EXPECT_EQ(FakeGCMAppHandler::MESSAGES_DELETED_EVENT, @@ -795,6 +819,9 @@ TEST_F(GCMDriverFunctionalTest, MessagesDeleted) { } TEST_F(GCMDriverFunctionalTest, LastTokenFetchTime) { + // GCM registration has to be performed otherwise GCM will not be started. + Register(kTestAppID1, ToSenderList("sender"), GCMDriverTest::WAIT); + EXPECT_EQ(base::Time(), driver()->GetLastTokenFetchTime()); base::Time fetch_time = base::Time::Now(); driver()->SetLastTokenFetchTime(fetch_time); @@ -871,22 +898,16 @@ bool GCMChannelStatusSyncerTest::CompareDelaySeconds( } TEST_F(GCMChannelStatusSyncerTest, DisableAndEnable) { - // Create GCMDriver first. GCM is not started. - CreateDriver(FakeGCMClient::NO_DELAY_START); - EXPECT_FALSE(driver()->IsStarted()); - - // By default, GCM is enabled. + // Create GCMDriver first. By default, GCM is enabled. + CreateDriver(); EXPECT_TRUE(driver()->gcm_enabled()); EXPECT_TRUE(syncer()->gcm_enabled()); // Remove delay such that the request could be executed immediately. syncer()->set_delay_removed_for_testing(true); - // GCM will be started after app handler is added. - AddAppHandlers(); - EXPECT_TRUE(driver()->IsStarted()); - // GCM is still enabled at this point. + AddAppHandlers(); EXPECT_TRUE(driver()->gcm_enabled()); EXPECT_TRUE(syncer()->gcm_enabled()); @@ -906,26 +927,19 @@ TEST_F(GCMChannelStatusSyncerTest, DisableAndEnable) { CompleteGCMChannelStatusRequest(true, 0); EXPECT_TRUE(driver()->gcm_enabled()); EXPECT_TRUE(syncer()->gcm_enabled()); - EXPECT_TRUE(driver()->IsStarted()); } TEST_F(GCMChannelStatusSyncerTest, DisableRestartAndEnable) { - // Create GCMDriver first. GCM is not started. - CreateDriver(FakeGCMClient::NO_DELAY_START); - EXPECT_FALSE(driver()->IsStarted()); - - // By default, GCM is enabled. + // Create GCMDriver first. By default, GCM is enabled. + CreateDriver(); EXPECT_TRUE(driver()->gcm_enabled()); EXPECT_TRUE(syncer()->gcm_enabled()); // Remove delay such that the request could be executed immediately. syncer()->set_delay_removed_for_testing(true); - // GCM will be started after app handler is added. - AddAppHandlers(); - EXPECT_TRUE(driver()->IsStarted()); - // GCM is still enabled at this point. + AddAppHandlers(); EXPECT_TRUE(driver()->gcm_enabled()); EXPECT_TRUE(syncer()->gcm_enabled()); @@ -936,11 +950,10 @@ TEST_F(GCMChannelStatusSyncerTest, DisableRestartAndEnable) { CompleteGCMChannelStatusRequest(false, 0); EXPECT_FALSE(driver()->gcm_enabled()); EXPECT_FALSE(syncer()->gcm_enabled()); - EXPECT_FALSE(driver()->IsStarted()); // Simulate browser start by recreating GCMDriver. ShutdownDriver(); - CreateDriver(FakeGCMClient::NO_DELAY_START); + CreateDriver(); // Remove delay such that the request could be executed immediately. syncer()->set_delay_removed_for_testing(true); @@ -948,12 +961,10 @@ TEST_F(GCMChannelStatusSyncerTest, DisableRestartAndEnable) { // GCM is still disabled. EXPECT_FALSE(driver()->gcm_enabled()); EXPECT_FALSE(syncer()->gcm_enabled()); - EXPECT_FALSE(driver()->IsStarted()); AddAppHandlers(); EXPECT_FALSE(driver()->gcm_enabled()); EXPECT_FALSE(syncer()->gcm_enabled()); - EXPECT_FALSE(driver()->IsStarted()); // Wait until the GCM channel status request gets triggered. PumpUILoop(); @@ -962,12 +973,11 @@ TEST_F(GCMChannelStatusSyncerTest, DisableRestartAndEnable) { CompleteGCMChannelStatusRequest(true, 0); EXPECT_TRUE(driver()->gcm_enabled()); EXPECT_TRUE(syncer()->gcm_enabled()); - EXPECT_TRUE(driver()->IsStarted()); } TEST_F(GCMChannelStatusSyncerTest, FirstTimePolling) { // Start GCM. - CreateDriver(FakeGCMClient::NO_DELAY_START); + CreateDriver(); AddAppHandlers(); // The 1st request should be triggered shortly without jittering. @@ -977,7 +987,7 @@ TEST_F(GCMChannelStatusSyncerTest, FirstTimePolling) { TEST_F(GCMChannelStatusSyncerTest, SubsequentPollingWithDefaultInterval) { // Create GCMDriver first. GCM is not started. - CreateDriver(FakeGCMClient::NO_DELAY_START); + CreateDriver(); // Remove delay such that the request could be executed immediately. syncer()->set_delay_removed_for_testing(true); @@ -1005,7 +1015,7 @@ TEST_F(GCMChannelStatusSyncerTest, SubsequentPollingWithDefaultInterval) { // Simulate browser start by recreating GCMDriver. ShutdownDriver(); - CreateDriver(FakeGCMClient::NO_DELAY_START); + CreateDriver(); AddAppHandlers(); // After start-up, the request should still be scheduled at the expected @@ -1019,7 +1029,7 @@ TEST_F(GCMChannelStatusSyncerTest, SubsequentPollingWithDefaultInterval) { TEST_F(GCMChannelStatusSyncerTest, SubsequentPollingWithUpdatedInterval) { // Create GCMDriver first. GCM is not started. - CreateDriver(FakeGCMClient::NO_DELAY_START); + CreateDriver(); // Remove delay such that the request could be executed immediately. syncer()->set_delay_removed_for_testing(true); @@ -1048,7 +1058,7 @@ TEST_F(GCMChannelStatusSyncerTest, SubsequentPollingWithUpdatedInterval) { // Simulate browser start by recreating GCMDriver. ShutdownDriver(); - CreateDriver(FakeGCMClient::NO_DELAY_START); + CreateDriver(); AddAppHandlers(); // After start-up, the request should still be scheduled at the expected -- 2.11.4.GIT