Commit c40442f0 authored by Bailey Forrest's avatar Bailey Forrest Committed by Commit Bot

[chromecast] Change LeScanManager to have ScanHandles

If a client wants to enable BLE scanning, it should take a ScanHandle.
When all ScanHandles are destroyed, scanning will be disabled.

This is to avoid clients from fighting with each other to determine if
BLE scanning should be enabled or not.

Bug: b/110432359
Test: cast_bluetooth_unittests, device_unittests, manual.
Change-Id: I23dbded8a5e1bd2f687b70d5d594140b4a23df8e
Reviewed-on: https://chromium-review.googlesource.com/1112585
Commit-Queue: Bailey Forrest <bcf@chromium.org>
Reviewed-by: default avatarStephen Lanham <slan@chromium.org>
Reviewed-by: default avatarReilly Grant <reillyg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#570564}
parent f7345e7e
......@@ -33,11 +33,23 @@ class LeScanManager {
virtual void AddObserver(Observer* o) = 0;
virtual void RemoveObserver(Observer* o) = 0;
// Enable or disable BLE scnaning. Can be called on any thread. |cb| is
// called on the thread that calls this method. |success| is false iff the
// operation failed.
using SetScanEnableCallback = base::OnceCallback<void(bool success)>;
virtual void SetScanEnable(bool enable, SetScanEnableCallback cb) = 0;
class ScanHandle {
public:
virtual ~ScanHandle() = default;
protected:
ScanHandle() = default;
private:
DISALLOW_COPY_AND_ASSIGN(ScanHandle);
};
// Request a handle to enable BLE scanning. Can be called on any thread. |cb|
// returns a handle. As long is there is at least one handle in existence, BLE
// scanning will be enabled. Returns nullptr if failed to enable scanning.
using RequestScanCallback =
base::OnceCallback<void(std::unique_ptr<ScanHandle> handle)>;
virtual void RequestScan(RequestScanCallback cb) = 0;
// Asynchronously get the most recent scan results. Can be called on any
// thread. |cb| is called on the calling thread with the results. If
......
......@@ -41,6 +41,19 @@ const int kMaxMessagesInQueue = 5;
} // namespace
class LeScanManagerImpl::ScanHandleImpl : public LeScanManager::ScanHandle {
public:
explicit ScanHandleImpl(LeScanManagerImpl* manager, int32_t id)
: on_destroyed_(BindToCurrentSequence(
base::BindOnce(&LeScanManagerImpl::NotifyScanHandleDestroyed,
manager->weak_factory_.GetWeakPtr(),
id))) {}
~ScanHandleImpl() override { std::move(on_destroyed_).Run(); }
private:
base::OnceClosure on_destroyed_;
};
LeScanManagerImpl::LeScanManagerImpl(
bluetooth_v2_shlib::LeScannerImpl* le_scanner)
: le_scanner_(le_scanner),
......@@ -64,24 +77,22 @@ void LeScanManagerImpl::RemoveObserver(Observer* observer) {
observers_->RemoveObserver(observer);
}
void LeScanManagerImpl::SetScanEnable(bool enable, SetScanEnableCallback cb) {
MAKE_SURE_IO_THREAD(SetScanEnable, enable,
BindToCurrentSequence(std::move(cb)));
bool success;
if (enable) {
success = le_scanner_->StartScan();
} else {
success = le_scanner_->StopScan();
void LeScanManagerImpl::RequestScan(RequestScanCallback cb) {
MAKE_SURE_IO_THREAD(RequestScan, BindToCurrentSequence(std::move(cb)));
if (scan_handle_ids_.empty()) {
if (!le_scanner_->StartScan()) {
LOG(ERROR) << "Failed to enable scanning";
std::move(cb).Run(nullptr);
return;
}
observers_->Notify(FROM_HERE, &Observer::OnScanEnableChanged, true);
}
if (!success) {
LOG(ERROR) << "Failed to " << (enable ? "enable" : "disable")
<< " ble scanning";
EXEC_CB_AND_RET(cb, false);
}
int32_t id = next_scan_handle_id_++;
auto handle = std::make_unique<ScanHandleImpl>(this, id);
scan_handle_ids_.insert(id);
observers_->Notify(FROM_HERE, &Observer::OnScanEnableChanged, enable);
EXEC_CB_AND_RET(cb, true);
std::move(cb).Run(std::move(handle));
}
void LeScanManagerImpl::GetScanResults(GetScanResultsCallback cb,
......@@ -91,27 +102,6 @@ void LeScanManagerImpl::GetScanResults(GetScanResultsCallback cb,
std::move(cb).Run(GetScanResultsInternal(std::move(scan_filter)));
}
// Returns a list of all scan results. The results are sorted by RSSI.
std::vector<LeScanResult> LeScanManagerImpl::GetScanResultsInternal(
base::Optional<ScanFilter> scan_filter) {
DCHECK(io_task_runner_->BelongsToCurrentThread());
std::vector<LeScanResult> results;
for (const auto& pair : addr_to_scan_results_) {
for (const auto& scan_result : pair.second) {
if (!scan_filter || scan_filter->Matches(scan_result)) {
results.push_back(scan_result);
}
}
}
std::sort(results.begin(), results.end(),
[](const LeScanResult& d1, const LeScanResult& d2) {
return d1.rssi > d2.rssi;
});
return results;
}
void LeScanManagerImpl::ClearScanResults() {
MAKE_SURE_IO_THREAD(ClearScanResults);
addr_to_scan_results_.clear();
......@@ -143,5 +133,40 @@ void LeScanManagerImpl::OnScanResult(
observers_->Notify(FROM_HERE, &Observer::OnNewScanResult, scan_result);
}
// Returns a list of all scan results. The results are sorted by RSSI.
std::vector<LeScanResult> LeScanManagerImpl::GetScanResultsInternal(
base::Optional<ScanFilter> scan_filter) {
DCHECK(io_task_runner_->BelongsToCurrentThread());
std::vector<LeScanResult> results;
for (const auto& pair : addr_to_scan_results_) {
for (const auto& scan_result : pair.second) {
if (!scan_filter || scan_filter->Matches(scan_result)) {
results.push_back(scan_result);
}
}
}
std::sort(results.begin(), results.end(),
[](const LeScanResult& d1, const LeScanResult& d2) {
return d1.rssi > d2.rssi;
});
return results;
}
void LeScanManagerImpl::NotifyScanHandleDestroyed(int32_t id) {
DCHECK(io_task_runner_->BelongsToCurrentThread());
size_t num_removed = scan_handle_ids_.erase(id);
DCHECK_EQ(num_removed, 1u);
if (scan_handle_ids_.empty()) {
if (!le_scanner_->StopScan()) {
LOG(ERROR) << "Failed to disable scanning";
} else {
observers_->Notify(FROM_HERE, &Observer::OnScanEnableChanged, false);
}
}
}
} // namespace bluetooth
} // namespace chromecast
......@@ -7,6 +7,7 @@
#include <list>
#include <map>
#include <set>
#include <string>
#include <vector>
......@@ -33,21 +34,25 @@ class LeScanManagerImpl : public LeScanManager,
// LeScanManager implementation:
void AddObserver(Observer* o) override;
void RemoveObserver(Observer* o) override;
void SetScanEnable(bool enable, SetScanEnableCallback cb) override;
void RequestScan(RequestScanCallback cb) override;
void GetScanResults(
GetScanResultsCallback cb,
base::Optional<ScanFilter> service_uuid = base::nullopt) override;
void ClearScanResults() override;
private:
class ScanHandleImpl;
// bluetooth_v2_shlib::LeScanner::Delegate implementation:
void OnScanResult(const bluetooth_v2_shlib::LeScanner::ScanResult&
scan_result_shlib) override;
// Returns a list of all BLE scan results. The results are sorted by RSSI.
// Must be called on |io_task_runner|.
std::vector<LeScanResult> GetScanResultsInternal(
base::Optional<ScanFilter> service_uuid);
// bluetooth_v2_shlib::LeScanner::Delegate implementation:
void OnScanResult(const bluetooth_v2_shlib::LeScanner::ScanResult&
scan_result_shlib) override;
void NotifyScanHandleDestroyed(int32_t id);
bluetooth_v2_shlib::LeScannerImpl* const le_scanner_;
scoped_refptr<base::SingleThreadTaskRunner> io_task_runner_;
......@@ -56,6 +61,9 @@ class LeScanManagerImpl : public LeScanManager,
std::map<bluetooth_v2_shlib::Addr, std::list<LeScanResult>>
addr_to_scan_results_;
int32_t next_scan_handle_id_ = 0;
std::set<int32_t> scan_handle_ids_;
base::WeakPtrFactory<LeScanManagerImpl> weak_factory_;
DISALLOW_COPY_AND_ASSIGN(LeScanManagerImpl);
......
......@@ -16,7 +16,7 @@
#include "chromecast/device/bluetooth/le/remote_descriptor.h"
#include "chromecast/device/bluetooth/le/remote_device.h"
#include "chromecast/device/bluetooth/le/remote_service.h"
#include "chromecast/device/bluetooth/shlib/mock_gatt_client.h"
#include "chromecast/device/bluetooth/shlib/mock_le_scanner.h"
#include "testing/gmock/include/gmock/gmock.h"
#include "testing/gtest/include/gtest/gtest.h"
......@@ -37,25 +37,9 @@ const bluetooth_v2_shlib::Addr kTestAddr2 = {
// This hack is needed because base::BindOnce does not support capture lambdas.
template <typename T>
void CopyResult(T* out, T in) {
*out = in;
*out = std::move(in);
}
class FakeLeScannerImpl : public bluetooth_v2_shlib::LeScannerImpl {
public:
FakeLeScannerImpl() {}
~FakeLeScannerImpl() override = default;
// bluetooth_v2_shlib::LeScannerImpl implementation:
bool IsSupported() override { return true; }
void SetDelegate(bluetooth_v2_shlib::LeScanner::Delegate* delegate) override {
}
bool StartScan() override { return true; }
bool StopScan() override { return true; }
private:
DISALLOW_COPY_AND_ASSIGN(FakeLeScannerImpl);
};
class MockLeScanManagerObserver : public LeScanManager::Observer {
public:
MOCK_METHOD1(OnScanEnableChanged, void(bool enabled));
......@@ -67,17 +51,12 @@ class LeScanManagerTest : public ::testing::Test {
LeScanManagerTest()
: io_task_runner_(base::CreateSingleThreadTaskRunnerWithTraits(
{base::TaskPriority::BACKGROUND, base::MayBlock()})),
le_scan_manager_(&fake_le_scan_manager_impl_) {}
~LeScanManagerTest() override {}
// testing::Test implementation:
void SetUp() override {
le_scan_manager_(&le_scanner_) {
le_scan_manager_.Initialize(io_task_runner_);
le_scan_manager_.AddObserver(&mock_observer_);
scoped_task_environment_.RunUntilIdle();
}
void TearDown() override {
~LeScanManagerTest() override {
le_scan_manager_.RemoveObserver(&mock_observer_);
le_scan_manager_.Finalize();
}
......@@ -88,7 +67,7 @@ class LeScanManagerTest : public ::testing::Test {
base::test::ScopedTaskEnvironment scoped_task_environment_;
scoped_refptr<base::SingleThreadTaskRunner> io_task_runner_;
FakeLeScannerImpl fake_le_scan_manager_impl_;
bluetooth_v2_shlib::MockLeScanner le_scanner_;
LeScanManagerImpl le_scan_manager_;
MockLeScanManagerObserver mock_observer_;
......@@ -98,17 +77,52 @@ class LeScanManagerTest : public ::testing::Test {
} // namespace
TEST_F(LeScanManagerTest, TestSetScanEnable) {
bool enabled = false;
TEST_F(LeScanManagerTest, TestEnableDisableScan) {
std::unique_ptr<LeScanManager::ScanHandle> scan_handle;
// Enable the LE scan. We expect the observer to be updated and to get a scan
// handle.
EXPECT_CALL(le_scanner_, StartScan()).WillOnce(Return(true));
EXPECT_CALL(mock_observer_, OnScanEnableChanged(true));
le_scan_manager_.RequestScan(
base::BindOnce(&CopyResult<typeof(scan_handle)>, &scan_handle));
scoped_task_environment_.RunUntilIdle();
ASSERT_TRUE(scan_handle);
// After deleting the last handle, we expect scan to be disabled.
EXPECT_CALL(le_scanner_, StopScan()).WillOnce(Return(true));
EXPECT_CALL(mock_observer_, OnScanEnableChanged(false));
scan_handle.reset();
scoped_task_environment_.RunUntilIdle();
}
TEST_F(LeScanManagerTest, TestMultipleHandles) {
static constexpr int kNumHandles = 20;
std::vector<std::unique_ptr<LeScanManager::ScanHandle>> scan_handles;
EXPECT_CALL(le_scanner_, StartScan()).WillOnce(Return(true));
EXPECT_CALL(mock_observer_, OnScanEnableChanged(true));
for (int i = 0; i < kNumHandles; ++i) {
std::unique_ptr<LeScanManager::ScanHandle> handle;
le_scan_manager_.RequestScan(
base::BindOnce(&CopyResult<typeof(handle)>, &handle));
scoped_task_environment_.RunUntilIdle();
ASSERT_TRUE(handle);
scan_handles.push_back(std::move(handle));
}
// Enable the LE scan. We expect the observer to be updated and the callback
// to be called with success=true.
le_scan_manager_.SetScanEnable(true, /* enable */
base::BindOnce(&CopyResult<bool>, &enabled));
EXPECT_CALL(le_scanner_, StopScan()).Times(0);
for (int i = 0; i < kNumHandles - 1; ++i) {
scan_handles.pop_back();
scoped_task_environment_.RunUntilIdle();
}
// After deleting the last handle, we expect scan to be disabled.
EXPECT_CALL(le_scanner_, StopScan()).WillOnce(Return(true));
EXPECT_CALL(mock_observer_, OnScanEnableChanged(false));
scan_handles.pop_back();
scoped_task_environment_.RunUntilIdle();
ASSERT_TRUE(enabled);
}
TEST_F(LeScanManagerTest, TestGetScanResultsEmpty) {
......@@ -122,6 +136,19 @@ TEST_F(LeScanManagerTest, TestGetScanResultsEmpty) {
ASSERT_EQ(0u, results.size());
}
TEST_F(LeScanManagerTest, TestEnableScanFails) {
std::unique_ptr<LeScanManager::ScanHandle> scan_handle;
// Observer should not be notified.
EXPECT_CALL(mock_observer_, OnScanEnableChanged(true)).Times(0);
EXPECT_CALL(le_scanner_, StartScan()).WillOnce(Return(false));
le_scan_manager_.RequestScan(
base::BindOnce(&CopyResult<typeof(scan_handle)>, &scan_handle));
scoped_task_environment_.RunUntilIdle();
EXPECT_FALSE(scan_handle);
}
TEST_F(LeScanManagerTest, TestGetScanResults) {
// Simulate some scan results.
bluetooth_v2_shlib::LeScanner::ScanResult raw_scan_result;
......@@ -256,22 +283,5 @@ TEST_F(LeScanManagerTest, TestOnNewScanResult) {
ASSERT_EQ(1, result.rssi);
}
TEST_F(LeScanManagerTest, TestOnScanEnableChanged) {
bool enabled = false;
ON_CALL(mock_observer_, OnScanEnableChanged(_))
.WillByDefault(
Invoke([&enabled](bool enabled_in) { enabled = enabled_in; }));
// Enable scanning.
le_scan_manager_.SetScanEnable(true /* enable */, base::DoNothing());
scoped_task_environment_.RunUntilIdle();
ASSERT_TRUE(enabled);
// Disable scanning.
le_scan_manager_.SetScanEnable(false /* enable */, base::DoNothing());
scoped_task_environment_.RunUntilIdle();
ASSERT_FALSE(enabled);
}
} // namespace bluetooth
} // namespace chromecast
......@@ -16,6 +16,12 @@ namespace bluetooth {
class MockLeScanManager : public LeScanManager {
public:
class MockScanHandle : public ScanHandle {
public:
MockScanHandle() = default;
~MockScanHandle() override = default;
};
MockLeScanManager();
~MockLeScanManager();
......@@ -28,9 +34,9 @@ class MockLeScanManager : public LeScanManager {
observer_ = nullptr;
}
MOCK_METHOD1(SetScanEnable, bool(bool enable));
void SetScanEnable(bool enable, SetScanEnableCallback cb) override {
std::move(cb).Run(SetScanEnable(enable));
MOCK_METHOD0(RequestScan, std::unique_ptr<ScanHandle>());
void RequestScan(RequestScanCallback cb) override {
std::move(cb).Run(RequestScan());
}
MOCK_METHOD1(
......
......@@ -213,9 +213,8 @@ void BluetoothAdapterCast::AddDiscoverySession(
// There is no active discovery session, and no pending requests. Enable
// scanning.
le_scan_manager_->SetScanEnable(
true, base::BindOnce(&BluetoothAdapterCast::OnScanEnabled,
weak_factory_.GetWeakPtr()));
le_scan_manager_->RequestScan(base::BindOnce(
&BluetoothAdapterCast::OnScanEnabled, weak_factory_.GetWeakPtr()));
}
void BluetoothAdapterCast::RemoveDiscoverySession(
......@@ -230,8 +229,7 @@ void BluetoothAdapterCast::RemoveDiscoverySession(
(void)discovery_filter;
// If there are pending requests, run the error call immediately.
if (pending_discovery_requests_.size() > 0u ||
pending_disable_discovery_request_) {
if (pending_discovery_requests_.size() > 0u) {
std::move(error_callback)
.Run(UMABluetoothDiscoverySessionOutcome::REMOVE_WITH_PENDING_REQUEST);
return;
......@@ -241,14 +239,14 @@ void BluetoothAdapterCast::RemoveDiscoverySession(
if (num_discovery_sessions_ > 1) {
num_discovery_sessions_--;
callback.Run();
return;
}
// This was the last active discovery session. Disable scanning.
pending_disable_discovery_request_.emplace(discovery_filter, callback,
std::move(error_callback));
le_scan_manager_->SetScanEnable(
false, base::BindOnce(&BluetoothAdapterCast::OnScanDisabled,
weak_factory_.GetWeakPtr()));
num_discovery_sessions_--;
DCHECK(scan_handle_);
scan_handle_.reset();
callback.Run();
}
void BluetoothAdapterCast::SetDiscoveryFilter(
......@@ -394,10 +392,11 @@ void BluetoothAdapterCast::AddDevice(
observer.DeviceAdded(this, device);
}
void BluetoothAdapterCast::OnScanEnabled(bool enabled) {
void BluetoothAdapterCast::OnScanEnabled(
std::unique_ptr<chromecast::bluetooth::LeScanManager::ScanHandle>
scan_handle) {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
if (!enabled) {
if (!scan_handle) {
LOG(WARNING) << "Failed to start scan.";
// Run the error callback.
......@@ -408,13 +407,14 @@ void BluetoothAdapterCast::OnScanEnabled(bool enabled) {
// If there is another pending request, try again.
if (pending_discovery_requests_.size() > 0u) {
le_scan_manager_->SetScanEnable(
true, base::BindOnce(&BluetoothAdapterCast::OnScanEnabled,
weak_factory_.GetWeakPtr()));
le_scan_manager_->RequestScan(base::BindOnce(
&BluetoothAdapterCast::OnScanEnabled, weak_factory_.GetWeakPtr()));
}
return;
}
scan_handle_ = std::move(scan_handle);
// The scan has been successfully enabled. Request the initial scan results
// from the scan manager.
le_scan_manager_->GetScanResults(base::BindOnce(
......@@ -428,26 +428,6 @@ void BluetoothAdapterCast::OnScanEnabled(bool enabled) {
}
}
void BluetoothAdapterCast::OnScanDisabled(bool success) {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
DCHECK(pending_disable_discovery_request_);
if (!success) {
LOG(WARNING) << "Failed to stop scan.";
// Run the error callback.
std::move(pending_disable_discovery_request_->error_callback)
.Run(UMABluetoothDiscoverySessionOutcome::FAILED);
pending_disable_discovery_request_ = base::nullopt;
return;
}
// The scan has been successfully disabled.
num_discovery_sessions_--;
pending_disable_discovery_request_->success_callback.Run();
pending_disable_discovery_request_ = base::nullopt;
}
void BluetoothAdapterCast::OnGetDevice(
scoped_refptr<chromecast::bluetooth::RemoteDevice> remote_device) {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
......
......@@ -159,8 +159,9 @@ class DEVICE_BLUETOOTH_EXPORT BluetoothAdapterCast
scoped_refptr<chromecast::bluetooth::RemoteDevice> remote_device);
// Called when the scanner has enabled scanning.
void OnScanEnabled(bool success);
void OnScanDisabled(bool success);
void OnScanEnabled(
std::unique_ptr<chromecast::bluetooth::LeScanManager::ScanHandle>
scan_handle);
void OnGetDevice(scoped_refptr<chromecast::bluetooth::RemoteDevice> device);
void OnGetScanResults(
std::vector<chromecast::bluetooth::LeScanResult> results);
......@@ -178,7 +179,6 @@ class DEVICE_BLUETOOTH_EXPORT BluetoothAdapterCast
};
std::queue<DiscoveryParams> pending_discovery_requests_;
base::Optional<DiscoveryParams> pending_disable_discovery_request_;
int num_discovery_sessions_ = 0;
......@@ -189,6 +189,9 @@ class DEVICE_BLUETOOTH_EXPORT BluetoothAdapterCast
chromecast::bluetooth::GattClientManager* const gatt_client_manager_;
chromecast::bluetooth::LeScanManager* const le_scan_manager_;
std::unique_ptr<chromecast::bluetooth::LeScanManager::ScanHandle>
scan_handle_;
bool powered_ = true;
bool initialized_ = false;
......
......@@ -11,6 +11,7 @@
#include "chromecast/device/bluetooth/le/remote_device.h"
#include "device/bluetooth/cast/bluetooth_adapter_cast.h"
using ::testing::ByMove;
using ::testing::Return;
namespace device {
......@@ -46,7 +47,11 @@ class BluetoothTestCast::GattClientManager
BluetoothTestCast::BluetoothTestCast()
: gatt_client_manager_(std::make_unique<GattClientManager>()) {
ON_CALL(le_scan_manager_, SetScanEnable).WillByDefault(Return(true));
ON_CALL(le_scan_manager_, RequestScan)
.WillByDefault(Return(ByMove(
std::unique_ptr<chromecast::bluetooth::LeScanManager::ScanHandle>(
std::make_unique<chromecast::bluetooth::MockLeScanManager::
MockScanHandle>()))));
}
BluetoothTestCast::~BluetoothTestCast() {
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment