components: Introduce AlarmTimer class and use it for GCM heartbeat

google_apis/gcm/engine/heartbeat_manager.h

Index: google_apis/gcm/engine/heartbeat_manager.h
diff --git a/google_apis/gcm/engine/heartbeat_manager.h b/google_apis/gcm/engine/heartbeat_manager.h
index 9266b98c8d4b21f02935002cd106dd314c27e40a..ea19bd14806bb287ba34cc47e24844e87ecb0418 100644
--- a/google_apis/gcm/engine/heartbeat_manager.h
+++ b/google_apis/gcm/engine/heartbeat_manager.h
@@ -7,10 +7,14 @@
 
 #include "base/callback.h"
 #include "base/logging.h"
+#include "base/memory/scoped_ptr.h"
 #include "base/memory/weak_ptr.h"
-#include "base/timer/timer.h"
 #include "google_apis/gcm/base/gcm_export.h"
 
+namespace base {
+class Timer;
+}
+
 namespace mcs_proto {
 class HeartbeatConfig;
 }
@@ -48,6 +52,10 @@ class GCM_EXPORT HeartbeatManager {
   // pending.
   base::TimeTicks GetNextHeartbeatTime() const;
 
+  // Sets the Timer that will be used for heartbeat messages.  Can only be
+  // called before the first time Start() is called.
+  void SetHeartbeatTimer(scoped_ptr<base::Timer> timer);

[Nicolas Zea, 2014/10/22 17:32:16]

Why not just pass it in at construction time?

[Chirantan Ekbote, 2014/10/22 18:47:14]

I originally tried that.  But to pass it in at construction time, we have to
pass it into the MCSClient constructor and then we need to change every location
that constructs one to also pass in a timer (there are a few such locations,
including the constructors for FakeMCSClient and TestMCSClient).  The timers
would also have to be constructed exactly the way the HeartbeatManager expects
so we would have to do

    new base::Timer(true, false)

or

    new timer::AlarmTimer(true, false)

which is really just an implementation detail of HeartbeatManager and it didn't
seem right to have that leaking everywhere.

[Nicolas Zea, 2014/10/23 17:49:02]

IMO its worse to have the possibility of someone changing the heartbeat timer at
runtime (e.g. you had to add that started_ boolean, which I'd be concerned about
not being updated properly). I'd rather keep the actual logic simple, and just
pass it in via the constructor, even if it means modifying all the places it
might be instantiated.

[Chirantan Ekbote, 2014/10/23 23:53:30]

Done.

+
  protected:
   // Helper method to send heartbeat on timer trigger.
   void OnHeartbeatTriggered();
@@ -56,6 +64,9 @@ class GCM_EXPORT HeartbeatManager {
   // Restarts the heartbeat timer.
   void RestartTimer();
 
+  // Whether the HeartbeatManager has started sending heartbeats.
+  bool started_;
+
   // Whether the last heartbeat ping sent has been acknowledged or not.
   bool waiting_for_ack_;
 
@@ -66,7 +77,7 @@ class GCM_EXPORT HeartbeatManager {
   int server_interval_ms_;
 
   // Timer for triggering heartbeats.
-  base::Timer heartbeat_timer_;
+  scoped_ptr<base::Timer> heartbeat_timer_;
 
   // Callbacks for interacting with the the connection.
   base::Closure send_heartbeat_callback_;