Revert 265792 "Updates to GCMClientImpl to use and update gservi..."

> Updates to GCMClientImpl to use and update gservices settings.
> Triggering periodic checkin based on checkin interval.
> Adding ability to skip a periodic checkin when it would be too early.
> 
> BUG=359254
> 
> Review URL: https://codereview.chromium.org/243813003

TBR=fgorski@chromium.org

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

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

3 files changed, 44 insertions, 213 deletions

diff --git a/google_apis/gcm/gcm_client_impl.cc b/google_apis/gcm/gcm_client_impl.cc
index 5deeb96..e152d63 100644
--- a/google_apis/gcm/gcm_client_impl.cc
+++ b/google_apis/gcm/gcm_client_impl.cc
@@ -19,7 +19,6 @@
 #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"
@@ -78,6 +77,7 @@ 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";
@@ -162,7 +162,6 @@ 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) {
 }
 
@@ -192,7 +191,6 @@ 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,12 +217,11 @@ 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;
-  last_checkin_time_ = result->last_checkin_time;
-  gservices_settings_->UpdateFromLoadResult(*result);
+  base::Time last_checkin_time = result->last_checkin_time;
   InitializeMCSClient(result.Pass());
 
   if (device_checkin_info_.IsValid()) {
-    SchedulePeriodicCheckin();
+    SchedulePeriodicCheckin(last_checkin_time);
     OnReady();
     return;
   }
