[GCM] Wiring heartbeat interval calls to GCMDriver

Wires the call to Add/RemoveHeartbeatInterval through:
* GCMDriver/Desktop/Android/Fake
* GCMDriverDesktop::IOWorker
* GCMClient/Impl to MCSClient
* Adding loading of persisted interval in MCSClient

BUG=481174

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

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

components/gcm_driver/fake_gcm_client.cc

@@ -148,6 +148,13 @@ std::string FakeGCMClient::GetInstanceIDData(const std::string& app_id) {
   return std::string();
 }
 
+void FakeGCMClient::AddHeartbeatInterval(const std::string& scope,
+                                         int interval_ms) {
+}
+
+void FakeGCMClient::RemoveHeartbeatInterval(const std::string& scope) {
+}
+
 void FakeGCMClient::PerformDelayedStart() {
   DCHECK(ui_thread_->RunsTasksOnCurrentThread());
 

components/gcm_driver/fake_gcm_client.h

@@ -60,6 +60,8 @@ class FakeGCMClient : public GCMClient {
                          const std::string& instance_id_data) override;
   void RemoveInstanceIDData(const std::string& app_id) override;
   std::string GetInstanceIDData(const std::string& app_id) override;
+  void AddHeartbeatInterval(const std::string& scope, int interval_ms) override;
+  void RemoveHeartbeatInterval(const std::string& scope) override;
 
   // Initiate the start that has been delayed.
   // Called on UI thread.

components/gcm_driver/fake_gcm_driver.cc

@@ -102,4 +102,11 @@ InstanceIDStore* FakeGCMDriver::GetInstanceIDStore() {
   return NULL;
 }
 
+void FakeGCMDriver::AddHeartbeatInterval(const std::string& scope,
+                                         int interval_ms) {
+}
+
+void FakeGCMDriver::RemoveHeartbeatInterval(const std::string& scope) {
+}
+
 }  // namespace gcm

components/gcm_driver/fake_gcm_driver.h

@@ -42,6 +42,8 @@ class FakeGCMDriver : public GCMDriver {
   void SetLastTokenFetchTime(const base::Time& time) override;
   void WakeFromSuspendForHeartbeat(bool wake) override;
   InstanceIDStore* GetInstanceIDStore() override;
+  void AddHeartbeatInterval(const std::string& scope, int interval_ms) override;
+  void RemoveHeartbeatInterval(const std::string& scope) override;
 
  protected:
   // GCMDriver implementation:

components/gcm_driver/gcm_client.h

@@ -320,6 +320,14 @@ class GCMClient {
   // Retrieves the Instance ID data for a specific app from the persistent
   // store.
   virtual std::string GetInstanceIDData(const std::string& app_id) = 0;
+
+  // Gets and sets custom heartbeat interval for the MCS connection.
+  // |scope| is used to identify the component that requests a custom interval
+  // to be set, and allows that component to later revoke the setting. It should
+  // be unique.
+  virtual void AddHeartbeatInterval(const std::string& scope,
+                                    int interval_ms) = 0;
+  virtual void RemoveHeartbeatInterval(const std::string& scope) = 0;
 };
 
 }  // namespace gcm

components/gcm_driver/gcm_client_impl.cc

@@ -556,6 +556,17 @@ std::string GCMClientImpl::GetInstanceIDData(const std::string& app_id) {
   return iter->second;
 }
 
+void GCMClientImpl::AddHeartbeatInterval(const std::string& scope,
+                                         int interval_ms) {
+  DCHECK(mcs_client_);
+  mcs_client_->AddHeartbeatInterval(scope, interval_ms);
+}
+
+void GCMClientImpl::RemoveHeartbeatInterval(const std::string& scope) {
+  DCHECK(mcs_client_);
+  mcs_client_->RemoveHeartbeatInterval(scope);
+}
+
 void GCMClientImpl::StartCheckin() {
   // Make sure no checkin is in progress.
   if (checkin_request_.get())

components/gcm_driver/gcm_client_impl.h

@@ -130,6 +130,8 @@ class GCMClientImpl
                          const std::string& instance_id_data) override;
   void RemoveInstanceIDData(const std::string& app_id) override;
   std::string GetInstanceIDData(const std::string& app_id) override;
