Chromoting: Synchronize connected/disconnected state between Java/C++

If the connection was dropped (network error or similar), the session
was not being cleaned up straight away. Instead, the cleanup was
delayed until the next time Java tried to connect, by which time it
had already popped open the progress dialog. This raised an
incorrect disconnection notification that closed the progress
dialog.

This CL ensures that disconnection causes an immediate notification to
Java, and the state is kept consistent between Java and C++ code.

BUG=395301

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

Cr-Commit-Position: refs/heads/master@{#288967}
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@288967 0039d316-1c4b-4281-b951-d872f2087c98

5 files changed, 47 insertions, 15 deletions

diff --git a/remoting/android/java/src/org/chromium/chromoting/Chromoting.java b/remoting/android/java/src/org/chromium/chromoting/Chromoting.java
index 1928f3c..f0901bc 100644
--- a/remoting/android/java/src/org/chromium/chromoting/Chromoting.java
+++ b/remoting/android/java/src/org/chromium/chromoting/Chromoting.java
@@ -474,7 +474,7 @@ public class Chromoting extends Activity implements JniInterface.ConnectionListe
                 // authenticate itself with the host using spake.
                 String sharedSecret = accessToken;
 
-                JniInterface.nativeOnThirdPartyTokenFetched(token, sharedSecret);
+                JniInterface.onThirdPartyTokenFetched(token, sharedSecret);
             }
         };
         return new ThirdPartyTokenFetcher(this, host.getTokenUrlPatterns(), callback);
diff --git a/remoting/android/java/src/org/chromium/chromoting/jni/JniInterface.java b/remoting/android/java/src/org/chromium/chromoting/jni/JniInterface.java
index f5ce54a..34bf0df 100644
--- a/remoting/android/java/src/org/chromium/chromoting/jni/JniInterface.java
+++ b/remoting/android/java/src/org/chromium/chromoting/jni/JniInterface.java
@@ -18,6 +18,7 @@ import android.view.KeyEvent;
 import android.view.View;
 import android.widget.CheckBox;
 import android.widget.TextView;
+import android.widget.Toast;
 
 import org.chromium.base.CalledByNative;
 import org.chromium.base.JNINamespace;
