Adding the base ChromotingInstance implementation and unittests. This class will be used by the ap…

remoting/test/test_chromoting_client.cc

Index: remoting/test/test_chromoting_client.cc
diff --git a/remoting/test/test_chromoting_client.cc b/remoting/test/test_chromoting_client.cc
new file mode 100644
index 0000000000000000000000000000000000000000..328a0eaba3a52c186fb3d99224ab0b3b72185ab2
--- /dev/null
+++ b/remoting/test/test_chromoting_client.cc
@@ -0,0 +1,350 @@
+// Copyright 2015 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/test/test_chromoting_client.h"
+
+#include <string>
+#include <vector>
+
+#include "base/logging.h"
+#include "base/thread_task_runner_handle.h"
+#include "jingle/glue/thread_wrapper.h"
+#include "net/base/request_priority.h"
+#include "net/socket/client_socket_factory.h"
+#include "remoting/base/url_request_context_getter.h"
+#include "remoting/client/audio_player.h"
+#include "remoting/client/chromoting_client.h"
+#include "remoting/client/client_context.h"
+#include "remoting/client/token_fetcher_proxy.h"
+#include "remoting/protocol/authentication_method.h"
+#include "remoting/protocol/chromium_port_allocator.h"
+#include "remoting/protocol/host_stub.h"
+#include "remoting/protocol/libjingle_transport_factory.h"
+#include "remoting/protocol/negotiating_client_authenticator.h"
+#include "remoting/protocol/network_settings.h"
+#include "remoting/protocol/session_config.h"
+#include "remoting/protocol/third_party_client_authenticator.h"
+#include "remoting/signaling/xmpp_signal_strategy.h"
+#include "remoting/test/remote_host_info_fetcher.h"
+#include "remoting/test/test_video_renderer.h"
+
+namespace {
+
+const char kRemotingCapabilities[] =

[Sergey Ulanov, 2015/03/11 00:09:51]

suggest renaming this to kLocalCapabilities.

[joedow, 2015/03/11 19:06:39]

Acknowledged.

+    "sendInitialResolution "
+    "rateLimitResizeRequests "
+    "googleDrive "
+    "videoRecorder "
+    "casting";

[Sergey Ulanov, 2015/03/11 00:09:52]

Do you really need all of this. I don't think this class implements any of these
capabilities, so you probably don't need any of them here.

[joedow, 2015/03/11 19:06:39]

Done.

+
+const char kXmppHostName[] = "talk.google.com";
+const int kXmppPortNumber = 5222;
+
+}  // namespace
+
+namespace remoting {
+namespace test {
+
+TestChromotingClient::TestChromotingClient()
+    : connection_to_host_state_(protocol::ConnectionToHost::INITIALIZING),
+      connection_error_code_(protocol::OK) {
+}
+
+TestChromotingClient::~TestChromotingClient() {
+  // Ensure any connections are closed and the members are destroyed in the
+  // appropriate order.
+  EndConnection();
+}
+
+void TestChromotingClient::StartConnection(
+    const std::string& user_name,
+    const std::string& access_token,
+    const RemoteHostInfo& remote_host_info) {
+  DCHECK(!user_name.empty());
+  DCHECK(!access_token.empty());
+  DCHECK(remote_host_info.IsReadyForConnection());
+
+  // Required to establish a connection to the host.
+  jingle_glue::JingleThreadWrapper::EnsureForCurrentMessageLoop();
+
+  scoped_refptr<URLRequestContextGetter> request_context_getter;
+  request_context_getter = new URLRequestContextGetter(
+      base::ThreadTaskRunnerHandle::Get(),   // network_runner
+      base::ThreadTaskRunnerHandle::Get());  // file_runner
+
+  client_context_.reset(new ClientContext(base::ThreadTaskRunnerHandle::Get()));
+
+  video_renderer_.reset(new TestVideoRenderer());
+
+  chromoting_client_.reset(new ChromotingClient(client_context_.get(),
+                                                this,  // client_user_interface.
+                                                video_renderer_.get(),
+                                                scoped_ptr<AudioPlayer>()));

[Sergey Ulanov, 2015/03/11 00:09:51]

this can be just nullptr.

[joedow, 2015/03/11 19:06:39]

Done.

+
+  if (test_connection_to_host_) {
+    chromoting_client_->SetConnectionToHostForTests(
+        test_connection_to_host_.Pass());
+  }
+
+  XmppSignalStrategy::XmppServerConfig xmpp_server_config;
+  xmpp_server_config.host = kXmppHostName;
+  xmpp_server_config.port = kXmppPortNumber;
+  xmpp_server_config.use_tls = true;
+  xmpp_server_config.username = user_name;
+  xmpp_server_config.auth_token = access_token;
+
+  // Set up the signal strategy.  This must outlive the client object.
+  signal_strategy_.reset(
+      new XmppSignalStrategy(net::ClientSocketFactory::GetDefaultFactory(),
+                             request_context_getter, xmpp_server_config));
+
+  protocol::NetworkSettings network_settings(
+      protocol::NetworkSettings::NAT_TRAVERSAL_FULL);
+
+  scoped_ptr<protocol::ChromiumPortAllocator> port_allocator(
+      protocol::ChromiumPortAllocator::Create(request_context_getter,
+                                              network_settings));
+
+  scoped_ptr<protocol::TransportFactory> transport_factory(
+      new protocol::LibjingleTransportFactory(
+          signal_strategy_.get(), port_allocator.Pass(), network_settings));
+
+  scoped_ptr<protocol::ThirdPartyClientAuthenticator::TokenFetcher>
+      token_fetcher(new TokenFetcherProxy(
+          base::Bind(&TestChromotingClient::FetchThirdPartyToken,
+                     remote_host_info.authorization_code,
+                     remote_host_info.shared_secret),
+          "FAKE_HOST_PUBLIC_KEY"));
+
+  fetch_secret_callback_ =

[Sergey Ulanov, 2015/03/11 00:09:52]

I don't think you need this. See my comment below.

[joedow, 2015/03/11 19:06:39]

Done.

+      base::Bind(&TestChromotingClient::FetchSecretFromString,
+                 remote_host_info.shared_secret);
+
+  std::vector<protocol::AuthenticationMethod> auth_methods;
+  auth_methods.push_back(protocol::AuthenticationMethod::ThirdParty());
+
+  scoped_ptr<protocol::Authenticator> authenticator(
+      new protocol::NegotiatingClientAuthenticator(
+          std::string(),  // client_pairing_id
+          std::string(),  // shared_secret
+          std::string(),  // authentication_tag
+          fetch_secret_callback_, token_fetcher.Pass(), auth_methods));

[Sergey Ulanov, 2015/03/11 00:09:52]

If i remember correctly fetch_secret_callback is needed only for pin-based
authentication, but you have only ThirdParty auth enabled, so you can just pass
a null callback here.

[joedow, 2015/03/11 19:06:39]

Done.

+
+  chromoting_client_->Start(signal_strategy_.get(), authenticator.Pass(),
+                            transport_factory.Pass(), remote_host_info.host_jid,
+                            kRemotingCapabilities);
+}
+
+void TestChromotingClient::EndConnection() {
+  // Clearing out the client will close the connection.
+  chromoting_client_.reset();
+
+  // The signal strategy object must outlive the client so destroy it next.
+  signal_strategy_.reset();
+
+  // The connection state should been updated when the chromoting client was
+  // destroyed however we will verify and update the connection state if not.

[Sergey Ulanov, 2015/03/11 00:09:52]

Maybe better to (D)CHECK here that ChromotingClient did update state?

[joedow, 2015/03/11 19:06:39]

Looks like the state that is set depends on whether an active connection was
established, I'd like to keep the state consistent here so I've updated the
comments.

+  if (connection_to_host_state_ != protocol::ConnectionToHost::CLOSED &&
+      connection_to_host_state_ != protocol::ConnectionToHost::FAILED &&
+      connection_error_code_ == protocol::OK) {
+    OnConnectionState(protocol::ConnectionToHost::CLOSED, protocol::OK);
+  }
+}
+
+void TestChromotingClient::AddRemoteConnectionObserver(
+    RemoteConnectionObserver* observer) {
+  DCHECK(observer);
+
+  connection_observers_.AddObserver(observer);
+}
+
+void TestChromotingClient::RemoveRemoteConnectionObserver(
+    RemoteConnectionObserver* observer) {
+  DCHECK(observer);
+
+  connection_observers_.RemoveObserver(observer);
+}
+
+void TestChromotingClient::SetConnectionToHostForTests(
+    scoped_ptr<protocol::ConnectionToHost> connection_to_host) {
+  test_connection_to_host_ = connection_to_host.Pass();
+}
+
+void TestChromotingClient::OnConnectionState(
+    protocol::ConnectionToHost::State state,
+    protocol::ErrorCode error_code) {
+  DVLOG(1) << "TestChromotingClient::OnConnectionState() Called";
+  DVLOG(2) << "--State: " << ConnectionStateToFriendlyString(state);

[Sergey Ulanov, 2015/03/11 00:09:51]

Please see the second part of this comment:
https://codereview.chromium.org/976233003/diff/40001/remoting/test/chromoting...

[joedow, 2015/03/11 19:06:39]

Done.

+  DVLOG(2) << "--ErrorCode: " << ProtocolErrorToFriendlyString(error_code);
+
+  connection_error_code_ = error_code;
+  connection_to_host_state_ = state;
+
+  FOR_EACH_OBSERVER(RemoteConnectionObserver, connection_observers_,
+                    ConnectionStateChanged(state, error_code));
+}
+
+void TestChromotingClient::OnConnectionReady(bool ready) {
+  DVLOG(1) << "TestChromotingClient::OnConnectionReady() Called";
+  DVLOG(2) << "--ready:" << ready;
+
+  FOR_EACH_OBSERVER(RemoteConnectionObserver, connection_observers_,
+                    ConnectionReady(ready));
+}
+
+void TestChromotingClient::OnRouteChanged(
+    const std::string& channel_name,
+    const protocol::TransportRoute& route) {
+  DVLOG(1) << "TestChromotingClient::OnRouteChanged() Called";
+  DVLOG(2) << "--channel_name:" << channel_name;
+  DVLOG(2) << "--route:" << protocol::TransportRoute::GetTypeString(route.type);
+
+  FOR_EACH_OBSERVER(RemoteConnectionObserver, connection_observers_,
+                    RouteChanged(channel_name, route));
+}
+
+void TestChromotingClient::SetCapabilities(const std::string& capabilities) {
+  DVLOG(1) << "TestChromotingClient::SetCapabilities() Called";
+  DVLOG(2) << "--Capabilities: " << capabilities;
+
+  FOR_EACH_OBSERVER(RemoteConnectionObserver, connection_observers_,
+                    CapabilitiesSet(capabilities));
+}
+
+void TestChromotingClient::SetPairingResponse(
+    const protocol::PairingResponse& pairing_response) {
+  DVLOG(1) << "TestChromotingClient::SetPairingResponse() Called";
+  DVLOG(2) << "--client_id: " << pairing_response.client_id();
+  DVLOG(2) << "--shared_secret: " << pairing_response.shared_secret();
+
+  FOR_EACH_OBSERVER(RemoteConnectionObserver, connection_observers_,
+                    PairingResponseSet(pairing_response));
+}
+
+void TestChromotingClient::DeliverHostMessage(
+    const protocol::ExtensionMessage& message) {
+  DVLOG(1) << "TestChromotingClient::DeliverHostMessage() called";
+  DVLOG(2) << "--type: " << message.type();
+  DVLOG(2) << "--data: " << message.data();
+
+  FOR_EACH_OBSERVER(RemoteConnectionObserver, connection_observers_,
+                    HostMessageReceived(message));
+}
+
+protocol::ClipboardStub* TestChromotingClient::GetClipboardStub() {
+  DVLOG(1) << "TestChromotingClient::GetClipboardStub() Called";
+  return this;
+}
+
+protocol::CursorShapeStub* TestChromotingClient::GetCursorShapeStub() {
+  DVLOG(1) << "TestChromotingClient::GetCursorShapeStub() Called";
+  return this;
+}
+
+void TestChromotingClient::InjectClipboardEvent(
+    const protocol::ClipboardEvent& event) {
+  DVLOG(1) << "TestChromotingClient::InjectClipboardEvent() Called";
+}
+
+void TestChromotingClient::SetCursorShape(
+    const protocol::CursorShapeInfo& cursor_shape) {
+  DVLOG(1) << "TestChromotingClient::SetCursorShape() Called";
+}
+
+void TestChromotingClient::FetchSecretFromString(
+    const std::string& shared_secret,
+    bool pairing_supported,
+    const protocol::SecretFetchedCallback& secret_fetched_callback) {
+  DVLOG(1) << "TestChromotingClient::FetchSecretFromString Called";
+  secret_fetched_callback.Run(shared_secret);
+}
+
+void TestChromotingClient::FetchThirdPartyToken(
+    const std::string& authorization_token,
+    const std::string& shared_secret,
+    const GURL& token_url,
+    const std::string& host_public_key,
+    const std::string& scope,
+    base::WeakPtr<TokenFetcherProxy> token_fetcher_proxy) {
+  DVLOG(1) << "TestChromotingClient::FetchThirdPartyToken Called";
+  DVLOG(2) << "--token_url: " << token_url;
+  DVLOG(2) << "--host_public_key: " << host_public_key;
+  DVLOG(2) << "--scope: " << scope;
+
+  if (token_fetcher_proxy) {
+    token_fetcher_proxy->OnTokenFetched(authorization_token, shared_secret);
+    token_fetcher_proxy.reset();
+  } else {
+    LOG(ERROR) << "Invalid token fetcher proxy passed in";
+  }
+}
+
+const char* TestChromotingClient::ConnectionStateToFriendlyString(

[Sergey Ulanov, 2015/03/11 00:09:51]

This doesn't need to be a method. Can be a static function. Same for
ProtocolErrorToFriendlyString().

[joedow, 2015/03/11 19:06:39]

Done.

+    protocol::ConnectionToHost::State state) {
+  switch (state) {
+    case protocol::ConnectionToHost::INITIALIZING:
+      return "INITIALIZING";
+
+    case protocol::ConnectionToHost::CONNECTING:
+      return "CONNECTING";
+
+    case protocol::ConnectionToHost::AUTHENTICATED:
+      return "AUTHENTICATED";
+
+    case protocol::ConnectionToHost::CONNECTED:
+      return "CONNECTED";
+
+    case protocol::ConnectionToHost::CLOSED:
+      return "CLOSED";
+
+    case protocol::ConnectionToHost::FAILED:
+      return "FAILED";
+
+    default:
+      LOG(ERROR) << "Unknown connection state: '" << state << "'";
+      return "UNKNOWN";
+  }
+}
+
+const char* TestChromotingClient::ProtocolErrorToFriendlyString(
+    protocol::ErrorCode error_code) {
+  switch (error_code) {
+    case protocol::OK:
+      return "NONE";
+
+    case protocol::PEER_IS_OFFLINE:
+      return "PEER_IS_OFFLINE";
+
+    case protocol::SESSION_REJECTED:
+      return "SESSION_REJECTED";
+
+    case protocol::AUTHENTICATION_FAILED:
+      return "AUTHENTICATION_FAILED";
+
+    case protocol::INCOMPATIBLE_PROTOCOL:
+      return "INCOMPATIBLE_PROTOCOL";
+
+    case protocol::HOST_OVERLOAD:
+      return "HOST_OVERLOAD";
+
+    case protocol::CHANNEL_CONNECTION_ERROR:
+      return "CHANNEL_CONNECTION_ERROR";
+
+    case protocol::SIGNALING_ERROR:
+      return "SIGNALING_ERROR";
+
+    case protocol::SIGNALING_TIMEOUT:
+      return "SIGNALING_TIMEOUT";
+
+    case protocol::UNKNOWN_ERROR:
+      return "UNKNOWN_ERROR";
+
+    default:
+      LOG(ERROR) << "Unrecognized error code: '" << error_code << "'";
+      return "UNKNOWN_ERROR";
+  }
+}
+
+}  // namespace test
+}  // namespace remoting