Fix ownership of BluetoothAdapter in BluetoothDeviceChooserController

BluetoothAdapter is a reference counted object and so
BluetoothDeviceChooserController should own it using a scoped_refptr.
Fixing this requires also fixing BluetoothAdapterFactoryWrapper's
AcquireAdapterCallback to take a scoped_refptr rather than a raw
pointer. A test for proper ownership has been added.

Bug: 1024121
Change-Id: I6342322e059f9cbff2a0d5f073f6bccfb0ca7c36
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1914536Reviewed-by: Matt Reynolds <mattreynolds@chromium.org>
Commit-Queue: Reilly Grant <reillyg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#715206}

content/browser/bluetooth/bluetooth_device_chooser_controller.cc

@@ -232,8 +232,8 @@ void RecordScanningDuration(const base::TimeDelta& duration) {
 BluetoothDeviceChooserController::BluetoothDeviceChooserController(
     WebBluetoothServiceImpl* web_bluetooth_service,
     RenderFrameHost* render_frame_host,
-    device::BluetoothAdapter* adapter)
-    : adapter_(adapter),
+    scoped_refptr<device::BluetoothAdapter> adapter)
+    : adapter_(std::move(adapter)),
       web_bluetooth_service_(web_bluetooth_service),
       render_frame_host_(render_frame_host),
       web_contents_(WebContents::FromRenderFrameHost(render_frame_host_)),

content/browser/bluetooth/bluetooth_device_chooser_controller.h

@@ -48,7 +48,7 @@ class CONTENT_EXPORT BluetoothDeviceChooserController final {
   BluetoothDeviceChooserController(
       WebBluetoothServiceImpl* web_bluetooth_service_,
       RenderFrameHost* render_frame_host,
-      device::BluetoothAdapter* adapter);
+      scoped_refptr<device::BluetoothAdapter> adapter);
   ~BluetoothDeviceChooserController();
 
   // This function performs the following checks before starting a discovery
