Skip to content

Commit

Permalink
device::BluetoothAdapterChromeOS: Implement reference counted sessions.
Browse files Browse the repository at this point in the history
Paving the way towards the new DiscoverySession API that's in the works,
this patch modifies BluetoothAdapterChromeOS to keep a count of device
discovery sessions that have been added and removed by clients, to prevent
concurrent sessions from cancelling eachother out. This patch also introduces
two new internal API methods (AddDiscoverySession and RemoveDiscoverySession)
which will be used by the new DiscoverySession class once we add it.

BUG=345006,239702
TEST=1. device_unittests.
     2. - Start two concurrent discovery sessions, one via chrome://settings and
          the other via the Aura Shell.
        - Close the aura shell menu. Device discovery should continue, until the
          "Add Bluetooth Device" overlay in chrome://settings is dismissed by
          clicking "Cancel".

Review URL: https://codereview.chromium.org/169663003

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@253517 0039d316-1c4b-4281-b951-d872f2087c98
  • Loading branch information
armansito@chromium.org committed Feb 26, 2014
1 parent 3f85526 commit 52f6b5f
Show file tree
Hide file tree
Showing 12 changed files with 795 additions and 95 deletions.
31 changes: 25 additions & 6 deletions chromeos/dbus/fake_bluetooth_adapter_client.cc
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,13 @@

namespace chromeos {

namespace {

// Default interval for delayed tasks.
const int kSimulationIntervalMs = 750;

} // namespace

const char FakeBluetoothAdapterClient::kAdapterPath[] =
"/fake/hci0";
const char FakeBluetoothAdapterClient::kAdapterName[] =
Expand Down Expand Up @@ -69,7 +76,8 @@ void FakeBluetoothAdapterClient::Properties::Set(
FakeBluetoothAdapterClient::FakeBluetoothAdapterClient()
: visible_(true),
second_visible_(false),
discovering_count_(0) {
discovering_count_(0),
simulation_interval_ms_(kSimulationIntervalMs) {
properties_.reset(new Properties(base::Bind(
&FakeBluetoothAdapterClient::OnPropertyChanged, base::Unretained(this))));

Expand Down Expand Up @@ -125,14 +133,14 @@ void FakeBluetoothAdapterClient::StartDiscovery(
const base::Closure& callback,
const ErrorCallback& error_callback) {
if (object_path != dbus::ObjectPath(kAdapterPath)) {
error_callback.Run(kNoResponseError, "");
PostDelayedTask(base::Bind(error_callback, kNoResponseError, ""));
return;
}

++discovering_count_;
VLOG(1) << "StartDiscovery: " << object_path.value() << ", "
<< "count is now " << discovering_count_;
callback.Run();
PostDelayedTask(callback);

if (discovering_count_ == 1) {
properties_->discovering.ReplaceValue(true);
Expand All @@ -149,20 +157,20 @@ void FakeBluetoothAdapterClient::StopDiscovery(
const base::Closure& callback,
const ErrorCallback& error_callback) {
if (object_path != dbus::ObjectPath(kAdapterPath)) {
error_callback.Run(kNoResponseError, "");
PostDelayedTask(base::Bind(error_callback, kNoResponseError, ""));
return;
}

if (!discovering_count_) {
LOG(WARNING) << "StopDiscovery called when not discovering";
error_callback.Run(kNoResponseError, "");
PostDelayedTask(base::Bind(error_callback, kNoResponseError, ""));
return;
}

--discovering_count_;
VLOG(1) << "StopDiscovery: " << object_path.value() << ", "
<< "count is now " << discovering_count_;
callback.Run();
PostDelayedTask(callback);

if (discovering_count_ == 0) {
FakeBluetoothDeviceClient* device_client =
Expand Down Expand Up @@ -194,6 +202,10 @@ void FakeBluetoothAdapterClient::RemoveDevice(
device_client->RemoveDevice(dbus::ObjectPath(kAdapterPath), device_path);
}

void FakeBluetoothAdapterClient::SetSimulationIntervalMs(int interval_ms) {
simulation_interval_ms_ = interval_ms;
}

void FakeBluetoothAdapterClient::SetVisible(
bool visible) {
if (visible && !visible_) {
Expand Down Expand Up @@ -247,4 +259,11 @@ void FakeBluetoothAdapterClient::OnPropertyChanged(
property_name));
}

void FakeBluetoothAdapterClient::PostDelayedTask(
const base::Closure& callback) {
base::MessageLoop::current()->PostDelayedTask(
FROM_HERE, callback,
base::TimeDelta::FromMilliseconds(simulation_interval_ms_));
}

} // namespace chromeos
10 changes: 10 additions & 0 deletions chromeos/dbus/fake_bluetooth_adapter_client.h
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,9 @@ class CHROMEOS_EXPORT FakeBluetoothAdapterClient
const base::Closure& callback,
const ErrorCallback& error_callback) OVERRIDE;

// Sets the current simulation timeout interval.
void SetSimulationIntervalMs(int interval_ms);

// Mark the adapter and second adapter as visible or invisible.
void SetVisible(bool visible);
void SetSecondVisible(bool visible);
Expand All @@ -73,6 +76,10 @@ class CHROMEOS_EXPORT FakeBluetoothAdapterClient
// Property callback passed when we create Properties* structures.
void OnPropertyChanged(const std::string& property_name);

// Posts the delayed task represented by |callback| onto the current
// message loop to be executed after |simulation_interval_ms_| milliseconds.
void PostDelayedTask(const base::Closure& callback);

// List of observers interested in event notifications from us.
ObserverList<Observer> observers_;

Expand All @@ -86,6 +93,9 @@ class CHROMEOS_EXPORT FakeBluetoothAdapterClient

// Number of times we've been asked to discover.
int discovering_count_;

// Current timeout interval used when posting delayed tasks.
int simulation_interval_ms_;
};

} // namespace chromeos
Expand Down
10 changes: 10 additions & 0 deletions device/bluetooth/bluetooth_adapter.cc
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,16 @@ BluetoothAdapter::~BluetoothAdapter() {
STLDeleteValues(&devices_);
}

void BluetoothAdapter::StartDiscovering(const base::Closure& callback,
const ErrorCallback& error_callback) {
AddDiscoverySession(callback, error_callback);
}

void BluetoothAdapter::StopDiscovering(const base::Closure& callback,
const ErrorCallback& error_callback) {
RemoveDiscoverySession(callback, error_callback);
}

BluetoothAdapter::DeviceList BluetoothAdapter::GetDevices() {
ConstDeviceList const_devices =
const_cast<const BluetoothAdapter *>(this)->GetDevices();
Expand Down
39 changes: 37 additions & 2 deletions device/bluetooth/bluetooth_adapter.h
Original file line number Diff line number Diff line change
Expand Up @@ -146,15 +146,15 @@ class BluetoothAdapter : public base::RefCounted<BluetoothAdapter> {
// callers should retrieve the current set of discovered devices by calling
// GetDevices() and checking for those with IsPaired() as false.
virtual void StartDiscovering(const base::Closure& callback,
const ErrorCallback& error_callback) = 0;
const ErrorCallback& error_callback);

// Requests that an earlier call to StartDiscovering() be cancelled; the
// adapter may not actually cease discovering devices if other callers
// have called StartDiscovering() and not yet called this method. On
// success |callback| will be called, on failure |error_callback| will be
// called instead.
virtual void StopDiscovering(const base::Closure& callback,
const ErrorCallback& error_callback) = 0;
const ErrorCallback& error_callback);

// Requests the list of devices from the adapter, all are returned
// including those currently connected and those paired. Use the
Expand All @@ -180,6 +180,41 @@ class BluetoothAdapter : public base::RefCounted<BluetoothAdapter> {
BluetoothAdapter();
virtual ~BluetoothAdapter();

// Internal methods for initiating and terminating device discovery sessions.
// An implementation of BluetoothAdapter keeps an internal reference count to
// make sure that the underlying controller is constantly searching for nearby
// devices and retrieving information from them as long as there are clients
// who have requested discovery. These methods behave in the following way:
//
// On a call to AddDiscoverySession:
// - If there is a pending request to the subsystem, queue this request to
// execute once the pending requests are done.
// - If the count is 0, issue a request to the subsystem to start
// device discovery. On success, increment the count to 1.
// - If the count is greater than 0, increment the count and return
// success.
// As long as the count is non-zero, the underlying controller will be
// discovering for devices. This means that Chrome will restart device
// scan and inquiry sessions if they ever end, unless these sessions
// terminate due to an unexpected reason.
//
// On a call to RemoveDiscoverySession:
// - If there is a pending request to the subsystem, queue this request to
// execute once the pending requests are done.
// - If the count is 0, return failure, as there is no active discovery
// session.
// - If the count is 1, issue a request to the subsystem to stop device
// discovery and decrement the count to 0 on success.
// - If the count is greater than 1, decrement the count and return
// success.
//
// These methods invoke |callback| for success and |error_callback| for
// failures.
virtual void AddDiscoverySession(const base::Closure& callback,
const ErrorCallback& error_callback) = 0;
virtual void RemoveDiscoverySession(const base::Closure& callback,
const ErrorCallback& error_callback) = 0;

// Devices paired with, connected to, discovered by, or visible to the
// adapter. The key is the Bluetooth address of the device and the value
// is the BluetoothDevice object whose lifetime is managed by the
Expand Down
Loading

0 comments on commit 52f6b5f

Please sign in to comment.