bluetooth: Implement simulateGATTConnectionResponse()

device/bluetooth/test/fake_peripheral.cc

Index: device/bluetooth/test/fake_peripheral.cc
diff --git a/device/bluetooth/test/fake_peripheral.cc b/device/bluetooth/test/fake_peripheral.cc
index acf93c736d8f69ce17be05c4cf339d7072f23f9d..335ff13f89913dc03369fe439a3956995cc9103f 100644
--- a/device/bluetooth/test/fake_peripheral.cc
+++ b/device/bluetooth/test/fake_peripheral.cc
@@ -4,13 +4,17 @@
 
 #include "device/bluetooth/test/fake_peripheral.h"
 
+#include "base/memory/weak_ptr.h"
+
 namespace bluetooth {
 
 FakePeripheral::FakePeripheral(FakeCentral* fake_central,
                                const std::string& address)
     : device::BluetoothDevice(fake_central),
       address_(address),
-      gatt_connected_(false) {}
+      system_connected_(false),
+      gatt_connected_(false),
+      weak_ptr_factory_(this) {}
 
 FakePeripheral::~FakePeripheral() {}
 
@@ -18,14 +22,20 @@ void FakePeripheral::SetName(base::Optional<std::string> name) {
   name_ = std::move(name);
 }
 
-void FakePeripheral::SetGattConnected(bool connected) {
-  gatt_connected_ = connected;
+void FakePeripheral::SetSystemConnected(bool connected) {
+  system_connected_ = connected;
 }
 
 void FakePeripheral::SetServiceUUIDs(UUIDSet service_uuids) {
   service_uuids_ = std::move(service_uuids);
 }
 
+void FakePeripheral::SetNextGATTConnectionResponse(uint16_t code) {
+  DCHECK(!next_connection_response_);
+  DCHECK(create_gatt_connection_error_callbacks_.empty());
+  next_connection_response_ = code;
+}
+
 uint32_t FakePeripheral::GetBluetoothClass() const {
   NOTREACHED();
   return 0;
@@ -92,7 +102,7 @@ bool FakePeripheral::IsConnected() const {
 }
 
 bool FakePeripheral::IsGattConnected() const {
-  return gatt_connected_;
+  return system_connected_ || gatt_connected_;

[scheib, 2017/05/30 20:54:13]

Our Android code at least doesn't work this way. Warrants at least a comment
explaining this impl.

If this was base only on gatt_connected_ then would you need to have the
CreateGattConnection method overridden? 

[ortuno, 2017/05/31 05:47:27]

Unsure what you mean by Android doesn't work this way? Neither Android nor macOS
support system connected devices, only Chrome OS does which is what we based our
code in for the Web Bluetooth Service.

Added comment to IsGattConnected() explaining the problem in more detail.
Correct.

[scheib, 2017/05/31 17:22:21]

OK. We discussed this some earlier. I think this is too tricky to leave in the
fake this way -- acting in a way the actual systems don't. Then having the rest
of the testing framework rely on this not-how-it-really-is behavior. IIRC You
pointed out earlier that if we make it work the way bluez does then we need to
iterate over all devices to find the pre-connected devices. Where would we need
to do that? At the JS implementation of the framework? I'd like to understand
what things would look like if the fake acted the way bluez does. Maybe best to
do over VC.

[scheib, 2017/06/01 05:03:29]

We've just had a VC chat. I understand more context now regarding bluez's
implementation and what our web bluetooth service impl is doing. I think the
comments could be made much clearer by referring to a document that describes
the current state of 'system connected' devices. 

The issue linked (630581) isn't easy to understand the current status of, and
doesn't do a clean job at introducing this concept. 

I suggest that issue be updated to indicate the suggested next steps for that
issue.

Here I think the comment should refer either an issue or a README.md section
describing the overall problem of system connected devices and suggested future
ways to clean things up. 

 }
 
 bool FakePeripheral::IsConnectable() const {
@@ -184,11 +194,43 @@ void FakePeripheral::ConnectToServiceInsecurely(
   NOTREACHED();
 }
 
+void FakePeripheral::CreateGattConnection(
+    const GattConnectionCallback& callback,
+    const ConnectErrorCallback& error_callback) {
+  create_gatt_connection_success_callbacks_.push_back(callback);
+  create_gatt_connection_error_callbacks_.push_back(error_callback);
+
+  // TODO(crbug.com/630581): Use BluetoothDevice::CreateGattConnection when
+  // system connected devices are supported.
+  if (gatt_connected_)
+    return DidConnectGatt();
+
+  CreateGattConnectionImpl();
+}
+
 void FakePeripheral::CreateGattConnectionImpl() {
-  NOTREACHED();
+  base::ThreadTaskRunnerHandle::Get()->PostTask(
+      FROM_HERE, base::Bind(&FakePeripheral::DispatchConnectionResponse,
+                            weak_ptr_factory_.GetWeakPtr()));
+}
+
+void FakePeripheral::DispatchConnectionResponse() {
+  DCHECK(!!next_connection_response_);
+
+  uint16_t code = next_connection_response_.value();
+  next_connection_response_.reset();
+
+  if (code == mojom::kHCISuccess) {
+    gatt_connected_ = true;
+    DidConnectGatt();
+  } else if (code == mojom::kHCIConnectionTimeout) {
+    DidFailToConnectGatt(ERROR_FAILED);
+  } else {
+    DidFailToConnectGatt(ERROR_UNKNOWN);
+  }
 }
 
 void FakePeripheral::DisconnectGatt() {
-  NOTREACHED();
 }
+
 }  // namespace bluetooth