@@ -127,7 +127,7 @@ class CONTENT_EXPORT BluetoothDeviceChooserController final {
   static int64_t scan_duration_;
 
   // The adapter used to get existing devices and start a discovery session.
-  device::BluetoothAdapter* adapter_;
+  scoped_refptr<device::BluetoothAdapter> adapter_;
   // The WebBluetoothServiceImpl that owns this instance.
   WebBluetoothServiceImpl* web_bluetooth_service_;
   // The RenderFrameHost that owns web_bluetooth_service_.

content/browser/bluetooth/web_bluetooth_service_impl.cc

@@ -644,7 +644,8 @@ void WebBluetoothServiceImpl::GetAvailability(
   }
 
   auto get_availability_impl = base::BindOnce(
-      [](GetAvailabilityCallback callback, device::BluetoothAdapter* adapter) {
+      [](GetAvailabilityCallback callback,
+         scoped_refptr<device::BluetoothAdapter> adapter) {
         std::move(callback).Run(adapter->IsPresent());
       },
       std::move(callback));
@@ -1236,7 +1237,7 @@ void WebBluetoothServiceImpl::RequestScanningStartImpl(
     mojo::AssociatedRemote<blink::mojom::WebBluetoothScanClient> client,
     blink::mojom::WebBluetoothRequestLEScanOptionsPtr options,
     RequestScanningStartCallback callback,
-    device::BluetoothAdapter* adapter) {
+    scoped_refptr<device::BluetoothAdapter> adapter) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
 
   // The renderer should never send invalid options.
@@ -1354,7 +1355,7 @@ void WebBluetoothServiceImpl::OnDiscoverySessionError() {
 void WebBluetoothServiceImpl::RequestDeviceImpl(
     blink::mojom::WebBluetoothRequestDeviceOptionsPtr options,
     RequestDeviceCallback callback,
-    device::BluetoothAdapter* adapter) {
+    scoped_refptr<device::BluetoothAdapter> adapter) {
   // The renderer should never send invalid options.
   if (IsRequestDeviceOptionsInvalid(options)) {
     CrashRendererAndClosePipe(bad_message::BDH_INVALID_OPTIONS);
@@ -1369,8 +1370,8 @@ void WebBluetoothServiceImpl::RequestDeviceImpl(
   // the new one to make sure they can't conflict.
   device_chooser_controller_.reset();
 
-  device_chooser_controller_.reset(
-      new BluetoothDeviceChooserController(this, render_frame_host_, adapter));
+  device_chooser_controller_.reset(new BluetoothDeviceChooserController(
+      this, render_frame_host_, std::move(adapter)));
 
   // TODO(crbug.com/730593): Remove AdaptCallbackForRepeating() by updating
   // the callee interface.

content/browser/bluetooth/web_bluetooth_service_impl.h

@@ -91,6 +91,8 @@ class CONTENT_EXPORT WebBluetoothServiceImpl
       BluetoothDeviceScanningPromptController* prompt_controller);
 
  private:
+  FRIEND_TEST_ALL_PREFIXES(WebBluetoothServiceImplTest,
+                           ClearStateDuringRequestDevice);
   FRIEND_TEST_ALL_PREFIXES(WebBluetoothServiceImplTest, PermissionAllowed);
   FRIEND_TEST_ALL_PREFIXES(WebBluetoothServiceImplTest,
                            PermissionPromptCanceled);
@@ -245,13 +247,13 @@ class CONTENT_EXPORT WebBluetoothServiceImpl
   void RequestDeviceImpl(
       blink::mojom::WebBluetoothRequestDeviceOptionsPtr options,
       RequestDeviceCallback callback,
-      device::BluetoothAdapter* adapter);
+      scoped_refptr<device::BluetoothAdapter> adapter);
 
   void RequestScanningStartImpl(
       mojo::AssociatedRemote<blink::mojom::WebBluetoothScanClient> client,
       blink::mojom::WebBluetoothRequestLEScanOptionsPtr options,
       RequestScanningStartCallback callback,
-      device::BluetoothAdapter* adapter);
+      scoped_refptr<device::BluetoothAdapter> adapter);
 
   void OnStartDiscoverySession(
       mojo::AssociatedRemote<blink::mojom::WebBluetoothScanClient> client,

content/browser/bluetooth/web_bluetooth_service_impl_unittest.cc

@@ -180,6 +180,20 @@ class WebBluetoothServiceImplTest : public RenderViewHostImplTestHarness {
   DISALLOW_COPY_AND_ASSIGN(WebBluetoothServiceImplTest);
 };
 
+TEST_F(WebBluetoothServiceImplTest, ClearStateDuringRequestDevice) {
+  auto options = blink::mojom::WebBluetoothRequestDeviceOptions::New();
+  options->accept_all_devices = true;
+
+  base::RunLoop loop;
+  service_->RequestDevice(
+      std::move(options),
+      base::BindLambdaForTesting(
+          [&loop](blink::mojom::WebBluetoothResult,
+                  blink::mojom::WebBluetoothDevicePtr) { loop.Quit(); }));
+  service_->ClearState();
+  loop.Run();
+}
+
 TEST_F(WebBluetoothServiceImplTest, PermissionAllowed) {
   blink::mojom::WebBluetoothLeScanFilterPtr filter = CreateScanFilter("a", "b");
   base::Optional<WebBluetoothServiceImpl::ScanFilters> filters;

device/bluetooth/bluetooth_adapter_factory_wrapper.cc

@@ -50,10 +50,9 @@ void BluetoothAdapterFactoryWrapper::AcquireAdapter(
   DCHECK(!GetAdapter(observer));
 
   AddAdapterObserver(observer);
-  if (adapter_.get()) {
+  if (adapter_) {
     base::ThreadTaskRunnerHandle::Get()->PostTask(
-        FROM_HERE,
-        base::BindOnce(std::move(callback), base::Unretained(adapter_.get())));
+        FROM_HERE, base::BindOnce(std::move(callback), adapter_));
     return;
   }
 
@@ -99,7 +98,7 @@ void BluetoothAdapterFactoryWrapper::OnGetAdapter(
   DCHECK(thread_checker_.CalledOnValidThread());
 
   set_adapter(adapter);
-  std::move(continuation).Run(adapter_.get());
+  std::move(continuation).Run(adapter_);
 }
 
 bool BluetoothAdapterFactoryWrapper::HasAdapter(

device/bluetooth/bluetooth_adapter_factory_wrapper.h

@@ -23,7 +23,8 @@ namespace device {
 // http://crbug.com/603291
 class DEVICE_BLUETOOTH_EXPORT BluetoothAdapterFactoryWrapper {
  public:
-  using AcquireAdapterCallback = base::OnceCallback<void(BluetoothAdapter*)>;
+  using AcquireAdapterCallback =
+      base::OnceCallback<void(scoped_refptr<BluetoothAdapter>)>;
 
   ~BluetoothAdapterFactoryWrapper();