bluetooth: Add Device connection logic and accompanying user interface.

device/bluetooth/device_unittest.cc

+// 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.
+
+#include "device/bluetooth/device.h"
+
+#include <memory>
+#include <string>
+#include <utility>
+
+#include "base/logging.h"
+#include "base/memory/ptr_util.h"
+#include "base/memory/weak_ptr.h"
+#include "base/run_loop.h"
+#include "device/bluetooth/test/device_connection_helper.h"
+#include "device/bluetooth/test/mock_bluetooth_adapter.h"
+#include "device/bluetooth/test/mock_bluetooth_device.h"
+#include "device/bluetooth/test/mock_bluetooth_gatt_connection.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+using ::testing::Return;
+
+namespace bluetooth {
+
+typedef testing::NiceMock<device::MockBluetoothAdapter>
+    NiceMockBluetoothAdapter;
+typedef testing::NiceMock<device::MockBluetoothDevice> NiceMockBluetoothDevice;
+typedef testing::NiceMock<device::MockBluetoothGattService>
+    NiceMockBluetoothGattService;
+typedef testing::NiceMock<device::MockBluetoothGattConnection>
+    NiceMockBluetoothGattConnection;

[dcheng, 2016/11/11 01:25:33]

Nit: prefer using A = B; to typedef B A; in new code.

[mbrunson, 2016/11/11 21:26:15]

Done.

+
+namespace {
+const char kTestLeDeviceAddress0[] = "11:22:33:44:55:66";
+const char kTestLeDeviceName0[] = "Test LE Device 0";
+
+const char kTestServiceId0[] = "service_id0";
+const char kTestServiceUuid0[] = "1234";
+
+const char kTestServiceId1[] = "service_id1";
+const char kTestServiceUuid1[] = "5678";
+
+class BluetoothInterfaceDeviceTest : public testing::Test {
+ public:
+  enum class Call { EXPECTED, NOT_EXPECTED };
+
+  BluetoothInterfaceDeviceTest()
+      : adapter_(new NiceMockBluetoothAdapter),
+        message_loop_(new base::MessageLoop),
+        weak_factory_(this) {
+    device_.reset(
+        new NiceMockBluetoothDevice(adapter_.get(), 0, kTestLeDeviceName0,
+                                    kTestLeDeviceAddress0, false, true));
+
+    ON_CALL(*adapter_, GetDevice(kTestLeDeviceAddress0))
+        .WillByDefault(Return(device_.get()));
+
+    std::unique_ptr<NiceMockBluetoothGattService> service1(

[dcheng, 2016/11/11 01:25:33]

auto service1 = base::MakeUnique<NiceMockBluetoothGattService>(...);

[mbrunson, 2016/11/11 21:26:15]

Done.

+        new NiceMockBluetoothGattService(
+            device_.get(), kTestServiceId0,
+            device::BluetoothUUID(kTestServiceUuid0), true /* is_primary */,
+            false /* is_local */));
+    std::unique_ptr<NiceMockBluetoothGattService> service2(

[dcheng, 2016/11/11 01:25:33]

Ditto.

[mbrunson, 2016/11/11 21:26:15]

Done.

+        new NiceMockBluetoothGattService(
+            device_.get(), kTestServiceId1,
+            device::BluetoothUUID(kTestServiceUuid1), true /* is_primary */,
+            false /* is_local */));
+
+    device_->AddMockService(std::move(service1));
+    device_->AddMockService(std::move(service2));
+
+    EXPECT_CALL(*device_, GetGattServices())
+        .WillRepeatedly(Invoke(device_.get(),
+                               &device::MockBluetoothDevice::GetMockServices));
+
+    std::unique_ptr<NiceMockBluetoothGattConnection> connection(
+        new NiceMockBluetoothGattConnection(adapter_, device_->GetAddress()));
+
+    // Owns itself.
+    device_service_ =
+        new Device(adapter_, std::move(connection), mojo::GetProxy(&proxy_));
+  }
+
+  void TearDown() override {
+    EXPECT_EQ(expected_success_callback_calls_, actual_success_callback_calls_);
+
+    if (!expect_device_service_deleted_) {
+      delete device_service_;

[dcheng, 2016/11/11 01:25:33]

This is confusing. Previous comments seem to imply it's self-owned, but
apparently not really?

[mbrunson, 2016/11/11 21:26:15]

There are a few tests that determine whether the Device actually deletes itself
if a disconnection occurs. If this isn't expected to occur, I delete it manually
here to prevent it from leaking.

+    }
+  }
+
+ protected:
+  void GetServicesCheckForPrecedingCalls(
+      Call expected,
+      size_t expected_service_count,
+      int num_of_preceding_calls,
+      DeviceConnectionHelper* helper,
+      const base::Closure& continuation,
+      std::vector<mojom::ServiceInfoPtr> services) {
+    helper->SetCallbackRan(true);
+    EXPECT_EQ(num_of_preceding_calls, callback_count_);
+    ++callback_count_;
+
+    if (expected == Call::EXPECTED)
+      ++actual_success_callback_calls_;
+
+    EXPECT_EQ(expected_service_count, services.size());
+    continuation.Run();
+  }
+
+  Device::GetServicesCallback GetGetServicesCheckForPrecedingCalls(
+      Call expected,
+      int num_of_preceding_calls,
+      const base::Closure& continuation) {
+    if (expected == Call::EXPECTED)
+      ++expected_success_callback_calls_;
+
+    return base::Bind(
+        &BluetoothInterfaceDeviceTest::GetServicesCheckForPrecedingCalls,
+        weak_factory_.GetWeakPtr(), expected, 2 /* expected_service_count */,
+        num_of_preceding_calls, base::Owned(new DeviceConnectionHelper(
+                                    device_service_->GetBindingForTesting())),
+        continuation);
+  }
+
+  scoped_refptr<NiceMockBluetoothAdapter> adapter_;
+  std::unique_ptr<NiceMockBluetoothDevice> device_;
+  Device* device_service_;
+  std::unique_ptr<base::MessageLoop> message_loop_;

[dcheng, 2016/11/11 01:25:33]

Nit: device_ and message_loop_ can just be members of the text fixture class
directly, I think.

[mbrunson, 2016/11/11 21:26:15]

Are you saying they should be put in the TEST_F sections or in the constructor?
They're already in the BluetoothInterfaceDeviceTest class.

[dcheng, 2016/11/15 07:24:02]

The unique_ptr indirection is unnecessary: they can just be
NiceMockBluetoothDevice device_ and base::MessageLoop message_loop_.

[mbrunson, 2016/11/16 03:32:03]

Done.

+  mojom::DevicePtr proxy_;
+
+  bool expect_device_service_deleted_ = false;
+  int expected_success_callback_calls_ = 0;
+  int actual_success_callback_calls_ = 0;
+  int callback_count_ = 0;
+
+  base::WeakPtrFactory<BluetoothInterfaceDeviceTest> weak_factory_;
+};
+}  // namespace
+
+TEST_F(BluetoothInterfaceDeviceTest, GetServices) {
+  EXPECT_CALL(*device_, IsGattServicesDiscoveryComplete())
+      .WillRepeatedly(Return(true));
+
+  base::RunLoop loop;
+  device_service_->GetServices(GetGetServicesCheckForPrecedingCalls(
+      Call::EXPECTED, 0 /* num_of_preceding_calls */, loop.QuitClosure()));
+
+  loop.Run();
+}
+
+TEST_F(BluetoothInterfaceDeviceTest, GetServicesNotDiscovered) {
+  EXPECT_CALL(*device_, IsGattServicesDiscoveryComplete())
+      .WillOnce(Return(false))
+      .WillOnce(Return(false))
+      .WillRepeatedly(Return(true));
+
+  base::RunLoop loop;
+
+  // Client: Sends multiple requests for services.
+  device_service_->GetServices(GetGetServicesCheckForPrecedingCalls(
+      Call::EXPECTED, 0 /* num_of_preceding_calls */, loop.QuitClosure()));
+  device_service_->GetServices(GetGetServicesCheckForPrecedingCalls(
+      Call::EXPECTED, 1 /* num_of_preceding_calls */, loop.QuitClosure()));
+
+  // Simulate: GattServicesDiscovered.
+  device_service_->GattServicesDiscovered(nullptr /* adapter */, device_.get());
+
+  // No more GetServices calls will complete.
+  device_service_->GattServicesDiscovered(nullptr /* adapter */, device_.get());
+
+  // Client: Sends more requests which run immediately.
+  device_service_->GetServices(GetGetServicesCheckForPrecedingCalls(
+      Call::EXPECTED, 2 /* num_of_preceding_calls */, loop.QuitClosure()));
+  device_service_->GetServices(GetGetServicesCheckForPrecedingCalls(
+      Call::EXPECTED, 3 /* num_of_preceding_calls */, loop.QuitClosure()));
+
+  // No more GetServices calls will complete.
+  device_service_->GattServicesDiscovered(nullptr /* adapter */, device_.get());
+
+  loop.Run();
+}
+
+TEST_F(BluetoothInterfaceDeviceTest,
+       GetServicesLostConnectionWithPendingRequests) {
+  EXPECT_CALL(*device_, IsGattServicesDiscoveryComplete())
+      .WillRepeatedly(Return(false));
+  // Client: Sends multiple requests for services.
+  base::RunLoop loop;
+  device_service_->GetServices(GetGetServicesCheckForPrecedingCalls(
+      Call::NOT_EXPECTED, 0 /* num_of_preceding_calls */, loop.QuitClosure()));
+  device_service_->GetServices(GetGetServicesCheckForPrecedingCalls(
+      Call::NOT_EXPECTED, 1 /* num_of_preceding_calls */, loop.QuitClosure()));
+  EXPECT_EQ(0, callback_count_);
+
+  // Simulate connection loss.
+  device_->SetConnected(false);
+  device_service_->DeviceChanged(nullptr /* adapter */, device_.get());
+  expect_device_service_deleted_ = true;
+}
+
+TEST_F(BluetoothInterfaceDeviceTest,
+       GetServicesForcedDisconnectionWithPendingRequests) {
+  EXPECT_CALL(*device_, IsGattServicesDiscoveryComplete())
+      .WillRepeatedly(Return(false));
+  // Client: Sends multiple requests for services.
+  base::RunLoop loop;
+  device_service_->GetServices(GetGetServicesCheckForPrecedingCalls(
+      Call::NOT_EXPECTED, 0 /* num_of_preceding_calls */, loop.QuitClosure()));
+  device_service_->GetServices(GetGetServicesCheckForPrecedingCalls(
+      Call::NOT_EXPECTED, 1 /* num_of_preceding_calls */, loop.QuitClosure()));
+  EXPECT_EQ(0, callback_count_);
+
+  // Simulate connection loss.
+  device_service_->Disconnect();
+  expect_device_service_deleted_ = true;
+}
+
+TEST_F(BluetoothInterfaceDeviceTest, GetServicesPipeClosedWithPendingRequests) {
+  EXPECT_CALL(*device_, IsGattServicesDiscoveryComplete())
+      .WillRepeatedly(Return(false));
+  // Client: Sends multiple requests for services.
+  base::RunLoop loop;
+  device_service_->GetServices(GetGetServicesCheckForPrecedingCalls(
+      Call::NOT_EXPECTED, 0 /* num_of_preceding_calls */, loop.QuitClosure()));
+  device_service_->GetServices(GetGetServicesCheckForPrecedingCalls(
+      Call::NOT_EXPECTED, 1 /* num_of_preceding_calls */, loop.QuitClosure()));
+  EXPECT_EQ(0, callback_count_);
+
+  // Simulate message pipe error.
+  proxy_.reset();
+  expect_device_service_deleted_ = true;
+}
+
+}  // namespace bluetooth