[GCM] Relanding updates to GCMClientImpl (g-services-settings)

This patch relands crrev.com/243813003 after crrev.com/252933002
fixes the problem with crrev.com/226923002

R=jianli@chromium.org,zea@chromium.org
BUG=359254

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

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@267430 0039d316-1c4b-4281-b951-d872f2087c98

3 files changed, 216 insertions, 44 deletions

diff --git a/google_apis/gcm/gcm_client_impl.cc b/google_apis/gcm/gcm_client_impl.cc
index 46295bd..2a59bbf 100644
--- a/google_apis/gcm/gcm_client_impl.cc
+++ b/google_apis/gcm/gcm_client_impl.cc
@@ -19,6 +19,7 @@
 #include "google_apis/gcm/engine/checkin_request.h"
 #include "google_apis/gcm/engine/connection_factory_impl.h"
 #include "google_apis/gcm/engine/gcm_store_impl.h"
+#include "google_apis/gcm/engine/gservices_settings.h"
 #include "google_apis/gcm/engine/mcs_client.h"
 #include "google_apis/gcm/monitoring/gcm_stats_recorder.h"
 #include "google_apis/gcm/protocol/mcs.pb.h"
@@ -77,7 +78,6 @@ enum MessageType {
 const char kMCSEndpointMain[] = "https://mtalk.google.com:5228";
 const char kMCSEndpointFallback[] = "https://mtalk.google.com:443";
 
-const int64 kDefaultCheckinInterval = 2 * 24 * 60 * 60LL;  // seconds = 2 days.
 const int kMaxRegistrationRetries = 5;
 const char kMessageTypeDataMessage[] = "gcm";
 const char kMessageTypeDeletedMessagesKey[] = "deleted_messages";
@@ -164,6 +164,7 @@ GCMClientImpl::GCMClientImpl(scoped_ptr<GCMInternalsBuilder> internals_builder)
       pending_registration_requests_deleter_(&pending_registration_requests_),
       pending_unregistration_requests_deleter_(
           &pending_unregistration_requests_),
+      periodic_checkin_ptr_factory_(this),
       weak_ptr_factory_(this) {
 }
 
@@ -193,6 +194,7 @@ void GCMClientImpl::Initialize(
   account_ids_ = account_ids;
 
   gcm_store_.reset(new GCMStoreImpl(path, blocking_task_runner));
+  gservices_settings_.reset(new GServicesSettings(gcm_store_.get()));
 
   delegate_ = delegate;
 
@@ -219,11 +221,12 @@ void GCMClientImpl::OnLoadCompleted(scoped_ptr<GCMStore::LoadResult> result) {
   registrations_ = result->registrations;
   device_checkin_info_.android_id = result->device_android_id;
   device_checkin_info_.secret = result->device_security_token;
-  base::Time last_checkin_time = result->last_checkin_time;
+  last_checkin_time_ = result->last_checkin_time;
+  gservices_settings_->UpdateFromLoadResult(*result);
   InitializeMCSClient(result.Pass());
 
   if (device_checkin_info_.IsValid()) {
-    SchedulePeriodicCheckin(last_checkin_time);
+    SchedulePeriodicCheckin();
     OnReady();
     return;
   }
@@ -294,12 +297,15 @@ void GCMClientImpl::ResetState() {
 }
 
 void GCMClientImpl::StartCheckin() {
-  CheckinRequest::RequestInfo request_info(
-    device_checkin_info_.android_id,
-    device_checkin_info_.secret,
-    std::string(),
-    account_ids_,
-    chrome_build_proto_);
+  // Make sure no checkin is in progress.
+  if (checkin_request_.get())
+    return;
+
+  CheckinRequest::RequestInfo request_info(device_checkin_info_.android_id,
+                                           device_checkin_info_.secret,
+                                           gservices_settings_->digest(),
+                                           account_ids_,
+                                           chrome_build_proto_);
   checkin_request_.reset(
       new CheckinRequest(request_info,
                          kDefaultBackoffPolicy,
@@ -335,30 +341,43 @@ void GCMClientImpl::OnCheckinCompleted(
   }
 
   if (device_checkin_info_.IsValid()) {
-    base::Time last_checkin_time = clock_->Now();
+    // First update G-services settings, as something might have changed.
+    gservices_settings_->UpdateFromCheckinResponse(checkin_response);
+    last_checkin_time_ = clock_->Now();
     gcm_store_->SetLastCheckinTime(
-        last_checkin_time,
+        last_checkin_time_,
         base::Bind(&GCMClientImpl::SetLastCheckinTimeCallback,
                    weak_ptr_factory_.GetWeakPtr()));
-    SchedulePeriodicCheckin(last_checkin_time);
+    SchedulePeriodicCheckin();
   }
 }
 
-void GCMClientImpl::SchedulePeriodicCheckin(
-    const base::Time& last_checkin_time) {
-  base::TimeDelta time_to_next_checkin = last_checkin_time +
-      base::TimeDelta::FromSeconds(kDefaultCheckinInterval) - clock_->Now();
-  if (time_to_next_checkin < base::TimeDelta::FromSeconds(0L))
-    time_to_next_checkin = base::TimeDelta::FromSeconds(0L);
-  // TODO(fgorski): Make sure that once dynamic events (like accounts list
-  // change) trigger checkin we reset the timer.
+void GCMClientImpl::SchedulePeriodicCheckin() {
+  // Make sure no checkin is in progress.
+  if (checkin_request_.get())
+    return;
+
+  // There should be only one periodic checkin pending at a time. Removing
+  // pending periodic checkin to schedule a new one.
+  periodic_checkin_ptr_factory_.InvalidateWeakPtrs();
+
+  base::TimeDelta time_to_next_checkin = GetTimeToNextCheckin();
+  if (time_to_next_checkin < base::TimeDelta())
+    time_to_next_checkin = base::TimeDelta();
+
   base::MessageLoop::current()->PostDelayedTask(
       FROM_HERE,
       base::Bind(&GCMClientImpl::StartCheckin,
-                 weak_ptr_factory_.GetWeakPtr()),
+                 periodic_checkin_ptr_factory_.GetWeakPtr()),
       time_to_next_checkin);
 }
 
+base::TimeDelta GCMClientImpl::GetTimeToNextCheckin() const {
+  return last_checkin_time_ +
+         base::TimeDelta::FromSeconds(gservices_settings_->checkin_interval()) -
+         clock_->Now();
+}
+
 void GCMClientImpl::SetLastCheckinTimeCallback(bool success) {
   // TODO(fgorski): This is one of the signals that store needs a rebuild.
   DCHECK(success);
diff --git a/google_apis/gcm/gcm_client_impl.h b/google_apis/gcm/gcm_client_impl.h
index 05d1023..f38a08d 100644
--- a/google_apis/gcm/gcm_client_impl.h
+++ b/google_apis/gcm/gcm_client_impl.h
@@ -29,6 +29,7 @@ class GURL;
 
 namespace base {
 class Clock;
+class Time;
 }  // namespace base
 
 namespace mcs_proto {
@@ -44,6 +45,7 @@ namespace gcm {
 class CheckinRequest;
 class ConnectionFactory;
 class GCMClientImplTest;
+class GServicesSettings;
 
 // Helper class for building GCM internals. Allows tests to inject fake versions
 // as necessary.
@@ -177,9 +179,12 @@ class GCM_EXPORT GCMClientImpl : public GCMClient {
   // Function also cleans up the pending checkin.
   void OnCheckinCompleted(
       const checkin_proto::AndroidCheckinResponse& checkin_response);
-  // Schedules next device checkin, based on |last_checkin_time| and
-  // checkin_interval specified in GServices settings.
-  void SchedulePeriodicCheckin(const base::Time& last_checkin_time);
+  // Schedules next periodic device checkin and makes sure there is at most one
+  // pending checkin at a time. This function is meant to be called after a
+  // successful checkin.
+  void SchedulePeriodicCheckin();
+  // Gets the time until next checkin.
+  base::TimeDelta GetTimeToNextCheckin() const;
   // Callback for setting last checkin time in the |gcm_store_|.
   void SetLastCheckinTimeCallback(bool success);
 
@@ -269,6 +274,15 @@ class GCM_EXPORT GCMClientImpl : public GCMClient {
   STLValueDeleter<PendingUnregistrationRequests>
       pending_unregistration_requests_deleter_;
 
+  // G-services settings that were provided by MCS.
+  scoped_ptr<GServicesSettings> gservices_settings_;
+
+  // Time of the last successful checkin.
+  base::Time last_checkin_time_;
+
+  // Factory for creating references when scheduling periodic checkin.
+  base::WeakPtrFactory<GCMClientImpl> periodic_checkin_ptr_factory_;
+
   // Factory for creating references in callbacks.
   base::WeakPtrFactory<GCMClientImpl> weak_ptr_factory_;
 
diff --git a/google_apis/gcm/gcm_client_impl_unittest.cc b/google_apis/gcm/gcm_client_impl_unittest.cc
index e6a24b0..25c0afd 100644
--- a/google_apis/gcm/gcm_client_impl_unittest.cc
+++ b/google_apis/gcm/gcm_client_impl_unittest.cc
@@ -8,12 +8,14 @@
 #include "base/files/scoped_temp_dir.h"
 #include "base/message_loop/message_loop.h"
 #include "base/run_loop.h"
-#include "base/test/simple_test_clock.h"
+#include "base/strings/string_number_conversions.h"
+#include "base/time/clock.h"
 #include "components/os_crypt/os_crypt_switches.h"
 #include "google_apis/gcm/base/mcs_message.h"
 #include "google_apis/gcm/base/mcs_util.h"
 #include "google_apis/gcm/engine/fake_connection_factory.h"
 #include "google_apis/gcm/engine/fake_connection_handler.h"
+#include "google_apis/gcm/engine/gservices_settings.h"
 #include "google_apis/gcm/monitoring/gcm_stats_recorder.h"
 #include "google_apis/gcm/protocol/android_checkin.pb.h"
 #include "google_apis/gcm/protocol/checkin.pb.h"
@@ -39,6 +41,8 @@ enum LastEvent {
 
 const uint64 kDeviceAndroidId = 54321;
 const uint64 kDeviceSecurityToken = 12345;
+const int64 kSettingsCheckinInterval = 16 * 60 * 60;
+const char kSettingsDefaultDigest[] = "default_digest";
 const char kAppId[] = "app_id";
 const char kSender[] = "project_id";
 const char kSender2[] = "project_id2";
@@ -115,9 +119,44 @@ void FakeMCSClient::SendMessage(const MCSMessage& message) {
   }
 }
 
+class AutoAdvancingTestClock : public base::Clock {
+ public:
+  explicit AutoAdvancingTestClock(base::TimeDelta auto_increment_time_delta);
+  virtual ~AutoAdvancingTestClock();
+
+  virtual base::Time Now() OVERRIDE;
+  void Advance(TimeDelta delta);
+  int call_count() const { return call_count_; }
+
+ private:
+  int call_count_;
+  base::TimeDelta auto_increment_time_delta_;
+  base::Time now_;
+
+  DISALLOW_COPY_AND_ASSIGN(AutoAdvancingTestClock);
+};
+
+AutoAdvancingTestClock::AutoAdvancingTestClock(
+    base::TimeDelta auto_increment_time_delta)
+    : call_count_(0), auto_increment_time_delta_(auto_increment_time_delta) {
+}
+
+AutoAdvancingTestClock::~AutoAdvancingTestClock() {
+}
+
+base::Time AutoAdvancingTestClock::Now() {
+  call_count_++;
+  now_ += auto_increment_time_delta_;
+  return now_;
+}
+
+void AutoAdvancingTestClock::Advance(base::TimeDelta delta) {
+  now_ += delta;
+}
+
 class FakeGCMInternalsBuilder : public GCMInternalsBuilder {
  public:
-  FakeGCMInternalsBuilder();
+  FakeGCMInternalsBuilder(base::TimeDelta clock_step);
   virtual ~FakeGCMInternalsBuilder();
 
   virtual scoped_ptr<base::Clock> BuildClock() OVERRIDE;
@@ -133,14 +172,19 @@ class FakeGCMInternalsBuilder : public GCMInternalsBuilder {
       scoped_refptr<net::HttpNetworkSession> network_session,
       net::NetLog* net_log,
       GCMStatsRecorder* recorder) OVERRIDE;
+
+ private:
+  base::TimeDelta clock_step_;
 };
 
-FakeGCMInternalsBuilder::FakeGCMInternalsBuilder() {}
+FakeGCMInternalsBuilder::FakeGCMInternalsBuilder(base::TimeDelta clock_step)
+    : clock_step_(clock_step) {
+}
 
 FakeGCMInternalsBuilder::~FakeGCMInternalsBuilder() {}
 
 scoped_ptr<base::Clock> FakeGCMInternalsBuilder::BuildClock() {
-  return make_scoped_ptr<base::Clock>(new base::SimpleTestClock());
+  return make_scoped_ptr<base::Clock>(new AutoAdvancingTestClock(clock_step_));
 }
 
 scoped_ptr<MCSClient> FakeGCMInternalsBuilder::BuildMCSClient(
@@ -174,10 +218,13 @@ class GCMClientImplTest : public testing::Test,
 
   virtual void SetUp() OVERRIDE;
 
-  void BuildGCMClient();
+  void BuildGCMClient(base::TimeDelta clock_step);
   void InitializeGCMClient();
   void ReceiveMessageFromMCS(const MCSMessage& message);
-  void CompleteCheckin(uint64 android_id, uint64 security_token);
+  void CompleteCheckin(uint64 android_id,
+                       uint64 security_token,
+                       const std::string& digest,
+                       const std::map<std::string, std::string>& settings);
   void CompleteRegistration(const std::string& registration_id);
   void CompleteUnregistration(const std::string& app_id);
 
@@ -234,18 +281,23 @@ class GCMClientImplTest : public testing::Test,
     return last_error_details_;
   }
 
+  const GServicesSettings* gservices_settings() const {
+    return gcm_client_->gservices_settings_.get();
+  }
+
   int64 CurrentTime();
 
- private:
   // Tooling.
   void PumpLoop();
   void PumpLoopUntilIdle();
   void QuitLoop();
-
-  base::SimpleTestClock* clock() const {
-    return reinterpret_cast<base::SimpleTestClock*>(gcm_client_->clock_.get());
+  void InitializeLoop();
+  bool CreateUniqueTempDir();
+  AutoAdvancingTestClock* clock() const {
+    return reinterpret_cast<AutoAdvancingTestClock*>(gcm_client_->clock_.get());
   }
 
+ private:
   // Variables used for verification.
   LastEvent last_event_;
   std::string last_app_id_;
@@ -281,11 +333,14 @@ void GCMClientImplTest::SetUp() {
   base::CommandLine::ForCurrentProcess()->AppendSwitch(
       os_crypt::switches::kUseMockKeychain);
 #endif  // OS_MACOSX
-  ASSERT_TRUE(temp_directory_.CreateUniqueTempDir());
-  run_loop_.reset(new base::RunLoop);
-  BuildGCMClient();
+  ASSERT_TRUE(CreateUniqueTempDir());
+  InitializeLoop();
+  BuildGCMClient(base::TimeDelta());
   InitializeGCMClient();
-  CompleteCheckin(kDeviceAndroidId, kDeviceSecurityToken);
+  CompleteCheckin(kDeviceAndroidId,
+                  kDeviceSecurityToken,
+                  std::string(),
+                  std::map<std::string, std::string>());
 }
 
 void GCMClientImplTest::PumpLoop() {
@@ -303,18 +358,42 @@ void GCMClientImplTest::QuitLoop() {
     run_loop_->Quit();
 }
 
-void GCMClientImplTest::BuildGCMClient() {
-  gcm_client_.reset(new GCMClientImpl(
-      make_scoped_ptr<GCMInternalsBuilder>(new FakeGCMInternalsBuilder())));
+void GCMClientImplTest::InitializeLoop() {
+  run_loop_.reset(new base::RunLoop);
+}
+
+bool GCMClientImplTest::CreateUniqueTempDir() {
+  return temp_directory_.CreateUniqueTempDir();
 }
 
-void GCMClientImplTest::CompleteCheckin(uint64 android_id,
-                                        uint64 security_token) {
+void GCMClientImplTest::BuildGCMClient(base::TimeDelta clock_step) {
+  gcm_client_.reset(new GCMClientImpl(make_scoped_ptr<GCMInternalsBuilder>(
+      new FakeGCMInternalsBuilder(clock_step))));
+}
+
+void GCMClientImplTest::CompleteCheckin(
+    uint64 android_id,
+    uint64 security_token,
+    const std::string& digest,
+    const std::map<std::string, std::string>& settings) {
   checkin_proto::AndroidCheckinResponse response;
   response.set_stats_ok(true);
   response.set_android_id(android_id);
   response.set_security_token(security_token);
 
+  // For testing G-services settings.
+  if (!digest.empty()) {
+    response.set_digest(digest);
+    for (std::map<std::string, std::string>::const_iterator it =
+             settings.begin();
+         it != settings.end();
+         ++it) {
+      checkin_proto::GservicesSetting* setting = response.add_setting();
+      setting->set_name(it->first);
+      setting->set_value(it->second);
+    }
+  }
+
   std::string response_string;
   response.SerializeToString(&response_string);
 
@@ -477,7 +556,7 @@ TEST_F(GCMClientImplTest, DISABLED_RegisterAppFromCache) {
   EXPECT_EQ(REGISTRATION_COMPLETED, last_event());
 
   // Recreate GCMClient in order to load from the persistent store.
-  BuildGCMClient();
+  BuildGCMClient(base::TimeDelta());
   InitializeGCMClient();
 
   EXPECT_TRUE(ExistsRegistration(kAppId));
@@ -604,4 +683,64 @@ TEST_F(GCMClientImplTest, SendMessage) {
             mcs_client()->last_data_message_stanza().app_data(0).value());
 }
 
+class GCMClientImplCheckinTest : public GCMClientImplTest {
+ public:
+  GCMClientImplCheckinTest();
+  virtual ~GCMClientImplCheckinTest();
+
+  virtual void SetUp() OVERRIDE;
+
+  std::map<std::string, std::string> GenerateSettings(int64 checkin_interval);
+};
+
+GCMClientImplCheckinTest::GCMClientImplCheckinTest() {
+}
+
+GCMClientImplCheckinTest::~GCMClientImplCheckinTest() {
+}
+
+void GCMClientImplCheckinTest::SetUp() {
+  testing::Test::SetUp();
+#if defined(OS_MACOSX)
+  base::CommandLine::ForCurrentProcess()->AppendSwitch(
+      os_crypt::switches::kUseMockKeychain);
+#endif  // OS_MACOSX
+  // Creating unique temp directory that will be used by GCMStore shared between
+  // GCM Client and G-services settings.
+  ASSERT_TRUE(CreateUniqueTempDir());
+  InitializeLoop();
+  // Time will be advancing one hour every time it is checked.
+  BuildGCMClient(base::TimeDelta::FromSeconds(kSettingsCheckinInterval));
+  InitializeGCMClient();
+}
+
+TEST_F(GCMClientImplCheckinTest, GServicesSettingsAfterInitialCheckin) {
+  std::map<std::string, std::string> settings;
+  settings["checkin_interval"] = base::Int64ToString(kSettingsCheckinInterval);
+  settings["checkin_url"] = "http://alternative.url/checkin";
+  settings["gcm_hostname"] = "http://alternative.gcm.host";
+  settings["gcm_secure_port"] = "443";
+  settings["gcm_registration_url"] = "http://alternative.url/registration";
+  CompleteCheckin(
+      kDeviceAndroidId, kDeviceSecurityToken, kSettingsDefaultDigest, settings);
+  EXPECT_EQ(kSettingsCheckinInterval, gservices_settings()->checkin_interval());
+}
+
+// This test only checks that periodic checkin happens.
+TEST_F(GCMClientImplCheckinTest, PeriodicCheckin) {
+  std::map<std::string, std::string> settings;
+  settings["checkin_interval"] = base::IntToString(kSettingsCheckinInterval);
+  settings["checkin_url"] = "http://alternative.url/checkin";
+  settings["gcm_hostname"] = "http://alternative.gcm.host";
+  settings["gcm_secure_port"] = "443";
+  settings["gcm_registration_url"] = "http://alternative.url/registration";
+  CompleteCheckin(
+      kDeviceAndroidId, kDeviceSecurityToken, kSettingsDefaultDigest, settings);
+  EXPECT_EQ(2, clock()->call_count());
+
+  PumpLoopUntilIdle();
+  CompleteCheckin(
+      kDeviceAndroidId, kDeviceSecurityToken, kSettingsDefaultDigest, settings);
+}
+
 }  // namespace gcm