Restructure chromoting_jni_instance handling of Java strings

This also adds some comments to explain how/why things are being done the way they are.

Review URL: https://chromiumcodereview.appspot.com/18612018

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

3 files changed, 222 insertions, 194 deletions

diff --git a/remoting/client/jni/chromoting_jni_instance.cc b/remoting/client/jni/chromoting_jni_instance.cc
index ba68da8..77fa9c8 100644
--- a/remoting/client/jni/chromoting_jni_instance.cc
+++ b/remoting/client/jni/chromoting_jni_instance.cc
@@ -23,139 +23,91 @@ ChromotingJNIInstance* ChromotingJNIInstance::GetInstance() {
 }
 
 ChromotingJNIInstance::ChromotingJNIInstance()
-    : username_cstr_(NULL),
-      auth_token_cstr_(NULL),
-      host_jid_cstr_(NULL),
-      host_id_cstr_(NULL),
-      host_pubkey_cstr_(NULL),
-      pin_cstr_(NULL) {
+    : connected_(false) {
   JNIEnv* env = base::android::AttachCurrentThread();
 
+  // The base and networks stacks must be registered with JNI in order to work
+  // on Android. An AtExitManager cleans this up at world's end.
   collector_.reset(new base::AtExitManager());
   base::android::RegisterJni(env);
   net::android::RegisterJni(env);
 
-  LOG(INFO) << "starting main message loop";
+  // On Android, the UI thread is managed by Java, so we need to attach and
+  // start a special type of message loop to allow Chromium code to run tasks.
+  LOG(INFO) << "Starting main message loop";
   ui_loop_.reset(new base::MessageLoopForUI());
   ui_loop_->Start();
 
-  LOG(INFO) << "spawning additional threads";
-  ui_runner_ = new AutoThreadTaskRunner(ui_loop_->message_loop_proxy(),
-                                        base::MessageLoop::QuitClosure());
-  net_runner_ = AutoThread::CreateWithType("native_net",
-                                           ui_runner_,
-                                           base::MessageLoop::TYPE_IO);
-  disp_runner_ = AutoThread::CreateWithType("native_disp",
-                                            ui_runner_,
-                                            base::MessageLoop::TYPE_DEFAULT);
+  LOG(INFO) << "Spawning additional threads";
+  // TODO(solb) Stop pretending to control the managed UI thread's lifetime.
+  ui_task_runner_ = new AutoThreadTaskRunner(ui_loop_->message_loop_proxy(),
+                                             base::MessageLoop::QuitClosure());
+  network_task_runner_ = AutoThread::CreateWithType("native_net",
+                                                    ui_task_runner_,
+                                                    base::MessageLoop::TYPE_IO);
+  display_task_runner_ = AutoThread::Create("native_disp",
+                                            ui_task_runner_);
 
-  url_requester_ = new URLRequestContextGetter(ui_runner_, net_runner_);
+  url_requester_ = new URLRequestContextGetter(ui_task_runner_,
+                                               network_task_runner_);
 
   class_ = static_cast<jclass>(env->NewGlobalRef(env->FindClass(JAVA_CLASS)));
 }
 
 ChromotingJNIInstance::~ChromotingJNIInstance() {
+  DCHECK(ui_task_runner_->BelongsToCurrentThread());
+  DCHECK(!connected_);
+
   JNIEnv* env = base::android::AttachCurrentThread();
   env->DeleteGlobalRef(class_);
-  // TODO(solb) detach all threads from JVM
+  // TODO(solb): crbug.com/259594 Detach all threads from JVM here.
 }
 
-void ChromotingJNIInstance::ConnectToHost(jstring username,
-                                          jstring auth_token,
-                                          jstring host_jid,
-                                          jstring host_id,
-                                          jstring host_pubkey) {
-  JNIEnv* env = base::android::AttachCurrentThread();
-
-  username_jstr_ = static_cast<jstring>(env->NewGlobalRef(username));
-  auth_token_jstr_ = static_cast<jstring>(env->NewGlobalRef(auth_token));
-  host_jid_jstr_ = static_cast<jstring>(env->NewGlobalRef(host_jid));
-  host_id_jstr_ = static_cast<jstring>(env->NewGlobalRef(host_id));
-  host_pubkey_jstr_ = static_cast<jstring>(env->NewGlobalRef(host_pubkey));
-
-  username_cstr_ = env->GetStringUTFChars(username_jstr_, NULL);
-  auth_token_cstr_ = env->GetStringUTFChars(auth_token_jstr_, NULL);
-  host_jid_cstr_ = env->GetStringUTFChars(host_jid_jstr_, NULL);
-  host_id_cstr_ = env->GetStringUTFChars(host_id_jstr_, NULL);
-  host_pubkey_cstr_ = env->GetStringUTFChars(host_pubkey_jstr_, NULL);
-
-  // We're a singleton, so Unretained is safe here.
-  disp_runner_->PostTask(FROM_HERE, base::Bind(
+void ChromotingJNIInstance::ConnectToHost(const char* username,
+                                          const char* auth_token,
+                                          const char* host_jid,
+                                          const char* host_id,
+                                          const char* host_pubkey) {
+  DCHECK(ui_task_runner_->BelongsToCurrentThread());
+  DCHECK(!connected_);
+  connected_ = true;
+
+  username_ = username;
+  auth_token_ = auth_token;
+  host_jid_ = host_jid;
+  host_id_ = host_id;
+  host_pubkey_ = host_pubkey;
+
+  display_task_runner_->PostTask(FROM_HERE, base::Bind(
       &ChromotingJNIInstance::ConnectToHostOnDisplayThread,
       base::Unretained(this)));
 }
 
 void ChromotingJNIInstance::DisconnectFromHost() {
-  JNIEnv* env = base::android::AttachCurrentThread();
+  DCHECK(ui_task_runner_->BelongsToCurrentThread());
+  DCHECK(connected_);
+  connected_ = false;
 
-  env->ReleaseStringUTFChars(username_jstr_, username_cstr_);
-  env->ReleaseStringUTFChars(auth_token_jstr_, auth_token_cstr_);
-  env->ReleaseStringUTFChars(host_jid_jstr_, host_jid_cstr_);
-  env->ReleaseStringUTFChars(host_id_jstr_, host_id_cstr_);
-  env->ReleaseStringUTFChars(host_pubkey_jstr_, host_pubkey_cstr_);
-
-  username_cstr_ = NULL;
-  auth_token_cstr_ = NULL;
-  host_jid_cstr_ = NULL;
-  host_id_cstr_ = NULL;
-  host_pubkey_cstr_ = NULL;
-
-  env->DeleteGlobalRef(username_jstr_);
-  env->DeleteGlobalRef(auth_token_jstr_);
-  env->DeleteGlobalRef(host_jid_jstr_);
-  env->DeleteGlobalRef(host_id_jstr_);
-  env->DeleteGlobalRef(host_pubkey_jstr_);
-
-  if (pin_cstr_) {
-    // AuthenticatedWithPin() has been called.
-    env->ReleaseStringUTFChars(pin_jstr_, pin_cstr_);
-    pin_cstr_ = NULL;
-    env->DeleteGlobalRef(pin_jstr_);
-  }
-
-  // We're a singleton, so Unretained is safe here.
-  net_runner_->PostTask(FROM_HERE, base::Bind(
+  network_task_runner_->PostTask(FROM_HERE, base::Bind(
       &ChromotingJNIInstance::DisconnectFromHostOnNetworkThread,
       base::Unretained(this)));
 }
 
-void ChromotingJNIInstance::AuthenticateWithPin(jstring pin) {
-  JNIEnv* env = base::android::AttachCurrentThread();
+void ChromotingJNIInstance::ProvideSecret(const char* pin) {
+  DCHECK(ui_task_runner_->BelongsToCurrentThread());
+  DCHECK(!pin_callback_.is_null());
 
-  pin_jstr_ = static_cast<jstring>(env->NewGlobalRef(pin));
-  pin_cstr_ = env->GetStringUTFChars(pin_jstr_, NULL);
-
-  net_runner_->PostTask(FROM_HERE, base::Bind(announce_secret_, pin_cstr_));
-}
-
-void ChromotingJNIInstance::FetchSecret(
-    bool pairable,
-    const protocol::SecretFetchedCallback& callback_encore) {
-  // All our work must be done on the UI thread.
-  if (!ui_runner_->BelongsToCurrentThread()) {
-    // We're a singleton, so Unretained is safe here.
-    ui_runner_->PostTask(FROM_HERE, base::Bind(
-        &ChromotingJNIInstance::FetchSecret,
-        base::Unretained(this),
-        pairable,
-        callback_encore));
-    return;
-  }
-
-  announce_secret_ = callback_encore;
-  JNIEnv* env = base::android::AttachCurrentThread();
-  env->CallStaticVoidMethod(
-      class_,
-      env->GetStaticMethodID(class_, "displayAuthenticationPrompt", "()V"));
+  // We invoke the string constructor to ensure |pin| gets copied *before* the
+  // asynchronous run, since Java might want it back as soon as we return.
+  network_task_runner_->PostTask(FROM_HERE,
+                                 base::Bind(pin_callback_, std::string(pin)));
 }
 
 void ChromotingJNIInstance::OnConnectionState(
     protocol::ConnectionToHost::State state,
     protocol::ErrorCode error) {
-  // All our work must be done on the UI thread.
-  if (!ui_runner_->BelongsToCurrentThread()) {
-    // We're a singleton, so Unretained is safe here.
-    ui_runner_->PostTask(FROM_HERE, base::Bind(
+  if (!ui_task_runner_->BelongsToCurrentThread()) {
+    ui_task_runner_->PostTask(FROM_HERE, base::Bind(
         &ChromotingJNIInstance::OnConnectionState,
         base::Unretained(this),
         state,
@@ -172,12 +124,15 @@ void ChromotingJNIInstance::OnConnectionState(
 }
 
 void ChromotingJNIInstance::OnConnectionReady(bool ready) {
+  // We ignore this message, since OnConnectionState() tells us the same thing.
 }
 
 void ChromotingJNIInstance::SetCapabilities(const std::string& capabilities) {}
 
 void ChromotingJNIInstance::SetPairingResponse(
-    const protocol::PairingResponse& response) {}
+    const protocol::PairingResponse& response) {
+  NOTIMPLEMENTED();
+}
 
 protocol::ClipboardStub* ChromotingJNIInstance::GetClipboardStub() {
   NOTIMPLEMENTED();
@@ -189,50 +144,44 @@ protocol::CursorShapeStub* ChromotingJNIInstance::GetCursorShapeStub() {
   return NULL;
 }
 
-// We don't use NOTIMPLEMENTED() here because NegotiatingClientAuthenticator
-// calls this even if it doesn't use the configuration method, and we don't
-// want to print an error on every run.
 scoped_ptr<protocol::ThirdPartyClientAuthenticator::TokenFetcher>
     ChromotingJNIInstance::GetTokenFetcher(const std::string& host_public_key) {
-  LOG(INFO) << "ChromotingJNIInstance::GetTokenFetcher(...) [unimplemented]";
+  // Return null to indicate that third-party authentication is unsupported.
   return scoped_ptr<protocol::ThirdPartyClientAuthenticator::TokenFetcher>();
 }
 
 void ChromotingJNIInstance::ConnectToHostOnDisplayThread() {
-  DCHECK(disp_runner_->BelongsToCurrentThread());
+  DCHECK(display_task_runner_->BelongsToCurrentThread());
 
-  if (!frames_.get()) {
-    frames_ = new FrameConsumerProxy(disp_runner_);
+  if (!frame_consumer_.get()) {
+    frame_consumer_ = new FrameConsumerProxy(display_task_runner_);
     // TODO(solb) Instantiate some FrameConsumer implementation and attach it.
   }
 
-  // We're a singleton, so Unretained is safe here.
-  net_runner_->PostTask(FROM_HERE, base::Bind(
+  network_task_runner_->PostTask(FROM_HERE, base::Bind(
       &ChromotingJNIInstance::ConnectToHostOnNetworkThread,
       base::Unretained(this)));
 }
 
 void ChromotingJNIInstance::ConnectToHostOnNetworkThread() {
-  DCHECK(net_runner_->BelongsToCurrentThread());
+  DCHECK(network_task_runner_->BelongsToCurrentThread());
 
   client_config_.reset(new ClientConfig());
-  client_config_->host_jid = host_jid_cstr_;
-  client_config_->host_public_key = host_pubkey_cstr_;
-  // We're a singleton, so Unretained is safe here.
+  client_config_->host_jid = host_jid_;
+  client_config_->host_public_key = host_pubkey_;
+
   client_config_->fetch_secret_callback = base::Bind(
       &ChromotingJNIInstance::FetchSecret,
       base::Unretained(this));
-  client_config_->authentication_tag = host_id_cstr_;
+  client_config_->authentication_tag = host_id_;
 
   // TODO(solb) Move these hardcoded values elsewhere:
   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"));
 
-  client_context_.reset(new ClientContext(net_runner_.get()));
+  client_context_.reset(new ClientContext(network_task_runner_.get()));
   client_context_->Start();
 
   connection_.reset(new protocol::ConnectionToHost(true));
@@ -241,37 +190,66 @@ void ChromotingJNIInstance::ConnectToHostOnNetworkThread() {
                                      client_context_.get(),
                                      connection_.get(),
                                      this,
-                                     frames_,
+                                     frame_consumer_,
                                      scoped_ptr<AudioPlayer>()));
 
-  chat_config_.reset(new XmppSignalStrategy::XmppServerConfig());
-  chat_config_->host = CHAT_SERVER;
-  chat_config_->port = CHAT_PORT;
-  chat_config_->use_tls = CHAT_USE_TLS;
+  signaling_config_.reset(new XmppSignalStrategy::XmppServerConfig());
+  signaling_config_->host = CHAT_SERVER;
+  signaling_config_->port = CHAT_PORT;
+  signaling_config_->use_tls = CHAT_USE_TLS;
 
-  chat_.reset(new XmppSignalStrategy(url_requester_,
-                                     username_cstr_,
-                                     auth_token_cstr_,
-                                     CHAT_AUTH_METHOD,
-                                     *chat_config_));
+  signaling_.reset(new XmppSignalStrategy(url_requester_,
+                                          username_,
+                                          auth_token_,
+                                          CHAT_AUTH_METHOD,
+                                          *signaling_config_));
 
-  netset_.reset(new NetworkSettings(NetworkSettings::NAT_TRAVERSAL_OUTGOING));
+  network_settings_.reset(new NetworkSettings(
+      NetworkSettings::NAT_TRAVERSAL_OUTGOING));
   scoped_ptr<protocol::TransportFactory> fact(
-      protocol::LibjingleTransportFactory::Create(*netset_, url_requester_));
+      protocol::LibjingleTransportFactory::Create(*network_settings_,
+                                                  url_requester_));
 
-  client_->Start(chat_.get(), fact.Pass());
+  client_->Start(signaling_.get(), fact.Pass());
 }
 
 void ChromotingJNIInstance::DisconnectFromHostOnNetworkThread() {
-  DCHECK(net_runner_->BelongsToCurrentThread());
+  DCHECK(network_task_runner_->BelongsToCurrentThread());
 
+  username_ = "";
+  auth_token_ = "";
+  host_jid_ = "";
+  host_id_ = "";
+  host_pubkey_ = "";
+
+  // |client_| must be torn down before |signaling_|.
+  pin_callback_.Reset();
   client_.reset();
   connection_.reset();
   client_context_.reset();
   client_config_.reset();
-  chat_.reset();  // This object must outlive client_.
-  chat_config_.reset();  // TODO(solb) Restructure to reuse between sessions.
-  netset_.reset();
+  signaling_.reset();
+  signaling_config_.reset();
+  network_settings_.reset();
+}
+
+void ChromotingJNIInstance::FetchSecret(
+    bool pairable,
+    const protocol::SecretFetchedCallback& callback) {
+  if (!ui_task_runner_->BelongsToCurrentThread()) {
+    ui_task_runner_->PostTask(FROM_HERE, base::Bind(
+        &ChromotingJNIInstance::FetchSecret,
+        base::Unretained(this),
+        pairable,
+        callback));
+    return;
+  }
+
+  pin_callback_ = callback;
+  JNIEnv* env = base::android::AttachCurrentThread();
+  env->CallStaticVoidMethod(
+      class_,
+      env->GetStaticMethodID(class_, "displayAuthenticationPrompt", "()V"));
 }
 
 }  // namespace remoting
diff --git a/remoting/client/jni/chromoting_jni_instance.h b/remoting/client/jni/chromoting_jni_instance.h
index 75ef56b..05ee17f 100644
--- a/remoting/client/jni/chromoting_jni_instance.h
+++ b/remoting/client/jni/chromoting_jni_instance.h
@@ -36,31 +36,35 @@ const int CHAT_PORT = 5222;
 const bool CHAT_USE_TLS = true;
 const char* const CHAT_AUTH_METHOD = "oauth2";
 
-// ClientUserInterface that makes and (indirectly) receives JNI calls.
+// ClientUserInterface that makes and (indirectly) receives JNI calls. It also
+// contains global resources on which the Chromoting components run
+// (e.g. message loops and task runners).
 class ChromotingJNIInstance : public ClientUserInterface {
  public:
+  // This class is instantiated at process initialization and persists until
+  // we close. It reuses many of its components between connections (i.e. when
+  // a DisconnectFromHost() call is followed by a ConnectToHost() one.
   static ChromotingJNIInstance* GetInstance();
 
-  // Call from UI thread.
+  // Initiates a connection with the specified host. This may only be called
+  // when |connected_| is false, and must be invoked on the UI thread.
   void ConnectToHost(
-      jstring username,
-      jstring auth_token,
-      jstring host_jid,
-      jstring host_id,
-      jstring host_pubkey);
-
-  // Call from UI thread.
+      const char* username,
+      const char* auth_token,
+      const char* host_jid,
+      const char* host_id,
+      const char* host_pubkey);
+
+  // Terminates the current connection (if it hasn't already failed) and clean
+  // up. This may only be called when |connected_|, and only from the UI thread.
   void DisconnectFromHost();
 
-  // Call from UI thread.
-  void AuthenticateWithPin(jstring pin);
-
-  // Called by client authenticator.
-  // Gets notified if the user needs to enter a PIN, and notifies Java in turn.
-  void FetchSecret(bool pairable,
-                   const protocol::SecretFetchedCallback& callback_encore);
+  // Provides the user's PIN and resumes the host authentication attempt. Call
+  // on the UI thread once the user has finished entering this PIN into the UI,
+  // but only after the UI has been asked to provide a PIN (via FetchSecret()).
+  void ProvideSecret(const char* pin);
 
-  // ClientUserInterface implementation:
+  // ClientUserInterface implementation.
   virtual void OnConnectionState(
       protocol::ConnectionToHost::State state,
       protocol::ErrorCode error) OVERRIDE;
@@ -75,6 +79,12 @@ class ChromotingJNIInstance : public ClientUserInterface {
 
  private:
   ChromotingJNIInstance();
+
+  // Any existing or attempted connection must have been terminated using
+  // DisconnectFromHost() before this singleton is destroyed. Because
+  // destruction only occurs at application exit after all connections have
+  // terminated, it is safe to make unretained cross-thread calls on the class.
+  // As a singleton, this object must be destroyed on the main (UI) thread.
   virtual ~ChromotingJNIInstance();
 
   void ConnectToHostOnDisplayThread();
@@ -82,41 +92,61 @@ class ChromotingJNIInstance : public ClientUserInterface {
 
   void DisconnectFromHostOnNetworkThread();
 
-  // Reusable between sessions:
-  jclass class_;  // Reference to the Java class into which we make JNI calls.
+  // Notifies the user interface that the user needs to enter a PIN. The
+  // current authentication attempt is put on hold until |callback| is invoked.
+  void FetchSecret(bool pairable,
+                   const protocol::SecretFetchedCallback& callback);
+
+  // The below variables are reused across consecutive sessions.
+
+  // Reference to the Java class into which we make JNI calls.
+  jclass class_;
+
+  // Used by the Chromium libraries to clean up the base and net libraries' JNI
+  // bindings. It must persist for the lifetime of the singleton.
   scoped_ptr<base::AtExitManager> collector_;
+
+  // Chromium code's connection to the Java message loop.
   scoped_ptr<base::MessageLoopForUI> ui_loop_;
-  scoped_refptr<AutoThreadTaskRunner> ui_runner_;
-  scoped_refptr<AutoThreadTaskRunner> net_runner_;
-  scoped_refptr<AutoThreadTaskRunner> disp_runner_;
+
+  // Runners that allow posting tasks to the various native threads.
+  scoped_refptr<AutoThreadTaskRunner> ui_task_runner_;
+  scoped_refptr<AutoThreadTaskRunner> network_task_runner_;
+  scoped_refptr<AutoThreadTaskRunner> display_task_runner_;
+
   scoped_refptr<net::URLRequestContextGetter> url_requester_;
-  scoped_refptr<FrameConsumerProxy> frames_;
+  scoped_refptr<FrameConsumerProxy> frame_consumer_;
+
+  // All below variables are specific to each connection.
+
+  // True iff ConnectToHost() has been called without a subsequent
+  // call to DisconnectFromHost() (i.e. while connecting, once connected, and
+  // between the time a connection fails and DisconnectFromHost() is called).
+  // To be used on the UI thread.
+  bool connected_;
 
-  // Specific to each session:
+  // This group of variables is to be used on the network thread.
   scoped_ptr<ClientConfig> client_config_;
   scoped_ptr<ClientContext> client_context_;
   scoped_ptr<protocol::ConnectionToHost> connection_;
   scoped_ptr<ChromotingClient> client_;
-  scoped_ptr<XmppSignalStrategy::XmppServerConfig> chat_config_;
-  scoped_ptr<XmppSignalStrategy> chat_;  // must outlive client_
-  scoped_ptr<NetworkSettings> netset_;
-  protocol::SecretFetchedCallback announce_secret_;
-
-  // Java string handles:
-  jstring username_jstr_;
-  jstring auth_token_jstr_;
-  jstring host_jid_jstr_;
-  jstring host_id_jstr_;
-  jstring host_pubkey_jstr_;
-  jstring pin_jstr_;
-
-  // C string pointers:
-  const char* username_cstr_;
-  const char* auth_token_cstr_;
-  const char* host_jid_cstr_;
-  const char* host_id_cstr_;
-  const char* host_pubkey_cstr_;
-  const char* pin_cstr_;
+  scoped_ptr<XmppSignalStrategy::XmppServerConfig> signaling_config_;
+  scoped_ptr<XmppSignalStrategy> signaling_;  // Must outlive client_
+  scoped_ptr<NetworkSettings> network_settings_;
+
+  // Pass this the user's PIN once we have it. To be assigned and accessed on
+  // the UI thread, but must be posted to the network thread to call it.
+  protocol::SecretFetchedCallback pin_callback_;
+
+  // These strings describe the current connection, and are not reused. They
+  // are initialized in ConnectionToHost(), but thereafter are only to be used
+  // on the network thread. (This is safe because ConnectionToHost()'s finishes
+  // using them on the UI thread before they are ever touched from network.)
+  std::string username_;
+  std::string auth_token_;
+  std::string host_jid_;
+  std::string host_id_;
+  std::string host_pubkey_;
 
   friend struct DefaultSingletonTraits<ChromotingJNIInstance>;
 
diff --git a/remoting/client/jni/jni_interface.cc b/remoting/client/jni/jni_interface.cc
index 205b05b..b78ff05 100644
--- a/remoting/client/jni/jni_interface.cc
+++ b/remoting/client/jni/jni_interface.cc
@@ -40,7 +40,8 @@ JNIEXPORT void JNICALL JNI_IMPLEMENTATION(loadNative)(JNIEnv* env,
   // them from DCHECKing out when they go looking.
   CommandLine::Init(0, NULL);
 
-  remoting::ChromotingJNIInstance::GetInstance(); // Initialize threads now.
+  // Create the singleton now so that the Chromoting threads will be set up.
+  remoting::ChromotingJNIInstance::GetInstance();
 }
 
 JNIEXPORT jstring JNICALL JNI_IMPLEMENTATION(getApiKey)(JNIEnv* env,
@@ -60,18 +61,32 @@ JNIEXPORT jstring JNICALL JNI_IMPLEMENTATION(getClientSecret)(JNIEnv* env,
       google_apis::GetOAuth2ClientSecret(google_apis::CLIENT_REMOTING).c_str());
 }
 
-JNIEXPORT void JNICALL JNI_IMPLEMENTATION(connectNative)(JNIEnv* env,
-                                                         jobject that,
-                                                         jstring username,
-                                                         jstring auth_token,
-                                                         jstring host_jid,
-                                                         jstring host_id,
-                                                         jstring host_pubkey) {
-  remoting::ChromotingJNIInstance::GetInstance()->ConnectToHost(username,
-                                                                auth_token,
-                                                                host_jid,
-                                                                host_id,
-                                                                host_pubkey);
+JNIEXPORT void JNICALL JNI_IMPLEMENTATION(connectNative)(
+    JNIEnv* env,
+    jobject that,
+    jstring username_jstr,
+    jstring auth_token_jstr,
+    jstring host_jid_jstr,
+    jstring host_id_jstr,
+    jstring host_pubkey_jstr) {
+  const char* username_cstr = env->GetStringUTFChars(username_jstr, NULL);
+  const char* auth_token_cstr = env->GetStringUTFChars(auth_token_jstr, NULL);
+  const char* host_jid_cstr = env->GetStringUTFChars(host_jid_jstr, NULL);
+  const char* host_id_cstr = env->GetStringUTFChars(host_id_jstr, NULL);
+  const char* host_pubkey_cstr = env->GetStringUTFChars(host_pubkey_jstr, NULL);
+
+  remoting::ChromotingJNIInstance::GetInstance()->ConnectToHost(
+      username_cstr,
+      auth_token_cstr,
+      host_jid_cstr,
+      host_id_cstr,
+      host_pubkey_cstr);
+
+  env->ReleaseStringUTFChars(username_jstr, username_cstr);
+  env->ReleaseStringUTFChars(auth_token_jstr, auth_token_cstr);
+  env->ReleaseStringUTFChars(host_jid_jstr, host_jid_cstr);
+  env->ReleaseStringUTFChars(host_id_jstr, host_id_cstr);
+  env->ReleaseStringUTFChars(host_pubkey_jstr, host_pubkey_cstr);
 }
 
 JNIEXPORT void JNICALL JNI_IMPLEMENTATION(disconnectNative)(JNIEnv* env,
@@ -79,10 +94,15 @@ JNIEXPORT void JNICALL JNI_IMPLEMENTATION(disconnectNative)(JNIEnv* env,
   remoting::ChromotingJNIInstance::GetInstance()->DisconnectFromHost();
 }
 
-JNIEXPORT void JNICALL JNI_IMPLEMENTATION(authenticationResponse)(JNIEnv* env,
-                                                                  jobject that,
-                                                                  jstring pin) {
-  remoting::ChromotingJNIInstance::GetInstance()->AuthenticateWithPin(pin);
+JNIEXPORT void JNICALL JNI_IMPLEMENTATION(authenticationResponse)(
+    JNIEnv* env,
+    jobject that,
+    jstring pin_jstr) {
+  const char* pin_cstr = env->GetStringUTFChars(pin_jstr, NULL);
+
+  remoting::ChromotingJNIInstance::GetInstance()->ProvideSecret(pin_cstr);
+
+  env->ReleaseStringUTFChars(pin_jstr, pin_cstr);
 }
 
 }  // extern "C"