@@ -294,15 +291,12 @@ void GCMClientImpl::ResetState() {
 }
 
 void GCMClientImpl::StartCheckin() {
-  // 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_);
+  CheckinRequest::RequestInfo request_info(
+    device_checkin_info_.android_id,
+    device_checkin_info_.secret,
+    std::string(),
+    account_ids_,
+    chrome_build_proto_);
   checkin_request_.reset(
       new CheckinRequest(request_info,
                          kDefaultBackoffPolicy,
@@ -338,43 +332,30 @@ void GCMClientImpl::OnCheckinCompleted(
   }
 
   if (device_checkin_info_.IsValid()) {
-    // First update G-services settings, as something might have changed.
-    gservices_settings_->UpdateFromCheckinResponse(checkin_response);
-    last_checkin_time_ = clock_->Now();
+    base::Time last_checkin_time = clock_->Now();
     gcm_store_->SetLastCheckinTime(
-        last_checkin_time_,
+        last_checkin_time,
         base::Bind(&GCMClientImpl::SetLastCheckinTimeCallback,
                    weak_ptr_factory_.GetWeakPtr()));
-    SchedulePeriodicCheckin();
+    SchedulePeriodicCheckin(last_checkin_time);
   }
 }
 
-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();
-
+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.
   base::MessageLoop::current()->PostDelayedTask(
       FROM_HERE,
       base::Bind(&GCMClientImpl::StartCheckin,
-                 periodic_checkin_ptr_factory_.GetWeakPtr()),
+                 weak_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 8df1ada..c4e6068 100644
--- a/google_apis/gcm/gcm_client_impl.h
+++ b/google_apis/gcm/gcm_client_impl.h
@@ -29,7 +29,6 @@ class GURL;
 
 namespace base {
 class Clock;
-class Time;
 }  // namespace base
 
 namespace mcs_proto {
@@ -45,7 +44,6 @@ namespace gcm {
 class CheckinRequest;
 class ConnectionFactory;
 class GCMClientImplTest;
-class GServicesSettings;
 
 // Helper class for building GCM internals. Allows tests to inject fake versions
 // as necessary.
@@ -178,12 +176,9 @@ class GCM_EXPORT GCMClientImpl : public GCMClient {
   // Function also cleans up the pending checkin.
   void OnCheckinCompleted(
       const checkin_proto::AndroidCheckinResponse& checkin_response);
-  // 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;
+  // Schedules next device checkin, based on |last_checkin_time| and
+  // checkin_interval specified in GServices settings.
+  void SchedulePeriodicCheckin(const base::Time& last_checkin_time);
   // Callback for setting last checkin time in the |gcm_store_|.
   void SetLastCheckinTimeCallback(bool success);
 
@@ -273,15 +268,6 @@ 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 948e725..7f6a937 100644
--- a/google_apis/gcm/gcm_client_impl_unittest.cc
+++ b/google_apis/gcm/gcm_client_impl_unittest.cc
@@ -8,14 +8,12 @@
 #include "base/files/scoped_temp_dir.h"
 #include "base/message_loop/message_loop.h"
 #include "base/run_loop.h"
-#include "base/strings/string_number_conversions.h"
-#include "base/time/clock.h"
+#include "base/test/simple_test_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"
@@ -41,8 +39,6 @@ enum LastEvent {
 
 const uint64 kDeviceAndroidId = 54321;
 const uint64 kDeviceSecurityToken = 12345;
-const int64 kSettingsCheckinInterval = 3600;
-const char kSettingsDefaultDigest[] = "default_digest";
 const char kAppId[] = "app_id";
 const char kSender[] = "project_id";
 const char kSender2[] = "project_id2";
@@ -119,44 +115,9 @@ 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(base::TimeDelta clock_step);
+  FakeGCMInternalsBuilder();
   virtual ~FakeGCMInternalsBuilder();
 
   virtual scoped_ptr<base::Clock> BuildClock() OVERRIDE;
@@ -171,19 +132,14 @@ class FakeGCMInternalsBuilder : public GCMInternalsBuilder {
       const net::BackoffEntry::Policy& backoff_policy,
       scoped_refptr<net::HttpNetworkSession> network_session,
       net::NetLog* net_log) OVERRIDE;
-
- private:
-  base::TimeDelta clock_step_;
 };
 
-FakeGCMInternalsBuilder::FakeGCMInternalsBuilder(base::TimeDelta clock_step)
-    : clock_step_(clock_step) {
-}
+FakeGCMInternalsBuilder::FakeGCMInternalsBuilder() {}
 
 FakeGCMInternalsBuilder::~FakeGCMInternalsBuilder() {}
 
 scoped_ptr<base::Clock> FakeGCMInternalsBuilder::BuildClock() {
-  return make_scoped_ptr<base::Clock>(new AutoAdvancingTestClock(clock_step_));
+  return make_scoped_ptr<base::Clock>(new base::SimpleTestClock());
 }
 
 scoped_ptr<MCSClient> FakeGCMInternalsBuilder::BuildMCSClient(
@@ -216,13 +172,10 @@ class GCMClientImplTest : public testing::Test,
 
   virtual void SetUp() OVERRIDE;
 
-  void BuildGCMClient(base::TimeDelta clock_step);
+  void BuildGCMClient();
   void InitializeGCMClient();
   void ReceiveMessageFromMCS(const MCSMessage& message);
-  void CompleteCheckin(uint64 android_id,
-                       uint64 security_token,
-                       const std::string& digest,
-                       const std::map<std::string, std::string>& settings);
+  void CompleteCheckin(uint64 android_id, uint64 security_token);
   void CompleteRegistration(const std::string& registration_id);
   void CompleteUnregistration(const std::string& app_id);
 
@@ -279,23 +232,18 @@ 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();
-  void InitializeLoop();
-  bool CreateUniqueTempDir();
-  AutoAdvancingTestClock* clock() const {
-    return reinterpret_cast<AutoAdvancingTestClock*>(gcm_client_->clock_.get());
+
+  base::SimpleTestClock* clock() const {
+    return reinterpret_cast<base::SimpleTestClock*>(gcm_client_->clock_.get());
   }
 
- private:
   // Variables used for verification.
   LastEvent last_event_;
   std::string last_app_id_;
@@ -331,14 +279,11 @@ void GCMClientImplTest::SetUp() {
   base::CommandLine::ForCurrentProcess()->AppendSwitch(
       os_crypt::switches::kUseMockKeychain);
 #endif  // OS_MACOSX
-  ASSERT_TRUE(CreateUniqueTempDir());
-  InitializeLoop();
-  BuildGCMClient(base::TimeDelta());
+  ASSERT_TRUE(temp_directory_.CreateUniqueTempDir());
+  run_loop_.reset(new base::RunLoop);
+  BuildGCMClient();
   InitializeGCMClient();
-  CompleteCheckin(kDeviceAndroidId,
-                  kDeviceSecurityToken,
-                  std::string(),
-                  std::map<std::string, std::string>());
+  CompleteCheckin(kDeviceAndroidId, kDeviceSecurityToken);
 }
 
 void GCMClientImplTest::PumpLoop() {
@@ -356,42 +301,18 @@ void GCMClientImplTest::QuitLoop() {
     run_loop_->Quit();
 }
 
-void GCMClientImplTest::InitializeLoop() {
-  run_loop_.reset(new base::RunLoop);
+void GCMClientImplTest::BuildGCMClient() {
+  gcm_client_.reset(new GCMClientImpl(
+      make_scoped_ptr<GCMInternalsBuilder>(new FakeGCMInternalsBuilder())));
 }
 
-bool GCMClientImplTest::CreateUniqueTempDir() {
-  return temp_directory_.CreateUniqueTempDir();
-}
-
-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) {
+void GCMClientImplTest::CompleteCheckin(uint64 android_id,
+                                        uint64 security_token) {
   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);
 
@@ -554,7 +475,7 @@ TEST_F(GCMClientImplTest, DISABLED_RegisterAppFromCache) {
   EXPECT_EQ(REGISTRATION_COMPLETED, last_event());
 
   // Recreate GCMClient in order to load from the persistent store.
-  BuildGCMClient(base::TimeDelta());
+  BuildGCMClient();
   InitializeGCMClient();
 
   EXPECT_TRUE(ExistsRegistration(kAppId));
@@ -681,61 +602,4 @@ 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(3600LL));
-  InitializeGCMClient();
-}
-
-TEST_F(GCMClientImplCheckinTest, GServicesSettingsAfterInitialCheckin) {
-  std::map<std::string, std::string> settings;
-  settings["checkin_interval"] = base::Int64ToString(kSettingsCheckinInterval);
-  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;
-  // Interval is smaller than the clock step.
-  settings["checkin_interval"] = "1800";
-  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