[Bluetooth WinRT] Explicitly open GATT service for shared access

It appears that GetCharacteristicsAsync() will implicitly open a GATT
service for exclusive access. We don't want this so this change adds an
explicit OpenAsync() call to choose shared access. The test fakes are
updated to only support shared access.

Bug: 968816
Change-Id: I30af95fe428fad075ae8cbddd3075979a834e65f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1637192Reviewed-by: Ovidio de Jesús Ruiz-Henríquez <odejesush@chromium.org>
Commit-Queue: Reilly Grant <reillyg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#665005}

device/bluetooth/bluetooth_gatt_discoverer_winrt.cc

@@ -37,12 +37,23 @@ using ABI::Windows::Devices::Bluetooth::GenericAttributeProfile::
     GattDeviceService;
 using ABI::Windows::Devices::Bluetooth::GenericAttributeProfile::
     GattDeviceServicesResult;
+using ABI::Windows::Devices::Bluetooth::GenericAttributeProfile::GattOpenStatus;
+using ABI::Windows::Devices::Bluetooth::GenericAttributeProfile::
+    GattOpenStatus_AlreadyOpened;
+using ABI::Windows::Devices::Bluetooth::GenericAttributeProfile::
+    GattOpenStatus_Success;
+using ABI::Windows::Devices::Bluetooth::GenericAttributeProfile::
+    GattSharingMode;
+using ABI::Windows::Devices::Bluetooth::GenericAttributeProfile::
+    GattSharingMode_SharedReadAndWrite;
 using ABI::Windows::Devices::Bluetooth::GenericAttributeProfile::
     IGattCharacteristic3;
 using ABI::Windows::Devices::Bluetooth::GenericAttributeProfile::
     IGattCharacteristicsResult;
 using ABI::Windows::Devices::Bluetooth::GenericAttributeProfile::
     IGattDescriptorsResult;
+using ABI::Windows::Devices::Bluetooth::GenericAttributeProfile::
+    IGattDeviceService;
 using ABI::Windows::Devices::Bluetooth::GenericAttributeProfile::
     IGattDeviceService3;
 using ABI::Windows::Devices::Bluetooth::GenericAttributeProfile::
@@ -196,15 +207,6 @@ void BluetoothGattDiscovererWinrt::OnGetGattServices(
 
   num_services_ = gatt_services_.size();
   for (const auto& gatt_service : gatt_services_) {
-    uint16_t service_attribute_handle;
-    hr = gatt_service->get_AttributeHandle(&service_attribute_handle);
-    if (FAILED(hr)) {
-      VLOG(2) << "Getting AttributeHandle failed: "
-              << logging::SystemErrorCodeToString(hr);
-      std::move(callback_).Run(false);
-      return;
-    }
-
     ComPtr<IGattDeviceService3> gatt_service_3;
     hr = gatt_service.As(&gatt_service_3);
     if (FAILED(hr)) {
@@ -214,31 +216,74 @@ void BluetoothGattDiscovererWinrt::OnGetGattServices(
       return;
     }
 
-    ComPtr<IAsyncOperation<GattCharacteristicsResult*>> get_characteristics_op;
-    hr = gatt_service_3->GetCharacteristicsAsync(&get_characteristics_op);
+    ComPtr<IAsyncOperation<GattOpenStatus>> open_op;
+    hr =
+        gatt_service_3->OpenAsync(GattSharingMode_SharedReadAndWrite, &open_op);
     if (FAILED(hr)) {
-      VLOG(2) << "GattDeviceService::GetCharacteristicsAsync() failed: "
+      VLOG(2) << "GattDeviceService::OpenAsync() failed: "
               << logging::SystemErrorCodeToString(hr);
       std::move(callback_).Run(false);
-      return;
     }
 
     hr = base::win::PostAsyncResults(
-        std::move(get_characteristics_op),
-        base::BindOnce(&BluetoothGattDiscovererWinrt::OnGetCharacteristics,
+        std::move(open_op),
+        base::BindOnce(&BluetoothGattDiscovererWinrt::OnServiceOpen,
                        weak_ptr_factory_.GetWeakPtr(),
-                       service_attribute_handle));
-
-    if (FAILED(hr)) {
-      VLOG(2) << "PostAsyncResults failed: "
-              << logging::SystemErrorCodeToString(hr);
-      std::move(callback_).Run(false);
-    }
+                       std::move(gatt_service)));
   }
 
   RunCallbackIfDone();
 }
 
