Refactor IqRequest.

remoting/jingle_glue/jingle_info_request.h

Index: remoting/jingle_glue/jingle_info_request.h
diff --git a/remoting/jingle_glue/jingle_info_request.h b/remoting/jingle_glue/jingle_info_request.h
index 6d5bcf652c81925877df40292a0deda268e4722e..538b1c4d9170b54926e6f74b4c00e29ab12cac63 100644
--- a/remoting/jingle_glue/jingle_info_request.h
+++ b/remoting/jingle_glue/jingle_info_request.h
@@ -12,10 +12,9 @@
 #include "base/basictypes.h"
 #include "base/callback.h"
 #include "base/memory/scoped_ptr.h"
+#include "remoting/jingle_glue/iq_sender.h"
 #include "third_party/libjingle/source/talk/base/sigslot.h"
 
-class Task;
-
 namespace buzz {
 class XmlElement;
 }  // namespace buzz
@@ -26,7 +25,7 @@ class SocketAddress;
 
 namespace remoting {
 
-class IqRequest;
+class SignalStrategy;
 
 // JingleInfoRequest handles requesting STUN/Relay infromation from

[Wez, 2011/11/03 02:09:30]

typo: infromation

(while you're here...)

[Sergey Ulanov, 2011/11/03 02:41:31]

Done.

 // the Google Talk network. The query is made when Send() is
@@ -47,7 +46,7 @@ class JingleInfoRequest : public sigslot::has_slots<> {
       const std::string&, const std::vector<std::string>&,
       const std::vector<talk_base::SocketAddress>&)> OnJingleInfoCallback;
 
-  explicit JingleInfoRequest(IqRequest* request);
+  explicit JingleInfoRequest(SignalStrategy* signal_strategy);

[Wez, 2011/11/03 02:09:30]

Why create with a SignalStrategy, rather than passing in an IqSender?

[Sergey Ulanov, 2011/11/03 02:41:31]

The idea is that IqSender is created by whoever needs to send an Iq message -
let me know if you disagree with this approach. It's also easier to test this
code when we pass around virtual interfaces.

[Wez, 2011/11/03 17:09:06]

When we discussed this previously, the model I had was:

IqRequest - Resource managing a single outstanding Iq request, providing control
over the lifetime of the callback.
IqSender - Manager class, instantiated once by each class that needs to dispatch
IqRequests, to manage those requests' lifetimes en masse, so that if the caller
is destroyed, teardown of the IqSender will ensure no further callbacks are
dispatched to the caller.  There may be many IqSenders using a single
SignalStrategy, and each IqSender can manage any number of requests.
SignalStrategy - The actual signalling mechanism.

The question I was asking here was simply that since this class only really
needs to see an IqSender, not a SignalStrategy, would it make more sense for the
caller to create the IqSender and pass it in?  But I think the current layout is
fine.

[Sergey Ulanov, 2011/11/03 17:56:00]

Yes, potentially we could pass IqSender here. It may be more efficient in some
cases, but given that we never create more than 3 IqSenders it doesn't make much
difference. I think passing signal_strategy is cleaner.

   virtual ~JingleInfoRequest();
 
   void Send(const OnJingleInfoCallback& callback);
@@ -57,6 +56,7 @@ class JingleInfoRequest : public sigslot::has_slots<> {
 
   void OnResponse(const buzz::XmlElement* stanza);
 
+  IqSender iq_sender_;
   scoped_ptr<IqRequest> request_;
   OnJingleInfoCallback on_jingle_info_cb_;