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

remoting/android/java/src/org/chromium/chromoting/jni/JniInterface.java

Index: remoting/android/java/src/org/chromium/chromoting/jni/JniInterface.java
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 f5ce54ad8299adfd6df49443fd105b263ca1f871..42af0b4aec8a511582f5d47c794780974db73f2b 100644
--- a/remoting/android/java/src/org/chromium/chromoting/jni/JniInterface.java
+++ b/remoting/android/java/src/org/chromium/chromoting/jni/JniInterface.java
@@ -191,6 +191,13 @@ public class JniInterface {
         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;
@@ -206,9 +213,15 @@ public class JniInterface {
 
     /** Reports 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 reportConnectionStatus(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();

[Jamie, 2014/07/30 17:22:04]

I don't like the fact that a method called reportConnectionStatus also closes
the connection. You could rename the method (handleStateChange?) or move the
disconnect out of this method.

[Lambros, 2014/08/12 01:27:35]

Renamed the method (this touches some C++ files as well).

+        }
     }
 
     /** Prompts the user to enter a PIN. Called on the UI thread. */
@@ -235,8 +248,10 @@ 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);
+                        }

[Jamie, 2014/07/30 17:22:04]

I think we should show an error in this case. It would make the code more
readable and be better UX.

[Lambros, 2014/08/12 01:27:35]

Done.

                     }
                 });
 
@@ -457,5 +472,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);
 }