+void BluetoothGattDiscovererWinrt::OnServiceOpen(
+    ComPtr<IGattDeviceService> gatt_service,
+    GattOpenStatus status) {
+  if (status != GattOpenStatus_Success &&
+      status != GattOpenStatus_AlreadyOpened) {
+    VLOG(2) << "Failed to open GATT service: " << status;
+    std::move(callback_).Run(false);
+    return;
+  }
+
+  uint16_t service_attribute_handle;
+  HRESULT hr = gatt_service->get_AttributeHandle(&service_attribute_handle);
+  if (FAILED(hr)) {
+    VLOG(2) << "Getting AttributeHandle failed: "
+            << logging::SystemErrorCodeToString(hr);
+    std::move(callback_).Run(false);
+    return;
+  }
+
+  ComPtr<IGattDeviceService3> gatt_service_3;
+  hr = gatt_service.As(&gatt_service_3);
+  if (FAILED(hr)) {
+    VLOG(2) << "Obtaining IGattDeviceService3 failed: "
+            << logging::SystemErrorCodeToString(hr);
+    std::move(callback_).Run(false);
+    return;
+  }
+
+  ComPtr<IAsyncOperation<GattCharacteristicsResult*>> get_characteristics_op;
+  hr = gatt_service_3->GetCharacteristicsAsync(&get_characteristics_op);
+  if (FAILED(hr)) {
+    VLOG(2) << "GattDeviceService::GetCharacteristicsAsync() failed: "
+            << logging::SystemErrorCodeToString(hr);
+    std::move(callback_).Run(false);
+    return;
+  }
+
+  hr = base::win::PostAsyncResults(
+      std::move(get_characteristics_op),
+      base::BindOnce(&BluetoothGattDiscovererWinrt::OnGetCharacteristics,
+                     weak_ptr_factory_.GetWeakPtr(), service_attribute_handle));
+
+  if (FAILED(hr)) {
+    VLOG(2) << "PostAsyncResults failed: "
+            << logging::SystemErrorCodeToString(hr);
+    std::move(callback_).Run(false);
+  }
+}
+
 void BluetoothGattDiscovererWinrt::OnGetCharacteristics(
     uint16_t service_attribute_handle,
     ComPtr<IGattCharacteristicsResult> characteristics_result) {

device/bluetooth/bluetooth_gatt_discoverer_winrt.h

@@ -63,6 +63,13 @@ class DEVICE_BLUETOOTH_EXPORT BluetoothGattDiscovererWinrt {
           ABI::Windows::Devices::Bluetooth::GenericAttributeProfile::
               IGattDeviceServicesResult> services_result);
 
+  void OnServiceOpen(
+      Microsoft::WRL::ComPtr<ABI::Windows::Devices::Bluetooth::
+                                 GenericAttributeProfile::IGattDeviceService>
+          gatt_service,
+      ABI::Windows::Devices::Bluetooth::GenericAttributeProfile::GattOpenStatus
+          status);
+
   void OnGetCharacteristics(
       uint16_t service_attribute_handle,
       Microsoft::WRL::ComPtr<

device/bluetooth/test/fake_gatt_device_service_winrt.cc

@@ -33,13 +33,19 @@ using ABI::Windows::Devices::Bluetooth::GenericAttributeProfile::
 using ABI::Windows::Devices::Bluetooth::GenericAttributeProfile::
     GattDeviceServicesResult;
 using ABI::Windows::Devices::Bluetooth::GenericAttributeProfile::GattOpenStatus;
+using ABI::Windows::Devices::Bluetooth::GenericAttributeProfile::
+    GattOpenStatus_AlreadyOpened;
+using ABI::Windows::Devices::Bluetooth::GenericAttributeProfile::
+    GattOpenStatus_Success;
 using ABI::Windows::Devices::Bluetooth::GenericAttributeProfile::
     GattSharingMode;
+using ABI::Windows::Devices::Bluetooth::GenericAttributeProfile::
+    GattSharingMode_SharedReadAndWrite;
 using ABI::Windows::Devices::Bluetooth::GenericAttributeProfile::IGattSession;
 using ABI::Windows::Devices::Enumeration::DeviceAccessStatus;
 using ABI::Windows::Devices::Enumeration::IDeviceAccessInformation;
-using ABI::Windows::Foundation::Collections::IVectorView;
 using ABI::Windows::Foundation::IAsyncOperation;
+using ABI::Windows::Foundation::Collections::IVectorView;
 using Microsoft::WRL::ComPtr;
 using Microsoft::WRL::Make;
 
@@ -109,11 +115,29 @@ HRESULT FakeGattDeviceServiceWinrt::RequestAccessAsync(
 HRESULT FakeGattDeviceServiceWinrt::OpenAsync(
     GattSharingMode sharing_mode,
     IAsyncOperation<GattOpenStatus>** operation) {
-  return E_NOTIMPL;
+  if (sharing_mode != GattSharingMode_SharedReadAndWrite)
+    return E_NOTIMPL;
+
+  GattOpenStatus status =
+      opened_ ? GattOpenStatus_AlreadyOpened : GattOpenStatus_Success;
+  opened_ = true;
+
+  auto async_op = Make<base::win::AsyncOperation<GattOpenStatus>>();
+  base::ThreadTaskRunnerHandle::Get()->PostTask(
+      FROM_HERE, base::BindOnce(async_op->callback(), status));
+  *operation = async_op.Detach();
+  return S_OK;
 }
 
 HRESULT FakeGattDeviceServiceWinrt::GetCharacteristicsAsync(
     IAsyncOperation<GattCharacteristicsResult*>** operation) {
+  // It has been observed that this method will implicitly call
+  // OpenAsync(Exclusive) if the service has not been opened already. Catch
+  // calls to an unopened service as we do not want to take an exclusive lock
+  // on a service.
+  if (!opened_)
+    return E_NOTIMPL;
+
   auto async_op = Make<base::win::AsyncOperation<GattCharacteristicsResult*>>();
   base::ThreadTaskRunnerHandle::Get()->PostTask(
       FROM_HERE, base::BindOnce(async_op->callback(),

device/bluetooth/test/fake_gatt_device_service_winrt.h

@@ -121,6 +121,7 @@ class FakeGattDeviceServiceWinrt
   Microsoft::WRL::ComPtr<FakeBluetoothLEDeviceWinrt> fake_device_;
   GUID uuid_;
   uint16_t attribute_handle_;
+  bool opened_ = false;
 
   std::vector<Microsoft::WRL::ComPtr<FakeGattCharacteristicWinrt>>
       fake_characteristics_;