cablev2: don't close Activity until transmission complete.

This change defers ending the Activity until all the fragments of the
reply have been sent. This _may_ help BLE reliability.

BUG=1002262

Change-Id: I3434817cbd60e3f91eeced083772077731cbf74a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2310192
Commit-Queue: Adam Langley <agl@chromium.org>
Commit-Queue: Martin Kreichgauer <martinkr@google.com>
Auto-Submit: Adam Langley <agl@chromium.org>
Reviewed-by: Martin Kreichgauer <martinkr@google.com>
Cr-Commit-Position: refs/heads/master@{#792361}

chrome/android/features/cablev2_authenticator/java/src/org/chromium/chrome/browser/webauth/authenticator/BLEHandler.java

@@ -90,6 +90,10 @@ class BLEHandler extends BluetoothGattServerCallback implements Closeable {
     private BluetoothGattDescriptor mCccd;
     private BluetoothGattCharacteristic mStatusChar;
     private Long mConnectedDevice;
+    // mCloseWhenDone indicates that |onComplete| should be called on
+    // |mAuthenticator| once the current series of notifications have been
+    // sent.
+    private boolean mCloseWhenDone;
 
     BLEHandler(CableAuthenticator authenticator, SingleThreadTaskRunner taskRunner) {
         mPendingFragments = new HashMap<Long, byte[][]>();
@@ -257,7 +261,9 @@ class BLEHandler extends BluetoothGattServerCallback implements Closeable {
     /**
      * Triggers a notification on the fidoStatus characteristic to the given device.
      */
-    public void sendNotification(BluetoothDevice device, byte[][] fragments) {
+    private void sendNotification(BluetoothDevice device, byte[][] fragments) {
+        assert mTaskRunner.belongsToCurrentThread();
+
         Log.i(TAG, "sendNotification sending " + fragments[0].length + ": " + hex(fragments[0]));
         Long client = addressToLong(device.getAddress());
         assert !mPendingFragments.containsKey(client);
@@ -273,7 +279,11 @@ class BLEHandler extends BluetoothGattServerCallback implements Closeable {
      * Like sendNotification(BluetoothDevice, byte[][]), but for a client ID passed from native
      * code.
      */
-    public void sendNotification(long client, byte[][] fragments) {
+    public void sendNotification(long client, byte[][] fragments, boolean closeWhenDone) {
+        assert mTaskRunner.belongsToCurrentThread();
+        // If the final reply has already been sent then no more should follow.
+        assert !mCloseWhenDone;
+
         String addr = longToAddress(client);
         Log.i(TAG, "sendNotification to " + addr);
         List<BluetoothDevice> devices = mManager.getConnectedDevices(BluetoothProfile.GATT);
@@ -288,6 +298,8 @@ class BLEHandler extends BluetoothGattServerCallback implements Closeable {
             Log.i(TAG, "can't find connected device " + addr);
             return;
         }
+
+        mCloseWhenDone = closeWhenDone;
         sendNotification(device, fragments);
     }
 
@@ -307,6 +319,9 @@ class BLEHandler extends BluetoothGattServerCallback implements Closeable {
 
             byte[][] remainingFragments = mPendingFragments.get(client);
             if (remainingFragments == null) {
+                if (mCloseWhenDone) {
+                    mAuthenticator.onComplete();
+                }
                 return;
             }
 

chrome/android/features/cablev2_authenticator/java/src/org/chromium/chrome/browser/webauth/authenticator/CableAuthenticator.java

@@ -87,8 +87,11 @@ class CableAuthenticator {
     public interface Callback {
         // Invoked when the authenticator has completed a handshake with a client device.
         void onAuthenticatorConnected();
-        // Invoked when the authenticator has finished. The UI should be dismissed at this point.
+        // Invoked when a transaction has completed. The response may still be transmitting and
+        // onComplete will follow.
         void onAuthenticatorResult(Result result);
+        // Invoked when the authenticator has finished. The UI should be dismissed at this point.
+        void onComplete();
     }
 
     public CableAuthenticator(Context context, CableAuthenticatorUI ui) {
@@ -143,6 +146,13 @@ class CableAuthenticator {
         return CableAuthenticatorJni.get().onBLEWrite(client, mtu, payload);
     }
 
+    /**
+     * Called by BLEHandler when transmission of a final reply is complete.
+     */
+    public void onComplete() {
+        mCallback.onComplete();
+    }
+
     // Calls from native code.
 
     /**
@@ -182,11 +192,11 @@ class CableAuthenticator {
      * Called by native code to send BLE data to a specified client.
      */
     @CalledByNative
-    public void sendNotification(long client, byte[][] fragments) {
+    public void sendNotification(long client, byte[][] fragments, boolean isTransactionEnd) {
         assert mTaskRunner.belongsToCurrentThread();
         assert mBleStarted;
 
-        mBleHandler.sendNotification(client, fragments);
+        mBleHandler.sendNotification(client, fragments, /*closeWhenDone=*/isTransactionEnd);
     }
 
     @CalledByNative

chrome/android/features/cablev2_authenticator/java/src/org/chromium/chrome/browser/webauth/authenticator/CableAuthenticatorUI.java

@@ -190,4 +190,9 @@ public class CableAuthenticatorUI extends Fragment
             getActivity().finish();
         });
     }
+
+    @Override
+    public void onComplete() {
+        getActivity().runOnUiThread(() -> { getActivity().finish(); });
+    }
 }

chrome/android/features/cablev2_authenticator/native/cablev2_authenticator_android.cc

@@ -295,9 +295,9 @@ class BLEClient {
         base::span<const uint8_t> message_bytes) = 0;
 
     // SendBLEMessages sends the given fragments to the target peer.
-    virtual void SendBLEMessages(
-        uint64_t target_addr,
-        std::vector<std::vector<uint8_t>> messages) = 0;
+    virtual void SendBLEMessages(uint64_t target_addr,
+                                 std::vector<std::vector<uint8_t>> messages,
+                                 bool is_transaction_complete) = 0;
   };
 
   BLEClient(uint64_t addr,
@@ -316,7 +316,7 @@ class BLEClient {
     return true;
   }
 
-  void Send(std::vector<uint8_t> data) {
+  void Send(std::vector<uint8_t> data, bool is_transaction_complete) {
     if (!crypter_->Encrypt(&data)) {
       FIDO_LOG(ERROR) << "Failed to encrypt response";
       return;
@@ -329,7 +329,8 @@ class BLEClient {
       return;
     }
 
-    delegate_->SendBLEMessages(addr_, std::move(fragments));
+    delegate_->SendBLEMessages(addr_, std::move(fragments),
+                               is_transaction_complete);
   }
 
   uint64_t addr() { return addr_; }
@@ -795,10 +796,11 @@ class CableInterface : public BLEClient::Delegate {
   }
 
   void SendBLEMessages(uint64_t target_addr,
-                       std::vector<std::vector<uint8_t>> messages) override {
+                       std::vector<std::vector<uint8_t>> messages,
+                       bool is_transaction_complete) override {
     Java_CableAuthenticator_sendNotification(
         env_, cable_authenticator_, target_addr,
-        ToJavaArrayOfByteArray(env_, messages));
+        ToJavaArrayOfByteArray(env_, messages), is_transaction_complete);
   }
 
   void OnMakeCredentialResponse(uint32_t ctap_status,
@@ -845,7 +847,7 @@ class CableInterface : public BLEClient::Delegate {
                       response_payload->end());
     }
 
-    ble_client_->Send(std::move(response));
+    ble_client_->Send(std::move(response), /*is_transaction_complete=*/true);
   }
 
   void OnGetAssertionResponse(uint32_t ctap_status,
@@ -886,7 +888,7 @@ class CableInterface : public BLEClient::Delegate {
                       response_payload->end());
     }
 
-    ble_client_->Send(std::move(response));
+    ble_client_->Send(std::move(response), /*is_transaction_complete=*/true);
   }
 
  private: