From 902c40650be8afef499d7de6e1fc60532f06cc2b Mon Sep 17 00:00:00 2001 From: johnme Date: Fri, 21 Aug 2015 15:06:17 -0700 Subject: Fix GCMClient delayed then immediate start. Commit 9f9a73672ab60c3da17bd4ff2660ce3a113cd8fa (https://codereview.chromium.org/1183843002) has a race condition, where calling GCMClient::Start(DELAYED_START) closely followed by GCMClient::Start(IMMEDIATE_START) fails to create the GCM store. This patch fixes that case. BUG=522902 Review URL: https://codereview.chromium.org/1310563003 Cr-Commit-Position: refs/heads/master@{#344875} --- components/gcm_driver/gcm_client_impl.cc | 16 ++++++++++++---- components/gcm_driver/gcm_client_impl_unittest.cc | 13 +++++++++++++ 2 files changed, 25 insertions(+), 4 deletions(-) diff --git a/components/gcm_driver/gcm_client_impl.cc b/components/gcm_driver/gcm_client_impl.cc index 6db36cb..1a95809 100644 --- a/components/gcm_driver/gcm_client_impl.cc +++ b/components/gcm_driver/gcm_client_impl.cc @@ -381,10 +381,18 @@ void GCMClientImpl::OnLoadCompleted(scoped_ptr result) { if (!result->success) { if (result->store_does_not_exist) { - // In the case that the store does not exist, set |state| back to - // INITIALIZED such that store loading could be triggered again when - // Start() is called with IMMEDIATE_START. - state_ = INITIALIZED; + if (start_mode_ == IMMEDIATE_START) { + // An immediate start was requested during the delayed start that just + // completed. Perform it now. + gcm_store_->Load(GCMStore::CREATE_IF_MISSING, + base::Bind(&GCMClientImpl::OnLoadCompleted, + weak_ptr_factory_.GetWeakPtr())); + } else { + // In the case that the store does not exist, set |state_| back to + // INITIALIZED such that store loading could be triggered again when + // Start() is called with IMMEDIATE_START. + state_ = INITIALIZED; + } } else { // Otherwise, destroy the store to try again. ResetStore(); diff --git a/components/gcm_driver/gcm_client_impl_unittest.cc b/components/gcm_driver/gcm_client_impl_unittest.cc index 876e2f9..3fbb2e7 100644 --- a/components/gcm_driver/gcm_client_impl_unittest.cc +++ b/components/gcm_driver/gcm_client_impl_unittest.cc @@ -1307,6 +1307,19 @@ TEST_F(GCMClientImplStartAndStopTest, ImmediateStartAndThenDelayStart) { EXPECT_EQ(GCMClientImpl::LOADED, gcm_client_state()); } +TEST_F(GCMClientImplStartAndStopTest, DelayedStartRace) { + // GCMClientImpl should be in INITIALIZED state at first. + EXPECT_EQ(GCMClientImpl::INITIALIZED, gcm_client_state()); + + // Delay start the GCM, then start it immediately while it's still loading. + gcm_client()->Start(GCMClient::DELAYED_START); + gcm_client()->Start(GCMClient::IMMEDIATE_START); + PumpLoopUntilIdle(); + EXPECT_EQ(GCMClientImpl::INITIAL_DEVICE_CHECKIN, gcm_client_state()); + DefaultCompleteCheckin(); + EXPECT_EQ(GCMClientImpl::READY, gcm_client_state()); +} + TEST_F(GCMClientImplStartAndStopTest, DelayedStart) { // GCMClientImpl should be in INITIALIZED state at first. EXPECT_EQ(GCMClientImpl::INITIALIZED, gcm_client_state()); -- cgit v1.1