Move signaling connection creation out of ChromotingHost.

remoting/host/heartbeat_sender.cc

Index: remoting/host/heartbeat_sender.cc
diff --git a/remoting/host/heartbeat_sender.cc b/remoting/host/heartbeat_sender.cc
index 2a7d8ac0ad0159442c605ff93c5ecb108c182a24..89dc05c93fe824ab43bd4426ee2557a95f0a16fb 100644
--- a/remoting/host/heartbeat_sender.cc
+++ b/remoting/host/heartbeat_sender.cc
@@ -34,68 +34,59 @@ 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;
-
-  full_jid_ = signal_strategy->GetLocalJid();
-
-  iq_sender_.reset(new IqSender(signal_strategy));
-
-  DoSendStanza();
-  timer_.Start(FROM_HERE, base::TimeDelta::FromMilliseconds(interval_ms_), this,
-               &HeartbeatSender::DoSendStanza);
-}
-
-void HeartbeatSender::OnSignallingDisconnected() {
-  DCHECK(message_loop_->BelongsToCurrentThread());
-  state_ = STOPPED;
-  request_.reset();
-  iq_sender_.reset();
-  timer_.Stop();
+void HeartbeatSender::OnSignalStrategyStateChange(SignalStrategy::State state) {
+  if (state == SignalStrategy::CONNECTED) {
+    DCHECK(state_ == INITIALIZED || state_ == STOPPED);
+    state_ = STARTED;
+
+    iq_sender_.reset(new IqSender(signal_strategy_));
+
+    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();
+  }
 }
 
-// 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;
@@ -106,8 +97,6 @@ void HeartbeatSender::DoSendStanza() {
 }
 
 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: "
@@ -167,7 +156,7 @@ XmlElement* HeartbeatSender::CreateSignature() {
   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);