@@ -186,11 +187,22 @@ public class JniInterface {
 
     /** Severs the connection and cleans up. Called on the UI thread. */
     public static void disconnectFromHost() {
-        if (!sConnected) return;
+        if (!sConnected) {
+            return;
+        }
 
         sConnectionListener.onConnectionState(ConnectionListener.State.CLOSED,
                 ConnectionListener.Error.OK);
 
+        disconnectFromHostWithoutNotification();
+    }
+
+    /** Same as disconnectFromHost() but without notifying the ConnectionListener. */
+    private static void disconnectFromHostWithoutNotification() {
+        if (!sConnected) {
+            return;
+        }
+
         nativeDisconnect();
         sConnectionListener = null;
         sConnected = false;
@@ -204,11 +216,17 @@ public class JniInterface {
     /** Performs the native portion of the cleanup. */
     private static native void nativeDisconnect();
 
-    /** Reports whenever the connection status changes. Called on the UI thread. */
+    /** Called by native code whenever the connection status changes. Called on the UI thread. */
     @CalledByNative
-    private static void reportConnectionStatus(int state, int error) {
-        sConnectionListener.onConnectionState(ConnectionListener.State.fromValue(state),
-                ConnectionListener.Error.fromValue(error));
+    private static void onConnectionState(int stateCode, int errorCode) {
+        ConnectionListener.State state = ConnectionListener.State.fromValue(stateCode);
+        ConnectionListener.Error error = ConnectionListener.Error.fromValue(errorCode);
+        sConnectionListener.onConnectionState(state, error);
+        if (state == ConnectionListener.State.FAILED || state == ConnectionListener.State.CLOSED) {
+            // Disconnect from the host here, otherwise the next time connectToHost() is called,
+            // it will try to disconnect, triggering an incorrect status notification.
+            disconnectFromHostWithoutNotification();
+        }
     }
 
     /** Prompts the user to enter a PIN. Called on the UI thread. */
@@ -235,8 +253,13 @@ public class JniInterface {
                     @Override
                     public void onClick(DialogInterface dialog, int which) {
                         Log.i("jniiface", "User provided a PIN code");
-                        nativeAuthenticationResponse(String.valueOf(pinTextView.getText()),
-                                pinCheckBox.isChecked(), Build.MODEL);
+                        if (sConnected) {
+                            nativeAuthenticationResponse(String.valueOf(pinTextView.getText()),
+                                    pinCheckBox.isChecked(), Build.MODEL);
+                        } else {
+                            String message = sContext.getString(R.string.error_network_error);
+                            Toast.makeText(sContext, message, Toast.LENGTH_LONG).show();
+                        }
                     }
                 });
 
@@ -457,5 +480,14 @@ public class JniInterface {
     /**
      * Notify the native code to continue authentication with the |token| and the |sharedSecret|.
      */
-    public static native void nativeOnThirdPartyTokenFetched(String token, String sharedSecret);
+    public static void onThirdPartyTokenFetched(String token, String sharedSecret) {
+        if (!sConnected) {
+            return;
+        }
+
+        nativeOnThirdPartyTokenFetched(token, sharedSecret);
+    }
+
+    /** Passes authentication data to the native handling code. */
+    private static native void nativeOnThirdPartyTokenFetched(String token, String sharedSecret);
 }
diff --git a/remoting/client/jni/chromoting_jni_instance.cc b/remoting/client/jni/chromoting_jni_instance.cc
index fb66b6c..253c54c 100644
--- a/remoting/client/jni/chromoting_jni_instance.cc
+++ b/remoting/client/jni/chromoting_jni_instance.cc
@@ -280,7 +280,7 @@ void ChromotingJniInstance::OnConnectionState(
 
   jni_runtime_->ui_task_runner()->PostTask(
       FROM_HERE,
-      base::Bind(&ChromotingJniRuntime::ReportConnectionStatus,
+      base::Bind(&ChromotingJniRuntime::OnConnectionState,
                  base::Unretained(jni_runtime_),
                  state,
                  error));
diff --git a/remoting/client/jni/chromoting_jni_runtime.cc b/remoting/client/jni/chromoting_jni_runtime.cc
index 60a8128..3cc2e080 100644
--- a/remoting/client/jni/chromoting_jni_runtime.cc
+++ b/remoting/client/jni/chromoting_jni_runtime.cc
@@ -235,13 +235,13 @@ void ChromotingJniRuntime::DisconnectFromHost() {
   }
 }
 
-void ChromotingJniRuntime::ReportConnectionStatus(
+void ChromotingJniRuntime::OnConnectionState(
     protocol::ConnectionToHost::State state,
     protocol::ErrorCode error) {
   DCHECK(ui_task_runner_->BelongsToCurrentThread());
 
   JNIEnv* env = base::android::AttachCurrentThread();
-  Java_JniInterface_reportConnectionStatus(env, state, error);
+  Java_JniInterface_onConnectionState(env, state, error);
 }
 
 void ChromotingJniRuntime::DisplayAuthenticationPrompt(bool pairing_supported) {
diff --git a/remoting/client/jni/chromoting_jni_runtime.h b/remoting/client/jni/chromoting_jni_runtime.h
index a5ca339..f905702 100644
--- a/remoting/client/jni/chromoting_jni_runtime.h
+++ b/remoting/client/jni/chromoting_jni_runtime.h
@@ -70,9 +70,9 @@ class ChromotingJniRuntime {
     return session_;
   }
 
-  // Notifies the user of the current connection status. Call on UI thread.
-  void ReportConnectionStatus(protocol::ConnectionToHost::State state,
-                              protocol::ErrorCode error);
+  // Notifies Java code of the current connection status. Call on UI thread.
+  void OnConnectionState(protocol::ConnectionToHost::State state,
+                         protocol::ErrorCode error);
 
   // Pops up a dialog box asking the user to enter a PIN. Call on UI thread.
   void DisplayAuthenticationPrompt(bool pairing_supported);