+  void AddHeartbeatInterval(const std::string& scope, int interval_ms) override;
+  void RemoveHeartbeatInterval(const std::string& scope) override;
 
   // GCMStatsRecorder::Delegate implemenation.
   void OnActivityRecorded() override;

components/gcm_driver/gcm_driver.h

@@ -169,6 +169,28 @@ class GCMDriver {
   // Supports saving the Instance ID data in the GCM store.
   virtual InstanceIDStore* GetInstanceIDStore() = 0;
 
+  // Adds or removes a custom client requested heartbeat interval. If multiple
+  // components set that setting, the lowest setting will be used. If the
+  // setting is outside of GetMax/MinClientHeartbeatIntervalMs() it will be
+  // ignored. If a new setting is less than the currently used, the connection
+  // will be reset with the new heartbeat. Client that no longer require
+  // aggressive heartbeats, should remove their requested interval. Heartbeats
+  // set this way survive connection/Chrome restart.
+  //
+  // GCM Driver can decide to postpone the action until Client is properly
+  // initialized, hence this setting can be called at any time.
+  //
+  // Server can overwrite the setting to a different value.
+  //
+  // |scope| is used to identify the component that requests a custom interval
+  // to be set, and allows that component to later revoke the setting.
+  // |interval_ms| should be between 2 minues and 15 minues (28 minues on
+  // cellular networks). For details check
+  // GetMin/MaxClientHeartbeatItnervalMs() in HeartbeatManager.
+  virtual void AddHeartbeatInterval(const std::string& scope,
+                                    int interval_ms) = 0;
+  virtual void RemoveHeartbeatInterval(const std::string& scope) = 0;
+
  protected:
   // Ensures that the GCM service starts (if necessary conditions are met).
   virtual GCMClient::Result EnsureStarted(GCMClient::StartMode start_mode) = 0;

components/gcm_driver/gcm_driver_android.cc

@@ -167,6 +167,13 @@ InstanceIDStore* GCMDriverAndroid::GetInstanceIDStore() {
   return NULL;
 }
 
+void GCMDriverAndroid::AddHeartbeatInterval(const std::string& scope,
+                                            int interval_ms) {
+}
+
+void GCMDriverAndroid::RemoveHeartbeatInterval(const std::string& scope) {
+}
+
 GCMClient::Result GCMDriverAndroid::EnsureStarted(
     GCMClient::StartMode start_mode) {
   // TODO(johnme): Maybe we should check if GMS is available?

components/gcm_driver/gcm_driver_android.h

@@ -65,6 +65,8 @@ class GCMDriverAndroid : public GCMDriver {
   void SetLastTokenFetchTime(const base::Time& time) override;
   void WakeFromSuspendForHeartbeat(bool wake) override;
   InstanceIDStore* GetInstanceIDStore() override;
+  void AddHeartbeatInterval(const std::string& scope, int interval_ms) override;
+  void RemoveHeartbeatInterval(const std::string& scope) override;
 
  protected:
   // GCMDriver implementation:

components/gcm_driver/gcm_driver_desktop.cc

@@ -91,6 +91,8 @@ class GCMDriverDesktop::IOWorker : public GCMClient::Delegate {
                          const std::string& instance_id_data);
   void RemoveInstanceIDData(const std::string& app_id);
   void GetInstanceIDData(const std::string& app_id);
+  void AddHeartbeatInterval(const std::string& scope, int interval_ms);
+  void RemoveHeartbeatInterval(const std::string& scope);
 
   // For testing purpose. Can be called from UI thread. Use with care.
   GCMClient* gcm_client_for_testing() const { return gcm_client_.get(); }
@@ -392,6 +394,18 @@ void GCMDriverDesktop::IOWorker::WakeFromSuspendForHeartbeat(bool wake) {
 #endif
 }
 
+void GCMDriverDesktop::IOWorker::AddHeartbeatInterval(const std::string& scope,
+                                                      int interval_ms) {
+  DCHECK(io_thread_->RunsTasksOnCurrentThread());
+  gcm_client_->AddHeartbeatInterval(scope, interval_ms);
+}
+
+void GCMDriverDesktop::IOWorker::RemoveHeartbeatInterval(
+    const std::string& scope) {
+  DCHECK(io_thread_->RunsTasksOnCurrentThread());
+  gcm_client_->RemoveHeartbeatInterval(scope);
+}
+
 GCMDriverDesktop::GCMDriverDesktop(
     scoped_ptr<GCMClientFactory> gcm_client_factory,
     const GCMClient::ChromeBuildInfo& chrome_build_info,
@@ -768,6 +782,49 @@ void GCMDriverDesktop::WakeFromSuspendForHeartbeat(bool wake) {
                  wake_from_suspend_enabled_));
 }
 
+void GCMDriverDesktop::AddHeartbeatInterval(const std::string& scope,
+                                            int interval_ms) {
+  DCHECK(ui_thread_->RunsTasksOnCurrentThread());
+
+  // The GCM service has not been initialized.
+  if (!delayed_task_controller_)
+    return;
+
+  if (!delayed_task_controller_->CanRunTaskWithoutDelay()) {
+    // The GCM service was initialized but has not started yet.
+    delayed_task_controller_->AddTask(
+        base::Bind(&GCMDriverDesktop::AddHeartbeatInterval,
+                   weak_ptr_factory_.GetWeakPtr(), scope, interval_ms));
+    return;
+  }
+
+  io_thread_->PostTask(
+      FROM_HERE,
+      base::Bind(&GCMDriverDesktop::IOWorker::AddHeartbeatInterval,
+                 base::Unretained(io_worker_.get()), scope, interval_ms));
+}
+
+void GCMDriverDesktop::RemoveHeartbeatInterval(const std::string& scope) {
+  DCHECK(ui_thread_->RunsTasksOnCurrentThread());
+
+  // The GCM service has not been initialized.
+  if (!delayed_task_controller_)
+    return;
+
+  if (!delayed_task_controller_->CanRunTaskWithoutDelay()) {
+    // The GCM service was initialized but has not started yet.
+    delayed_task_controller_->AddTask(
+        base::Bind(&GCMDriverDesktop::RemoveHeartbeatInterval,
+                   weak_ptr_factory_.GetWeakPtr(), scope));
+    return;
+  }
+
+  io_thread_->PostTask(
+      FROM_HERE,
+      base::Bind(&GCMDriverDesktop::IOWorker::RemoveHeartbeatInterval,
+                 base::Unretained(io_worker_.get()), scope));
+}
+
 void GCMDriverDesktop::SetAccountTokens(
     const std::vector<GCMClient::AccountTokenInfo>& account_tokens) {
   DCHECK(ui_thread_->RunsTasksOnCurrentThread());

components/gcm_driver/gcm_driver_desktop.h

@@ -85,6 +85,8 @@ class GCMDriverDesktop : public GCMDriver,
   void SetLastTokenFetchTime(const base::Time& time) override;
   void WakeFromSuspendForHeartbeat(bool wake) override;
   InstanceIDStore* GetInstanceIDStore() override;
+  void AddHeartbeatInterval(const std::string& scope, int interval_ms) override;
+  void RemoveHeartbeatInterval(const std::string& scope) override;
 
   // InstanceIDStore overrides:
   void AddInstanceIDData(const std::string& app_id,

google_apis/gcm/engine/gcm_store.cc

@@ -26,6 +26,7 @@ void GCMStore::LoadResult::Reset() {
   last_token_fetch_time = base::Time::FromInternalValue(0LL);
   last_checkin_accounts.clear();
   account_mappings.clear();
+  heartbeat_intervals.clear();
   success = false;
   instance_id_data.clear();
 }

google_apis/gcm/engine/mcs_client.cc

@@ -276,6 +276,12 @@ void MCSClient::Initialize(
         collapse_key_map_[collapse_key] = packet_info;
     }
   }
+
+  // Establish if there is any custom client interval persisted from the last
+  // run and set it on the heartbeat manager.
+  custom_heartbeat_intervals_.swap(load_result->heartbeat_intervals);
+  int min_interval_ms = GetMinHeartbeatIntervalMs();
+  heartbeat_manager_.SetClientHeartbeatIntervalMs(min_interval_ms);
 }
 
 void MCSClient::Login(uint64 android_id, uint64 security_token) {
@@ -380,19 +386,25 @@ void MCSClient::AddHeartbeatInterval(const std::string& scope,
     return;
 
   custom_heartbeat_intervals_[scope] = interval_ms;
-  // TODO(fgorski): Save in the gcm store as well.
+  gcm_store_->AddHeartbeatInterval(scope, interval_ms,
+                                   base::Bind(&MCSClient::OnGCMUpdateFinished,
+                                              weak_ptr_factory_.GetWeakPtr()));
 
-  int min_interval_ms = GetMinCustomHeartbeatInterval();
+  int min_interval_ms = GetMinHeartbeatIntervalMs();
   heartbeat_manager_.SetClientHeartbeatIntervalMs(min_interval_ms);
 }
 
 void MCSClient::RemoveHeartbeatInterval(const std::string& scope) {
   custom_heartbeat_intervals_.erase(scope);
-  int min_interval = GetMinCustomHeartbeatInterval();
+  gcm_store_->RemoveHeartbeatInterval(
+      scope, base::Bind(&MCSClient::OnGCMUpdateFinished,
+                        weak_ptr_factory_.GetWeakPtr()));
+
+  int min_interval = GetMinHeartbeatIntervalMs();
   heartbeat_manager_.SetClientHeartbeatIntervalMs(min_interval);
 }
 
-int MCSClient::GetMinCustomHeartbeatInterval() {
+int MCSClient::GetMinHeartbeatIntervalMs() {
   if (custom_heartbeat_intervals_.empty())
     return kNoCustomHeartbeat;
 

google_apis/gcm/engine/mcs_client.h

@@ -160,8 +160,8 @@ class GCM_EXPORT MCSClient {
   void AddHeartbeatInterval(const std::string& scope, int interval_ms);
   void RemoveHeartbeatInterval(const std::string& scope);
 
-  HeartbeatManager& GetHeartbeatManagerForTesting() {
-    return heartbeat_manager_;
+  HeartbeatManager* GetHeartbeatManagerForTesting() {
+    return &heartbeat_manager_;
   }
 
  private:
@@ -225,8 +225,9 @@ class GCM_EXPORT MCSClient {
   // any associated state).
   MCSPacketInternal PopMessageForSend();
 
-  // Gets the minimum interval from the map of scopes to intervals.
-  int GetMinCustomHeartbeatInterval();
+  // Gets the minimum interval from the map of scopes to intervals in
+  // milliseconds.
+  int GetMinHeartbeatIntervalMs();
 
   // Local version string. Sent on login.
   const std::string version_string_;

google_apis/gcm/engine/mcs_client_unittest.cc

@@ -113,8 +113,11 @@ class MCSClientTest : public testing::Test {
   void InitializeClient();
   void StoreCredentials();
   void LoginClient(const std::vector<std::string>& acknowledged_ids);
+  void LoginClientWithHeartbeat(
+      const std::vector<std::string>& acknowledged_ids,
+      int heartbeat_interval_ms);
   void AddExpectedLoginRequest(const std::vector<std::string>& acknowledged_ids,
-                               int custom_heartbeat_interval);
+                               int heartbeat_interval_ms);
 
   base::SimpleTestClock* clock() { return &clock_; }
   TestMCSClient* mcs_client() const { return mcs_client_.get(); }
@@ -209,7 +212,13 @@ void MCSClientTest::InitializeClient() {
 
 void MCSClientTest::LoginClient(
     const std::vector<std::string>& acknowledged_ids) {
-  AddExpectedLoginRequest(acknowledged_ids, 0);
+  LoginClientWithHeartbeat(acknowledged_ids, 0);
+}
+
+void MCSClientTest::LoginClientWithHeartbeat(
+    const std::vector<std::string>& acknowledged_ids,
+    int heartbeat_interval_ms) {
+  AddExpectedLoginRequest(acknowledged_ids, heartbeat_interval_ms);
   mcs_client_->Login(kAndroidId, kSecurityToken);
   run_loop_->Run();
   run_loop_.reset(new base::RunLoop());
@@ -217,15 +226,15 @@ void MCSClientTest::LoginClient(
 
 void MCSClientTest::AddExpectedLoginRequest(
     const std::vector<std::string>& acknowledged_ids,
-    int custom_heartbeat_interval) {
+    int heartbeat_interval_ms) {
   scoped_ptr<mcs_proto::LoginRequest> login_request =
       BuildLoginRequest(kAndroidId, kSecurityToken, "");
   for (size_t i = 0; i < acknowledged_ids.size(); ++i)
     login_request->add_received_persistent_id(acknowledged_ids[i]);
-  if (custom_heartbeat_interval) {
+  if (heartbeat_interval_ms) {
     mcs_proto::Setting* setting = login_request->add_setting();
     setting->set_name("hbping");
-    setting->set_value(base::IntToString(custom_heartbeat_interval));
+    setting->set_value(base::IntToString(heartbeat_interval_ms));
   }
   GetFakeHandler()->ExpectOutgoingMessage(
       MCSMessage(kLoginRequestTag, login_request.Pass()));
@@ -898,97 +907,272 @@ TEST_F(MCSClientTest, CollapseKeysDifferentUser) {
   PumpLoop();
 }
 
-// Tests adding and removing custom heartbeat interval.
-TEST_F(MCSClientTest, CustomHeartbeatInterval) {
+// Test case for setting a custom heartbeat interval, when it is too short.
+// Covers both connection restart and storing of custom intervals.
+TEST_F(MCSClientTest, CustomHeartbeatIntervalTooShort) {
   BuildMCSClient();
   InitializeClient();
   LoginClient(std::vector<std::string>());
+  PumpLoop();
+  StoreCredentials();
 
-  TestConnectionListener test_connection_listener;
-  connection_factory()->SetConnectionListener(&test_connection_listener);
-
-  HeartbeatManager& hb_manager = mcs_client()->GetHeartbeatManagerForTesting();
+  HeartbeatManager* hb_manager = mcs_client()->GetHeartbeatManagerForTesting();
   // By default custom client interval is not set.
-  EXPECT_FALSE(hb_manager.HasClientHeartbeatInterval());
+  EXPECT_FALSE(hb_manager->HasClientHeartbeatInterval());
 
   const std::string component_1 = "component1";
   int interval_ms = 30 * 1000;  // 30 seconds, too low.
   mcs_client()->AddHeartbeatInterval(component_1, interval_ms);
   // Setting was too low so it was ignored.
-  EXPECT_FALSE(hb_manager.HasClientHeartbeatInterval());
+  EXPECT_FALSE(hb_manager->HasClientHeartbeatInterval());
+
+  // Restore and check again to make sure that nothing was set in store.
+  BuildMCSClient();
+  InitializeClient();
+  LoginClientWithHeartbeat(std::vector<std::string>(), 0);
+  PumpLoop();
+
+  hb_manager = mcs_client()->GetHeartbeatManagerForTesting();
+  EXPECT_FALSE(hb_manager->HasClientHeartbeatInterval());
+}
 
-  interval_ms = 60 * 60 * 1000;  // 1 hour, too high.
+// Test case for setting a custom heartbeat interval, when it is too long.
+// Covers both connection restart and storing of custom intervals.
+TEST_F(MCSClientTest, CustomHeartbeatIntervalTooLong) {
+  BuildMCSClient();
+  InitializeClient();
+  LoginClient(std::vector<std::string>());
+  PumpLoop();
+  StoreCredentials();
+
+  HeartbeatManager* hb_manager = mcs_client()->GetHeartbeatManagerForTesting();
+
+  const std::string component_1 = "component1";
+  int interval_ms = 60 * 60 * 1000;  // 1 hour, too high.
   mcs_client()->AddHeartbeatInterval(component_1, interval_ms);
   // Setting was too high, again it was ignored.
-  EXPECT_FALSE(hb_manager.HasClientHeartbeatInterval());
+  EXPECT_FALSE(hb_manager->HasClientHeartbeatInterval());
+
+  // Restore and check again to make sure that nothing was set in store.
+  BuildMCSClient();
+  InitializeClient();
+  LoginClientWithHeartbeat(std::vector<std::string>(), 0);
+  PumpLoop();
+
+  // Setting was too high, again it was ignored.
+  hb_manager = mcs_client()->GetHeartbeatManagerForTesting();
+  EXPECT_FALSE(hb_manager->HasClientHeartbeatInterval());
+}
 
-  int expected_disconnect_counter = 0;
-  EXPECT_EQ(expected_disconnect_counter,
-            test_connection_listener.get_disconnect_counter());
+// Tests adding and removing custom heartbeat interval.
+// Covers both connection restart and storing of custom intervals.
+TEST_F(MCSClientTest, CustomHeartbeatIntervalSingleInterval) {
+  BuildMCSClient();
+  InitializeClient();
+  LoginClient(std::vector<std::string>());
+  PumpLoop();
+  StoreCredentials();
+
+  TestConnectionListener test_connection_listener;
+  connection_factory()->SetConnectionListener(&test_connection_listener);
+
+  HeartbeatManager* hb_manager = mcs_client()->GetHeartbeatManagerForTesting();
+
+  const std::string component_1 = "component1";
+  int interval_ms = 5 * 60 * 1000;  // 5 minutes. A valid setting.
 
-  interval_ms = 5 * 60 * 1000;  // 5 minutes. A valid setting.
   AddExpectedLoginRequest(std::vector<std::string>(), interval_ms);
   mcs_client()->AddHeartbeatInterval(component_1, interval_ms);
-  // Setting was OK. HearbeatManager should get that setting now.
-  EXPECT_TRUE(hb_manager.HasClientHeartbeatInterval());
-  EXPECT_EQ(interval_ms, hb_manager.GetClientHeartbeatIntervalMs());
+  PumpLoop();
+
+  // Interval was OK. HearbeatManager should get that setting now.
+  EXPECT_TRUE(hb_manager->HasClientHeartbeatInterval());
+  EXPECT_EQ(interval_ms, hb_manager->GetClientHeartbeatIntervalMs());
+  EXPECT_EQ(1, test_connection_listener.get_disconnect_counter());
+
+  // Check that setting was persisted and will take effect upon restart.
+  BuildMCSClient();
+  InitializeClient();
+  LoginClientWithHeartbeat(std::vector<std::string>(), interval_ms);
+  PumpLoop();
+
+  // HB manger uses the shortest persisted interval after restart.
+  hb_manager = mcs_client()->GetHeartbeatManagerForTesting();
+  EXPECT_TRUE(hb_manager->HasClientHeartbeatInterval());
+  EXPECT_EQ(interval_ms, hb_manager->GetClientHeartbeatIntervalMs());
+
+  mcs_client()->RemoveHeartbeatInterval(component_1);
+  PumpLoop();
+
+  // Check that setting was persisted and will take effect upon restart.
+  BuildMCSClient();
+  InitializeClient();
+  LoginClientWithHeartbeat(std::vector<std::string>(), 0);
+  PumpLoop();
+
+  hb_manager = mcs_client()->GetHeartbeatManagerForTesting();
+  EXPECT_FALSE(hb_manager->HasClientHeartbeatInterval());
+}
+
+// Tests adding custom heartbeat interval before connection is initialized.
+TEST_F(MCSClientTest, CustomHeartbeatIntervalSetBeforeInitialize) {
+  BuildMCSClient();
+
+  const std::string component_1 = "component1";
+  int interval_ms = 5 * 60 * 1000;  // 5 minutes. A valid setting.
+  mcs_client()->AddHeartbeatInterval(component_1, interval_ms);
+  InitializeClient();
+  LoginClientWithHeartbeat(std::vector<std::string>(), interval_ms);
+  HeartbeatManager* hb_manager = mcs_client()->GetHeartbeatManagerForTesting();
+  EXPECT_TRUE(hb_manager->HasClientHeartbeatInterval());
+}
+
+// Tests adding custom heartbeat interval after connection is initialized, but
+// but before login is sent.
+TEST_F(MCSClientTest, CustomHeartbeatIntervalSetBeforeLogin) {
+  BuildMCSClient();
+
+  const std::string component_1 = "component1";
+  int interval_ms = 5 * 60 * 1000;  // 5 minutes. A valid setting.
+  InitializeClient();
+  mcs_client()->AddHeartbeatInterval(component_1, interval_ms);
+  LoginClientWithHeartbeat(std::vector<std::string>(), interval_ms);
+  HeartbeatManager* hb_manager = mcs_client()->GetHeartbeatManagerForTesting();
+  EXPECT_TRUE(hb_manager->HasClientHeartbeatInterval());
+}
+
+// Tests situation when two heartbeat intervals are set and second is longer.
+// Covers both connection restart and storing of custom intervals.
+TEST_F(MCSClientTest, CustomHeartbeatIntervalSecondIntervalLonger) {
+  BuildMCSClient();
+  InitializeClient();
+  LoginClient(std::vector<std::string>());
+  PumpLoop();
+  StoreCredentials();
+
+  TestConnectionListener test_connection_listener;
+  connection_factory()->SetConnectionListener(&test_connection_listener);
+
+  HeartbeatManager* hb_manager = mcs_client()->GetHeartbeatManagerForTesting();
 
-  ++expected_disconnect_counter;
-  EXPECT_EQ(expected_disconnect_counter,
-            test_connection_listener.get_disconnect_counter());
+  const std::string component_1 = "component1";
+  int interval_ms = 5 * 60 * 1000;  // 5 minutes. A valid setting.
+
+  AddExpectedLoginRequest(std::vector<std::string>(), interval_ms);
+  mcs_client()->AddHeartbeatInterval(component_1, interval_ms);
+  PumpLoop();
 
   const std::string component_2 = "component2";
   int other_interval_ms = 10 * 60 * 1000;  // 10 minutes. A valid setting.
   mcs_client()->AddHeartbeatInterval(component_2, other_interval_ms);
-  // Setting was OK, but higher than the previous setting and HearbeatManager
-  // will not be updated.
-  EXPECT_TRUE(hb_manager.HasClientHeartbeatInterval());
-  EXPECT_EQ(interval_ms, hb_manager.GetClientHeartbeatIntervalMs());
-  // No connection reset expected.
-  EXPECT_EQ(expected_disconnect_counter,
-            test_connection_listener.get_disconnect_counter());
+  PumpLoop();
+
+  // Interval was OK, but longer. HearbeatManager will use the first one.
+  EXPECT_TRUE(hb_manager->HasClientHeartbeatInterval());
+  EXPECT_EQ(interval_ms, hb_manager->GetClientHeartbeatIntervalMs());
+  EXPECT_EQ(1, test_connection_listener.get_disconnect_counter());
+
+  // Check that setting was persisted and will take effect upon restart.
+  BuildMCSClient();
+  InitializeClient();
+  LoginClientWithHeartbeat(std::vector<std::string>(), interval_ms);
+  PumpLoop();
 
-  other_interval_ms = 3 * 60 * 1000;  // 3 minutes. A valid setting.
+  // HB manger uses the shortest persisted interval after restart.
+  hb_manager = mcs_client()->GetHeartbeatManagerForTesting();
+  EXPECT_TRUE(hb_manager->HasClientHeartbeatInterval());
+  EXPECT_EQ(interval_ms, hb_manager->GetClientHeartbeatIntervalMs());
+}
+
+// Tests situation when two heartbeat intervals are set and second is shorter.
+// Covers both connection restart and storing of custom intervals.
+TEST_F(MCSClientTest, CustomHeartbeatIntervalSecondIntervalShorter) {
+  BuildMCSClient();
+  InitializeClient();
+  LoginClient(std::vector<std::string>());
+  PumpLoop();
+  StoreCredentials();
+
+  TestConnectionListener test_connection_listener;
+  connection_factory()->SetConnectionListener(&test_connection_listener);
+
+  HeartbeatManager* hb_manager = mcs_client()->GetHeartbeatManagerForTesting();
+
+  const std::string component_1 = "component1";
+  int interval_ms = 5 * 60 * 1000;  // 5 minutes. A valid setting.
+
+  AddExpectedLoginRequest(std::vector<std::string>(), interval_ms);
+  mcs_client()->AddHeartbeatInterval(component_1, interval_ms);
+  PumpLoop();
+
+  const std::string component_2 = "component2";
+  int other_interval_ms = 3 * 60 * 1000;  // 3 minutes. A valid setting.
   AddExpectedLoginRequest(std::vector<std::string>(), other_interval_ms);
   mcs_client()->AddHeartbeatInterval(component_2, other_interval_ms);
-  // Setting was OK and lower then present setting. HearbeatManager should get
-  // that setting now.
-  EXPECT_TRUE(hb_manager.HasClientHeartbeatInterval());
-  EXPECT_EQ(other_interval_ms, hb_manager.GetClientHeartbeatIntervalMs());
-  ++expected_disconnect_counter;
-  EXPECT_EQ(expected_disconnect_counter,
-            test_connection_listener.get_disconnect_counter());
+  PumpLoop();
+  // Interval was OK. HearbeatManager should get that setting now.
+  EXPECT_TRUE(hb_manager->HasClientHeartbeatInterval());
+  EXPECT_EQ(other_interval_ms, hb_manager->GetClientHeartbeatIntervalMs());
+  EXPECT_EQ(2, test_connection_listener.get_disconnect_counter());
 
-  mcs_client()->RemoveHeartbeatInterval(component_2);
-  // Removing the lowest setting reverts to second lowest.
-  EXPECT_TRUE(hb_manager.HasClientHeartbeatInterval());
-  EXPECT_EQ(interval_ms, hb_manager.GetClientHeartbeatIntervalMs());
-  // No connection reset expected.
-  EXPECT_EQ(expected_disconnect_counter,
-            test_connection_listener.get_disconnect_counter());
+  // Check that setting was persisted and will take effect upon restart.
+  BuildMCSClient();
+  InitializeClient();
+  LoginClientWithHeartbeat(std::vector<std::string>(), other_interval_ms);
+  PumpLoop();
 
-  mcs_client()->RemoveHeartbeatInterval(component_1);
-  // Removing all of the intervals, removes it from the HeartbeatManager.
-  EXPECT_FALSE(hb_manager.HasClientHeartbeatInterval());
-  // No connection reset expected.
-  EXPECT_EQ(expected_disconnect_counter,
-            test_connection_listener.get_disconnect_counter());
+  // HB manger uses the shortest persisted interval after restart.
+  hb_manager = mcs_client()->GetHeartbeatManagerForTesting();
+  EXPECT_TRUE(hb_manager->HasClientHeartbeatInterval());
+  EXPECT_EQ(other_interval_ms, hb_manager->GetClientHeartbeatIntervalMs());
+}
 
-  mcs_client()->AddHeartbeatInterval(component_2, other_interval_ms);
+// Tests situation shorter of two intervals is removed.
+// Covers both connection restart and storing of custom intervals.
+TEST_F(MCSClientTest, CustomHeartbeatIntervalRemoveShorterInterval) {
+  BuildMCSClient();
+  InitializeClient();
+  LoginClient(std::vector<std::string>());
+  PumpLoop();
+  StoreCredentials();
+
+  TestConnectionListener test_connection_listener;
+  connection_factory()->SetConnectionListener(&test_connection_listener);
+
+  HeartbeatManager* hb_manager = mcs_client()->GetHeartbeatManagerForTesting();
+
+  const std::string component_1 = "component1";
+  int interval_ms = 5 * 60 * 1000;  // 5 minutes. A valid setting.
+
+  AddExpectedLoginRequest(std::vector<std::string>(), interval_ms);
   mcs_client()->AddHeartbeatInterval(component_1, interval_ms);
-  EXPECT_TRUE(hb_manager.HasClientHeartbeatInterval());
-  EXPECT_EQ(other_interval_ms, hb_manager.GetClientHeartbeatIntervalMs());
-  // No connection reset expected.
-  EXPECT_EQ(expected_disconnect_counter,
-            test_connection_listener.get_disconnect_counter());
+  PumpLoop();
 
-  // Removing interval other than lowest does not change anything.
-  mcs_client()->RemoveHeartbeatInterval(component_1);
-  EXPECT_TRUE(hb_manager.HasClientHeartbeatInterval());
-  EXPECT_EQ(other_interval_ms, hb_manager.GetClientHeartbeatIntervalMs());
+  const std::string component_2 = "component2";
+  int other_interval_ms = 3 * 60 * 1000;  // 3 minutes. A valid setting.
+  AddExpectedLoginRequest(std::vector<std::string>(), other_interval_ms);
+  mcs_client()->AddHeartbeatInterval(component_2, other_interval_ms);
+  PumpLoop();
+
+  mcs_client()->RemoveHeartbeatInterval(component_2);
+  PumpLoop();
+
+  // Removing the lowest setting reverts to second lowest.
+  EXPECT_TRUE(hb_manager->HasClientHeartbeatInterval());
+  EXPECT_EQ(interval_ms, hb_manager->GetClientHeartbeatIntervalMs());
   // No connection reset expected.
-  EXPECT_EQ(expected_disconnect_counter,
-            test_connection_listener.get_disconnect_counter());
+  EXPECT_EQ(2, test_connection_listener.get_disconnect_counter());
+
+  // Check that setting was persisted and will take effect upon restart.
+  BuildMCSClient();
+  InitializeClient();
+  LoginClientWithHeartbeat(std::vector<std::string>(), interval_ms);
+  PumpLoop();
+
+  // HB manger uses the shortest persisted interval after restart.
+  hb_manager = mcs_client()->GetHeartbeatManagerForTesting();
+  EXPECT_TRUE(hb_manager->HasClientHeartbeatInterval());
+  EXPECT_EQ(interval_ms, hb_manager->GetClientHeartbeatIntervalMs());
 }
 
 } // namespace