Commit eb40b23d authored by gdk@chromium.org's avatar gdk@chromium.org

Fix missing callback chaining in UsbService.

On OS_CHROMEOS when the permission_broker checks a set of devices the
UsbService needs to wait for the async work to complete before responding.
This worked on OS_CHROMEOS desktop builds because the stub implementation of
PermissionBrokerClient wasn't racing to modify the devices vector.

BUG=chromium-os:38836

Review URL: https://chromiumcodereview.appspot.com/12226137

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@182320 0039d316-1c4b-4281-b951-d872f2087c98
parent 357ca592
...@@ -309,7 +309,7 @@ bool UsbFindDevicesFunction::Prepare() { ...@@ -309,7 +309,7 @@ bool UsbFindDevicesFunction::Prepare() {
} }
void UsbFindDevicesFunction::AsyncWorkStart() { void UsbFindDevicesFunction::AsyncWorkStart() {
scoped_ptr<base::ListValue> result(new base::ListValue()); result_.reset(new base::ListValue());
if (device_for_test_) { if (device_for_test_) {
UsbDeviceResource* const resource = new UsbDeviceResource( UsbDeviceResource* const resource = new UsbDeviceResource(
...@@ -317,8 +317,8 @@ void UsbFindDevicesFunction::AsyncWorkStart() { ...@@ -317,8 +317,8 @@ void UsbFindDevicesFunction::AsyncWorkStart() {
device_for_test_); device_for_test_);
Device device; Device device;
result->Append(PopulateDevice(manager_->Add(resource), 0, 0)); result_->Append(PopulateDevice(manager_->Add(resource), 0, 0));
SetResult(result.release()); SetResult(result_.release());
AsyncWorkCompleted(); AsyncWorkCompleted();
return; return;
} }
...@@ -341,20 +341,23 @@ void UsbFindDevicesFunction::AsyncWorkStart() { ...@@ -341,20 +341,23 @@ void UsbFindDevicesFunction::AsyncWorkStart() {
return; return;
} }
vector<scoped_refptr<UsbDevice> > devices; service->FindDevices(vendor_id, product_id, &devices_, base::Bind(
service->FindDevices(vendor_id, product_id, &devices); &UsbFindDevicesFunction::OnCompleted, this));
for (size_t i = 0; i < devices.size(); ++i) { }
UsbDevice* const device = devices[i];
void UsbFindDevicesFunction::OnCompleted() {
for (size_t i = 0; i < devices_.size(); ++i) {
UsbDevice* const device = devices_[i];
UsbDeviceResource* const resource = new UsbDeviceResource( UsbDeviceResource* const resource = new UsbDeviceResource(
extension_->id(), device); extension_->id(), device);
Device js_device; Device js_device;
result->Append(PopulateDevice(manager_->Add(resource), result_->Append(PopulateDevice(manager_->Add(resource),
vendor_id, parameters_->options.vendor_id,
product_id)); parameters_->options.product_id));
} }
SetResult(result.release()); SetResult(result_.release());
AsyncWorkCompleted(); AsyncWorkCompleted();
} }
......
...@@ -70,6 +70,10 @@ class UsbFindDevicesFunction : public UsbAsyncApiFunction { ...@@ -70,6 +70,10 @@ class UsbFindDevicesFunction : public UsbAsyncApiFunction {
virtual void AsyncWorkStart() OVERRIDE; virtual void AsyncWorkStart() OVERRIDE;
private: private:
void OnCompleted();
scoped_ptr<base::ListValue> result_;
std::vector<scoped_refptr<UsbDevice> > devices_;
scoped_ptr<extensions::api::usb::FindDevices::Params> parameters_; scoped_ptr<extensions::api::usb::FindDevices::Params> parameters_;
}; };
......
...@@ -70,30 +70,38 @@ void UsbService::Cleanup() { ...@@ -70,30 +70,38 @@ void UsbService::Cleanup() {
void UsbService::FindDevices(const uint16 vendor_id, void UsbService::FindDevices(const uint16 vendor_id,
const uint16 product_id, const uint16 product_id,
vector<scoped_refptr<UsbDevice> >* devices) { vector<scoped_refptr<UsbDevice> >* devices,
const base::Callback<void()>& callback) {
DCHECK(event_handler_) << "FindDevices called after event handler stopped."; DCHECK(event_handler_) << "FindDevices called after event handler stopped.";
#if defined(OS_CHROMEOS) #if defined(OS_CHROMEOS)
chromeos::PermissionBrokerClient* client = chromeos::PermissionBrokerClient* client =
chromeos::DBusThreadManager::Get()->GetPermissionBrokerClient(); chromeos::DBusThreadManager::Get()->GetPermissionBrokerClient();
DCHECK(client) << "Could not get permission broker client."; DCHECK(client) << "Could not get permission broker client.";
if (!client) if (!client) {
callback.Run();
return; return;
}
client->RequestUsbAccess(vendor_id, client->RequestUsbAccess(vendor_id,
product_id, product_id,
base::Bind(&UsbService::FindDevicesImpl, base::Bind(&UsbService::FindDevicesImpl,
base::Unretained(this), base::Unretained(this),
vendor_id, vendor_id,
product_id, product_id,
devices)); devices,
callback));
#else #else
FindDevicesImpl(vendor_id, product_id, devices, true); FindDevicesImpl(vendor_id, product_id, devices, callback, true);
#endif // defined(OS_CHROMEOS) #endif // defined(OS_CHROMEOS)
} }
void UsbService::FindDevicesImpl(const uint16 vendor_id, void UsbService::FindDevicesImpl(const uint16 vendor_id,
const uint16 product_id, const uint16 product_id,
vector<scoped_refptr<UsbDevice> >* devices, vector<scoped_refptr<UsbDevice> >* devices,
const base::Callback<void()>& callback,
bool success) { bool success) {
base::ScopedClosureRunner run_callback(callback);
devices->clear(); devices->clear();
// If the permission broker was unable to obtain permission for the specified // If the permission broker was unable to obtain permission for the specified
......
...@@ -33,10 +33,11 @@ class UsbService : public ProfileKeyedService { ...@@ -33,10 +33,11 @@ class UsbService : public ProfileKeyedService {
// Find all of the devices attached to the system that are identified by // Find all of the devices attached to the system that are identified by
// |vendor_id| and |product_id|, inserting them into |devices|. Clears // |vendor_id| and |product_id|, inserting them into |devices|. Clears
// |devices| before use. // |devices| before use. Calls |callback| once |devices| is populated.
void FindDevices(const uint16 vendor_id, void FindDevices(const uint16 vendor_id,
const uint16 product_id, const uint16 product_id,
std::vector<scoped_refptr<UsbDevice> >* devices); std::vector<scoped_refptr<UsbDevice> >* devices,
const base::Callback<void()>& callback);
// This function should not be called by normal code. It is invoked by a // This function should not be called by normal code. It is invoked by a
// UsbDevice's Close function and disposes of the associated platform handle. // UsbDevice's Close function and disposes of the associated platform handle.
...@@ -73,6 +74,7 @@ class UsbService : public ProfileKeyedService { ...@@ -73,6 +74,7 @@ class UsbService : public ProfileKeyedService {
void FindDevicesImpl(const uint16 vendor_id, void FindDevicesImpl(const uint16 vendor_id,
const uint16 product_id, const uint16 product_id,
std::vector<scoped_refptr<UsbDevice> >* devices, std::vector<scoped_refptr<UsbDevice> >* devices,
const base::Callback<void()>& callback,
bool success); bool success);
// Populates |output| with the result of enumerating all attached USB devices. // Populates |output| with the result of enumerating all attached USB devices.
......
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