components: Introduce AlarmTimer class and use it for GCM heartbeat

google_apis/gcm/engine/heartbeat_manager.h

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #ifndef GOOGLE_APIS_GCM_ENGINE_HEARTBEAT_MANAGER_H_
 #define GOOGLE_APIS_GCM_ENGINE_HEARTBEAT_MANAGER_H_
 
 #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;
 }
 
 namespace gcm {
 
 // A heartbeat management class, capable of sending and handling heartbeat
 // receipt/failures and triggering reconnection as necessary.
 class GCM_EXPORT HeartbeatManager {
  public:
@@ skipping 17 matching lines @@
 
   // Updates the current heartbeat interval.
   void UpdateHeartbeatConfig(const mcs_proto::HeartbeatConfig& config);
 
   // Returns the next scheduled heartbeat time. A null time means
   // no heartbeat is pending. If non-null and less than the
   // current time (in ticks), the heartbeat has been triggered and an ack is
   // 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();
 
  private:
   // 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_;
 
   // The current heartbeat interval.
   int heartbeat_interval_ms_;
   // The most recent server-provided heartbeat interval (0 if none has been
   // provided).
   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_;
   base::Closure trigger_reconnect_callback_;
 
   base::WeakPtrFactory<HeartbeatManager> weak_ptr_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(HeartbeatManager);
 };
 
 }  // namespace gcm
 
 #endif  // GOOGLE_APIS_GCM_ENGINE_HEARTBEAT_MANAGER_H_