Also need to account for GATT overhead on writes.

r754176 was incomplete: it should have trimmed the MTU for writes from
the phone was well as the advertised MTU.

BUG=1002262

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

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

@@ -73,6 +73,9 @@ class BLEHandler extends BluetoothGattServerCallback implements Closeable {
     private static final String STATE_FILE_NAME = "cablev2_authenticator";
     private static final String STATE_VALUE_NAME = "keys";
 
+    // The (G)ATT op-code and attribute handle take three bytes.
+    private static final int GATT_MTU_OVERHEAD = 3;
+
     /**
      * The pending fragments to send to each client. If present, the value is
      * never an empty array.
@@ -173,9 +176,19 @@ class BLEHandler extends BluetoothGattServerCallback implements Closeable {
     @Override
     public void onMtuChanged(BluetoothDevice device, int mtu) {
         Long client = addressToLong(device.getAddress());
+        // At least six bytes is required: three bytes of GATT overhead and
+        // three bytes of CTAP2 framing. If the requested MTU is less than
+        // this, try six bytes anyway. The operations will probably fail but
+        // there's nothing we can do about it.
+        if (mtu < 6) {
+            mtu = 6;
+        }
+
+        int clientMTU = mtu - GATT_MTU_OVERHEAD;
+
         mTaskRunner.postTask(() -> {
-            mKnownMtus.put(client, mtu);
-            BLEHandlerJni.get().recordClientMtu(client, mtu);
+            mKnownMtus.put(client, clientMTU);
+            BLEHandlerJni.get().recordClientMtu(client, clientMTU);
         });
     }
 
@@ -195,17 +208,10 @@ class BLEHandler extends BluetoothGattServerCallback implements Closeable {
                 mTaskRunner.postTask(() -> {
                     Integer mtu = mKnownMtus.get(client);
                     if (mtu == null) {
-                        // If no MTU is negotiated, the GATT default is just 23 bytes.
-                        mtu = 23;
-                    }
-                    if (mtu < 4) {
-                        Log.i(TAG, "MTU unusably small");
-                        mServer.sendResponse(
-                                device, requestId, BluetoothGatt.GATT_FAILURE, 0, null);
-                        return;
+                        // If no MTU is negotiated, the GATT default is just 23
+                        // bytes. Subtract three bytes of GATT overhead.
+                        mtu = 23 - GATT_MTU_OVERHEAD;
                     }
-                    // The (G)ATT op-code and attribute handle take three bytes.
-                    mtu -= 3;
                     byte[] mtuBytes = {(byte) (mtu >> 8), (byte) (mtu & 0xff)};
                     mServer.sendResponse(
                             device, requestId, BluetoothGatt.GATT_SUCCESS, 0, mtuBytes);

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

@@ -754,11 +754,15 @@ class CableInterface : public Client::Delegate {
                                          const JavaParamRef<jbyteArray>& data) {
     auto it = clients_.find(client_addr);
     if (it == clients_.end()) {
-      DCHECK(known_mtus_.find(client_addr) != known_mtus_.end());
-      uint16_t mtu = known_mtus_[client_addr];
-      if (mtu == 0) {
-        mtu = 512;
-      }
+      // If no MTU is negotiated, the GATT default is just 23 bytes. Subtract
+      // three bytes of GATT overhead.
+      static constexpr uint16_t kDefaultMTU = 23 - 3;
+      // TODO: once we only handle a single client, the Java code can ensure
+      // that an MTU has always been set and this code wont need to know a
+      // default.
+      const auto& mtu_it = known_mtus_.find(client_addr);
+      const uint16_t mtu =
+          mtu_it == known_mtus_.end() ? kDefaultMTU : mtu_it->second;
       it = clients_
                .emplace(client_addr,
                         new Client(client_addr, mtu, &auth_state_, this))