Move signaling connection creation out of ChromotingHost.

remoting/host/register_support_host_request.cc

 // 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.
 
 #include "remoting/host/register_support_host_request.h"
 
 #include "base/bind.h"
 #include "base/logging.h"
 #include "base/message_loop.h"
 #include "base/string_number_conversions.h"
@@ skipping 18 matching lines @@
 const char kSignatureTag[] = "signature";
 const char kSignatureTimeAttr[] = "time";
 
 // Strings used to parse responses received from the bot.
 const char kRegisterQueryResultTag[] = "register-support-host-result";
 const char kSupportIdTag[] = "support-id";
 const char kSupportIdLifetimeTag[] = "support-id-lifetime";
 }
 
 RegisterSupportHostRequest::RegisterSupportHostRequest()
-    : message_loop_(NULL) {
+    : signal_strategy_(NULL) {
 }
 
 RegisterSupportHostRequest::~RegisterSupportHostRequest() {
+  if (signal_strategy_)
+    signal_strategy_->RemoveListener(this);
 }
 
-bool RegisterSupportHostRequest::Init(HostConfig* config,
+bool RegisterSupportHostRequest::Init(SignalStrategy* signal_strategy,
+                                      HostConfig* config,
                                       const RegisterCallback& callback) {
-  callback_ = callback;
-
   if (!key_pair_.Load(config)) {

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

nit: Consider DCHECK()ing |signal_strategy|, since it is dereferenced below.

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

Fixed in a different CL.

     return false;
   }
+
+  callback_ = callback;
+  signal_strategy_ = signal_strategy;
+  signal_strategy_->AddListener(this);
+  iq_sender_.reset(new IqSender(signal_strategy_));
+
   return true;
 }
 
-void RegisterSupportHostRequest::OnSignallingConnected(
-    SignalStrategy* signal_strategy) {
-  DCHECK(!callback_.is_null());
+void RegisterSupportHostRequest::OnSignalStrategyStateChange(
+    SignalStrategy::State state) {
+  if (state == SignalStrategy::CONNECTED) {
+    DCHECK(!callback_.is_null());
 
-  message_loop_ = MessageLoop::current();
-
-  iq_sender_.reset(new IqSender(signal_strategy));
-  request_.reset(iq_sender_->SendIq(
-      buzz::STR_SET, kChromotingBotJid,
-      CreateRegistrationRequest(signal_strategy->GetLocalJid()),
-      base::Bind(&RegisterSupportHostRequest::ProcessResponse,
-                 base::Unretained(this))));
+    request_.reset(iq_sender_->SendIq(
+        buzz::STR_SET, kChromotingBotJid,
+        CreateRegistrationRequest(signal_strategy_->GetLocalJid()),
+        base::Bind(&RegisterSupportHostRequest::ProcessResponse,
+                   base::Unretained(this))));
+  } else if (state == SignalStrategy::DISCONNECTED) {
+    request_.reset();
+    if (!callback_.is_null()) {
+      // We will reach here if signaling fails to connect.

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

Or if signalling disconnects before we've received the response?

It feels like we should clean up |request_| and remove our signalling listener
when we receive the response, so that this DISCONNECTED handler is purely for
the failure case.

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

Done.

+      RegisterCallback callback = callback_;
+      callback_.Reset();
+      callback.Run(false, std::string(), base::TimeDelta());
+    }
+  }
 }
 
-void RegisterSupportHostRequest::OnSignallingDisconnected() {
-  if (!message_loop_) {
-    // We will reach here with |message_loop_| NULL if the Host's
-    // XMPP connection attempt fails.
-    CHECK(!callback_.is_null());
-    DCHECK(!request_.get());
-    callback_.Run(false, std::string(), base::TimeDelta());
-    return;
-  }
-  DCHECK_EQ(message_loop_, MessageLoop::current());
-  request_.reset();
-  iq_sender_.reset();
+bool RegisterSupportHostRequest::OnSignalStrategyIncomingStanza(
+    const buzz::XmlElement* stanza) {
+  return false;
 }
 
-// Ignore any notifications other than signalling
-// connected/disconnected events.
-void RegisterSupportHostRequest::OnAccessDenied() { }
-void RegisterSupportHostRequest::OnClientAuthenticated(
-    const std::string& jid) { }
-void RegisterSupportHostRequest::OnClientDisconnected(
-    const std::string& jid) { }
-void RegisterSupportHostRequest::OnShutdown() { }
-
 XmlElement* RegisterSupportHostRequest::CreateRegistrationRequest(
     const std::string& jid) {
   XmlElement* query = new XmlElement(
       QName(kChromotingXmlNamespace, kRegisterQueryTag));
   XmlElement* public_key = new XmlElement(
       QName(kChromotingXmlNamespace, kPublicKeyTag));
   public_key->AddText(key_pair_.GetPublicKey());
   query->AddElement(public_key);
   query->AddElement(CreateSignature(jid));
   return query;
@@ skipping 69 matching lines @@
     return false;
   }
 
   *support_id = support_id_element->BodyText();
   *lifetime = base::TimeDelta::FromSeconds(lifetime_int);
   return true;
 }
 
 
 void RegisterSupportHostRequest::ProcessResponse(const XmlElement* response) {
-  DCHECK_EQ(message_loop_, MessageLoop::current());
   std::string support_id;
   base::TimeDelta lifetime;
   bool success = ParseResponse(response, &support_id, &lifetime);
-  callback_.Run(success, support_id, lifetime);
+  RegisterCallback callback = callback_;
+  callback_.Reset();
+  callback.Run(success, support_id, lifetime);
 }
 
 }  // namespace remoting