Reporting of policy errors via host-offline-reason: part 3

remoting/host/minimum_heartbeat_supporter.cc

+// Copyright 2014 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/minimum_heartbeat_supporter.h"
+
+#include "base/bind.h"
+#include "base/time/time.h"
+#include "net/base/network_change_notifier.h"
+#include "net/socket/client_socket_factory.h"
+#include "remoting/base/auto_thread_task_runner.h"
+#include "remoting/base/logging.h"
+#include "remoting/base/url_request_context_getter.h"
+#include "remoting/host/ack_or_timeout_reporter.h"
+#include "remoting/host/chromoting_host_context.h"
+#include "remoting/host/dns_blackhole_checker.h"
+#include "remoting/host/heartbeat_sender.h"
+#include "remoting/host/signaling_connector.h"
+#include "remoting/signaling/xmpp_signal_strategy.h"
+
+namespace remoting {
+
+MinimumHeartbeatSupporter::MinimumHeartbeatSupporter(
+    scoped_refptr<AutoThreadTaskRunner> network_task_runner,
+    scoped_ptr<net::NetworkChangeNotifier> network_change_notifier,
+    scoped_ptr<XmppSignalStrategy> signal_strategy,
+    scoped_ptr<SignalingConnector> signaling_connector,
+    scoped_ptr<HeartbeatSender> heartbeat_sender)
+    : network_task_runner_(network_task_runner),
+      network_change_notifier_(network_change_notifier.Pass()),
+      signal_strategy_(signal_strategy.Pass()),
+      signaling_connector_(signaling_connector.Pass()),
+      heartbeat_sender_(heartbeat_sender.Pass()) {
+}
+
+scoped_refptr<MinimumHeartbeatSupporter> MinimumHeartbeatSupporter::Create(
+    const base::Closure& on_heartbeat_successful_callback,
+    const base::Closure& on_unknown_host_id_error_callback,
+    const base::Closure& on_auth_failed_callback,
+    const ChromotingHostContext& host_context,
+    const XmppSignalStrategy::XmppServerConfig& xmpp_server_config,
+    const std::string& talkgadget_prefix,
+    const std::string& host_id,
+    const scoped_refptr<const RsaKeyPair>& key_pair,
+    const std::string& directory_bot_jid,
+    const std::string& oauth_refresh_token,
+    bool use_service_account) {
+  DCHECK(host_context.network_task_runner()->BelongsToCurrentThread());
+
+  // Create a NetworkChangeNotifier for use by the signaling connector.
+  scoped_ptr<net::NetworkChangeNotifier> network_change_notifier(
+      net::NetworkChangeNotifier::Create());
+
+  scoped_ptr<XmppSignalStrategy> signal_strategy(new XmppSignalStrategy(
+      net::ClientSocketFactory::GetDefaultFactory(),
+      host_context.url_request_context_getter(), xmpp_server_config));
+
+  scoped_ptr<DnsBlackholeChecker> dns_blackhole_checker(new DnsBlackholeChecker(
+      host_context.url_request_context_getter(), talkgadget_prefix));
+
+  scoped_ptr<SignalingConnector> signaling_connector(new SignalingConnector(
+      signal_strategy.get(), dns_blackhole_checker.Pass(),
+      on_auth_failed_callback));
+
+  if (!oauth_refresh_token.empty()) {
+    scoped_ptr<OAuthTokenGetter::OAuthCredentials> oauth_credentials(
+        new OAuthTokenGetter::OAuthCredentials(xmpp_server_config.username,
+                                               oauth_refresh_token,
+                                               use_service_account));
+
+    scoped_ptr<OAuthTokenGetter> oauth_token_getter(
+        new OAuthTokenGetter(oauth_credentials.Pass(),
+                             host_context.url_request_context_getter(), false));
+
+    signaling_connector->EnableOAuth(oauth_token_getter.Pass());
+  }
+
+  scoped_ptr<HeartbeatSender> heartbeat_sender(new HeartbeatSender(
+      on_heartbeat_successful_callback, on_unknown_host_id_error_callback,
+      host_id, signal_strategy.get(), key_pair, directory_bot_jid));
+
+  return new MinimumHeartbeatSupporter(
+      host_context.network_task_runner(), network_change_notifier.Pass(),
+      signal_strategy.Pass(), signaling_connector.Pass(),
+      heartbeat_sender.Pass());
+}
+
+MinimumHeartbeatSupporter::~MinimumHeartbeatSupporter() {
+  DCHECK(network_task_runner_->BelongsToCurrentThread());
+
+  HOST_LOG << "MinimumHeartbeatSupporter destroyed.";
+}
+
+void MinimumHeartbeatSupporter::SendHostOfflineReason(
+    const std::string& host_offline_reason,
+    const base::TimeDelta& timeout) {
+  DCHECK(network_task_runner_->BelongsToCurrentThread());
+
+  HOST_LOG << "SendHostOfflineReason: trying to send " << host_offline_reason
+           << " to the bot";
+
+  ReportAckOrTimeout(
+      // Passing partially bound |SetHostOfflineReason| as |function_that_acks|

[Sergey Ulanov, 2014/12/15 22:39:19]

nit: Add . at the end. Comments should be full sentences, it's not a full
sentence without .

[Sergey Ulanov, 2014/12/15 22:39:19]

nit: SetHostOfflineReason() instead of |SetHostOfflineReason| please. Pipes are
normally used to denote |variable_names| while () marks a function name.

[Łukasz Anforowicz, 2014/12/16 00:59:52]

N/A after removing ReportAckOrTimeout.

[Łukasz Anforowicz, 2014/12/16 00:59:52]

N/A after removing ReportAckOrTimeout.

+      base::Bind(&HeartbeatSender::SetHostOfflineReason,

[Sergey Ulanov, 2014/12/15 22:39:19]

If you replace ReportAckOrTimeout() with CreateCallbackWithTimeout() this code
would look much simpler and more readable, e.g.:

  heartbeat_sender_->SetHostOfflineReason(
      CreateCallbackWithTimeout(
          timeout, network_task_runner_,
          base::Bind(&MinimumHeartbeatSupporter::OnAckOrTimeout, this));

[Łukasz Anforowicz, 2014/12/16 00:59:52]

N/A after removing ReportAckOrTimeout.

+                 base::Unretained(heartbeat_sender_.get()),
+                 host_offline_reason),
+      timeout, network_task_runner_,
+      // The callback below will keep |this| alive until either ack or timeout.
+      base::Bind(&MinimumHeartbeatSupporter::OnAckOrTimeout, this));
+}
+
+void MinimumHeartbeatSupporter::OnAckOrTimeout(AckOrTimeout ack_or_timeout) {
+  DCHECK(network_task_runner_->BelongsToCurrentThread());
+  switch (ack_or_timeout) {
+    case Ack:
+      HOST_LOG << "SendHostOfflineReason: got ack";
+      break;
+    case Timeout:
+      HOST_LOG << "SendHostOfflineReason: timed out";
+      break;
+    default:
+      NOTREACHED();
+      break;
+  }
+}
+
+}  // namespace remoting