Move signaling connection creation out of ChromotingHost.

remoting/host/heartbeat_sender.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/heartbeat_sender.h"
 
 #include "base/bind.h"
 #include "base/logging.h"
 #include "base/message_loop_proxy.h"
 #include "base/string_number_conversions.h"
@@ skipping 16 matching lines @@
 const char kHostIdAttr[] = "hostid";
 const char kHeartbeatSignatureTag[] = "signature";
 const char kSignatureTimeAttr[] = "time";
 
 const char kHeartbeatResultTag[] = "heartbeat-result";
 const char kSetIntervalTag[] = "set-interval";
 
 const int64 kDefaultHeartbeatIntervalMs = 5 * 60 * 1000;  // 5 minutes.
 }
 
-HeartbeatSender::HeartbeatSender(base::MessageLoopProxy* message_loop,
-                                 MutableHostConfig* config)
-
+HeartbeatSender::HeartbeatSender()
     : state_(CREATED),
-      message_loop_(message_loop),
-      config_(config),
+      signal_strategy_(NULL),
       interval_ms_(kDefaultHeartbeatIntervalMs) {
-  DCHECK(config_);
 }
 
 HeartbeatSender::~HeartbeatSender() {
   DCHECK(state_ == CREATED || state_ == INITIALIZED || state_ == STOPPED);

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

nit: |state_| != STARTED is really what you mean here?

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

Heh, I originally wrote it as |state_| != STARTED, but Albert objected:
http://codereview.chromium.org/3087003/diff/9001/remoting/host/heartbeat_send...
:)

In any case, I don't think we don't need this DCHECK anymore. Removed it.

+  if (signal_strategy_)
+    signal_strategy_->RemoveListener(this);
 }
 
-bool HeartbeatSender::Init() {
+bool HeartbeatSender::Init(SignalStrategy* signal_strategy,
+                           MutableHostConfig* config) {
   DCHECK(state_ == CREATED);
 
-  if (!config_->GetString(kHostIdConfigPath, &host_id_)) {
+  if (!config->GetString(kHostIdConfigPath, &host_id_)) {
     LOG(ERROR) << "host_id is not defined in the config.";
     return false;
   }
 
-  if (!key_pair_.Load(config_)) {
+  if (!key_pair_.Load(config)) {
     return false;
   }
 
+  DCHECK(signal_strategy);
+  signal_strategy_ = signal_strategy;
+  signal_strategy_->AddListener(this);

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

What happens if the SignalStrategy is already connected by the time we reach
here?

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

Added OnSignalStrategyStateChange() call below to handle this case. 

+
   state_ = INITIALIZED;
 
   return true;
 }
 
-void HeartbeatSender::OnSignallingConnected(SignalStrategy* signal_strategy) {
-  DCHECK(message_loop_->BelongsToCurrentThread());
-  DCHECK(state_ == INITIALIZED || state_ == STOPPED);
-  state_ = STARTED;
+void HeartbeatSender::OnSignalStrategyStateChange(SignalStrategy::State state) {
+  if (state == SignalStrategy::CONNECTED) {
+    DCHECK(state_ == INITIALIZED || state_ == STOPPED);
+    state_ = STARTED;
 
-  full_jid_ = signal_strategy->GetLocalJid();
+    iq_sender_.reset(new IqSender(signal_strategy_));
 
-  iq_sender_.reset(new IqSender(signal_strategy));
-
-  DoSendStanza();
-  timer_.Start(FROM_HERE, base::TimeDelta::FromMilliseconds(interval_ms_), this,
-               &HeartbeatSender::DoSendStanza);
+    DoSendStanza();
+    timer_.Start(FROM_HERE, base::TimeDelta::FromMilliseconds(interval_ms_),
+                 this, &HeartbeatSender::DoSendStanza);
+  } else if (state == SignalStrategy::DISCONNECTED) {
+    state_ = STOPPED;

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

Surely this should be INITIALIZED?

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

STOPPED is correct here. Anyway, in my next CL I removed INITIALIZED and CREATED
states - they are useless now.

+    request_.reset();
+    iq_sender_.reset();
+    timer_.Stop();
+  }
 }
 
-void HeartbeatSender::OnSignallingDisconnected() {
-  DCHECK(message_loop_->BelongsToCurrentThread());
-  state_ = STOPPED;
-  request_.reset();
-  iq_sender_.reset();
-  timer_.Stop();
-}
-
-// Ignore any notifications other than signalling
-// connected/disconnected events.
-void HeartbeatSender::OnAccessDenied() { }
-void HeartbeatSender::OnClientAuthenticated(const std::string& jid) { }
-void HeartbeatSender::OnClientDisconnected(const std::string& jid) { }
-void HeartbeatSender::OnShutdown() { }
-
 void HeartbeatSender::DoSendStanza() {
-  DCHECK(message_loop_->BelongsToCurrentThread());
   DCHECK_EQ(state_, STARTED);
 
   VLOG(1) << "Sending heartbeat stanza to " << kChromotingBotJid;
   request_.reset(iq_sender_->SendIq(
       buzz::STR_SET, kChromotingBotJid, CreateHeartbeatMessage(),
       base::Bind(&HeartbeatSender::ProcessResponse,
                  base::Unretained(this))));
 }
 
 void HeartbeatSender::ProcessResponse(const XmlElement* response) {
-  DCHECK(message_loop_->BelongsToCurrentThread());
-
   std::string type = response->Attr(buzz::QN_TYPE);
   if (type == buzz::STR_ERROR) {
     LOG(ERROR) << "Received error in response to heartbeat: "
                << response->Str();
     return;
   }
 
   // This method must only be called for error or result stanzas.
   DCHECK_EQ(std::string(buzz::STR_RESULT), type);
 
@@ skipping 39 matching lines @@
 
 XmlElement* HeartbeatSender::CreateSignature() {
   XmlElement* signature_tag = new XmlElement(
       QName(kChromotingXmlNamespace, kHeartbeatSignatureTag));
 
   int64 time = static_cast<int64>(base::Time::Now().ToDoubleT());
   std::string time_str(base::Int64ToString(time));
   signature_tag->AddAttr(
       QName(kChromotingXmlNamespace, kSignatureTimeAttr), time_str);
 
-  std::string message = full_jid_ + ' ' + time_str;
+  std::string message = signal_strategy_->GetLocalJid() + ' ' + time_str;
   std::string signature(key_pair_.GetSignature(message));
   signature_tag->AddText(signature);
 
   return signature_tag;
 }
 
 }  // namespace remoting