Refactor WebBluetoothServiceClient in the web_bluetooth.mojom

third_party/WebKit/public/platform/modules/bluetooth/web_bluetooth.mojom

 // Copyright 2016 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 module blink.mojom;
 
 import "device/bluetooth/public/interfaces/uuid.mojom";
 
 // Bluetooth Terminology:
 //
@@ skipping 139 matching lines @@
 };
 
 struct WebBluetoothRemoteGATTDescriptor {
   string instance_id; // See Instance ID Note above.
   bluetooth.mojom.UUID uuid;
 };
 
 // Web Bluetooth Interface that Blink can use to perform
 // Bluetooth GATT Operations on Bluetooth Devices.
 interface WebBluetoothService {
-  // Sets the client for this WebBluetoothService. The service will notify the
-  // client of device events e.g. when a Characteristic's value changes or when
-  // a device disconnects.
-  SetClient(associated WebBluetoothServiceClient client);
-
   RequestDevice(WebBluetoothRequestDeviceOptions options)
     => (WebBluetoothResult result, WebBluetoothDevice? device);
 
   // Creates a GATT Connection to a Bluetooth Device identified by |device_id|
   // if a connection to the device didn't exist already. If a GATT connection
   // existed already then this function increases the ref count to keep that
   // connection alive.
-  RemoteServerConnect(WebBluetoothDeviceId device_id) => (WebBluetoothResult result);
+  RemoteServerConnect(WebBluetoothDeviceId device_id) => (
+    WebBluetoothResult result,
+    associated WebBluetoothServerClient&? client_request);
 
   // If a GATT connection exists for Device identified by |device_id| then
   // decreases the ref count for that connection.
   RemoteServerDisconnect(WebBluetoothDeviceId device_id);
 
   // Returns the Services of a GATT Device identified by |device_id|.
   // If |services_uuid| is present, filters services by |services_uuid|.
   // Otherwise returns all non-blocklisted services.
   // If |quantity| == WebBluetoothGATTQueryQuantity::SINGLE, at most one
   // service will be returned.
@@ skipping 31 matching lines @@
   // |characteristic_instance_id|. The callback is run with
   // WebBluetoothResult::SUCCESS if the value was successfully
   // written.
   RemoteCharacteristicWriteValue(
     string characteristic_instance_id,
     array<uint8> value) => (WebBluetoothResult result);
 
   // Starts notifications for the characteristic identified by
   // |characteristic_instance_id|.
   RemoteCharacteristicStartNotifications(
-    string characteristic_instance_id) => (WebBluetoothResult result);
+    string characteristic_instance_id) => (
+      WebBluetoothResult result,
+      associated WebBluetoothCharacteristicClient&? client_request);

[ortuno, 2017/02/24 03:28:52]

As you mentioned now that we no longer have a global client we are missing
events for read requests. We could solve this by simply firing an event on the
readValue callback. But we would introduce a bug in which if you subscribed to
notifications and then called readValue you would get notified twice. I think
there are two ways to solve this:

1. Add logic to web_bluetooth_service_impl to not notify if there are pending
reads.
2. Change GattCharacteristicValueChanged to only get called for notifications.

I think option 2 is the way to go: For Android[1], macOS[2] and Windows[3] it's
only removing a line. For BlueZ there is a bit more work to do since we would
have to keep track of pending requests and run callbacks when the value changes.
Similar to what we do on macOS[4]

[1]
https://cs.chromium.org/chromium/src/device/bluetooth/bluetooth_remote_gatt_c...
[2]
https://cs.chromium.org/chromium/src/device/bluetooth/bluetooth_remote_gatt_c...
[3]
https://cs.chromium.org/chromium/src/device/bluetooth/bluetooth_remote_gatt_c...
[4]
https://cs.chromium.org/chromium/src/device/bluetooth/bluetooth_remote_gatt_c...

[juncai, 2017/03/01 02:04:12]

I will do this in a separate CL. 

[juncai, 2017/03/02 03:23:38]

I opened an issue at:
https://bugs.chromium.org/p/chromium/issues/detail?id=697702

 
   // Stops notifications for the characteristic identified by
   // |characteristic_instance_id|.
   RemoteCharacteristicStopNotifications(
     string characteristic_instance_id) => ();
 
   // Returns the Descriptors of a GATT Characteristic identified by
   // |characteristics_instance_id|.
   // If |descriptor_uuid| is present, filters descriptors by
   // |descriptor_uuid|. Otherwise returns all non-blocklisted descriptors.
@@ skipping 18 matching lines @@
 
   // Writes a value to the descriptor identified by
   // |descriptor_instance_id|. The callback is run with
   // WebBluetoothResult::SUCCESS if the value was successfully
   // written.
   RemoteDescriptorWriteValue(
     string descriptor_instance_id,
     array<uint8> value) => (WebBluetoothResult result);
 };
 
-// Classes should implement this interface and pass an associated pointer
-// bound to them to the WebBluetoothService by using SetClient. Classes
-// that do this will be notified of device events e.g. device disconnection.
-interface WebBluetoothServiceClient {
-  // The characteristic identified by |characteristic_instance_id| has received
-  // a notification of value change.
-  RemoteCharacteristicValueChanged(string characteristic_instance_id,
-                                   array<uint8> value);
+interface WebBluetoothCharacteristicClient {
+  RemoteCharacteristicValueChanged(array<uint8> value);
+};
 
-  // The device identified by |device_id| has been disconnected.
-  GattServerDisconnected(WebBluetoothDeviceId device_id);
+interface WebBluetoothServerClient {
+  GattServerDisconnected();
 };