Reporting of policy errors via host-offline-reason: part 2b

remoting/host/heartbeat_sender.h

 // Copyright (c) 2012 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 REMOTING_HOST_HEARTBEAT_SENDER_H_
 #define REMOTING_HOST_HEARTBEAT_SENDER_H_
 
 #include <string>
 
+#include "base/callback.h"

[Lambros, 2014/11/18 02:33:52]

callback_forward.h :)

[Łukasz Anforowicz, 2014/11/18 17:30:45]

What about the base::Closure by-value / not-a-pointer-or-reference fields being
added below?  (I wish Rietveld would show line numbers so I can more easily
refer to other parts of the code being reviewed :-/).

[Lambros, 2014/11/18 18:47:21]

Yes you're right, didn't see those. I only scanned for "Callback" :)

 #include "base/compiler_specific.h"
 #include "base/gtest_prod_util.h"
 #include "base/memory/ref_counted.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/threading/thread_checker.h"
 #include "base/timer/timer.h"
 #include "remoting/base/rsa_key_pair.h"
 #include "remoting/signaling/signal_strategy.h"
 
 namespace base {
@@ skipping 60 matching lines @@
 // stanza of this form:
 //
 //  <iq type="set" from="remoting@bot.talk.google.com"
 //      to="user@gmail.com/chromoting123123" id="5" xmlns="jabber:client">
 //    <rem:heartbeat-result xmlns:rem="google:remoting">
 //      <rem:expected-sequence-id>654</rem:expected-sequence-id>
 //    </rem:heartbeat-result>
 //  </iq>
 class HeartbeatSender : public SignalStrategy::Listener {
  public:
-  class Listener {
-   public:
-    virtual ~Listener() { }
-
-    // Invoked after the first successful heartbeat.
-    virtual void OnHeartbeatSuccessful() = 0;
-
-    // Invoked when the host ID is permanently not recognized by the server.
-    virtual void OnUnknownHostIdError() = 0;
-  };
-
   // |signal_strategy| and |delegate| must outlive this
   // object. Heartbeats will start when the supplied SignalStrategy
   // enters the CONNECTED state.
-  HeartbeatSender(Listener* listener,
+  HeartbeatSender(const base::Closure& on_heartbeat_successful_callback,
+                  const base::Closure& on_unknown_host_id_error,
                   const std::string& host_id,
                   SignalStrategy* signal_strategy,
                   scoped_refptr<RsaKeyPair> key_pair,
                   const std::string& directory_bot_jid);
   virtual ~HeartbeatSender();
 
   // Sets host offline reason for future heartbeat stanzas,
   // as well as intiates sending a stanza right away.
   //
   // See rem:host-offline-reason class-level comments for discussion
@@ skipping 25 matching lines @@
   void ProcessResponse(bool is_offline_heartbeat_response,
                        IqRequest* request,
                        const buzz::XmlElement* response);
   void SetInterval(int interval);
   void SetSequenceId(int sequence_id);
 
   // Helper methods used by DoSendStanza() to generate heartbeat stanzas.
   scoped_ptr<buzz::XmlElement> CreateHeartbeatMessage();
   scoped_ptr<buzz::XmlElement> CreateSignature();
 
-  Listener* listener_;
+  base::Closure on_heartbeat_successful_callback_;
+  base::Closure on_unknown_host_id_error_;
   std::string host_id_;
   SignalStrategy* signal_strategy_;
   scoped_refptr<RsaKeyPair> key_pair_;
   std::string directory_bot_jid_;
   scoped_ptr<IqSender> iq_sender_;
   scoped_ptr<IqRequest> request_;
   int interval_ms_;
   base::RepeatingTimer<HeartbeatSender> timer_;
   base::OneShotTimer<HeartbeatSender> timer_resend_;
   int sequence_id_;
   bool sequence_id_was_set_;
   int sequence_id_recent_set_num_;
   bool heartbeat_succeeded_;
   int failed_startup_heartbeat_count_;
   std::string host_offline_reason_;
   base::Closure host_offline_reason_ack_callback_;
-  // TODO(lukasza): Consistent usage of listener-vs-callback. This is
-  // inconsistent today, because 1) host-offline-reason changes really
-  // needed to use callbacks (for ability to wrap them in
-  // CancellableCallback and for ability to hold a ref-count to
-  // MinimumHeartbeatSupporter throughout the lifetime of a callback)
-  // and 2) refactoring for consistently using callbacks everywhere
-  // spans multiple files and would obscure the core changes for
-  // host-offline-reason code.
 
   base::ThreadChecker thread_checker_;
 
   DISALLOW_COPY_AND_ASSIGN(HeartbeatSender);
 };
 
 }  // namespace remoting
 
 #endif  // REMOTING_HOST_HEARTBEAT_SENDER_H_