[fido] Only WriteWithoutResponse on macOS

This change restricts the usage of WriteWithoutResponse to macOS. On
macOS this is required for performance reasons, but on other platforms
this can lead to misbehavior. This results in requests not being
handled correctly.

Bug: 763303
Change-Id: I5b609fd70eafdf4b9abad680218a9e04ac302703
Reviewed-on: https://chromium-review.googlesource.com/1167515
Commit-Queue: Jan Wilken Dörrie <jdoerrie@chromium.org>
Reviewed-by: Giovanni Ortuño Urquidi <ortuno@chromium.org>
Reviewed-by: Balazs Engedy <engedy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#582555}

device/bluetooth/test/mock_bluetooth_gatt_characteristic.h

@@ -12,6 +12,7 @@
 #include <vector>
 
 #include "base/callback.h"
+#include "base/containers/span.h"
 #include "base/macros.h"
 #include "device/bluetooth/bluetooth_remote_gatt_characteristic.h"
 #include "device/bluetooth/bluetooth_remote_gatt_descriptor.h"
@@ -71,6 +72,7 @@ class MockBluetoothGattCharacteristic
                     const base::Closure&,
                     const ErrorCallback&));
 #endif
+  MOCK_METHOD1(WriteWithoutResponse, bool(base::span<const uint8_t>));
 
   void AddMockDescriptor(
       std::unique_ptr<MockBluetoothGattDescriptor> mock_descriptor);

device/fido/ble/fido_ble_connection.cc

@@ -10,6 +10,7 @@
 #include "base/bind.h"
 #include "base/callback_helpers.h"
 #include "base/logging.h"
+#include "build/build_config.h"
 #include "device/bluetooth/bluetooth_gatt_connection.h"
 #include "device/bluetooth/bluetooth_gatt_notify_session.h"
 #include "device/bluetooth/bluetooth_remote_gatt_characteristic.h"
@@ -234,19 +235,21 @@ void FidoBleConnection::WriteControlPoint(const std::vector<uint8_t>& data,
     return;
   }
 
+#if defined(OS_MACOSX)
   // Attempt a write without response for performance reasons. Fall back to a
   // confirmed write in case of failure, e.g. when the characteristic does not
   // provide the required property.
   if (control_point->WriteWithoutResponse(data)) {
     DVLOG(2) << "Write without response succeeded.";
     std::move(callback).Run(true);
-  } else {
-    auto copyable_callback =
-        base::AdaptCallbackForRepeating(std::move(callback));
-    control_point->WriteRemoteCharacteristic(
-        data, base::Bind(OnWrite, copyable_callback),
-        base::Bind(OnWriteError, copyable_callback));
+    return;
   }
+#endif  // defined(OS_MACOSX)
+
+  auto copyable_callback = base::AdaptCallbackForRepeating(std::move(callback));
+  control_point->WriteRemoteCharacteristic(
+      data, base::Bind(OnWrite, copyable_callback),
+      base::Bind(OnWriteError, copyable_callback));
 }
 
 void FidoBleConnection::WriteServiceRevision(ServiceRevision service_revision,

device/fido/ble/fido_ble_connection_unittest.cc

@@ -251,6 +251,16 @@ class FidoBleConnectionTest : public ::testing::Test {
   }
 
   void SetNextWriteControlPointResponse(bool success) {
+// For performance reasons we try writes without responses first on macOS.
+#if defined(OS_MACOSX)
+    EXPECT_CALL(*u2f_control_point_, WriteWithoutResponse)
+        .WillOnce(Return(success));
+    if (success)
+      return;
+#else
+    EXPECT_CALL(*u2f_control_point_, WriteWithoutResponse).Times(0);
+#endif  // defined(OS_MACOSX)
+
     EXPECT_CALL(*u2f_control_point_, WriteRemoteCharacteristic(_, _, _))
         .WillOnce(Invoke([success](const auto& data, const auto& callback,
                                    const auto& error_callback) {