Support for connecting to localhost on the chromoting test driver.

remoting/test/test_chromoting_client.cc

 // 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/strings/string_split.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"

[joedow, 2015/07/16 03:23:19]

This header seems like it is not needed

[tonychun, 2015/07/16 23:12:08]

Done.

 #include "remoting/test/test_video_renderer.h"
 
 namespace {
-
-const char kAppRemotingCapabilities[] =
-    "rateLimitResizeRequests desktopShape sendInitialResolution googleDrive";
-
 const char kXmppHostName[] = "talk.google.com";
 const int kXmppPortNumber = 5222;
 
 // Used as the TokenFetcherCallback for App Remoting sessions.
 void 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<remoting::TokenFetcherProxy> token_fetcher_proxy) {
   VLOG(2)  << "FetchThirdPartyToken("
            << "token_url: " << token_url << ", "
            << "host_public_key: " << host_public_key << ", "
            << "scope: " << scope << ") Called";
 
   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";
   }
 }
 
+void FetchSecret(
+    const std::string& client_secret,
+    bool pairing_supported,

[joedow, 2015/07/16 03:23:19]

is pairing_supported needed?  You bind true to it but don't use it.

[tonychun, 2015/07/16 23:12:08]

Removed the unused parameter.

+    bool pairing_expected,
+    const remoting::protocol::SecretFetchedCallback& secret_fetched_callback) {
+  secret_fetched_callback.Run(client_secret);
+}
+
 const char* ConnectionStateToFriendlyString(
     remoting::protocol::ConnectionToHost::State state) {
   switch (state) {
     case remoting::protocol::ConnectionToHost::INITIALIZING:
       return "INITIALIZING";
 
     case remoting::protocol::ConnectionToHost::CONNECTING:
       return "CONNECTING";
 
     case remoting::protocol::ConnectionToHost::AUTHENTICATED:
@@ skipping 69 matching lines @@
   TestChromotingClient();
 }
 
 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());
-
+    const ConnectionInfo& connection_info) {
   // 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()));
 
   // Check to see if the user passed in a customized video renderer.
   if (!video_renderer_) {
     video_renderer_.reset(new TestVideoRenderer());
   }
 
   chromoting_client_.reset(new ChromotingClient(client_context_.get(),
                                                 this,  // client_user_interface.
                                                 video_renderer_.get(),
                                                 nullptr));  // audio_player
 
   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;
+  xmpp_server_config.username = connection_info.user_name;
+  xmpp_server_config.auth_token = connection_info.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,
           protocol::TransportRole::CLIENT));
 
   scoped_ptr<protocol::ThirdPartyClientAuthenticator::TokenFetcher>
       token_fetcher(new TokenFetcherProxy(
-          base::Bind(&FetchThirdPartyToken, remote_host_info.authorization_code,
-                     remote_host_info.shared_secret),
-          "FAKE_HOST_PUBLIC_KEY"));
+          base::Bind(&FetchThirdPartyToken,
+                connection_info.authorization_code,

[joedow, 2015/07/16 03:23:19]

indentation seems off here, either 4 spaces or line up with the ampersand on
FetchThirdPartyToken

[tonychun, 2015/07/16 23:12:08]

Done.

+                connection_info.shared_secret),
+          connection_info.public_key));
 
+  // Parse |auth_methods_str| for authentication methods.

[joedow, 2015/07/16 03:23:19]

Why build this here?  This feels like something that could be built when
generating the ConnectionInfo struct, then you wouldn't need the string parsing
here, you could just create the vector there and assign it here.

[tonychun, 2015/07/16 23:12:08]

Done.

+  std::vector<std::string> parts;
+  base::SplitString(connection_info.auth_methods_str, ',', &parts);
   std::vector<protocol::AuthenticationMethod> auth_methods;
-  auth_methods.push_back(protocol::AuthenticationMethod::ThirdParty());
+  for (std::string part : parts) {
+    protocol::AuthenticationMethod auth_method =
+        protocol::AuthenticationMethod::FromString(part);
+    if (auth_method.is_valid()) {
+      auth_methods.push_back(auth_method);
+    }
+  }
 
-  // FetchSecretCallback is used for PIN based auth which we aren't using so we
-  // can pass a null callback here.
   protocol::FetchSecretCallback fetch_secret_callback;
+  if (!connection_info.pin.empty()) {
+    fetch_secret_callback = base::Bind(&FetchSecret, connection_info.pin, true);
+  }
   scoped_ptr<protocol::Authenticator> authenticator(
       new protocol::NegotiatingClientAuthenticator(
-          std::string(),  // client_pairing_id
-          std::string(),  // shared_secret
-          std::string(),  // authentication_tag
+          std::string(),            // client_pairing_id
+          std::string(),            // shared_secret

[joedow, 2015/07/16 03:23:19]

you can pass the connection_info members for pairing_id and shared_secret here
can't you?  They are just going to be empty strings plus this function will
support other connection methods as long as the info struct is set up correctly.

[tonychun, 2015/07/16 23:12:08]

I've added a new member in ConnectionSetupInfo (newly named) called pairing_id.

+          connection_info.host_id,  // authentication_tag
           fetch_secret_callback, token_fetcher.Pass(), auth_methods));
 
   chromoting_client_->Start(signal_strategy_.get(), authenticator.Pass(),
-                            transport_factory.Pass(), remote_host_info.host_jid,
-                            kAppRemotingCapabilities);
+                            transport_factory.Pass(), connection_info.host_jid,
+                            connection_info.capabilities);
 }
 
 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 will be updated when the chromoting client was
@@ skipping 104 matching lines @@
   VLOG(1) << "TestChromotingClient::InjectClipboardEvent() Called";
 }
 
 void TestChromotingClient::SetCursorShape(
     const protocol::CursorShapeInfo& cursor_shape) {
   VLOG(1) << "TestChromotingClient::SetCursorShape() Called";
 }
 
 }  // namespace test
 }  // namespace remoting