device/bluetooth: drop WriteWithoutResponse

This removes the BluetoothRemoteGattCharacteristic::WriteWithoutResponse
method. There are no longer any users of this method.

BluetoothRemoteGattCharacteristic::WriteRemoteCharacteristic can now be
used instead since it now takes a |write_type| parameter.

This also removes the last use of IsWritableWithoutResponse() so it can
be removed as well.

Bug: 831524
Change-Id: Ib6c929db61bcee732cf0cd1c5822bde8cfa9a8f2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2191234Reviewed-by: Reilly Grant <reillyg@chromium.org>
Reviewed-by: Vincent Scheib <scheib@chromium.org>
Commit-Queue: David Lechner <david@pybricks.com>
Cr-Commit-Position: refs/heads/master@{#770282}

device/bluetooth/bluetooth_remote_gatt_characteristic.cc

@@ -107,12 +107,6 @@ void BluetoothRemoteGattCharacteristic::StartNotifySession(
 }
 #endif
 
-bool BluetoothRemoteGattCharacteristic::WriteWithoutResponse(
-    base::span<const uint8_t> value) {
-  NOTIMPLEMENTED();
-  return false;
-}
-
 bool BluetoothRemoteGattCharacteristic::AddDescriptor(
     std::unique_ptr<BluetoothRemoteGattDescriptor> descriptor) {
   if (!descriptor)

device/bluetooth/bluetooth_remote_gatt_characteristic.h

@@ -175,15 +175,6 @@ class DEVICE_BLUETOOTH_EXPORT BluetoothRemoteGattCharacteristic
       ErrorCallback error_callback) = 0;
 #endif
 
-  // Sends a write request to a remote characteristic with the value |value|
-  // without waiting for a response. This method returns false to signal
-  // failures. When attempting to write the remote characteristic true is
-  // returned without a guarantee of success. This method only applies to remote
-  // characteristics and will fail for those that are locally hosted.
-  // This method is currently implemented only on macOS.
-  // TODO(https://crbug.com/831524): Implement it on other platforms as well.
-  virtual bool WriteWithoutResponse(base::span<const uint8_t> value);
-
  protected:
   using DescriptorMap =
       base::flat_map<std::string,

device/bluetooth/bluetooth_remote_gatt_characteristic_mac.h

@@ -51,7 +51,6 @@ class DEVICE_BLUETOOTH_EXPORT BluetoothRemoteGattCharacteristicMac
       const std::vector<uint8_t>& value,
       base::OnceClosure callback,
       ErrorCallback error_callback) override;
-  bool WriteWithoutResponse(base::span<const uint8_t> value) override;
 
  protected:
   void SubscribeToNotifications(BluetoothRemoteGattDescriptor* ccc_descriptor,
@@ -87,8 +86,6 @@ class DEVICE_BLUETOOTH_EXPORT BluetoothRemoteGattCharacteristicMac
   bool IsReadable() const;
   // Returns true if the characteristic is writable.
   bool IsWritable() const;
-  // Returns true if the characteristic is writable without response.
-  bool IsWritableWithoutResponse() const;
   // Returns true if the characteristic supports notifications or indications.
   bool SupportsNotificationsOrIndications() const;
   // Returns the write type (with or without responses).

device/bluetooth/bluetooth_remote_gatt_characteristic_mac.mm

@@ -229,26 +229,6 @@ void BluetoothRemoteGattCharacteristicMac::DeprecatedWriteRemoteCharacteristic(
   }
 }
 
