[GCM] Start GCMChannelStatusSyncer when GCM is disabled

We need to start GCMChannelStatusSyncer when GCM is disabled in order
to poll the server to find out when it is reenabled.

Also if server returns empty response, we should not treat it as error
and trigger the backoff logic.

BUG=423415
TEST=new tests added

Committed: https://crrev.com/9e9dd3b7798b3500d70941af04b1325ef9a0b544
Cr-Commit-Position: refs/heads/master@{#299518}

R=fgorski@chromium.org

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

Cr-Commit-Position: refs/heads/master@{#299854}

chrome/browser/services/gcm/gcm_desktop_utils.cc

@@ -4,6 +4,7 @@
 
 #include "chrome/browser/services/gcm/gcm_desktop_utils.h"
 
+#include "base/command_line.h"
 #include "base/logging.h"
 #include "base/sequenced_task_runner.h"
 #include "base/threading/sequenced_worker_pool.h"
@@ -20,6 +21,8 @@ namespace gcm {
 
 namespace {
 
+const char kChannelStatusRelativePath[] = "/experimentstatus";
+
 GCMClient::ChromePlatform GetPlatform() {
 #if defined(OS_WIN)
   return GCMClient::PLATFORM_WIN;
@@ -72,13 +75,9 @@ GCMClient::ChromeBuildInfo GetChromeBuildInfo() {
 }
 
 std::string GetChannelStatusRequestUrl() {
-  chrome::VersionInfo::Channel channel = chrome::VersionInfo::GetChannel();
-  if (channel == chrome::VersionInfo::CHANNEL_STABLE ||
-      channel == chrome::VersionInfo::CHANNEL_BETA) {
-    return ProfileSyncService::kSyncServerUrl;
-  }
-
-  return ProfileSyncService::kDevServerUrl;
+  GURL sync_url(
+      ProfileSyncService::GetSyncServiceURL(*CommandLine::ForCurrentProcess()));
+  return sync_url.spec() + kChannelStatusRelativePath;
 }
 
 std::string GetUserAgent() {

components/gcm_driver/gcm_channel_status_request.cc

@@ -93,15 +93,20 @@ bool GCMChannelStatusRequest::ParseResponse(const net::URLFetcher* source) {
   }
 
   std::string response_string;
-  if (!source->GetResponseAsString(&response_string) ||
-      response_string.empty()) {
+  if (!source->GetResponseAsString(&response_string)) {
     LOG(ERROR) << "GCM channel response failed to be retrieved.";
     return false;
   }
 
+  // Empty response means to keep the existing values.
+  if (response_string.empty()) {
+    callback_.Run(false, false, 0);
+    return true;
+  }
+
   sync_pb::ExperimentStatusResponse response_proto;
   if (!response_proto.ParseFromString(response_string)) {
-    LOG(ERROR) << "GCM channel response failed to be parse as proto.";
+    LOG(ERROR) << "GCM channel response failed to be parsed as proto.";
     return false;
   }
 
@@ -120,7 +125,7 @@ bool GCMChannelStatusRequest::ParseResponse(const net::URLFetcher* source) {
   if (poll_interval_seconds < kMinPollIntervalSeconds)
     poll_interval_seconds = kMinPollIntervalSeconds;
 
-  callback_.Run(enabled, poll_interval_seconds);
+  callback_.Run(true, enabled, poll_interval_seconds);
 
   return true;
 }

components/gcm_driver/gcm_channel_status_request.h

@@ -26,7 +26,14 @@ namespace gcm {
 class GCMChannelStatusRequest : public net::URLFetcherDelegate {
  public:
   // Callback completing the channel status request.
-  typedef base::Callback<void(bool enabled, int poll_interval_seconds)>
+  // |update_received|: use the existing values if it is false which means no
+  //                    update is received.
+  // |enabled|: indicates if GCM is enabled (allowed to run) or not.
+  // |poll_interval_seconds|: the interval in seconds to start next poll
+  //                          request.
+  typedef base::Callback<void(bool update_received,
+                              bool enabled,
+                              int poll_interval_seconds)>
       GCMChannelStatusRequestCallback;
 
   GCMChannelStatusRequest(

components/gcm_driver/gcm_channel_status_request_unittest.cc

@@ -29,13 +29,16 @@ class GCMChannelStatusRequestTest : public testing::Test {
                                   const std::string& response_body);
   void SetResponseProtoData(GCMStatus status, int poll_interval_seconds);
   void CompleteFetch();
-  void OnRequestCompleted(bool enabled, int poll_interval_seconds);
+  void OnRequestCompleted(bool update_received,
+                          bool enabled,
+                          int poll_interval_seconds);
 
   scoped_ptr<GCMChannelStatusRequest> request_;
   base::MessageLoop message_loop_;
   net::TestURLFetcherFactory url_fetcher_factory_;
   scoped_refptr<net::TestURLRequestContextGetter> url_request_context_getter_;
   bool request_callback_invoked_;
+  bool update_received_;
   bool enabled_;
   int poll_interval_seconds_;
 };
@@ -44,6 +47,7 @@ GCMChannelStatusRequestTest::GCMChannelStatusRequestTest()
     : url_request_context_getter_(new net::TestURLRequestContextGetter(
            message_loop_.message_loop_proxy())),
       request_callback_invoked_(false),
+      update_received_(false),
       enabled_(true),
       poll_interval_seconds_(0) {
 }
@@ -97,8 +101,9 @@ void GCMChannelStatusRequestTest::CompleteFetch() {
 }
 
 void GCMChannelStatusRequestTest::OnRequestCompleted(
-    bool enabled, int poll_interval_seconds) {
+    bool update_received, bool enabled, int poll_interval_seconds) {
   request_callback_invoked_ = true;
+  update_received_ = update_received;
   enabled_ = enabled;
   poll_interval_seconds_ = poll_interval_seconds;
 }
@@ -116,7 +121,8 @@ TEST_F(GCMChannelStatusRequestTest, ResponseEmpty) {
   SetResponseStatusAndString(net::HTTP_OK, "");
   CompleteFetch();
 
-  EXPECT_FALSE(request_callback_invoked_);
+  EXPECT_TRUE(request_callback_invoked_);
+  EXPECT_FALSE(update_received_);
 }
 
 TEST_F(GCMChannelStatusRequestTest, ResponseNotInProtoFormat) {
@@ -132,7 +138,8 @@ TEST_F(GCMChannelStatusRequestTest, ResponseEmptyProtoData) {
   SetResponseProtoData(NOT_SPECIFIED, 0);
   CompleteFetch();
 
-  EXPECT_FALSE(request_callback_invoked_);
+  EXPECT_TRUE(request_callback_invoked_);
+  EXPECT_FALSE(update_received_);
 }
 
 TEST_F(GCMChannelStatusRequestTest, ResponseWithDisabledStatus) {
@@ -141,6 +148,7 @@ TEST_F(GCMChannelStatusRequestTest, ResponseWithDisabledStatus) {
   CompleteFetch();
 
   EXPECT_TRUE(request_callback_invoked_);
+  EXPECT_TRUE(update_received_);
   EXPECT_FALSE(enabled_);
   EXPECT_EQ(
       GCMChannelStatusRequest::default_poll_interval_seconds(),
@@ -153,6 +161,7 @@ TEST_F(GCMChannelStatusRequestTest, ResponseWithEnabledStatus) {
   CompleteFetch();
 
   EXPECT_TRUE(request_callback_invoked_);
+  EXPECT_TRUE(update_received_);
   EXPECT_TRUE(enabled_);
   EXPECT_EQ(
       GCMChannelStatusRequest::default_poll_interval_seconds(),
@@ -169,6 +178,7 @@ TEST_F(GCMChannelStatusRequestTest, ResponseWithPollInterval) {
   CompleteFetch();
 
   EXPECT_TRUE(request_callback_invoked_);
+  EXPECT_TRUE(update_received_);
   EXPECT_TRUE(enabled_);
   EXPECT_EQ(poll_interval_seconds, poll_interval_seconds_);
 }
@@ -183,6 +193,7 @@ TEST_F(GCMChannelStatusRequestTest, ResponseWithShortPollInterval) {
   CompleteFetch();
 
   EXPECT_TRUE(request_callback_invoked_);
+  EXPECT_TRUE(update_received_);
   EXPECT_TRUE(enabled_);
   EXPECT_EQ(GCMChannelStatusRequest::min_poll_interval_seconds(),
             poll_interval_seconds_);
@@ -196,6 +207,7 @@ TEST_F(GCMChannelStatusRequestTest, ResponseWithDisabledStatusAndPollInterval) {
   CompleteFetch();
 
   EXPECT_TRUE(request_callback_invoked_);
+  EXPECT_TRUE(update_received_);
   EXPECT_FALSE(enabled_);
   EXPECT_EQ(poll_interval_seconds, poll_interval_seconds_);
 }

components/gcm_driver/gcm_channel_status_syncer.cc

@@ -5,12 +5,14 @@
 #include "components/gcm_driver/gcm_channel_status_syncer.h"
 
 #include "base/bind.h"
+#include "base/command_line.h"
 #include "base/location.h"
 #include "base/logging.h"
 #include "base/message_loop/message_loop.h"
 #include "base/prefs/pref_registry_simple.h"
 #include "base/prefs/pref_service.h"
 #include "base/rand_util.h"
+#include "base/strings/string_number_conversions.h"
 #include "components/gcm_driver/gcm_channel_status_request.h"
 #include "components/gcm_driver/gcm_driver.h"
 #include "components/pref_registry/pref_registry_syncable.h"
@@ -35,8 +37,21 @@ const int kFirstTimeDelaySeconds = 1 * 60;  // 1 minute.
 // The fuzzing variation added to the polling delay.
 const int kGCMChannelRequestTimeJitterSeconds = 15 * 60;  // 15 minues.
 
+// The minimum poll interval that can be overridden to.
+const int kMinCustomPollIntervalMinutes = 2;
+
+// Custom poll interval could not be used more than the limit below.
+const int kMaxNumberToUseCustomPollInterval = 10;
+
 }  // namespace
 
+namespace switches {
+
+// Override the default poll interval for testing purpose.
+const char kCustomPollIntervalMinutes[] = "gcm-channel-poll-interval";
+
+}  // namepsace switches
+
 // static
 void GCMChannelStatusSyncer::RegisterPrefs(PrefRegistrySimple* registry) {
   registry->RegisterBooleanPref(kGCMChannelStatus, true);
@@ -79,9 +94,11 @@ GCMChannelStatusSyncer::GCMChannelStatusSyncer(
       channel_status_request_url_(channel_status_request_url),
       user_agent_(user_agent),
       request_context_(request_context),
+      started_(false),
       gcm_enabled_(true),
       poll_interval_seconds_(
           GCMChannelStatusRequest::default_poll_interval_seconds()),
+      custom_poll_interval_use_count_(0),
       delay_removed_for_testing_(false),
       weak_ptr_factory_(this) {
   gcm_enabled_ = prefs_->GetBoolean(kGCMChannelStatus);
@@ -91,6 +108,19 @@ GCMChannelStatusSyncer::GCMChannelStatusSyncer(
     poll_interval_seconds_ =
         GCMChannelStatusRequest::min_poll_interval_seconds();
   }
+  if (CommandLine::ForCurrentProcess()->HasSwitch(
+          switches::kCustomPollIntervalMinutes)) {
+    std::string value(CommandLine::ForCurrentProcess()->GetSwitchValueASCII(
+        switches::kCustomPollIntervalMinutes));
+    int minutes = 0;
+    if (base::StringToInt(value, &minutes)) {
+      DCHECK_GE(minutes, kMinCustomPollIntervalMinutes);
+      if (minutes >= kMinCustomPollIntervalMinutes) {
+        poll_interval_seconds_ = minutes * 60;
+        custom_poll_interval_use_count_ = kMaxNumberToUseCustomPollInterval;
+      }
+    }
+  }
   last_check_time_ = base::Time::FromInternalValue(
       prefs_->GetInt64(kGCMChannelLastCheckTime));
 }
@@ -100,18 +130,21 @@ GCMChannelStatusSyncer::~GCMChannelStatusSyncer() {
 
 void GCMChannelStatusSyncer::EnsureStarted() {
   // Bail out if the request is already scheduled or started.
-  if (weak_ptr_factory_.HasWeakPtrs() || request_)
+  if (started_)
     return;
+  started_ = true;
 
   ScheduleRequest();
 }
 
 void GCMChannelStatusSyncer::Stop() {
+  started_ = false;
   request_.reset();
   weak_ptr_factory_.InvalidateWeakPtrs();
 }
 
-void GCMChannelStatusSyncer::OnRequestCompleted(bool enabled,
+void GCMChannelStatusSyncer::OnRequestCompleted(bool update_received,
+                                                bool enabled,
                                                 int poll_interval_seconds) {
   DCHECK(request_);
   request_.reset();
@@ -121,23 +154,31 @@ void GCMChannelStatusSyncer::OnRequestCompleted(bool enabled,
   prefs_->SetInt64(kGCMChannelLastCheckTime,
                    last_check_time_.ToInternalValue());
 
-  if (gcm_enabled_ != enabled) {
-    gcm_enabled_ = enabled;
-    prefs_->SetBoolean(kGCMChannelStatus, enabled);
-    if (gcm_enabled_)
-      driver_->Enable();
-    else
-      driver_->Disable();
-  }
-
-  DCHECK_GE(poll_interval_seconds,
-            GCMChannelStatusRequest::min_poll_interval_seconds());
-  if (poll_interval_seconds_ != poll_interval_seconds) {
-    poll_interval_seconds_ = poll_interval_seconds;
-    prefs_->SetInteger(kGCMChannelPollIntervalSeconds, poll_interval_seconds_);
+  if (update_received) {
+    if (gcm_enabled_ != enabled) {
+      gcm_enabled_ = enabled;
+      prefs_->SetBoolean(kGCMChannelStatus, enabled);
+      if (gcm_enabled_)
+        driver_->Enable();
+      else
+        driver_->Disable();
+    }
+
+    // Skip updating poll interval if the custom one is still in effect.
+    if (!custom_poll_interval_use_count_) {
+      DCHECK_GE(poll_interval_seconds,
+                GCMChannelStatusRequest::min_poll_interval_seconds());
+      if (poll_interval_seconds_ != poll_interval_seconds) {
+        poll_interval_seconds_ = poll_interval_seconds;
+        prefs_->SetInteger(kGCMChannelPollIntervalSeconds,
+                           poll_interval_seconds_);
+      }
+    }
   }
 
-  ScheduleRequest();
+  // Do not schedule next request if syncer is stopped.
+  if (started_)
+    ScheduleRequest();
 }
 
 void GCMChannelStatusSyncer::ScheduleRequest() {
@@ -147,6 +188,9 @@ void GCMChannelStatusSyncer::ScheduleRequest() {
       base::Bind(&GCMChannelStatusSyncer::StartRequest,
                  weak_ptr_factory_.GetWeakPtr()),
       current_request_delay_interval_);
+
+  if (custom_poll_interval_use_count_)
+    custom_poll_interval_use_count_--;
 }
 
 void GCMChannelStatusSyncer::StartRequest() {
@@ -180,7 +224,9 @@ base::TimeDelta GCMChannelStatusSyncer::GetRequestDelayInterval() const {
     delay_seconds = kFirstTimeDelaySeconds;
   } else {
     // Otherwise, add a fuzzing variation to the delay.
-    delay_seconds += base::RandInt(0, kGCMChannelRequestTimeJitterSeconds);
+    // The fuzzing variation is off when the custom interval is used.
+    if (!custom_poll_interval_use_count_)
+      delay_seconds += base::RandInt(0, kGCMChannelRequestTimeJitterSeconds);
   }
 
   return base::TimeDelta::FromSeconds(delay_seconds);

components/gcm_driver/gcm_channel_status_syncer.h

@@ -59,7 +59,9 @@ class GCMChannelStatusSyncer {
 
  private:
   // Called when a request is completed.
-  void OnRequestCompleted(bool enabled, int poll_interval_seconds);
+  void OnRequestCompleted(bool update_received,
+                          bool enabled,
+                          int poll_interval_seconds);
 
   // Schedules next request to start after appropriate delay.
   void ScheduleRequest();
@@ -80,10 +82,17 @@ class GCMChannelStatusSyncer {
   scoped_refptr<net::URLRequestContextGetter> request_context_;
   scoped_ptr<GCMChannelStatusRequest> request_;
 
+  bool started_;
   bool gcm_enabled_;
   int poll_interval_seconds_;
   base::Time last_check_time_;
 
+  // If non-zero, |poll_interval_seconds_| is overriden by the command line
+  // options for testing purpose. Each time when the custom poll interval is
+  // used, this count is subtracted by one. When it reaches zero, the default
+  // poll interval will be used instead.
+  int custom_poll_interval_use_count_;
+
   // The flag that indicates if the delay, including fuzzing variation and poll
   // interval, is removed for testing purpose.
   bool delay_removed_for_testing_;

components/gcm_driver/gcm_driver_desktop.cc

@@ -432,6 +432,7 @@ void GCMDriverDesktop::RemoveAppHandler(const std::string& app_id) {
   // remove the last app handler - account mapper.
   if (app_handlers().size() == 1) {
     Stop();
+    gcm_channel_status_syncer_->Stop();
   }
 }
 
@@ -471,8 +472,6 @@ void GCMDriverDesktop::Stop() {
   if (!gcm_started_)
     return;
 
-  gcm_channel_status_syncer_->Stop();
-
   account_mapper_->ShutdownHandler();
   GCMDriver::RemoveAppHandler(kGCMAccountMapperAppId);
 
@@ -650,8 +649,14 @@ GCMClient::Result GCMDriverDesktop::EnsureStarted() {
   if (gcm_started_)
     return GCMClient::SUCCESS;
 
-  if (!gcm_enabled_)
+  if (!gcm_enabled_) {
+    // Poll for channel status in order to find out when it is re-enabled when
+    // GCM is currently disabled.
+    if (GCMDriver::IsAllowedForAllUsers())
+      gcm_channel_status_syncer_->EnsureStarted();
+
     return GCMClient::GCM_DISABLED;
+  }
 
   // Have any app requested the service?
   if (app_handlers().empty())

components/gcm_driver/gcm_driver_desktop_unittest.cc

@@ -1017,6 +1017,8 @@ GCMChannelStatusSyncerTest::~GCMChannelStatusSyncerTest() {
 void GCMChannelStatusSyncerTest::SetUp() {
   GCMDriverTest::SetUp();
 
+  url_fetcher_factory_.set_remove_fetcher_on_delete(true);
+
   // Turn on all-user support.
   ASSERT_TRUE(base::FieldTrialList::CreateFieldTrial("GCM", "Enabled"));
 }
@@ -1091,7 +1093,7 @@ TEST_F(GCMChannelStatusSyncerTest, DisableAndEnable) {
   EXPECT_TRUE(driver()->IsStarted());
 }
 
-TEST_F(GCMChannelStatusSyncerTest, DisableAndRestart) {
+TEST_F(GCMChannelStatusSyncerTest, DisableRestartAndEnable) {
   // Create GCMDriver first. GCM is not started.
   CreateDriver(FakeGCMClient::NO_DELAY_START);
   EXPECT_FALSE(driver()->IsStarted());
@@ -1124,6 +1126,9 @@ TEST_F(GCMChannelStatusSyncerTest, DisableAndRestart) {
   ShutdownDriver();
   CreateDriver(FakeGCMClient::NO_DELAY_START);
 
+  // Remove delay such that the request could be executed immediately.
+  syncer()->set_delay_removed_for_testing(true);
+
   // GCM is still disabled.
   EXPECT_FALSE(driver()->gcm_enabled());
   EXPECT_FALSE(syncer()->gcm_enabled());
@@ -1133,6 +1138,15 @@ TEST_F(GCMChannelStatusSyncerTest, DisableAndRestart) {
   EXPECT_FALSE(driver()->gcm_enabled());
   EXPECT_FALSE(syncer()->gcm_enabled());
   EXPECT_FALSE(driver()->IsStarted());
+
+  // Wait until the GCM channel status request gets triggered.
+  PumpUILoop();
+
+  // Complete the request that re-enables the GCM.
+  CompleteGCMChannelStatusRequest(true, 0);
+  EXPECT_TRUE(driver()->gcm_enabled());
+  EXPECT_TRUE(syncer()->gcm_enabled());
+  EXPECT_TRUE(driver()->IsStarted());
 }
 
 TEST_F(GCMChannelStatusSyncerTest, FirstTimePolling) {