[bluetooth] Fix GattServicesDiscovered on Win

This change fixes GattServicesDiscovered on Windows to only be called
once all services, characteristics and descriptors are discovered.
Furthermore it removes the deprecated GattDiscoveryCompleteForService
notification.

Bug: 507419, 821766
Change-Id: I234409cb159a022e0443b8c2d2a4b1c90fac9b15
Reviewed-on: https://chromium-review.googlesource.com/850357Reviewed-by: Giovanni Ortuño Urquidi <ortuno@chromium.org>
Reviewed-by: Conley Owens <cco3@chromium.org>
Commit-Queue: Jan Wilken Dörrie <jdoerrie@chromium.org>
Cr-Commit-Position: refs/heads/master@{#545016}

device/bluetooth/bluetooth_remote_gatt_service_win.cc

@@ -5,6 +5,7 @@
 #include "device/bluetooth/bluetooth_remote_gatt_service_win.h"
 
 #include <memory>
+#include <utility>
 
 #include "base/bind.h"
 #include "base/memory/ptr_util.h"
@@ -22,16 +23,14 @@ BluetoothRemoteGattServiceWin::BluetoothRemoteGattServiceWin(
     uint16_t service_attribute_handle,
     bool is_primary,
     BluetoothRemoteGattServiceWin* parent_service,
-    scoped_refptr<base::SequencedTaskRunner>& ui_task_runner)
+    scoped_refptr<base::SequencedTaskRunner> ui_task_runner)
     : device_(device),
       service_path_(service_path),
       service_uuid_(service_uuid),
       service_attribute_handle_(service_attribute_handle),
       is_primary_(is_primary),
       parent_service_(parent_service),
-      ui_task_runner_(ui_task_runner),
-      discovery_complete_notified_(false),
-      discovery_pending_count_(0),
+      ui_task_runner_(std::move(ui_task_runner)),
       weak_ptr_factory_(this) {
   DCHECK(ui_task_runner_->RunsTasksInCurrentSequence());
   DCHECK(!service_path_.empty());
@@ -107,8 +106,10 @@ void BluetoothRemoteGattServiceWin::GattCharacteristicDiscoveryComplete(
 
   discovery_completed_included_characteristics_.insert(
       characteristic->GetIdentifier());
+  SetDiscoveryComplete(included_characteristics_.size() ==
+                       discovery_completed_included_characteristics_.size());
   adapter_->NotifyGattCharacteristicAdded(characteristic);
-  NotifyGattDiscoveryCompleteForServiceIfNecessary();
+  NotifyGattServiceDiscoveryCompleteIfNecessary();
 }
 
 void BluetoothRemoteGattServiceWin::Update() {
@@ -130,11 +131,21 @@ void BluetoothRemoteGattServiceWin::OnGetIncludedCharacteristics(
   if (--discovery_pending_count_ != 0)
     return;
 
-  // Report discovery complete.
-  SetDiscoveryComplete(true);
   UpdateIncludedCharacteristics(characteristics.get(), num);
-  NotifyGattDiscoveryCompleteForServiceIfNecessary();
-  device_->GattServiceDiscoveryComplete(this);
+  SetDiscoveryComplete(included_characteristics_.size() ==
+                       discovery_completed_included_characteristics_.size());
+
+  // In case there are new included characterisitics that haven't been
+  // discovered yet, observers should be notified once that the discovery of
+  // these characteristics is complete. Hence the discovery complete flag is
+  // reset.
+  if (!IsDiscoveryComplete()) {
+    discovery_complete_notified_ = false;
+    return;
+  }
+
+  adapter_->NotifyGattServiceChanged(this);
+  NotifyGattServiceDiscoveryCompleteIfNecessary();
 }
 
 void BluetoothRemoteGattServiceWin::UpdateIncludedCharacteristics(
@@ -179,23 +190,18 @@ void BluetoothRemoteGattServiceWin::UpdateIncludedCharacteristics(
                                         std::move(characteristic_object));
     }
   }
-
-  if (IsDiscoveryComplete())
-    adapter_->NotifyGattServiceChanged(this);
 }
 
 void BluetoothRemoteGattServiceWin::
-    NotifyGattDiscoveryCompleteForServiceIfNecessary() {
-  if (discovery_completed_included_characteristics_.size() ==
-          included_characteristics_.size() &&
-      IsDiscoveryComplete() && !discovery_complete_notified_) {
-    adapter_->NotifyGattDiscoveryComplete(this);
+    NotifyGattServiceDiscoveryCompleteIfNecessary() {
+  if (IsDiscoveryComplete() && !discovery_complete_notified_) {
     discovery_complete_notified_ = true;
+    device_->GattServiceDiscoveryComplete(this);
   }
 }
 
 bool BluetoothRemoteGattServiceWin::IsCharacteristicDiscovered(
-    BTH_LE_UUID& uuid,
+    const BTH_LE_UUID& uuid,
     uint16_t attribute_handle) {
   BluetoothUUID bt_uuid =
       BluetoothTaskManagerWin::BluetoothLowEnergyUuidToBluetoothUuid(uuid);

device/bluetooth/bluetooth_remote_gatt_service_win.h

@@ -7,7 +7,9 @@
 
 #include <memory>
 #include <set>
+#include <string>
 #include <unordered_map>
+#include <vector>
 
 #include "base/files/file.h"
 #include "base/memory/weak_ptr.h"
@@ -34,7 +36,7 @@ class DEVICE_BLUETOOTH_EXPORT BluetoothRemoteGattServiceWin
       uint16_t service_attribute_handle,
       bool is_primary,
       BluetoothRemoteGattServiceWin* parent_service,
-      scoped_refptr<base::SequencedTaskRunner>& ui_task_runner);
+      scoped_refptr<base::SequencedTaskRunner> ui_task_runner);
   ~BluetoothRemoteGattServiceWin() override;
 
   // Override BluetoothRemoteGattService interfaces.
@@ -68,12 +70,13 @@ class DEVICE_BLUETOOTH_EXPORT BluetoothRemoteGattServiceWin
       PBTH_LE_GATT_CHARACTERISTIC characteristics,
       uint16_t num);
 
-  // Sends GattDiscoveryCompleteForService notification if necessary.
-  void NotifyGattDiscoveryCompleteForServiceIfNecessary();
+  // Sends GattServiceDiscoveryComplete notification if necessary.
+  void NotifyGattServiceDiscoveryCompleteIfNecessary();
 
   // Checks if the characteristic with |uuid| and |attribute_handle| has already
   // been discovered as included characteristic.
-  bool IsCharacteristicDiscovered(BTH_LE_UUID& uuid, uint16_t attribute_handle);
+  bool IsCharacteristicDiscovered(const BTH_LE_UUID& uuid,
+                                  uint16_t attribute_handle);
 
   // Checks if |characteristic| still exists in this service according to newly
   // retreived |num| of included |characteristics|.
@@ -113,11 +116,11 @@ class DEVICE_BLUETOOTH_EXPORT BluetoothRemoteGattServiceWin
 
   // Flag indicates if discovery complete notification has been send out to
   // avoid duplicate notification.
-  bool discovery_complete_notified_;
+  bool discovery_complete_notified_ = false;
 
   // Counts the number of asynchronous operations that are discovering
   // characteristics.
-  int discovery_pending_count_;
+  int discovery_pending_count_ = 0;
 
   base::WeakPtrFactory<BluetoothRemoteGattServiceWin> weak_ptr_factory_;
   DISALLOW_COPY_AND_ASSIGN(BluetoothRemoteGattServiceWin);