|
@@ -0,0 +1,160 @@
|
|
|
+From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
|
|
|
+From: Reilly Grant <[email protected]>
|
|
|
+Date: Wed, 7 Oct 2020 23:26:36 +0000
|
|
|
+Subject: usb: Prevent parallel calls to UsbDevice::Open
|
|
|
+
|
|
|
+This change adds a check to prevent a Mojo client from calling Open()
|
|
|
+multiple times while the open is in progress.
|
|
|
+
|
|
|
+Bug: 1135857
|
|
|
+Change-Id: Ib467de9129673710b883d9e186c32c359f8592d8
|
|
|
+Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2454846
|
|
|
+Auto-Submit: Reilly Grant <[email protected]>
|
|
|
+Reviewed-by: Matt Reynolds <[email protected]>
|
|
|
+Commit-Queue: Reilly Grant <[email protected]>
|
|
|
+Cr-Commit-Position: refs/heads/master@{#814940}
|
|
|
+
|
|
|
+diff --git a/services/device/usb/mojo/device_impl.cc b/services/device/usb/mojo/device_impl.cc
|
|
|
+index 6507b470afd2756878cd0990cfa7da2b9c730036..3dc07f879d70bc7755e59be1864a17f9a8e0ef10 100644
|
|
|
+--- a/services/device/usb/mojo/device_impl.cc
|
|
|
++++ b/services/device/usb/mojo/device_impl.cc
|
|
|
+@@ -160,6 +160,7 @@ void DeviceImpl::OnOpen(base::WeakPtr<DeviceImpl> self,
|
|
|
+ return;
|
|
|
+ }
|
|
|
+
|
|
|
++ self->opening_ = false;
|
|
|
+ self->device_handle_ = std::move(handle);
|
|
|
+ if (self->device_handle_ && self->client_)
|
|
|
+ self->client_->OnDeviceOpened();
|
|
|
+@@ -175,16 +176,19 @@ void DeviceImpl::OnPermissionGrantedForOpen(OpenCallback callback,
|
|
|
+ device_->Open(base::BindOnce(
|
|
|
+ &DeviceImpl::OnOpen, weak_factory_.GetWeakPtr(), std::move(callback)));
|
|
|
+ } else {
|
|
|
++ opening_ = false;
|
|
|
+ std::move(callback).Run(mojom::UsbOpenDeviceError::ACCESS_DENIED);
|
|
|
+ }
|
|
|
+ }
|
|
|
+
|
|
|
+ void DeviceImpl::Open(OpenCallback callback) {
|
|
|
+- if (device_handle_) {
|
|
|
++ if (opening_ || device_handle_) {
|
|
|
+ std::move(callback).Run(mojom::UsbOpenDeviceError::ALREADY_OPEN);
|
|
|
+ return;
|
|
|
+ }
|
|
|
+
|
|
|
++ opening_ = true;
|
|
|
++
|
|
|
+ if (!device_->permission_granted()) {
|
|
|
+ device_->RequestPermission(
|
|
|
+ base::BindOnce(&DeviceImpl::OnPermissionGrantedForOpen,
|
|
|
+diff --git a/services/device/usb/mojo/device_impl.h b/services/device/usb/mojo/device_impl.h
|
|
|
+index 60ccbcc2e1e3ed964341a7bb8581c142eb27cf40..ab0a3797ae0031708b42f9f1cbc5bddb6f42efac 100644
|
|
|
+--- a/services/device/usb/mojo/device_impl.h
|
|
|
++++ b/services/device/usb/mojo/device_impl.h
|
|
|
+@@ -104,7 +104,9 @@ class DeviceImpl : public mojom::UsbDevice, public device::UsbDevice::Observer {
|
|
|
+ ScopedObserver<device::UsbDevice, device::UsbDevice::Observer> observer_;
|
|
|
+
|
|
|
+ // The device handle. Will be null before the device is opened and after it
|
|
|
+- // has been closed.
|
|
|
++ // has been closed. |opening_| is set to true while the asynchronous open is
|
|
|
++ // in progress.
|
|
|
++ bool opening_ = false;
|
|
|
+ scoped_refptr<UsbDeviceHandle> device_handle_;
|
|
|
+
|
|
|
+ mojo::SelfOwnedReceiverRef<mojom::UsbDevice> receiver_;
|
|
|
+diff --git a/services/device/usb/mojo/device_impl_unittest.cc b/services/device/usb/mojo/device_impl_unittest.cc
|
|
|
+index f0cd0eab4cf04d6f922c1748274d8f5f07a9452e..81045373bb3fa306b78f580f87424eacba36cd3d 100644
|
|
|
+--- a/services/device/usb/mojo/device_impl_unittest.cc
|
|
|
++++ b/services/device/usb/mojo/device_impl_unittest.cc
|
|
|
+@@ -22,6 +22,7 @@
|
|
|
+ #include "base/memory/ref_counted_memory.h"
|
|
|
+ #include "base/run_loop.h"
|
|
|
+ #include "base/stl_util.h"
|
|
|
++#include "base/test/bind_test_util.h"
|
|
|
+ #include "base/test/task_environment.h"
|
|
|
+ #include "mojo/public/cpp/bindings/interface_request.h"
|
|
|
+ #include "mojo/public/cpp/bindings/receiver.h"
|
|
|
+@@ -265,7 +266,10 @@ class USBDeviceImplTest : public testing::Test {
|
|
|
+ void OpenMockHandle(UsbDevice::OpenCallback& callback) {
|
|
|
+ EXPECT_FALSE(is_device_open_);
|
|
|
+ is_device_open_ = true;
|
|
|
+- std::move(callback).Run(mock_handle_);
|
|
|
++ // Simulate the asynchronous device opening process.
|
|
|
++ base::SequencedTaskRunnerHandle::Get()->PostDelayedTask(
|
|
|
++ FROM_HERE, base::BindOnce(std::move(callback), mock_handle_),
|
|
|
++ base::TimeDelta::FromMilliseconds(1));
|
|
|
+ }
|
|
|
+
|
|
|
+ void CloseMockHandle() {
|
|
|
+@@ -515,17 +519,39 @@ TEST_F(USBDeviceImplTest, OpenFailure) {
|
|
|
+ GetMockDeviceProxy(device_client.CreateInterfacePtrAndBind());
|
|
|
+
|
|
|
+ EXPECT_CALL(mock_device(), OpenInternal(_))
|
|
|
+- .WillOnce(Invoke([](UsbDevice::OpenCallback& callback) {
|
|
|
++ .WillOnce([](UsbDevice::OpenCallback& callback) {
|
|
|
+ std::move(callback).Run(nullptr);
|
|
|
+- }));
|
|
|
++ });
|
|
|
+ EXPECT_CALL(device_client, OnDeviceOpened()).Times(0);
|
|
|
+ EXPECT_CALL(device_client, OnDeviceClosed()).Times(0);
|
|
|
+
|
|
|
+- base::RunLoop loop;
|
|
|
+- device->Open(base::BindOnce(&ExpectOpenAndThen,
|
|
|
+- mojom::UsbOpenDeviceError::ACCESS_DENIED,
|
|
|
+- loop.QuitClosure()));
|
|
|
+- loop.Run();
|
|
|
++ {
|
|
|
++ base::RunLoop loop;
|
|
|
++ device->Open(
|
|
|
++ base::BindLambdaForTesting([&](mojom::UsbOpenDeviceError result) {
|
|
|
++ EXPECT_EQ(result, mojom::UsbOpenDeviceError::ACCESS_DENIED);
|
|
|
++ loop.Quit();
|
|
|
++ }));
|
|
|
++ loop.Run();
|
|
|
++ }
|
|
|
++
|
|
|
++ // A second attempt can succeed.
|
|
|
++ EXPECT_CALL(mock_device(), OpenInternal(_));
|
|
|
++ EXPECT_CALL(device_client, OnDeviceOpened());
|
|
|
++ EXPECT_CALL(device_client, OnDeviceClosed());
|
|
|
++
|
|
|
++ {
|
|
|
++ base::RunLoop loop;
|
|
|
++ device->Open(
|
|
|
++ base::BindLambdaForTesting([&](mojom::UsbOpenDeviceError result) {
|
|
|
++ EXPECT_EQ(result, mojom::UsbOpenDeviceError::OK);
|
|
|
++ loop.Quit();
|
|
|
++ }));
|
|
|
++ loop.Run();
|
|
|
++ }
|
|
|
++
|
|
|
++ device.reset();
|
|
|
++ base::RunLoop().RunUntilIdle();
|
|
|
+ }
|
|
|
+
|
|
|
+ TEST_F(USBDeviceImplTest, OpenDelayedFailure) {
|
|
|
+@@ -549,6 +575,24 @@ TEST_F(USBDeviceImplTest, OpenDelayedFailure) {
|
|
|
+ std::move(saved_callback).Run(nullptr);
|
|
|
+ }
|
|
|
+
|
|
|
++TEST_F(USBDeviceImplTest, MultipleOpenNotAllowed) {
|
|
|
++ MockUsbDeviceClient device_client;
|
|
|
++ mojo::Remote<mojom::UsbDevice> device =
|
|
|
++ GetMockDeviceProxy(device_client.CreateInterfacePtrAndBind());
|
|
|
++
|
|
|
++ base::RunLoop loop;
|
|
|
++ device->Open(
|
|
|
++ base::BindLambdaForTesting([&](mojom::UsbOpenDeviceError result) {
|
|
|
++ EXPECT_EQ(result, mojom::UsbOpenDeviceError::OK);
|
|
|
++ }));
|
|
|
++ device->Open(
|
|
|
++ base::BindLambdaForTesting([&](mojom::UsbOpenDeviceError result) {
|
|
|
++ EXPECT_EQ(result, mojom::UsbOpenDeviceError::ALREADY_OPEN);
|
|
|
++ loop.Quit();
|
|
|
++ }));
|
|
|
++ loop.Run();
|
|
|
++}
|
|
|
++
|
|
|
+ TEST_F(USBDeviceImplTest, Close) {
|
|
|
+ MockUsbDeviceClient device_client;
|
|
|
+ mojo::Remote<mojom::UsbDevice> device =
|