-bool BluetoothRemoteGattCharacteristicMac::WriteWithoutResponse(
-    base::span<const uint8_t> value) {
-  if (!IsWritableWithoutResponse()) {
-    DVLOG(1) << *this << ": Characteristic not writable without response.";
-    return false;
-  }
-  if (HasPendingRead() || HasPendingWrite()) {
-    DVLOG(1) << *this << ": Characteristic write already in progress.";
-    return false;
-  }
-
-  DVLOG(1) << *this << ": Write characteristic without response.";
-  base::scoped_nsobject<NSData> nsdata_value(
-      [[NSData alloc] initWithBytes:value.data() length:value.size()]);
-  [GetCBPeripheral() writeValue:nsdata_value
-              forCharacteristic:cb_characteristic_
-                           type:CBCharacteristicWriteWithoutResponse];
-  return true;
-}
-
 void BluetoothRemoteGattCharacteristicMac::SubscribeToNotifications(
     BluetoothRemoteGattDescriptor* ccc_descriptor,
     base::OnceClosure callback,
@@ -447,10 +427,6 @@ bool BluetoothRemoteGattCharacteristicMac::IsWritable() const {
          (properties & PROPERTY_WRITE_WITHOUT_RESPONSE);
 }
 
-bool BluetoothRemoteGattCharacteristicMac::IsWritableWithoutResponse() const {
-  return (GetProperties() & PROPERTY_WRITE_WITHOUT_RESPONSE);
-}
-
 bool BluetoothRemoteGattCharacteristicMac::SupportsNotificationsOrIndications()
     const {
   BluetoothGattCharacteristic::Properties properties = GetProperties();

device/bluetooth/bluetooth_remote_gatt_characteristic_unittest.cc

@@ -4564,192 +4564,6 @@ TEST_F(
   base::RunLoop().RunUntilIdle();
 }
 
-#if defined(OS_MACOSX)
-#define MAYBE_WriteWithoutResponse_PropertyNotPresent \
-  WriteWithoutResponse_PropertyNotPresent
-#else
-#define MAYBE_WriteWithoutResponse_PropertyNotPresent \
-  DISABLED_WriteWithoutResponse_PropertyNotPresent
-#endif
-// Tests that WriteWithoutResponse fails when a characteristic does not have the
-// required property.
-// TODO(https://crbug.com/831524): Enable for other platforms once supported.
-#if defined(OS_WIN)
-TEST_P(BluetoothRemoteGattCharacteristicTestWinrtOnly,
-       WriteWithoutResponse_PropertyNotPresent) {
-#else
-TEST_F(BluetoothRemoteGattCharacteristicTest,
-       MAYBE_WriteWithoutResponse_PropertyNotPresent) {
-#endif
-  if (!PlatformSupportsLowEnergy()) {
-    LOG(WARNING) << "Low Energy Bluetooth unavailable, skipping unit test.";
-    return;
-  }
-  ASSERT_NO_FATAL_FAILURE(FakeCharacteristicBoilerplate(
-      BluetoothRemoteGattCharacteristic::PROPERTY_WRITE));
-
-  std::vector<uint8_t> test_vector = {0, 1, 2, 3, 4, 0xf, 0xf0, 0xff};
-  EXPECT_FALSE(characteristic1_->WriteWithoutResponse(test_vector));
-}
-
-#if defined(OS_MACOSX)
-#define MAYBE_WriteWithoutResponse_PendingWrite \
-  WriteWithoutResponse_PendingWrite
-#else
-#define MAYBE_WriteWithoutResponse_PendingWrite \
-  DISABLED_WriteWithoutResponse_PendingWrite
-#endif
-// Tests that WriteWithoutResponse fails when a characteristic already has a
-// pending write.
-// TODO(https://crbug.com/831524): Enable for other platforms once supported.
-#if defined(OS_WIN)
-TEST_P(BluetoothRemoteGattCharacteristicTestWinrtOnly,
-       WriteWithoutResponse_PendingWrite) {
-#else
-TEST_F(BluetoothRemoteGattCharacteristicTest,
-       MAYBE_WriteWithoutResponse_PendingWrite) {
-#endif
-  if (!PlatformSupportsLowEnergy()) {
-    LOG(WARNING) << "Low Energy Bluetooth unavailable, skipping unit test.";
-    return;
-  }
-  ASSERT_NO_FATAL_FAILURE(FakeCharacteristicBoilerplate(
-      BluetoothRemoteGattCharacteristic::PROPERTY_WRITE |
-      BluetoothRemoteGattCharacteristic::PROPERTY_WRITE_WITHOUT_RESPONSE));
-
-  std::vector<uint8_t> test_vector = {0, 1, 2, 3, 4, 0xf, 0xf0, 0xff};
-  characteristic1_->WriteRemoteCharacteristic(
-      test_vector, WriteType::kWithResponse, GetCallback(Call::EXPECTED),
-      GetGattErrorCallback(Call::NOT_EXPECTED));
-
-  // Explicitly make sure that the callback has not been invoked yet. Since
-  // WriteRemoteCharacteristic is still pending, this results in an
-  // error for WriteWithoutResponse.
-  EXPECT_EQ(0, callback_count_);
-  EXPECT_FALSE(characteristic1_->WriteWithoutResponse(test_vector));
-
-  // Test that the failed WriteWithoutResponse request does not interfere with
-  // the pending WriteRemoteCharacteristic request.
-  SimulateGattCharacteristicWrite(characteristic1_);
-  base::RunLoop().RunUntilIdle();
-  EXPECT_EQ(1, callback_count_);
-  EXPECT_EQ(test_vector, last_write_value_);
-}
-
-#if defined(OS_MACOSX)
-#define MAYBE_DeprecatedWriteWithoutResponse_PendingWrite \
-  DeprecatedWriteWithoutResponse_PendingWrite
-#else
-#define MAYBE_DeprecatedWriteWithoutResponse_PendingWrite \
-  DISABLED_DeprecatedWriteWithoutResponse_PendingWrite
-#endif
-// Tests that WriteWithoutResponse fails when a characteristic already has a
-// pending write.
-// TODO(https://crbug.com/831524): Enable for other platforms once supported.
-#if defined(OS_WIN)
-TEST_P(BluetoothRemoteGattCharacteristicTestWinrtOnly,
-       DeprecatedWriteWithoutResponse_PendingWrite) {
-#else
-TEST_F(BluetoothRemoteGattCharacteristicTest,
-       MAYBE_DeprecatedWriteWithoutResponse_PendingWrite) {
-#endif
-  if (!PlatformSupportsLowEnergy()) {
-    LOG(WARNING) << "Low Energy Bluetooth unavailable, skipping unit test.";
-    return;
-  }
-  ASSERT_NO_FATAL_FAILURE(FakeCharacteristicBoilerplate(
-      BluetoothRemoteGattCharacteristic::PROPERTY_WRITE |
-      BluetoothRemoteGattCharacteristic::PROPERTY_WRITE_WITHOUT_RESPONSE));
-
-  std::vector<uint8_t> test_vector = {0, 1, 2, 3, 4, 0xf, 0xf0, 0xff};
-  characteristic1_->DeprecatedWriteRemoteCharacteristic(
-      test_vector, GetCallback(Call::EXPECTED),
-      GetGattErrorCallback(Call::NOT_EXPECTED));
-
-  // Explicitly make sure that the callback has not been invoked yet. Since
-  // DeprecatedWriteRemoteCharacteristic is still pending, this results in an
-  // error for WriteWithoutResponse.
-  EXPECT_EQ(0, callback_count_);
-  EXPECT_FALSE(characteristic1_->WriteWithoutResponse(test_vector));
-
-  // Test that the failed WriteWithoutReponse request does not interfere with
-  // the pending DeprecatedWriteRemoteCharacteristic request.
-  SimulateGattCharacteristicWrite(characteristic1_);
-  base::RunLoop().RunUntilIdle();
-  EXPECT_EQ(1, callback_count_);
-  EXPECT_EQ(test_vector, last_write_value_);
-}
-
-#if defined(OS_MACOSX)
-#define MAYBE_WriteWithoutResponse_PendingRead WriteWithoutResponse_PendingRead
-#else
-#define MAYBE_WriteWithoutResponse_PendingRead \
-  DISABLED_WriteWithoutResponse_PendingRead
-#endif
-// Tests that WriteWithoutResponse fails when a characteristic already has a
-// pending read.
-// TODO(https://crbug.com/831524): Enable for other platforms once supported.
-#if defined(OS_WIN)
-TEST_P(BluetoothRemoteGattCharacteristicTestWinrtOnly,
-       WriteWithoutResponse_PendingRead) {
-#else
-TEST_F(BluetoothRemoteGattCharacteristicTest,
-       MAYBE_WriteWithoutResponse_PendingRead) {
-#endif
-  if (!PlatformSupportsLowEnergy()) {
-    LOG(WARNING) << "Low Energy Bluetooth unavailable, skipping unit test.";
-    return;
-  }
-  ASSERT_NO_FATAL_FAILURE(FakeCharacteristicBoilerplate(
-      BluetoothRemoteGattCharacteristic::PROPERTY_READ |
-      BluetoothRemoteGattCharacteristic::PROPERTY_WRITE_WITHOUT_RESPONSE));
-
-  characteristic1_->ReadRemoteCharacteristic(
-      GetReadValueCallback(Call::EXPECTED),
-      GetGattErrorCallback(Call::NOT_EXPECTED));
-
-  // Explicitly make sure that the callback has not been invoked yet. Since
-  // ReadRemoteCharacteristic is still pending, this results in an error for
-  // WriteWithoutResponse.
-  EXPECT_EQ(0, callback_count_);
-  std::vector<uint8_t> test_vector = {0, 1, 2, 3, 4, 0xf, 0xf0, 0xff};
-  EXPECT_FALSE(characteristic1_->WriteWithoutResponse(test_vector));
-
-  // Test that the failed WriteWithoutResponse request does not interfere with
-  // the pending ReadRemoteCharacteristic request.
-  SimulateGattCharacteristicRead(characteristic1_, test_vector);
-  base::RunLoop().RunUntilIdle();
-  EXPECT_EQ(1, callback_count_);
-  EXPECT_EQ(test_vector, last_read_value_);
-}
-
-#if defined(OS_MACOSX)
-#define MAYBE_WriteWithoutResponse_Success WriteWithoutResponse_Success
-#else
-#define MAYBE_WriteWithoutResponse_Success DISABLED_WriteWithoutResponse_Success
-#endif
-// Tests that WriteWithoutResponse indicates success if the proper conditions
-// are met.
-// TODO(https://crbug.com/831524): Enable for other platforms once supported.
-#if defined(OS_WIN)
-TEST_P(BluetoothRemoteGattCharacteristicTestWinrtOnly,
-       WriteWithoutResponse_Success) {
-#else
-TEST_F(BluetoothRemoteGattCharacteristicTest,
-       MAYBE_WriteWithoutResponse_Success) {
-#endif
-  if (!PlatformSupportsLowEnergy()) {
-    LOG(WARNING) << "Low Energy Bluetooth unavailable, skipping unit test.";
-    return;
-  }
-  ASSERT_NO_FATAL_FAILURE(FakeCharacteristicBoilerplate(
-      BluetoothRemoteGattCharacteristic::PROPERTY_WRITE_WITHOUT_RESPONSE));
-
-  std::vector<uint8_t> test_vector = {1, 2, 3, 4};
-  EXPECT_TRUE(characteristic1_->WriteWithoutResponse(test_vector));
-  EXPECT_EQ(test_vector, last_write_value_);
-}
-
 #if defined(OS_ANDROID)
 #define MAYBE_StartNotifySessionDuringDisconnect \
   StartNotifySessionDuringDisconnect

device/bluetooth/bluetooth_remote_gatt_characteristic_winrt.cc

@@ -332,56 +332,6 @@ void BluetoothRemoteGattCharacteristicWinrt::UpdateDescriptors(
   std::swap(descriptors, descriptors_);
 }
 
-bool BluetoothRemoteGattCharacteristicWinrt::WriteWithoutResponse(
-    base::span<const uint8_t> value) {
-  if (!(GetProperties() & PROPERTY_WRITE_WITHOUT_RESPONSE))
-    return false;
-
-  if (pending_read_callbacks_ || pending_write_callbacks_)
-    return false;
-
-  ComPtr<IGattCharacteristic3> characteristic_3;
-  HRESULT hr = characteristic_.As(&characteristic_3);
-  if (FAILED(hr)) {
-    BLUETOOTH_LOG(DEBUG) << "As IGattCharacteristic3 failed: "
-                         << logging::SystemErrorCodeToString(hr);
-    return false;
-  }
-
-  ComPtr<IBuffer> buffer;
-  hr = base::win::CreateIBufferFromData(value.data(), value.size(), &buffer);
-  if (FAILED(hr)) {
-    BLUETOOTH_LOG(DEBUG) << "base::win::CreateIBufferFromData failed: "
-                         << logging::SystemErrorCodeToString(hr);
-    return false;
-  }
-
-  ComPtr<IAsyncOperation<GattWriteResult*>> write_value_op;
-  // Note: As we are ignoring the result WriteValueWithOptionAsync() would work
-  // as well, but re-using WriteValueWithResultAndOptionAsync() does simplify
-  // the testing code and there is no difference in production.
-  hr = characteristic_3->WriteValueWithResultAndOptionAsync(
-      buffer.Get(), GattWriteOption_WriteWithoutResponse, &write_value_op);
-  if (FAILED(hr)) {
-    BLUETOOTH_LOG(DEBUG)
-        << "GattCharacteristic::WriteValueWithResultAndOptionAsync failed: "
-        << logging::SystemErrorCodeToString(hr);
-    return false;
-  }
-
-  // While we are ignoring the response, we still post the async_op in order to
-  // extend its lifetime until the operation has completed.
-  hr =
-      base::win::PostAsyncResults(std::move(write_value_op), base::DoNothing());
-  if (FAILED(hr)) {
-    BLUETOOTH_LOG(DEBUG) << "PostAsyncResults failed: "
-                         << logging::SystemErrorCodeToString(hr);
-    return false;
-  }
-
-  return true;
-}
-
 IGattCharacteristic*
 BluetoothRemoteGattCharacteristicWinrt::GetCharacteristicForTesting() {
   return characteristic_.Get();

device/bluetooth/bluetooth_remote_gatt_characteristic_winrt.h

@@ -55,7 +55,6 @@ class DEVICE_BLUETOOTH_EXPORT BluetoothRemoteGattCharacteristicWinrt
       const std::vector<uint8_t>& value,
       base::OnceClosure callback,
       ErrorCallback error_callback) override;
-  bool WriteWithoutResponse(base::span<const uint8_t> value) override;
 
   void UpdateDescriptors(BluetoothGattDiscovererWinrt* gatt_discoverer);
 

device/bluetooth/test/fake_remote_gatt_characteristic.cc

@@ -190,16 +190,6 @@ void FakeRemoteGattCharacteristic::PrepareWriteRemoteCharacteristic(
 }
 #endif
 
-bool FakeRemoteGattCharacteristic::WriteWithoutResponse(
-    base::span<const uint8_t> value) {
-  if (properties_ & PROPERTY_WRITE_WITHOUT_RESPONSE) {
-    last_written_value_.emplace(value.begin(), value.end());
-    return true;
-  }
-
-  return false;
-}
-
 void FakeRemoteGattCharacteristic::SubscribeToNotifications(
     device::BluetoothRemoteGattDescriptor* ccc_descriptor,
 #if defined(OS_CHROMEOS)

device/bluetooth/test/fake_remote_gatt_characteristic.h

@@ -98,7 +98,6 @@ class FakeRemoteGattCharacteristic
                                         base::OnceClosure callback,
                                         ErrorCallback error_callback) override;
 #endif
-  bool WriteWithoutResponse(base::span<const uint8_t> value) override;
 
  protected:
 #if defined(OS_CHROMEOS)

device/bluetooth/test/mock_bluetooth_gatt_characteristic.h

@@ -105,7 +105,6 @@ class MockBluetoothGattCharacteristic
                     base::OnceClosure&,
                     ErrorCallback&));
 #endif
-  MOCK_METHOD1(WriteWithoutResponse, bool(base::span<const uint8_t>));
 
   void AddMockDescriptor(
       std::unique_ptr<MockBluetoothGattDescriptor> mock_descriptor);