bluetooth: CDP to support add and remove characteristic

Add new commands to CDP BluetoothEmulation to support adding and
removing characteristic from a service.

This CL also ensures fake_central_ is reset on disconnect to avoid
confusing timeout error in some test scenarios that fake central has
been torn down.

Bug: 41484719, 398027231
Change-Id: I2cccb5d29fabcdcc1fad2f41b782c8aba1b81e8e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6408115
Reviewed-by: Matt Reynolds <mattreynolds@chromium.org>
Commit-Queue: Jack Hsieh <chengweih@chromium.org>
Reviewed-by: Andrey Kosyakov <caseq@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1439848}

content/browser/devtools/protocol/bluetooth_emulation_handler.cc

@@ -76,7 +76,26 @@ mojo::StructPtr<bluetooth::mojom::ScanRecord> ToScanRecord(
   return out_record;
 }
 
-bluetooth::mojom::CentralState ToCentralState(const String& state_string) {
+mojo::StructPtr<bluetooth::mojom::CharacteristicProperties>
+ToCharacteristicProperties(
+    BluetoothEmulation::CharacteristicProperties* in_properties) {
+  mojo::StructPtr<bluetooth::mojom::CharacteristicProperties> out_properties =
+      bluetooth::mojom::CharacteristicProperties::New();
+  out_properties->broadcast = in_properties->GetBroadcast().value_or(false);
+  out_properties->read = in_properties->GetRead().value_or(false);
+  out_properties->write_without_response =
+      in_properties->GetWriteWithoutResponse().value_or(false);
+  out_properties->write = in_properties->GetWrite().value_or(false);
+  out_properties->notify = in_properties->GetNotify().value_or(false);
+  out_properties->indicate = in_properties->GetIndicate().value_or(false);
+  out_properties->authenticated_signed_writes =
+      in_properties->GetAuthenticatedSignedWrites().value_or(false);
+  out_properties->extended_properties =
+      in_properties->GetExtendedProperties().value_or(false);
+  return out_properties;
+}
+
+bluetooth::mojom::CentralState ToCentralState(const std::string& state_string) {
   if (state_string ==
       protocol::BluetoothEmulation::CentralStateEnum::PoweredOff) {
     return bluetooth::mojom::CentralState::POWERED_OFF;
@@ -127,7 +146,7 @@ void BluetoothEmulationHandler::Wire(UberDispatcher* dispatcher) {
   BluetoothEmulation::Dispatcher::wire(dispatcher, this);
 }
 
-Response BluetoothEmulationHandler::Enable(const String& in_state,
+Response BluetoothEmulationHandler::Enable(const std::string& in_state,
                                            bool in_le_supported) {
   if (emulation_enabled_) {
     return Response::ServerError("BluetoothEmulation already enabled");
@@ -144,6 +163,7 @@ Response BluetoothEmulationHandler::Enable(const String& in_state,
         base::MakeRefCounted<bluetooth::FakeCentral>(
             ToCentralState(in_state),
             fake_central_.BindNewPipeAndPassReceiver()));
+    fake_central_.reset_on_disconnect();
     // While there's a possibility the client might not be fully settled on the
     // fake central side upon return, this is acceptable. Client events are
     // expected to be delivered only after at least one peripheral has been
@@ -175,7 +195,7 @@ Response BluetoothEmulationHandler::Disable() {
 }
 
 void BluetoothEmulationHandler::SetSimulatedCentralState(
-    const String& in_state,
+    const std::string& in_state,
     std::unique_ptr<SetSimulatedCentralStateCallback> callback) {
   if (!is_enabled()) {
     std::move(callback)->sendFailure(
@@ -189,8 +209,8 @@ void BluetoothEmulationHandler::SetSimulatedCentralState(
 }
 
 void BluetoothEmulationHandler::SimulatePreconnectedPeripheral(
-    const String& in_address,
-    const String& in_name,
+    const std::string& in_address,
+    const std::string& in_name,
     std::unique_ptr<
         protocol::Array<protocol::BluetoothEmulation::ManufacturerData>>
         in_manufacturer_data,
@@ -226,8 +246,8 @@ void BluetoothEmulationHandler::SimulateAdvertisement(
 }
 
 void BluetoothEmulationHandler::SimulateGATTOperationResponse(
-    const String& in_address,
-    const String& in_type,
+    const std::string& in_address,
+    const std::string& in_type,
     int in_code,
     std::unique_ptr<SimulateGATTOperationResponseCallback> callback) {
   if (!is_enabled()) {
@@ -260,8 +280,8 @@ void BluetoothEmulationHandler::SimulateGATTOperationResponse(
 }
 
 void BluetoothEmulationHandler::AddService(
-    const String& in_address,
-    const String& in_uuid,
+    const std::string& in_address,
+    const std::string& in_serviceUuid,
     std::unique_ptr<AddServiceCallback> callback) {
   if (!is_enabled()) {
     std::move(callback)->sendFailure(
@@ -269,10 +289,10 @@ void BluetoothEmulationHandler::AddService(
     return;
   }
 
-  device::BluetoothUUID uuid(in_uuid);
+  device::BluetoothUUID uuid(in_serviceUuid);
   if (!uuid.IsValid()) {
     std::move(callback)->sendFailure(Response::InvalidParams(
-        base::StrCat({in_uuid, " is not a valid UUID"})));
+        base::StrCat({in_serviceUuid, " is not a valid UUID"})));
     return;
   }
 
@@ -290,13 +310,13 @@ void BluetoothEmulationHandler::AddService(
             std::move(callback)->sendSuccess(*identifier);
           },
           std::move(callback),
-          base::StrCat({"Failed to add service ", in_uuid, " to peripheral ",
-                        in_address})));
+          base::StrCat({"Failed to add service ", in_serviceUuid,
+                        " to peripheral ", in_address})));
 }
 
 void BluetoothEmulationHandler::RemoveService(
-    const String& in_address,
-    const String& in_id,
+    const std::string& in_address,
+    const std::string& in_serviceId,
     std::unique_ptr<RemoveServiceCallback> callback) {
   if (!is_enabled()) {
     std::move(callback)->sendFailure(
@@ -305,7 +325,7 @@ void BluetoothEmulationHandler::RemoveService(
   }
 
   fake_central_->RemoveFakeService(
-      in_id, in_address,
+      in_serviceId, in_address,
       base::BindOnce(
           [](std::unique_ptr<RemoveServiceCallback> callback,
              const std::string& error_message, bool success) {
@@ -317,8 +337,75 @@ void BluetoothEmulationHandler::RemoveService(
             std::move(callback)->sendSuccess();
           },
           std::move(callback),
-          base::StrCat({"Failed to remove service represented by ", in_id,
-                        " from peripheral ", in_address})));
+          base::StrCat({"Failed to remove service represented by ",
+                        in_serviceId, " from peripheral ", in_address})));
+}
+
+void BluetoothEmulationHandler::AddCharacteristic(
+    const std::string& in_address,
+    const std::string& in_serviceId,
+    const std::string& in_characteristicUuid,
+    std::unique_ptr<protocol::BluetoothEmulation::CharacteristicProperties>
+        in_properties,
+    std::unique_ptr<AddCharacteristicCallback> callback) {
+  if (!is_enabled()) {
+    std::move(callback)->sendFailure(
+        Response::ServerError("BluetoothEmulation not enabled"));
+    return;
+  }
+
+  device::BluetoothUUID uuid(in_characteristicUuid);
+  if (!uuid.IsValid()) {
+    std::move(callback)->sendFailure(Response::InvalidParams(
+        base::StrCat({in_characteristicUuid, " is not a valid UUID"})));
+    return;
+  }
+
+  fake_central_->AddFakeCharacteristic(
+      uuid, ToCharacteristicProperties(in_properties.get()), in_serviceId,
+      in_address,
+      base::BindOnce(
+          [](std::unique_ptr<AddCharacteristicCallback> callback,
+             const std::string& error_message,
+             const std::optional<std::string>& identifier) {
+            if (!identifier) {
+              std::move(callback)->sendFailure(
+                  Response::ServerError(error_message));
+              return;
+            }
+            std::move(callback)->sendSuccess(*identifier);
+          },
+          std::move(callback),
+          base::StrCat({"Failed to add characteristic ", in_characteristicUuid,
+                        " to service ", in_serviceId})));
+}
+
+void BluetoothEmulationHandler::RemoveCharacteristic(
+    const std::string& in_address,
+    const std::string& in_serviceId,
+    const std::string& in_characteristicId,
+    std::unique_ptr<RemoveCharacteristicCallback> callback) {
+  if (!is_enabled()) {
+    std::move(callback)->sendFailure(
+        Response::ServerError("BluetoothEmulation not enabled"));
+    return;
+  }
+
+  fake_central_->RemoveFakeCharacteristic(
+      in_characteristicId, in_serviceId, in_address,
+      base::BindOnce(
+          [](std::unique_ptr<RemoveCharacteristicCallback> callback,
+             const std::string& error_message, bool success) {
+            if (!success) {
+              std::move(callback)->sendFailure(
+                  Response::ServerError(error_message));
+              return;
+            }
+            std::move(callback)->sendSuccess();
+          },
+          std::move(callback),
+          base::StrCat({"Failed to remove characteristic represented by ",
+                        in_characteristicId, " from service ", in_serviceId})));
 }
 
 void BluetoothEmulationHandler::DispatchGATTOperationEvent(

content/browser/devtools/protocol/bluetooth_emulation_handler.h

@@ -62,12 +62,25 @@ class CONTENT_EXPORT BluetoothEmulationHandler
       std::unique_ptr<SimulateGATTOperationResponseCallback> callback) override;
 
   void AddService(const std::string& in_address,
-                  const std::string& in_uuid,
+                  const std::string& in_serviceUuid,
                   std::unique_ptr<AddServiceCallback> callback) override;
   void RemoveService(const std::string& in_address,
-                     const std::string& in_id,
+                     const std::string& in_serviceId,
                      std::unique_ptr<RemoveServiceCallback> callback) override;
 
+  void AddCharacteristic(
+      const std::string& in_address,
+      const std::string& in_serviceId,
+      const std::string& in_characteristicUuid,
+      std::unique_ptr<protocol::BluetoothEmulation::CharacteristicProperties>
+          in_properties,
+      std::unique_ptr<AddCharacteristicCallback> callback) override;
+  void RemoveCharacteristic(
+      const std::string& in_address,
+      const std::string& in_serviceId,
+      const std::string& in_characteristicId,
+      std::unique_ptr<RemoveCharacteristicCallback> callback) override;
+
   // bluetooth::mojom::FakeCentralClient
   void DispatchGATTOperationEvent(
       bluetooth::mojom::GATTOperationType type,

content/browser/devtools/protocol_config.json

@@ -187,7 +187,9 @@
                     "simulateAdvertisement",
                     "simulateGATTOperationResponse",
                     "addService",
-                    "removeService"
+                    "removeService",
+                    "addCharacteristic",
+                    "removeCharacteristic"
                 ]
             }
         ]

third_party/blink/public/devtools_protocol/browser_protocol.pdl

@@ -13266,6 +13266,19 @@ experimental domain BluetoothEmulation
       integer rssi
       ScanRecord scanRecord
 
+  # Describes the properties of a characteristic. This follows Bluetooth Core
+  # Specification BT 4.2 Vol 3 Part G 3.3.1. Characteristic Properties.
+  type CharacteristicProperties extends object
+    properties
+      optional boolean broadcast
+      optional boolean read
+      optional boolean writeWithoutResponse
+      optional boolean write
+      optional boolean notify
+      optional boolean indicate
+      optional boolean authenticatedSignedWrites
+      optional boolean extendedProperties
+
   # Enable the BluetoothEmulation domain.
   command enable
     parameters
@@ -13307,20 +13320,41 @@ experimental domain BluetoothEmulation
       GATTOperationType type
       integer code
 
-  # Adds a service with |uuid| to the peripheral with |address|.
+  # Adds a service with |serviceUuid| to the peripheral with |address|.
   command addService
     parameters
       string address
       string serviceUuid
     returns
       # An identifier that uniquely represents this service.
-      string id
+      string serviceId
 
-  # Removes the service respresented by |id| from the peripheral with |address|.
+  # Removes the service respresented by |serviceId| from the peripheral with
+  # |address|.
   command removeService
     parameters
       string address
-      string id
+      string serviceId
+
+  # Adds a characteristic with |characteristicUuid| and |properties| to the
+  # service represented by |serviceId| in the peripheral with |address|.
+  command addCharacteristic
+    parameters
+      string address
+      string serviceId
+      string characteristicUuid
+      CharacteristicProperties properties
+    returns
+      # An identifier that uniquely represents this characteristic.
+      string characteristicId
+
+  # Removes the characteristic respresented by |characteristicId| from the
+  # service respresented by |serviceId| in the peripheral with |address|.
+  command removeCharacteristic
+    parameters
+      string address
+      string serviceId
+      string characteristicId
 
   # Event for when a GATT operation of |type| to the peripheral with |address|
   # happened.

third_party/blink/web_tests/inspector-protocol/bluetooth/characteristic-add-remove-expected.txt

@@ -0,0 +1,6 @@
+Tests Bluetooth adding and removing characteristic from a service
+After adding measurement interval characteristic: 00002a21-0000-1000-8000-00805f9b34fb,read,notify
+After adding date time characteristic: 00002a21-0000-1000-8000-00805f9b34fb,read,notify,00002a08-0000-1000-8000-00805f9b34fb,read
+After removing date time characteristic: 00002a21-0000-1000-8000-00805f9b34fb,read,notify
+After removing measurement interval characteristic: No Characteristics found in service.
+

third_party/blink/web_tests/inspector-protocol/bluetooth/characteristic-add-remove.js

@@ -0,0 +1,117 @@
+(async function(/** @type {import('test_runner').TestRunner} */ testRunner) {
+  const {session, dp} = await testRunner.startBlank(
+      'Tests Bluetooth adding and removing characteristic from a service');
+  const bp = testRunner.browserP();
+  await dp.Page.enable();
+  await dp.Runtime.enable();
+  const BluetoothHelper =
+      await testRunner.loadScript('resources/bluetooth-helper.js')
+  const helper = new BluetoothHelper(testRunner, dp, session);
+  await helper.setupPreconnectedPeripheral();
+  await helper.requestDevice({
+    acceptAllDevices: true,
+    optionalServices: [BluetoothHelper.HEART_RATE_SERVICE_UUID]
+  });
+  await helper.setupGattOperationHandler();
+  const {result: {serviceId: heartRateServiceId}} =
+      await bp.BluetoothEmulation.addService({
+        address: helper.peripheralAddress(),
+        serviceUuid: BluetoothHelper.HEART_RATE_SERVICE_UUID,
+      });
+
+  const getCharacteristics = async (serviceUuid) => {
+    const devices = await navigator.bluetooth.getDevices();
+    const server = await devices[0].gatt.connect();
+    const toPropertyStrings = (properties) => {
+      let propertyStrings = [];
+      if (properties.broadcast) {
+        propertyStrings.push('broadcast');
+      }
+      if (properties.read) {
+        propertyStrings.push('read');
+      }
+      if (properties.write_without_response) {
+        propertyStrings.push('write_without_response');
+      }
+      if (properties.write) {
+        propertyStrings.push('write');
+      }
+      if (properties.notify) {
+        propertyStrings.push('notify');
+      }
+      if (properties.indicate) {
+        propertyStrings.push('indicate');
+      }
+      if (properties.authenticated_signed_writes) {
+        propertyStrings.push('authenticated_signed_writes');
+      }
+      if (properties.reliableWrite) {
+        propertyStrings.push('reliableWrite');
+      }
+      if (properties.writableAuxiliaries) {
+        propertyStrings.push('writableAuxiliaries');
+      }
+      return propertyStrings;
+    };
+
+    let characteristics = [];
+    try {
+      const service = await server.getPrimaryService(serviceUuid);
+      characteristics = await service.getCharacteristics();
+    } catch (e) {
+      return e.message;
+    }
+    return characteristics.map(
+        c => [c.uuid].concat(toPropertyStrings(c.properties)));
+  };
+
+  // Start the test.
+  const {result: {characteristicId: measurementIntervalCharacteristicId}} =
+      await bp.BluetoothEmulation.addCharacteristic({
+        address: helper.peripheralAddress(),
+        serviceId: heartRateServiceId,
+        characteristicUuid:
+            BluetoothHelper.MEASUREMENT_INTERVAL_CHARACTERISTIC_UUID,
+        properties: {
+          read: true,
+          write: false,
+          notify: true,
+        }
+      });
+  testRunner.log(`After adding measurement interval characteristic: ${
+      await session.evaluateAsync(
+          getCharacteristics, BluetoothHelper.HEART_RATE_SERVICE_UUID)}`);
+
+  const {result: {characteristicId: dateTimeCharacteristicId}} =
+      await bp.BluetoothEmulation.addCharacteristic({
+        address: helper.peripheralAddress(),
+        serviceId: heartRateServiceId,
+        characteristicUuid: BluetoothHelper.DATE_TIME_CHARACTERISTIC_UUID,
+        properties: {
+          read: true,
+        }
+      });
+  testRunner.log(`After adding date time characteristic: ${
+      await session.evaluateAsync(
+          getCharacteristics, BluetoothHelper.HEART_RATE_SERVICE_UUID)}`);
+
+  await bp.BluetoothEmulation.removeCharacteristic({
+    address: helper.peripheralAddress(),
+    serviceId: heartRateServiceId,
+    characteristicId: dateTimeCharacteristicId
+  });
+  testRunner.log(`After removing date time characteristic: ${
+      await session.evaluateAsync(
+          getCharacteristics, BluetoothHelper.HEART_RATE_SERVICE_UUID)}`);
+
+  await bp.BluetoothEmulation.removeCharacteristic({
+    address: helper.peripheralAddress(),
+    serviceId: heartRateServiceId,
+    characteristicId: measurementIntervalCharacteristicId
+  });
+  testRunner.log(`After removing measurement interval characteristic: ${
+      await session.evaluateAsync(
+          getCharacteristics, BluetoothHelper.HEART_RATE_SERVICE_UUID)}`);
+
+  testRunner.completeTest();
+});

third_party/blink/web_tests/inspector-protocol/bluetooth/characteristic-invalid-uuid-expected.txt

@@ -0,0 +1,9 @@
+Tests Bluetooth adding an invalid characteristic UUID
+{
+    error : {
+        code : -32602
+        message : abc is not a valid UUID
+    }
+    id : <number>
+}
+

third_party/blink/web_tests/inspector-protocol/bluetooth/characteristic-invalid-uuid.js

@@ -0,0 +1,29 @@
+(async function(/** @type {import('test_runner').TestRunner} */ testRunner) {
+  const {session, dp} = await testRunner.startBlank(
+      'Tests Bluetooth adding an invalid characteristic UUID');
+  const bp = testRunner.browserP();
+  const BluetoothHelper =
+      await testRunner.loadScript('resources/bluetooth-helper.js')
+  const helper = new BluetoothHelper(testRunner, dp, session);
+  await helper.setupPreconnectedPeripheral();
+  const {result: {serviceId: heartRateServiceId}} =
+      await bp.BluetoothEmulation.addService({
+        address: helper.peripheralAddress(),
+        serviceUuid: BluetoothHelper.HEART_RATE_SERVICE_UUID,
+      });
+
+  // Start the test.
+  const result = await bp.BluetoothEmulation.addCharacteristic({
+    address: helper.peripheralAddress(),
+    serviceId: heartRateServiceId,
+    characteristicUuid: 'abc',
+    properties: {
+      read: true,
+      write: false,
+      notify: true,
+    }
+  });
+  testRunner.log(result);
+
+  testRunner.completeTest();
+});

third_party/blink/web_tests/inspector-protocol/bluetooth/characteristic-remove-unknown-id-expected.txt

@@ -0,0 +1,9 @@
+Tests Bluetooth removing an unknown characteristic id
+{
+    error : {
+        code : -32000
+        message : Failed to remove characteristic represented by unknown characteristic id from service 09:09:09:09:09:09_1
+    }
+    id : <number>
+}
+

third_party/blink/web_tests/inspector-protocol/bluetooth/characteristic-remove-unknown-id.js

@@ -0,0 +1,24 @@
+(async function(/** @type {import('test_runner').TestRunner} */ testRunner) {
+  const {session, dp} = await testRunner.startBlank(
+      'Tests Bluetooth removing an unknown characteristic id');
+  const bp = testRunner.browserP();
+  const BluetoothHelper =
+      await testRunner.loadScript('resources/bluetooth-helper.js')
+  const helper = new BluetoothHelper(testRunner, dp, session);
+  await helper.setupPreconnectedPeripheral();
+  const {result: {serviceId: heartRateServiceId}} =
+      await bp.BluetoothEmulation.addService({
+        address: helper.peripheralAddress(),
+        serviceUuid: BluetoothHelper.HEART_RATE_SERVICE_UUID,
+      });
+
+  // Start the test.
+  const result = await bp.BluetoothEmulation.removeCharacteristic({
+    address: helper.peripheralAddress(),
+    serviceId: heartRateServiceId,
+    characteristicId: 'unknown characteristic id'
+  });
+  testRunner.log(result);
+
+  testRunner.completeTest();
+});

third_party/blink/web_tests/inspector-protocol/bluetooth/resources/bluetooth-helper.js

@@ -5,6 +5,10 @@ class BluetoothHelper {
   static PRECONNECTED_PERIPHERAL_NAME = 'BLE Test Device';
   static HEART_RATE_SERVICE_UUID = '0000180d-0000-1000-8000-00805f9b34fb';
   static BATTERY_SERVICE_UUID = '0000180f-0000-1000-8000-00805f9b34fb';
+  static MEASUREMENT_INTERVAL_CHARACTERISTIC_UUID =
+      '00002a21-0000-1000-8000-00805f9b34fb';
+  static DATE_TIME_CHARACTERISTIC_UUID = '00002a08-0000-1000-8000-00805f9b34fb';
+
   static HCI_SUCCESS = 0x0000;
 
   constructor(testRunner, protocol, session) {

third_party/blink/web_tests/inspector-protocol/bluetooth/service-add-remove.js

@@ -29,7 +29,7 @@
   };
 
   // Start the test.
-  const {result: {id: heartRateServiceId}} =
+  const {result: {serviceId: heartRateServiceId}} =
       await bp.BluetoothEmulation.addService({
         address: helper.peripheralAddress(),
         serviceUuid: BluetoothHelper.HEART_RATE_SERVICE_UUID,
@@ -37,7 +37,7 @@
   testRunner.log(`After adding heart rate service: ${
       await session.evaluateAsync(getPrimaryServices)}`);
 
-  const {result: {id: batteryServiceId}} =
+  const {result: {serviceId: batteryServiceId}} =
       await bp.BluetoothEmulation.addService({
         address: helper.peripheralAddress(),
         serviceUuid: BluetoothHelper.BATTERY_SERVICE_UUID,
@@ -47,14 +47,14 @@
 
   await bp.BluetoothEmulation.removeService({
     address: helper.peripheralAddress(),
-    id: batteryServiceId,
+    serviceId: batteryServiceId,
   });
   testRunner.log(`After removing battery service: ${
       await session.evaluateAsync(getPrimaryServices)}`);
 
   await bp.BluetoothEmulation.removeService({
     address: helper.peripheralAddress(),
-    id: heartRateServiceId,
+    serviceId: heartRateServiceId,
   });
   testRunner.log(`After removing heart rate service: ${
       await session.evaluateAsync(getPrimaryServices)}`);

third_party/blink/web_tests/inspector-protocol/bluetooth/service-remove-unknown-id.js

@@ -9,7 +9,7 @@
 
   // Start the test.
   const result = await bp.BluetoothEmulation.removeService(
-      {address: helper.peripheralAddress(), id: 'unknown service id'});
+      {address: helper.peripheralAddress(), serviceId: 'unknown service id'});
   testRunner.log(result);
 
   testRunner.completeTest();