Refactor IqRequest.

remoting/jingle_glue/jingle_info_request.h

 // Copyright (c) 2011 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_JINGLE_GLUE_JINGLE_INFO_REQUEST_H_
 #define REMOTING_JINGLE_GLUE_JINGLE_INFO_REQUEST_H_
 
 #include <set>
 #include <string>
 #include <vector>
 
 #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
 
 namespace talk_base {
 class SocketAddress;
 }  // namespace talk_base
 
 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
 // called. The callback given to Send() is called when response to the
 // request is received.
 //
 // This class is not threadsafe and should be used on the same thread it is
 // created on.
 //
 // TODO(ajwong): Add support for a timeout.
 class JingleInfoRequest : public sigslot::has_slots<> {
  public:
   // Callback to receive the Jingle configuration settings.  The argumetns are

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

typo: argumetns

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

Done.

   // passed by pointer so the receive may call swap on them.  The receiver does
   // NOT own the arguments, which are guaranteed only to be alive for the
   // duration of the callback.
   typedef base::Callback<void (
       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);
 
  private:
   struct PendingDnsRequest;
 
   void OnResponse(const buzz::XmlElement* stanza);
 
+  IqSender iq_sender_;
   scoped_ptr<IqRequest> request_;
   OnJingleInfoCallback on_jingle_info_cb_;
 
   DISALLOW_COPY_AND_ASSIGN(JingleInfoRequest);
 };
 
 }  // namespace remoting
 
 #endif  // REMOTING_JINGLE_GLUE_JINGLE_INFO_REQUEST_H_