Move signaling connection creation out of ChromotingHost.

remoting/host/register_support_host_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_HOST_REGISTER_SUPPORT_HOST_REQUEST_H_
 #define REMOTING_HOST_REGISTER_SUPPORT_HOST_REQUEST_H_
 
 #include <string>
 
 #include "base/callback.h"
 #include "base/memory/ref_counted.h"
 #include "base/memory/scoped_ptr.h"
+#include "remoting/jingle_glue/signal_strategy.h"
 #include "remoting/host/host_key_pair.h"
-#include "remoting/host/host_status_observer.h"
 #include "testing/gtest/include/gtest/gtest_prod.h"
 
 class MessageLoop;
 
 namespace buzz {
 class XmlElement;
 }  // namespace buzz
 
 namespace base {
 class TimeDelta;
 }  // namespace base
 
 namespace remoting {
 
 class IqRequest;
 class IqSender;
 
 // RegisterSupportHostRequest sends support host registeration request
-// to the Chromoting Bot. It listens to the status of the host using
-// HostStatusObserver interface and sends the request when signalling
-// channel is connected. When a response is received from the bot, it
-// calls the callback specified in the Init() method.
-class RegisterSupportHostRequest : public HostStatusObserver {
+// to the Chromoting Bot. It listens to signalling channel status and
+// sends the request when signalling channel is connected. When a

[Wez, 2012/01/03 16:25:04]

nit: ... sends a request to register the host for a Support-Id, as soon as the
associated SignalStrategy becomes connected.

[Sergey Ulanov, 2012/01/03 21:51:02]

Done.

+// response is received from the bot, it calls the callback specified
+// in the Init() method.
+class RegisterSupportHostRequest : public SignalStrategy::Listener {
  public:
   // First parameter is set to true on success. Second parameter is
   // the new SessionID received from the bot. Third parameter is the
   // amount of time until that id expires.

[Wez, 2012/01/03 16:25:04]

nit: Could use a zero time-to-expiry to indicate failure.

[Sergey Ulanov, 2012/01/03 21:51:02]

I think that using a separate parameter is cleaner. In either case it's not
related to this CL.

   typedef base::Callback<void(bool, const std::string&,
                               const base::TimeDelta&)> RegisterCallback;
 
   RegisterSupportHostRequest();
   virtual ~RegisterSupportHostRequest();
 
   // Provide the configuration to use to register the host, and a

[Wez, 2012/01/03 16:25:04]

nit: Initializes the registration to use a supplied SignalStrategy and
HostConfig, and to notify |callback| upon completion or failure.

[Sergey Ulanov, 2012/01/03 21:51:02]

Done.

   // callback to invoke when a registration response is received.
   // |callback| is called when registration response is received from
-  // the server. Ownership of |callback| is given to the request
-  // object. Caller must ensure that the callback object is valid
-  // while signalling connection exists. Returns false on falure
-  // (e.g. config is invalid). Callback is never called if the bot
-  // malfunctions and doesn't respond to the request.
-  bool Init(HostConfig* config, const RegisterCallback& callback);
+  // the server. Returns false on falure (e.g. config is
+  // invalid). Callback is never called if the bot malfunctions and

[Wez, 2012/01/03 16:25:04]

nit: Would it simplify the interface to notify the callback on failure?

[Sergey Ulanov, 2012/01/03 21:51:02]

Having result here is useful to detect config errors before starting the host.
But I refactored this code in the next CL and removed this method.

+  // doesn't respond to the request.

[Wez, 2012/01/03 16:25:04]

If the caller is responsible for implementing a sensible timeout then that
should be mentioned in the description of the class?

[Sergey Ulanov, 2012/01/03 21:51:02]

Normally GTalk would reply with an error when the bot is down, so it is very
unlikely that we don't get any response. Ideally this class should have timeout
and call the callback with error when timeout expires. I've added TODO to fix
it.

+  bool Init(SignalStrategy* signal_strategy_,
+            HostConfig* config,
+            const RegisterCallback& callback);
 
   // HostStatusObserver implementation.
-  virtual void OnSignallingConnected(SignalStrategy* signal_strategy) OVERRIDE;
-  virtual void OnSignallingDisconnected() OVERRIDE;
-  virtual void OnClientAuthenticated(const std::string& jid) OVERRIDE;
-  virtual void OnClientDisconnected(const std::string& jid) OVERRIDE;
-  virtual void OnAccessDenied() OVERRIDE;
-  virtual void OnShutdown() OVERRIDE;
+  virtual void OnSignalStrategyStateChange(
+      SignalStrategy::State state) OVERRIDE;
+  virtual bool OnSignalStrategyIncomingStanza(
+      const buzz::XmlElement* stanza) OVERRIDE;
 
  private:
   void DoSend();
 
   // Caller owns the result.
   buzz::XmlElement* CreateRegistrationRequest(const std::string& jid);
   buzz::XmlElement* CreateSignature(const std::string& jid);
 
   void ProcessResponse(const buzz::XmlElement* response);
   bool ParseResponse(const buzz::XmlElement* response,
                      std::string* support_id, base::TimeDelta* lifetime);
 
-  MessageLoop* message_loop_;
+  SignalStrategy* signal_strategy_;
   RegisterCallback callback_;
   scoped_ptr<IqSender> iq_sender_;
   scoped_ptr<IqRequest> request_;
   HostKeyPair key_pair_;
 
   DISALLOW_COPY_AND_ASSIGN(RegisterSupportHostRequest);
 };
 
 }  // namespace remoting
 
 #endif  // REMOTING_HOST_REGISTER_SUPPORT_HOST_REQUEST_H_