Cleanups in ChromotingClient

1. Moved creation of ConnectionToHost to ChromotingClient. Previously
   ConnectionToHost had to be passed in the constructor.
2. Authentication now is created outside of ChromotingClient. This
   allowed to remove ClientConfig struct (it was used mostly to
   pass authentication parameters). This will also allow passing
   a fake authenticator for tests.
3. Removed ClientConfig.

Review URL: https://codereview.chromium.org/384523003

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@282757 0039d316-1c4b-4281-b951-d872f2087c98

3 files changed, 36 insertions, 52 deletions

diff --git a/remoting/client/jni/chromoting_jni_instance.cc b/remoting/client/jni/chromoting_jni_instance.cc
index 54b8588..8b63f3d2 100644
--- a/remoting/client/jni/chromoting_jni_instance.cc
+++ b/remoting/client/jni/chromoting_jni_instance.cc
@@ -23,6 +23,7 @@
 #include "remoting/jingle_glue/server_log_entry.h"
 #include "remoting/protocol/host_stub.h"
 #include "remoting/protocol/libjingle_transport_factory.h"
+#include "remoting/protocol/negotiating_client_authenticator.h"
 
 namespace remoting {
 
@@ -48,6 +49,7 @@ ChromotingJniInstance::ChromotingJniInstance(ChromotingJniRuntime* jni_runtime,
                                              const char* pairing_secret)
     : jni_runtime_(jni_runtime),
       host_id_(host_id),
+      host_jid_(host_jid),
       create_pairing_(false),
       stats_logging_enabled_(false),
       weak_factory_(this) {
@@ -61,23 +63,24 @@ ChromotingJniInstance::ChromotingJniInstance(ChromotingJniRuntime* jni_runtime,
   xmpp_config_.auth_token = auth_token;
   xmpp_config_.auth_service = "oauth2";
 
-  // Initialize ClientConfig.
-  client_config_.host_jid = host_jid;
-  client_config_.host_public_key = host_pubkey;
-
-  client_config_.fetch_secret_callback =
-      base::Bind(&ChromotingJniInstance::FetchSecret, this);
-  client_config_.authentication_tag = host_id_;
+  // Initialize |authenticator_|.
+  scoped_ptr<protocol::ThirdPartyClientAuthenticator::TokenFetcher>
+      token_fetcher(new TokenFetcherProxy(
+          base::Bind(&ChromotingJniInstance::FetchThirdPartyToken,
+                     weak_factory_.GetWeakPtr()),
+          host_pubkey));
 
-  client_config_.client_pairing_id = pairing_id;
-  client_config_.client_paired_secret = pairing_secret;
+  std::vector<protocol::AuthenticationMethod> auth_methods;
+  auth_methods.push_back(protocol::AuthenticationMethod::Spake2Pair());
+  auth_methods.push_back(protocol::AuthenticationMethod::Spake2(
+      protocol::AuthenticationMethod::HMAC_SHA256));
+  auth_methods.push_back(protocol::AuthenticationMethod::Spake2(
+      protocol::AuthenticationMethod::NONE));
 
-  client_config_.authentication_methods.push_back(
-      protocol::AuthenticationMethod::FromString("spake2_pair"));
-  client_config_.authentication_methods.push_back(
-      protocol::AuthenticationMethod::FromString("spake2_hmac"));
-  client_config_.authentication_methods.push_back(
-      protocol::AuthenticationMethod::FromString("spake2_plain"));
+  authenticator_.reset(new protocol::NegotiatingClientAuthenticator(
+      pairing_id, pairing_secret, host_id_,
+      base::Bind(&ChromotingJniInstance::FetchSecret, this),
+      token_fetcher.Pass(), auth_methods));
 
   // Post a task to start connection
   jni_runtime_->display_task_runner()->PostTask(
@@ -193,7 +196,7 @@ void ChromotingJniInstance::SendMouseEvent(
   if (button != protocol::MouseEvent::BUTTON_UNDEFINED)
     event.set_button_down(button_down);
 
-  connection_->input_stub()->InjectMouseEvent(event);
+  client_->input_stub()->InjectMouseEvent(event);
 }
 
 void ChromotingJniInstance::SendMouseWheelEvent(int delta_x, int delta_y) {
@@ -208,7 +211,7 @@ void ChromotingJniInstance::SendMouseWheelEvent(int delta_x, int delta_y) {
   protocol::MouseEvent event;
   event.set_wheel_delta_x(delta_x);
   event.set_wheel_delta_y(delta_y);
-  connection_->input_stub()->InjectMouseEvent(event);
+  client_->input_stub()->InjectMouseEvent(event);
 }
 
 bool ChromotingJniInstance::SendKeyEvent(int key_code, bool key_down) {
@@ -232,7 +235,7 @@ void ChromotingJniInstance::SendTextEvent(const std::string& text) {
 
   protocol::TextEvent event;
   event.set_text(text);
-  connection_->input_stub()->InjectTextEvent(event);
+  client_->input_stub()->InjectTextEvent(event);
 }
 
 void ChromotingJniInstance::RecordPaintTime(int64 paint_time_ms) {
@@ -260,7 +263,7 @@ void ChromotingJniInstance::OnConnectionState(
     protocol::PairingRequest request;
     DCHECK(!device_name_.empty());
     request.set_client_name(device_name_);
-    connection_->host_stub()->RequestPairing(request);
+    client_->host_stub()->RequestPairing(request);
   }
 
   jni_runtime_->ui_task_runner()->PostTask(
@@ -310,15 +313,6 @@ protocol::CursorShapeStub* ChromotingJniInstance::GetCursorShapeStub() {
   return this;
 }
 
-scoped_ptr<protocol::ThirdPartyClientAuthenticator::TokenFetcher>
-    ChromotingJniInstance::GetTokenFetcher(const std::string& host_public_key) {
-  return scoped_ptr<protocol::ThirdPartyClientAuthenticator::TokenFetcher>(
-      new TokenFetcherProxy(
-          base::Bind(&ChromotingJniInstance::FetchThirdPartyToken,
-                     weak_factory_.GetWeakPtr()),
-          host_public_key));
-}
-
 void ChromotingJniInstance::InjectClipboardEvent(
     const protocol::ClipboardEvent& event) {
   NOTIMPLEMENTED();
@@ -360,8 +354,6 @@ void ChromotingJniInstance::ConnectToHostOnNetworkThread() {
       jni_runtime_->network_task_runner().get()));
   client_context_->Start();
 
-  connection_.reset(new protocol::ConnectionToHost(true));
-
   SoftwareVideoRenderer* renderer =
       new SoftwareVideoRenderer(client_context_->main_task_runner(),
                                 client_context_->decode_task_runner(),
@@ -369,10 +361,10 @@ void ChromotingJniInstance::ConnectToHostOnNetworkThread() {
   view_->set_frame_producer(renderer);
   video_renderer_.reset(renderer);
 
-  client_.reset(new ChromotingClient(
-      client_config_, client_context_.get(), connection_.get(),
-      this, video_renderer_.get(), scoped_ptr<AudioPlayer>()));
-
+  client_.reset(new ChromotingClient(client_context_.get(),
+                                     this,
+                                     video_renderer_.get(),
+                                     scoped_ptr<AudioPlayer>()));
 
   signaling_.reset(new XmppSignalStrategy(
       net::ClientSocketFactory::GetDefaultFactory(),
@@ -396,7 +388,8 @@ void ChromotingJniInstance::ConnectToHostOnNetworkThread() {
           port_allocator.PassAs<cricket::HttpPortAllocatorBase>(),
           network_settings));
 
-  client_->Start(signaling_.get(), transport_factory.Pass());
+  client_->Start(signaling_.get(), authenticator_.Pass(),
+                 transport_factory.Pass(), host_jid_, std::string());
 }
 
 void ChromotingJniInstance::DisconnectFromHostOnNetworkThread() {
@@ -407,7 +400,7 @@ void ChromotingJniInstance::DisconnectFromHostOnNetworkThread() {
   stats_logging_enabled_ = false;
 
   // |client_| must be torn down before |signaling_|.
-  connection_.reset();
+  client_.reset();
   client_.reset();
   client_status_logger_.reset();
 }
@@ -422,11 +415,8 @@ void ChromotingJniInstance::FetchSecret(
     return;
   }
 
-  if (!client_config_.client_pairing_id.empty()) {
-    // We attempted to connect using an existing pairing that was rejected.
-    // Unless we forget about the stale credentials, we'll continue trying them.
-    jni_runtime_->CommitPairingCredentials(host_id_, "", "");
-  }
+  // Delete pairing credentials if they exist.
+  jni_runtime_->CommitPairingCredentials(host_id_, "", "");
 
   pin_callback_ = callback;
   jni_runtime_->DisplayAuthenticationPrompt(pairable);
@@ -456,7 +446,7 @@ void ChromotingJniInstance::SendKeyEventInternal(int usb_key_code,
   protocol::KeyEvent event;
   event.set_usb_keycode(usb_key_code);
   event.set_pressed(key_down);
-  connection_->input_stub()->InjectKeyEvent(event);
+  client_->input_stub()->InjectKeyEvent(event);
 }
 
 void ChromotingJniInstance::EnableStatsLogging(bool enabled) {
diff --git a/remoting/client/jni/chromoting_jni_instance.h b/remoting/client/jni/chromoting_jni_instance.h
index 65ffee7..f7067f0 100644
--- a/remoting/client/jni/chromoting_jni_instance.h
+++ b/remoting/client/jni/chromoting_jni_instance.h
@@ -12,14 +12,12 @@
 #include "base/memory/weak_ptr.h"
 #include "base/message_loop/message_loop.h"
 #include "remoting/client/chromoting_client.h"
-#include "remoting/client/client_config.h"
 #include "remoting/client/client_context.h"
 #include "remoting/client/client_user_interface.h"
 #include "remoting/client/frame_consumer_proxy.h"
 #include "remoting/client/jni/jni_frame_consumer.h"
 #include "remoting/jingle_glue/xmpp_signal_strategy.h"
 #include "remoting/protocol/clipboard_stub.h"
-#include "remoting/protocol/connection_to_host.h"
 #include "remoting/protocol/cursor_shape_stub.h"
 
 namespace remoting {
@@ -106,8 +104,6 @@ class ChromotingJniInstance
       const protocol::ExtensionMessage& message) OVERRIDE;
   virtual protocol::ClipboardStub* GetClipboardStub() OVERRIDE;
   virtual protocol::CursorShapeStub* GetCursorShapeStub() OVERRIDE;
-  virtual scoped_ptr<protocol::ThirdPartyClientAuthenticator::TokenFetcher>
-      GetTokenFetcher(const std::string& host_public_key) OVERRIDE;
 
   // CursorShapeStub implementation.
   virtual void InjectClipboardEvent(
@@ -149,6 +145,7 @@ class ChromotingJniInstance
 
   // ID of the host we are connecting to.
   std::string host_id_;
+  std::string host_jid_;
 
   // This group of variables is to be used on the display thread.
   scoped_refptr<FrameConsumerProxy> frame_consumer_;
@@ -156,10 +153,9 @@ class ChromotingJniInstance
   scoped_ptr<base::WeakPtrFactory<JniFrameConsumer> > view_weak_factory_;
 
   // This group of variables is to be used on the network thread.
-  ClientConfig client_config_;
   scoped_ptr<ClientContext> client_context_;
   scoped_ptr<VideoRenderer> video_renderer_;
-  scoped_ptr<protocol::ConnectionToHost> connection_;
+  scoped_ptr<protocol::Authenticator> authenticator_;
   scoped_ptr<ChromotingClient> client_;
   XmppSignalStrategy::XmppServerConfig xmpp_config_;
   scoped_ptr<XmppSignalStrategy> signaling_;  // Must outlive client_
diff --git a/remoting/client/jni/chromoting_jni_runtime.cc b/remoting/client/jni/chromoting_jni_runtime.cc
index c271973..a4df219 100644
--- a/remoting/client/jni/chromoting_jni_runtime.cc
+++ b/remoting/client/jni/chromoting_jni_runtime.cc
@@ -258,10 +258,8 @@ void ChromotingJniRuntime::CommitPairingCredentials(const std::string& host,
 
   JNIEnv* env = base::android::AttachCurrentThread();
   ScopedJavaLocalRef<jstring> j_host = ConvertUTF8ToJavaString(env, host);
-  ScopedJavaLocalRef<jbyteArray> j_id = ToJavaByteArray(
-      env, reinterpret_cast<const uint8*>(id.data()), id.size());
-  ScopedJavaLocalRef<jbyteArray> j_secret = ToJavaByteArray(
-      env, reinterpret_cast<const uint8*>(secret.data()), secret.size());
+  ScopedJavaLocalRef<jstring> j_id = ConvertUTF8ToJavaString(env, id);
+  ScopedJavaLocalRef<jstring> j_secret = ConvertUTF8ToJavaString(env,secret);
 
   Java_JniInterface_commitPairingCredentials(
       env, j_host.obj(), j_id.obj(), j_secret.obj());