[CrOS Tether] Create BleScanner, a class which scan BLE advertisements and identifies nearby device…

chromeos/components/tether/ble_scanner_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 "chromeos/components/tether/ble_scanner.h"
+
+#include "base/logging.h"
+#include "chromeos/components/tether/ble_constants.h"
+#include "chromeos/components/tether/mock_local_device_data_provider.h"
+#include "components/cryptauth/mock_eid_generator.h"
+#include "components/cryptauth/proto/cryptauth_api.pb.h"
+#include "components/cryptauth/remote_device_test_util.h"
+#include "device/bluetooth/test/mock_bluetooth_adapter.h"
+#include "device/bluetooth/test/mock_bluetooth_device.h"
+#include "device/bluetooth/test/mock_bluetooth_discovery_session.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+using testing::_;
+using testing::DoAll;
+using testing::Eq;
+using testing::Invoke;
+using testing::NiceMock;
+using testing::SaveArg;
+using testing::Return;
+
+namespace chromeos {
+
+namespace tether {
+
+namespace {
+class MockBleScannerObserver : public BleScanner::Observer {
+ public:
+  MockBleScannerObserver() {}
+
+  void OnReceivedAdvertisementFromDevice(
+      const device::BluetoothDevice* bluetooth_device,
+      cryptauth::RemoteDevice remote_device) override {
+    bluetooth_devices_.push_back(bluetooth_device);
+    remote_devices_.push_back(remote_device);
+  }
+
+  int GetNumCalls() { return static_cast<int>(bluetooth_devices_.size()); }
+
+  std::vector<const device::BluetoothDevice*>& bluetooth_devices() {
+    return bluetooth_devices_;
+  }
+
+  std::vector<cryptauth::RemoteDevice>& remote_devices() {
+    return remote_devices_;
+  }
+
+ private:
+  std::vector<const device::BluetoothDevice*> bluetooth_devices_;
+  std::vector<cryptauth::RemoteDevice> remote_devices_;
+};
+
+class MockServiceDataBluetoothDevice : public device::MockBluetoothDevice {

[Ryan Hansberry, 2017/01/04 00:01:04]

nit: Maybe rename this MockBluetoothDeviceWithServiceData?

[Kyle Horimoto, 2017/01/04 18:35:43]

Done.

+ public:
+  MockServiceDataBluetoothDevice(device::MockBluetoothAdapter* adapter,
+                                 const std::string& service_data)
+      : device::MockBluetoothDevice(adapter,
+                                    /* bluetooth_class */ 0,
+                                    "name",
+                                    "11:22:33:44:55:66",
+                                    false,
+                                    false) {
+    for (size_t i = 0; i < service_data.size(); i++) {
+      service_data_.push_back(static_cast<uint8_t>(service_data[i]));
+    }
+  }
+
+  const std::vector<uint8_t>* service_data() { return &service_data_; }
+
+ private:
+  std::vector<uint8_t> service_data_;
+};
+
+const std::string fake_local_public_key = "fakeLocalPublicKey";
+
+const std::string current_eid_data = "currentEidData";
+const int64_t current_eid_start_ms = 1000L;
+const int64_t current_eid_end_ms = 2000L;
+
+const std::string adjacent_eid_data = "adjacentEidData";
+const int64_t adjacent_eid_start_ms = 2000L;
+const int64_t adjacent_eid_end_ms = 3000L;
+
+const std::string fake_beacon_seed1_data = "fakeBeaconSeed1Data";

[Ryan Hansberry, 2017/01/04 00:01:04]

nit: instead of '1' and '2', used 'current' and 'adjacent', respectively.

[Kyle Horimoto, 2017/01/04 18:35:43]

I'm going to keep it as 1 and 2. There is no notion of current or adjacent for
the seeds themselves, only for the EidData.

+const int64_t fake_beacon_seed1_start_ms = 1000L;

[Ryan Hansberry, 2017/01/04 00:01:04]

Directly 'current_eid_start_ms' and etc. instead of essentially redeclaring
them.

[Kyle Horimoto, 2017/01/04 18:35:43]

Ditto.

[Ryan Hansberry, 2017/01/04 20:02:20]

I understand your reasoning for not renaming the seeds to current or adjacent,
but why not set variables like 'fake_beacon_seed1_start_ms' to
'current_eid_start_ms' instead of reusing the literal value 1000?

[Kyle Horimoto, 2017/01/04 20:06:56]

Done.

+const int64_t fake_beacon_seed1_end_ms = 2000L;
+
+const std::string fake_beacon_seed2_data = "fakeBeaconSeed2Data";
+const int64_t fake_beacon_seed2_start_ms = 2000L;
+const int64_t fake_beacon_seed2_end_ms = 3000L;
+
+std::unique_ptr<cryptauth::EidGenerator::EidData>
+CreateFakeBackgroundScanFilter() {
+  cryptauth::EidGenerator::DataWithTimestamp current(
+      current_eid_data, current_eid_start_ms, current_eid_end_ms);
+
+  std::unique_ptr<cryptauth::EidGenerator::DataWithTimestamp> adjacent =
+      base::MakeUnique<cryptauth::EidGenerator::DataWithTimestamp>(
+          adjacent_eid_data, adjacent_eid_start_ms, adjacent_eid_end_ms);
+
+  return base::MakeUnique<cryptauth::EidGenerator::EidData>(
+      current, std::move(adjacent));
+}
+
+std::vector<cryptauth::BeaconSeed> CreateFakeBeaconSeeds() {
+  cryptauth::BeaconSeed seed1;
+  seed1.set_data(fake_beacon_seed1_data);
+  seed1.set_start_time_millis(fake_beacon_seed1_start_ms);
+  seed1.set_start_time_millis(fake_beacon_seed1_end_ms);
+
+  cryptauth::BeaconSeed seed2;
+  seed2.set_data(fake_beacon_seed2_data);
+  seed2.set_start_time_millis(fake_beacon_seed2_start_ms);
+  seed2.set_start_time_millis(fake_beacon_seed2_end_ms);
+
+  std::vector<cryptauth::BeaconSeed> seeds = {seed1, seed2};
+  return seeds;
+}
+}  // namespace
+
+class BleScannerTest : public testing::Test {
+ protected:
+  class TestDelegate : public BleScanner::Delegate {
+   public:
+    TestDelegate()
+        : is_bluetooth_adapter_available_(true),
+          last_get_adapter_callback_(nullptr) {}
+
+    ~TestDelegate() override {}
+
+    bool IsBluetoothAdapterAvailable() const override {
+      return is_bluetooth_adapter_available_;
+    }
+
+    void set_is_bluetooth_adapter_available(
+        bool is_bluetooth_adapter_available) {
+      is_bluetooth_adapter_available_ = is_bluetooth_adapter_available;
+    }
+
+    void GetAdapter(const device::BluetoothAdapterFactory::AdapterCallback&
+                        callback) override {
+      last_get_adapter_callback_ = callback;
+    }
+
+    const device::BluetoothAdapterFactory::AdapterCallback
+    last_get_adapter_callback() {
+      return last_get_adapter_callback_;
+    }
+
+    const std::vector<uint8_t>* GetServiceDataForUUID(
+        const device::BluetoothUUID& service_uuid,
+        device::BluetoothDevice* device) override {
+      if (device::BluetoothUUID(kAdvertisingServiceUuid) == service_uuid) {
+        return reinterpret_cast<MockServiceDataBluetoothDevice*>(device)

[Ryan Hansberry, 2017/01/04 00:01:04]

For my own learning: Why did you choose reinterpret_cast instead of static_cast?

[Kyle Horimoto, 2017/01/04 18:35:44]

reinterpret_cast should be used when we are converting a type to another version
of the same type (i.e., MockBluetoothDeviceWithServiceData is a
MockBluetoothDevice). static_cast should be used when you are changing the
interpretation of the bytes (e.g., when casting a long to an int).

[Ryan Hansberry, 2017/01/04 20:02:20]

Thanks!

+            ->service_data();
+      }
+
+      return nullptr;
+    }
+
+   private:
+    bool is_bluetooth_adapter_available_;
+    device::BluetoothAdapterFactory::AdapterCallback last_get_adapter_callback_;
+  };
+
+  BleScannerTest()
+      : test_devices_(cryptauth::GenerateTestRemoteDevices(3)),
+        test_beacon_seeds_(CreateFakeBeaconSeeds()) {}
+
+  void SetUp() override {
+    test_delegate_ = new TestDelegate();
+    EXPECT_TRUE(test_delegate_->IsBluetoothAdapterAvailable());
+    EXPECT_FALSE(test_delegate_->last_get_adapter_callback());
+
+    mock_eid_generator_ = base::MakeUnique<cryptauth::MockEidGenerator>();
+    mock_eid_generator_->set_background_scan_filter(
+        CreateFakeBackgroundScanFilter());
+
+    mock_local_device_data_provider_ =
+        base::MakeUnique<MockLocalDeviceDataProvider>();
+    mock_local_device_data_provider_->SetPublicKey(
+        base::MakeUnique<std::string>(fake_local_public_key));
+    mock_local_device_data_provider_->SetBeaconSeeds(
+        base::MakeUnique<std::vector<cryptauth::BeaconSeed>>(
+            test_beacon_seeds_));
+
+    mock_adapter_ =
+        make_scoped_refptr(new NiceMock<device::MockBluetoothAdapter>());
+    stored_discovery_filter_.reset();
+    stored_discovery_callback_.Reset();
+    stored_discovery_errback_.Reset();
+    ON_CALL(*mock_adapter_, StartDiscoverySessionWithFilterRaw(_, _, _))
+        .WillByDefault(Invoke(
+            this, &BleScannerTest::SaveStartDiscoverySessionWithFilterArgs));
+    ON_CALL(*mock_adapter_, IsPowered()).WillByDefault(Return(true));
+
+    mock_discovery_session_ = nullptr;
+
+    ble_scanner_ = base::WrapUnique(new BleScanner(
+        base::WrapUnique(test_delegate_), mock_eid_generator_.get(),

[Ryan Hansberry, 2017/01/04 00:01:04]

nit: place this arg on a separate line.

[Kyle Horimoto, 2017/01/04 18:35:43]

Same - this is what the Chromium lint tool chose.

[Ryan Hansberry, 2017/01/04 20:02:20]

My bad, still getting used to the style.

+        mock_local_device_data_provider_.get()));
+
+    mock_observer_ = base::MakeUnique<MockBleScannerObserver>();
+    ble_scanner_->AddObserver(mock_observer_.get());
+  }
+
+  void SaveStartDiscoverySessionWithFilterArgs(
+      const device::BluetoothDiscoveryFilter* discovery_filter,
+      const device::BluetoothAdapter::DiscoverySessionCallback& callback,
+      const device::BluetoothAdapter::ErrorCallback& errback) {
+    stored_discovery_filter_ =
+        base::MakeUnique<device::BluetoothDiscoveryFilter>(
+            device::BluetoothTransport::BLUETOOTH_TRANSPORT_LE);
+    stored_discovery_filter_->CopyFrom(*discovery_filter);
+    stored_discovery_callback_ = callback;
+    stored_discovery_errback_ = errback;
+  }
+
+  void InvokeAdapterCallback() {
+    const device::BluetoothAdapterFactory::AdapterCallback
+        last_get_adapter_callback = test_delegate_->last_get_adapter_callback();
+    ASSERT_TRUE(last_get_adapter_callback);

[Ryan Hansberry, 2017/01/04 00:01:04]

Is there any kind of ASSERT_NOT_NULL? This reads like it's a boolean. Not a big
deal if not.

[Kyle Horimoto, 2017/01/04 18:35:44]

No, this is the conventional approach within Chromium.

+
+    // Because the adapter has just been initialized, the discovery session
+    // should not have been started yet.
+    EXPECT_FALSE(stored_discovery_filter_);
+    EXPECT_TRUE(stored_discovery_callback_.is_null());
+    EXPECT_TRUE(stored_discovery_errback_.is_null());
+
+    EXPECT_CALL(*mock_adapter_, AddObserver(ble_scanner_.get()));
+    last_get_adapter_callback.Run(mock_adapter_);
+
+    // Once the adapter callback is returned, a discovery session should be
+    // started via that adapter.
+    AssertDiscoverySessionRequested();
+  }
+
+  void AssertDiscoverySessionRequested() {
+    // First, ensure that the correct discovery filter was passed.
+    EXPECT_TRUE(stored_discovery_filter_);
+    EXPECT_EQ(device::BluetoothTransport::BLUETOOTH_TRANSPORT_LE,
+              stored_discovery_filter_->GetTransport());
+    int16_t observed_rssi;
+    ASSERT_TRUE(stored_discovery_filter_->GetRSSI(&observed_rssi));
+    EXPECT_EQ(-90, observed_rssi);

[Ryan Hansberry, 2017/01/04 00:01:04]

Please use the constant defined in BleScanner.

[Kyle Horimoto, 2017/01/04 18:35:44]

That is an implementation detail of the class, so I don't want to expose it
publicly. I used a constant kExpectedDiscoveryRSSI for this test.

+
+    // Now, ensure that both a callback and errback were pased.

[Ryan Hansberry, 2017/01/04 00:01:04]

passed*

[Kyle Horimoto, 2017/01/04 18:35:44]

Done.

+    EXPECT_FALSE(stored_discovery_callback_.is_null());
+    EXPECT_FALSE(stored_discovery_errback_.is_null());
+  }
+
+  void InvokeDiscoveryStartedCallback() {
+    EXPECT_FALSE(stored_discovery_callback_.is_null());
+
+    mock_discovery_session_ = new device::MockBluetoothDiscoverySession();
+    stored_discovery_callback_.Run(base::WrapUnique(mock_discovery_session_));
+  }
+
+  std::vector<cryptauth::RemoteDevice> test_devices_;
+  std::vector<cryptauth::BeaconSeed> test_beacon_seeds_;
+
+  std::unique_ptr<MockBleScannerObserver> mock_observer_;
+
+  TestDelegate* test_delegate_;
+  std::unique_ptr<cryptauth::MockEidGenerator> mock_eid_generator_;
+  std::unique_ptr<MockLocalDeviceDataProvider> mock_local_device_data_provider_;
+
+  scoped_refptr<NiceMock<device::MockBluetoothAdapter>> mock_adapter_;
+  device::MockBluetoothDiscoverySession* mock_discovery_session_;
+
+  std::unique_ptr<device::BluetoothDiscoveryFilter> stored_discovery_filter_;
+  device::BluetoothAdapter::DiscoverySessionCallback stored_discovery_callback_;
+  device::BluetoothAdapter::ErrorCallback stored_discovery_errback_;
+
+  std::unique_ptr<BleScanner> ble_scanner_;
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(BleScannerTest);
+};
+
+TEST_F(BleScannerTest, TestNoBluetoothAdapter) {
+  test_delegate_->set_is_bluetooth_adapter_available(false);
+  EXPECT_FALSE(ble_scanner_->RegisterScanFilterForDevice(test_devices_[0]));
+  EXPECT_FALSE(
+      ble_scanner_->IsDeviceRegistered(test_devices_[0].GetDeviceId()));
+  EXPECT_FALSE(test_delegate_->last_get_adapter_callback());
+  EXPECT_FALSE(mock_observer_->GetNumCalls());
+}
+
+TEST_F(BleScannerTest, TestNoLocalBeaconSeeds) {
+  mock_local_device_data_provider_->SetBeaconSeeds(nullptr);
+  EXPECT_FALSE(ble_scanner_->RegisterScanFilterForDevice(test_devices_[0]));
+  EXPECT_FALSE(
+      ble_scanner_->IsDeviceRegistered(test_devices_[0].GetDeviceId()));
+  EXPECT_FALSE(test_delegate_->last_get_adapter_callback());
+  EXPECT_FALSE(mock_observer_->GetNumCalls());
+}
+
+TEST_F(BleScannerTest, TestNoBackgroundScanFilter) {
+  mock_eid_generator_->set_background_scan_filter(nullptr);
+  EXPECT_FALSE(ble_scanner_->RegisterScanFilterForDevice(test_devices_[0]));
+  EXPECT_FALSE(
+      ble_scanner_->IsDeviceRegistered(test_devices_[0].GetDeviceId()));
+  EXPECT_FALSE(test_delegate_->last_get_adapter_callback());
+  EXPECT_FALSE(mock_observer_->GetNumCalls());
+}
+
+TEST_F(BleScannerTest, TestAdapterDoesNotInitialize) {
+  EXPECT_TRUE(ble_scanner_->RegisterScanFilterForDevice(test_devices_[0]));
+  EXPECT_TRUE(ble_scanner_->IsDeviceRegistered(test_devices_[0].GetDeviceId()));
+  EXPECT_TRUE(test_delegate_->last_get_adapter_callback());
+
+  // Do not call the last GetAdapter() callback. The device should still be able
+  // to be unregistered.
+  EXPECT_TRUE(ble_scanner_->UnregisterScanFilterForDevice(test_devices_[0]));
+  EXPECT_FALSE(
+      ble_scanner_->IsDeviceRegistered(test_devices_[0].GetDeviceId()));
+  EXPECT_FALSE(mock_observer_->GetNumCalls());
+}
+
+TEST_F(BleScannerTest, TestAdapterDoesNotInitialize_MultipleDevicesRegistered) {
+  EXPECT_TRUE(ble_scanner_->RegisterScanFilterForDevice(test_devices_[0]));
+  EXPECT_TRUE(ble_scanner_->RegisterScanFilterForDevice(test_devices_[1]));
+  EXPECT_TRUE(ble_scanner_->IsDeviceRegistered(test_devices_[0].GetDeviceId()));
+  EXPECT_TRUE(ble_scanner_->IsDeviceRegistered(test_devices_[1].GetDeviceId()));
+  EXPECT_TRUE(test_delegate_->last_get_adapter_callback());
+
+  // Do not call the last GetAdapter() callback. The devices should still be
+  // able to be unregistered.
+  EXPECT_TRUE(ble_scanner_->UnregisterScanFilterForDevice(test_devices_[0]));
+  EXPECT_TRUE(ble_scanner_->UnregisterScanFilterForDevice(test_devices_[1]));
+  EXPECT_FALSE(
+      ble_scanner_->IsDeviceRegistered(test_devices_[0].GetDeviceId()));
+  EXPECT_FALSE(
+      ble_scanner_->IsDeviceRegistered(test_devices_[1].GetDeviceId()));
+  EXPECT_FALSE(mock_observer_->GetNumCalls());
+}
+
+TEST_F(BleScannerTest, TestDiscoverySessionFailsToStart) {
+  EXPECT_TRUE(ble_scanner_->RegisterScanFilterForDevice(test_devices_[0]));
+  EXPECT_TRUE(ble_scanner_->IsDeviceRegistered(test_devices_[0].GetDeviceId()));
+
+  InvokeAdapterCallback();
+  stored_discovery_errback_.Run();
+
+  EXPECT_TRUE(ble_scanner_->IsDeviceRegistered(test_devices_[0].GetDeviceId()));
+  EXPECT_TRUE(ble_scanner_->UnregisterScanFilterForDevice(test_devices_[0]));
+  EXPECT_FALSE(mock_observer_->GetNumCalls());
+}
+
+TEST_F(BleScannerTest, TestDiscoveryStartsButNoDevicesFound) {
+  EXPECT_TRUE(ble_scanner_->RegisterScanFilterForDevice(test_devices_[0]));
+  EXPECT_TRUE(ble_scanner_->IsDeviceRegistered(test_devices_[0].GetDeviceId()));
+
+  InvokeAdapterCallback();
+  InvokeDiscoveryStartedCallback();
+
+  // No devices found.
+
+  EXPECT_TRUE(ble_scanner_->IsDeviceRegistered(test_devices_[0].GetDeviceId()));
+  EXPECT_TRUE(ble_scanner_->UnregisterScanFilterForDevice(test_devices_[0]));
+  EXPECT_FALSE(mock_observer_->GetNumCalls());
+}
+
+TEST_F(BleScannerTest, TestDiscovery) {

[Ryan Hansberry, 2017/01/04 00:01:04]

nit: I think it would be very valuable to split this test up. It's quite large,
and the values of calls like mock_eid_generator_->num_identify_calls() are
affected by previous tests, which means you need to understand how all these
separate tests interact in order to understand this test method as a whole.

[Kyle Horimoto, 2017/01/04 18:35:44]

Done.

+  size_t kMinNumBytesInServiceData = 4;
+  std::string empty_service_data = "";
+  std::string short_service_data = "abc";
+  std::string valid_service_data_for_other_device = "abcd";
+  std::string valid_service_data_for_registered_device = "abcde";
+  ASSERT_TRUE(short_service_data.size() < kMinNumBytesInServiceData);
+  ASSERT_TRUE(valid_service_data_for_other_device.size() >=
+              kMinNumBytesInServiceData);
+  ASSERT_TRUE(valid_service_data_for_registered_device.size() >=
+              kMinNumBytesInServiceData);
+
+  EXPECT_TRUE(ble_scanner_->RegisterScanFilterForDevice(test_devices_[0]));
+  EXPECT_TRUE(ble_scanner_->IsDeviceRegistered(test_devices_[0].GetDeviceId()));
+
+  InvokeAdapterCallback();
+  InvokeDiscoveryStartedCallback();
+
+  // Device with no service data connected. Service data is required to identify
+  // the advertising device.
+  MockServiceDataBluetoothDevice device1(mock_adapter_.get(),
+                                         empty_service_data);
+  ble_scanner_->DeviceAdded(mock_adapter_.get(), &device1);
+  EXPECT_FALSE(mock_eid_generator_->num_identify_calls());
+  EXPECT_FALSE(mock_observer_->GetNumCalls());

[Ryan Hansberry, 2017/01/04 00:01:04]

Please use EXPECT_EQ(0, ...) instead, for all these calls that return numbers.

[Kyle Horimoto, 2017/01/04 18:35:44]

EXPECT_TRUE/EXPECT_FALSE are the convention in Chromium for these cases.

+
+  // Device with short service data connected. Service data of at least 4 bytes
+  // is required to identify the advertising device.
+  MockServiceDataBluetoothDevice device2(mock_adapter_.get(),
+                                         short_service_data);
+  ble_scanner_->DeviceAdded(mock_adapter_.get(), &device2);
+  EXPECT_FALSE(mock_eid_generator_->num_identify_calls());
+  EXPECT_FALSE(mock_observer_->GetNumCalls());
+
+  // Device with valid service data connected, but the local device data
+  // cannot be fetched.
+  mock_local_device_data_provider_->SetPublicKey(nullptr);
+  mock_local_device_data_provider_->SetBeaconSeeds(nullptr);
+  MockServiceDataBluetoothDevice device3(mock_adapter_.get(),
+                                         valid_service_data_for_other_device);
+  ble_scanner_->DeviceAdded(mock_adapter_.get(), &device3);
+  EXPECT_FALSE(mock_eid_generator_->num_identify_calls());
+  EXPECT_FALSE(mock_observer_->GetNumCalls());
+
+  // Device with valid service data connected, but there was no registered
+  // device corresponding to

[Ryan Hansberry, 2017/01/04 00:01:04]

Please complete this comment.

[Kyle Horimoto, 2017/01/04 18:35:44]

Done.

+  mock_local_device_data_provider_->SetPublicKey(
+      base::MakeUnique<std::string>(fake_local_public_key));
+  mock_local_device_data_provider_->SetBeaconSeeds(
+      base::MakeUnique<std::vector<cryptauth::BeaconSeed>>(test_beacon_seeds_));
+  MockServiceDataBluetoothDevice device4(mock_adapter_.get(),
+                                         valid_service_data_for_other_device);
+  ble_scanner_->DeviceAdded(mock_adapter_.get(), &device4);
+  EXPECT_EQ(1, mock_eid_generator_->num_identify_calls());
+  EXPECT_FALSE(mock_observer_->GetNumCalls());
+
+  // Registered device connects.
+  MockServiceDataBluetoothDevice device5(
+      mock_adapter_.get(), valid_service_data_for_registered_device);
+  mock_eid_generator_->set_identified_device(&test_devices_[0]);
+  ble_scanner_->DeviceAdded(mock_adapter_.get(), &device5);
+  EXPECT_EQ(2, mock_eid_generator_->num_identify_calls());
+  EXPECT_EQ(1, mock_observer_->GetNumCalls());
+  EXPECT_EQ(1, static_cast<int>(mock_observer_->bluetooth_devices().size()));
+  EXPECT_EQ(&device5, mock_observer_->bluetooth_devices()[0]);
+  EXPECT_EQ(1, static_cast<int>(mock_observer_->remote_devices().size()));
+  EXPECT_EQ(test_devices_[0], mock_observer_->remote_devices()[0]);
+
+  EXPECT_TRUE(ble_scanner_->IsDeviceRegistered(test_devices_[0].GetDeviceId()));
+  EXPECT_TRUE(ble_scanner_->UnregisterScanFilterForDevice(test_devices_[0]));
+  EXPECT_EQ(2, mock_eid_generator_->num_identify_calls());
+  EXPECT_EQ(1, mock_observer_->GetNumCalls());
+}
+
+TEST_F(BleScannerTest, TestDiscovery_MultipleObservers) {
+  MockBleScannerObserver extra_observer;
+  ble_scanner_->AddObserver(&extra_observer);
+
+  EXPECT_TRUE(ble_scanner_->RegisterScanFilterForDevice(test_devices_[0]));
+  EXPECT_TRUE(ble_scanner_->IsDeviceRegistered(test_devices_[0].GetDeviceId()));
+
+  InvokeAdapterCallback();
+  InvokeDiscoveryStartedCallback();
+
+  MockServiceDataBluetoothDevice device(mock_adapter_.get(), "fakeServiceData");
+  mock_eid_generator_->set_identified_device(&test_devices_[0]);
+  ble_scanner_->DeviceAdded(mock_adapter_.get(), &device);
+
+  EXPECT_EQ(1, mock_observer_->GetNumCalls());
+  EXPECT_EQ(1, static_cast<int>(mock_observer_->bluetooth_devices().size()));
+  EXPECT_EQ(&device, mock_observer_->bluetooth_devices()[0]);
+  EXPECT_EQ(1, static_cast<int>(mock_observer_->remote_devices().size()));
+  EXPECT_EQ(test_devices_[0], mock_observer_->remote_devices()[0]);
+
+  EXPECT_EQ(1, extra_observer.GetNumCalls());
+  EXPECT_EQ(1, static_cast<int>(extra_observer.bluetooth_devices().size()));
+  EXPECT_EQ(&device, extra_observer.bluetooth_devices()[0]);
+  EXPECT_EQ(1, static_cast<int>(extra_observer.remote_devices().size()));
+  EXPECT_EQ(test_devices_[0], extra_observer.remote_devices()[0]);
+
+  // Now, unregister both observers.
+  ble_scanner_->RemoveObserver(mock_observer_.get());
+  ble_scanner_->RemoveObserver(&extra_observer);
+
+  // Now, simulate another scan being received. The observers should not be
+  // notified since they are unregistered, so they should still have a call
+  // count of 1.
+  ble_scanner_->DeviceAdded(mock_adapter_.get(), &device);
+  EXPECT_EQ(1, mock_observer_->GetNumCalls());
+  EXPECT_EQ(1, extra_observer.GetNumCalls());
+
+  EXPECT_TRUE(ble_scanner_->IsDeviceRegistered(test_devices_[0].GetDeviceId()));
+  EXPECT_TRUE(ble_scanner_->UnregisterScanFilterForDevice(test_devices_[0]));
+}
+
+TEST_F(BleScannerTest, TestRegistrationLimit) {
+  EXPECT_TRUE(ble_scanner_->RegisterScanFilterForDevice(test_devices_[0]));
+  EXPECT_TRUE(ble_scanner_->IsDeviceRegistered(test_devices_[0].GetDeviceId()));
+  EXPECT_TRUE(ble_scanner_->RegisterScanFilterForDevice(test_devices_[1]));
+  EXPECT_TRUE(ble_scanner_->IsDeviceRegistered(test_devices_[1].GetDeviceId()));
+
+  // Attempt to register another device. Registration should fail since the
+  // maximum number of devices have already been registered.
+  ASSERT_EQ(2, kMaxConcurrentAdvertisements);
+  EXPECT_FALSE(ble_scanner_->RegisterScanFilterForDevice(test_devices_[2]));
+  EXPECT_FALSE(
+      ble_scanner_->IsDeviceRegistered(test_devices_[2].GetDeviceId()));
+
+  // Unregistering a device which is not registered should also return false.
+  EXPECT_FALSE(ble_scanner_->UnregisterScanFilterForDevice(test_devices_[2]));
+  EXPECT_FALSE(
+      ble_scanner_->IsDeviceRegistered(test_devices_[2].GetDeviceId()));
+
+  // Unregister device 0.
+  EXPECT_TRUE(ble_scanner_->IsDeviceRegistered(test_devices_[0].GetDeviceId()));
+  EXPECT_TRUE(ble_scanner_->UnregisterScanFilterForDevice(test_devices_[0]));
+  EXPECT_FALSE(
+      ble_scanner_->IsDeviceRegistered(test_devices_[0].GetDeviceId()));
+
+  // Now, device 2 can be registered.
+  EXPECT_TRUE(ble_scanner_->RegisterScanFilterForDevice(test_devices_[2]));
+  EXPECT_TRUE(ble_scanner_->IsDeviceRegistered(test_devices_[2].GetDeviceId()));
+
+  // Now, unregister the devices.
+  EXPECT_TRUE(ble_scanner_->IsDeviceRegistered(test_devices_[1].GetDeviceId()));
+  EXPECT_TRUE(ble_scanner_->UnregisterScanFilterForDevice(test_devices_[1]));
+  EXPECT_FALSE(
+      ble_scanner_->IsDeviceRegistered(test_devices_[1].GetDeviceId()));
+
+  EXPECT_TRUE(ble_scanner_->IsDeviceRegistered(test_devices_[2].GetDeviceId()));
+  EXPECT_TRUE(ble_scanner_->UnregisterScanFilterForDevice(test_devices_[2]));
+  EXPECT_FALSE(
+      ble_scanner_->IsDeviceRegistered(test_devices_[2].GetDeviceId()));
+}
+
+TEST_F(BleScannerTest, TestAdapterPoweredChanged) {
+  // This test starts with the adapter powered on, then turns power off before
+  // discovery starts, then turns power back on and allows discovery to
+  // complete, then turns power back off again, and finally turns it back on to
+  // complete another discovery session.
+  EXPECT_CALL(*mock_adapter_, IsPowered())

[Ryan Hansberry, 2017/01/04 00:01:04]

This is pretty difficult to read. I can't easily tell where each of these
different calls will happen. Is it possible to explicitly pepper these
throughout this test?

[Kyle Horimoto, 2017/01/04 18:35:44]

That's not valid to do within GoogleMock.

Relevant information from [1] is copied below:
Google Mock requires expectations to be set before the mock functions are
called, otherwise the behavior is undefined.

[1]
https://github.com/google/googletest/blob/master/googlemock/docs/ForDummies.md

[Ryan Hansberry, 2017/01/04 20:02:20]

Thanks for the link, that's really helpful.

+      .WillOnce(Return(true))
+      .WillOnce(Return(false))
+      .WillOnce(Return(true))
+      .WillOnce(Return(false))
+      .WillOnce(Return(true));
+
+  EXPECT_TRUE(ble_scanner_->RegisterScanFilterForDevice(test_devices_[0]));
+  EXPECT_TRUE(ble_scanner_->IsDeviceRegistered(test_devices_[0].GetDeviceId()));
+  InvokeAdapterCallback();
+
+  // The discovery session should have been requested but not yet initialized.
+  EXPECT_TRUE(stored_discovery_filter_.get());
+  EXPECT_FALSE(mock_discovery_session_);
+
+  // Turn the adapter off before the discovery session starts.
+  ble_scanner_->AdapterPoweredChanged(mock_adapter_.get(), false);
+
+  // Turn the adapter back on, and finish initializing discovery this time.
+  ble_scanner_->AdapterPoweredChanged(mock_adapter_.get(), true);
+  InvokeDiscoveryStartedCallback();
+
+  // The session should have been started.
+  device::MockBluetoothDiscoverySession* session1 = mock_discovery_session_;
+  EXPECT_NE(nullptr, session1);
+
+  // Now turn the adapter off again.
+  ble_scanner_->AdapterPoweredChanged(mock_adapter_.get(), false);
+
+  // Turn the adapter back on.
+  ble_scanner_->AdapterPoweredChanged(mock_adapter_.get(), true);
+  InvokeDiscoveryStartedCallback();
+
+  // A new session should have started, so the session objects should not be the
+  // same.
+  device::MockBluetoothDiscoverySession* session2 = mock_discovery_session_;
+  EXPECT_NE(nullptr, session2);
+  EXPECT_NE(session1, session2);
+
+  // Unregister device.
+  EXPECT_TRUE(ble_scanner_->IsDeviceRegistered(test_devices_[0].GetDeviceId()));
+  EXPECT_TRUE(ble_scanner_->UnregisterScanFilterForDevice(test_devices_[0]));
+  EXPECT_FALSE(
+      ble_scanner_->IsDeviceRegistered(test_devices_[0].GetDeviceId()));
+}
+
+}  // namespace tether
+
+}  // namespace chromeos