bluetooth: Move requestDevice to mojo

content/browser/bluetooth/web_bluetooth_service_impl.cc

Index: content/browser/bluetooth/web_bluetooth_service_impl.cc
diff --git a/content/browser/bluetooth/web_bluetooth_service_impl.cc b/content/browser/bluetooth/web_bluetooth_service_impl.cc
index 0a8728f10d54b3c631d23bbbcc2d1a65819dc5a5..b50f2320ce3bd14da021a7dda48f2e433f53c46f 100644
--- a/content/browser/bluetooth/web_bluetooth_service_impl.cc
+++ b/content/browser/bluetooth/web_bluetooth_service_impl.cc
@@ -4,7 +4,7 @@
 
 // ID Not In Map Note:
 // A service, characteristic, or descriptor ID not in the corresponding
-// BluetoothDispatcherHost map [service_id_to_device_address_,
+// WebBluetoothServiceImpl map [service_id_to_device_address_,
 // characteristic_id_to_service_id_, descriptor_to_characteristic_] implies a
 // hostile renderer because a renderer obtains the corresponding ID from this
 // class and it will be added to the map at that time.
@@ -13,11 +13,15 @@
 
 #include <algorithm>
 
+#include "base/strings/utf_string_conversions.h"
 #include "base/thread_task_runner_handle.h"
+#include "content/browser/bluetooth/bluetooth_adapter_factory_wrapper.h"
 #include "content/browser/bluetooth/bluetooth_blacklist.h"
-#include "content/browser/bluetooth/bluetooth_dispatcher_host.h"
+#include "content/browser/bluetooth/bluetooth_device_provider.h"
+#include "content/browser/bluetooth/bluetooth_metrics.h"
 #include "content/browser/bluetooth/frame_connected_bluetooth_devices.h"
 #include "content/browser/renderer_host/render_process_host_impl.h"
+#include "content/public/browser/browser_thread.h"
 #include "content/public/browser/navigation_handle.h"
 #include "content/public/browser/render_frame_host.h"
 #include "content/public/browser/web_contents.h"
@@ -170,13 +174,11 @@ WebBluetoothServiceImpl::WebBluetoothServiceImpl(
       weak_ptr_factory_(this) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
   CHECK(web_contents());
-
-  GetBluetoothDispatcherHost()->AddAdapterObserver(this);
 }
 
 WebBluetoothServiceImpl::~WebBluetoothServiceImpl() {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
-  GetBluetoothDispatcherHost()->RemoveAdapterObserver(this);
+  GetBluetoothAdapterFactoryWrapper()->ReleaseAdapter(this);
 }
 
 void WebBluetoothServiceImpl::SetClientConnectionErrorHandler(
@@ -193,11 +195,21 @@ void WebBluetoothServiceImpl::DidFinishNavigation(
   }
 }
 
-void WebBluetoothServiceImpl::AdapterPresentChanged(
+void WebBluetoothServiceImpl::AdapterPoweredChanged(
     device::BluetoothAdapter* adapter,
-    bool present) {
-  if (!present) {
-    ClearState();

[Jeffrey Yasskin, 2016/05/13 04:41:59]

Is removing this going to bring https://crbug.com/604318 back?

[ortuno, 2016/05/13 20:11:18]

Noup! That bug happened because the adapter in BluetoothDispatcherHost was
destroyed before notify sessions in WebBluetoothServiceImpl were destroyed. Now
that we destroy the adapter after we stop notify session in ClearState(), we
shouldn't see that error. Now that I think about it we do destroy the adapter
before we stop notify session in the destructor. Fixed that.

+    bool powered) {
+  DCHECK_CURRENTLY_ON(BrowserThread::UI);
+  if (device_provider_.get()) {
+    device_provider_->AdapterPoweredChanged(powered);
+  }
+}
+
+void WebBluetoothServiceImpl::DeviceAdded(device::BluetoothAdapter* adapter,
+                                          device::BluetoothDevice* device) {
+  DCHECK_CURRENTLY_ON(BrowserThread::UI);
+  if (device_provider_.get()) {
+    VLOG(1) << "Adding device to device provider: " << device->GetAddress();
+    device_provider_->AddFilteredDevice(*device);
   }
 }
 
@@ -275,14 +287,35 @@ void WebBluetoothServiceImpl::SetClient(
   client_.Bind(std::move(client));
 }
 
+void WebBluetoothServiceImpl::RequestDevice(
+    blink::mojom::WebBluetoothRequestDeviceOptionsPtr options,
+    const RequestDeviceCallback& callback) {
+  RecordWebBluetoothFunctionCall(UMAWebBluetoothFunction::REQUEST_DEVICE);
+  RecordRequestDeviceOptions(options);
+
+  if (!GetBluetoothAdapterFactoryWrapper()->GetAdapter(this)) {
+    if (GetBluetoothAdapterFactoryWrapper()->IsBluetoothAdapterAvailable()) {
+      GetBluetoothAdapterFactoryWrapper()->AcquireAdapter(
+          this, base::Bind(&WebBluetoothServiceImpl::RequestDeviceImpl,
+                           weak_ptr_factory_.GetWeakPtr(),
+                           base::Passed(std::move(options)), callback));
+      return;
+    }
+    RecordRequestDeviceOutcome(UMARequestDeviceOutcome::NO_BLUETOOTH_ADAPTER);
+    callback.Run(blink::mojom::WebBluetoothError::NO_BLUETOOTH_ADAPTER,
+                 nullptr /* device */);
+    return;
+  }
+  RequestDeviceImpl(std::move(options), callback);
+}
+
 void WebBluetoothServiceImpl::RemoteServerConnect(
     const mojo::String& device_id,
     const RemoteServerConnectCallback& callback) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
   RecordWebBluetoothFunctionCall(UMAWebBluetoothFunction::CONNECT_GATT);
 
-  const CacheQueryResult query_result =
-      GetBluetoothDispatcherHost()->QueryCacheForDevice(GetOrigin(), device_id);
+  const CacheQueryResult query_result = QueryCacheForDevice(device_id);
 
   if (query_result.outcome != CacheQueryOutcome::SUCCESS) {
     RecordConnectGATTOutcome(query_result.outcome);
@@ -335,16 +368,14 @@ void WebBluetoothServiceImpl::RemoteServerGetPrimaryService(
   RecordWebBluetoothFunctionCall(UMAWebBluetoothFunction::GET_PRIMARY_SERVICE);
   RecordGetPrimaryServiceService(device::BluetoothUUID(service_uuid));
 
-  if (!GetBluetoothDispatcherHost()
-           ->allowed_devices_map_.IsOriginAllowedToAccessService(
-               GetOrigin(), device_id, service_uuid)) {
+  if (!allowed_devices_map_.IsOriginAllowedToAccessService(
+          GetOrigin(), device_id, service_uuid)) {
     callback.Run(blink::mojom::WebBluetoothError::NOT_ALLOWED_TO_ACCESS_SERVICE,
                  nullptr /* service */);
     return;
   }
 
-  const CacheQueryResult query_result =
-      GetBluetoothDispatcherHost()->QueryCacheForDevice(GetOrigin(), device_id);
+  const CacheQueryResult query_result = QueryCacheForDevice(device_id);
 
   if (query_result.outcome == CacheQueryOutcome::BAD_RENDERER) {
     binding_.Close();
@@ -613,6 +644,26 @@ void WebBluetoothServiceImpl::RemoteCharacteristicStopNotifications(
       weak_ptr_factory_.GetWeakPtr(), characteristic_instance_id, callback));
 }
 
+void WebBluetoothServiceImpl::RequestDeviceImpl(
+    blink::mojom::WebBluetoothRequestDeviceOptionsPtr options,
+    const RequestDeviceCallback& callback) {
+  // requestDevice() can only be called when processing a user-gesture and
+  // any user gesture outside of a chooser should close the chooser so we should
+  // never get a request with an open chooser.
+  CHECK(!device_provider_.get());
+
+  device_provider_.reset(new BluetoothDeviceProvider(
+      render_frame_host_, GetBluetoothAdapterFactoryWrapper()->GetAdapter(this),
+      GetBluetoothAdapterFactoryWrapper()->GetScanDuration()));
+
+  device_provider_->GetDevice(
+      options->Clone(), base::Bind(&WebBluetoothServiceImpl::OnGetDeviceSuccess,
+                                   weak_ptr_factory_.GetWeakPtr(),
+                                   base::Passed(options->Clone()), callback),

[Jeffrey Yasskin, 2016/05/13 04:41:59]

Instead of cloning the options, would it make sense to have GetDevice() pass
them on to the success callback?

[ortuno, 2016/05/13 20:11:18]

Done.

+      base::Bind(&WebBluetoothServiceImpl::OnGetDeviceFailed,
+                 weak_ptr_factory_.GetWeakPtr(), callback));
+}
+
 void WebBluetoothServiceImpl::RemoteServerGetPrimaryServiceImpl(
     const std::string& service_uuid,
     const RemoteServerGetPrimaryServiceCallback& callback,
@@ -648,6 +699,60 @@ void WebBluetoothServiceImpl::RemoteServerGetPrimaryServiceImpl(
                std::move(service_ptr));
 }
 
+void WebBluetoothServiceImpl::OnGetDeviceSuccess(
+    blink::mojom::WebBluetoothRequestDeviceOptionsPtr options,
+    const RequestDeviceCallback& callback,
+    const std::string& device_address) {
+  blink::mojom::WebBluetoothDevicePtr device_ptr =

[Jeffrey Yasskin, 2016/05/13 04:41:59]

I'd probably create this just before it's used for the first time, so it's
clearer that it doesn't accumulate data through the whole function.

[ortuno, 2016/05/13 20:11:18]

Done.

+      blink::mojom::WebBluetoothDevice::New();
+
+  const device::BluetoothDevice* const device =
+      GetBluetoothAdapterFactoryWrapper()->GetAdapter(this)->GetDevice(

[Jeffrey Yasskin, 2016/05/13 04:41:59]

Consider wrapping the "GetBluetoothAdapterFactoryWrapper()->GetAdapter(this)"
into a "GetAdapter()" member function.

[ortuno, 2016/05/13 20:11:18]

Done.

+          device_address);
+  if (device == nullptr) {
+    VLOG(1) << "Device " << device_address << " no longer in adapter";
+    RecordRequestDeviceOutcome(UMARequestDeviceOutcome::CHOSEN_DEVICE_VANISHED);
+    callback.Run(blink::mojom::WebBluetoothError::CHOSEN_DEVICE_VANISHED,
+                 nullptr /* device */);
+    device_provider_.reset();

[Jeffrey Yasskin, 2016/05/13 04:41:59]

Can you reset this at the beginning of the function so we can't accidentally
return without destroying it?

[ortuno, 2016/05/13 20:11:18]

Done.

+    return;
+  }
+
+  const std::string device_id_for_origin =
+      allowed_devices_map_.AddDevice(GetOrigin(), device_address, options);
+
+  VLOG(1) << "Device: " << device->GetName();
+  VLOG(1) << "UUIDs: ";
+
+  mojo::Array<mojo::String> filtered_uuids;
+  for (const device::BluetoothUUID& uuid : device->GetUUIDs()) {
+    if (allowed_devices_map_.IsOriginAllowedToAccessService(
+            GetOrigin(), device_id_for_origin, uuid.canonical_value())) {
+      VLOG(1) << "\t Allowed: " << uuid.canonical_value();
+      filtered_uuids.push_back(uuid.canonical_value());
+    } else {
+      VLOG(1) << "\t Not Allowed: " << uuid.canonical_value();
+    }
+  }
+
+  device_ptr->id = device_id_for_origin;
+  device_ptr->name = base::UTF16ToUTF8(device->GetName());
+  device_ptr->uuids = std::move(filtered_uuids);
+
+  RecordRequestDeviceOutcome(UMARequestDeviceOutcome::SUCCESS);
+  callback.Run(blink::mojom::WebBluetoothError::SUCCESS, std::move(device_ptr));
+  device_provider_.reset();
+}
+
+void WebBluetoothServiceImpl::OnGetDeviceFailed(
+    const RequestDeviceCallback& callback,
+    blink::mojom::WebBluetoothError error) {
+  // Errors are recorded by the BluetoothDeviceProvider that called this

[Jeffrey Yasskin, 2016/05/13 04:41:59]

Or just "by *device_provider_."?

[ortuno, 2016/05/13 20:11:18]

Done.

+  // function.
+  callback.Run(error, nullptr /* device */);
+  device_provider_.reset();
+}
+
 void WebBluetoothServiceImpl::OnCreateGATTConnectionSuccess(
     const std::string& device_id,
     base::TimeTicks start_time,
@@ -733,6 +838,29 @@ void WebBluetoothServiceImpl::OnStopNotifySessionComplete(
   callback.Run();
 }
 
+CacheQueryResult WebBluetoothServiceImpl::QueryCacheForDevice(
+    const std::string& device_id) {
+  const std::string& device_address =
+      allowed_devices_map_.GetDeviceAddress(GetOrigin(), device_id);
+  if (device_address.empty()) {
+    CrashRendererAndClosePipe(bad_message::BDH_DEVICE_NOT_ALLOWED_FOR_ORIGIN);
+    return CacheQueryResult(CacheQueryOutcome::BAD_RENDERER);
+  }
+
+  CacheQueryResult result;
+  result.device =
+      GetBluetoothAdapterFactoryWrapper()->GetAdapter(this)->GetDevice(
+          device_address);
+
+  // When a device can't be found in the BluetoothAdapter, that generally
+  // indicates that it's gone out of range. We reject with a NetworkError in
+  // that case.
+  if (result.device == nullptr) {
+    result.outcome = CacheQueryOutcome::NO_DEVICE;
+  }
+  return result;
+}
+
 CacheQueryResult WebBluetoothServiceImpl::QueryCacheForService(
     const std::string& service_instance_id) {
   auto device_iter = service_id_to_device_address_.find(service_instance_id);
@@ -744,23 +872,14 @@ CacheQueryResult WebBluetoothServiceImpl::QueryCacheForService(
   }
 
   const std::string& device_id =
-      GetBluetoothDispatcherHost()->allowed_devices_map_.GetDeviceId(
-          GetOrigin(), device_iter->second);
+      allowed_devices_map_.GetDeviceId(GetOrigin(), device_iter->second);
   // Kill the renderer if origin is not allowed to access the device.
   if (device_id.empty()) {
     CrashRendererAndClosePipe(bad_message::BDH_DEVICE_NOT_ALLOWED_FOR_ORIGIN);
     return CacheQueryResult(CacheQueryOutcome::BAD_RENDERER);
   }
 
-  CacheQueryResult result =
-      GetBluetoothDispatcherHost()->QueryCacheForDevice(GetOrigin(), device_id);
-
-  // TODO(ortuno): Remove once QueryCacheForDevice closes binding_.
-  // http://crbug.com/508771
-  if (result.outcome == CacheQueryOutcome::BAD_RENDERER) {
-    binding_.Close();
-  }
-
+  CacheQueryResult result = QueryCacheForDevice(device_id);
   if (result.outcome != CacheQueryOutcome::SUCCESS) {
     return result;
   }
@@ -768,10 +887,9 @@ CacheQueryResult WebBluetoothServiceImpl::QueryCacheForService(
   result.service = result.device->GetGattService(service_instance_id);
   if (result.service == nullptr) {
     result.outcome = CacheQueryOutcome::NO_SERVICE;
-  } else if (!GetBluetoothDispatcherHost()
-                  ->allowed_devices_map_.IsOriginAllowedToAccessService(
-                      GetOrigin(), device_id,
-                      result.service->GetUUID().canonical_value())) {
+  } else if (!allowed_devices_map_.IsOriginAllowedToAccessService(
+                 GetOrigin(), device_id,
+                 result.service->GetUUID().canonical_value())) {
     CrashRendererAndClosePipe(bad_message::BDH_SERVICE_NOT_ALLOWED_FOR_ORIGIN);
     return CacheQueryResult(CacheQueryOutcome::BAD_RENDERER);
   }
@@ -809,10 +927,11 @@ RenderProcessHost* WebBluetoothServiceImpl::GetRenderProcessHost() {
   return render_frame_host_->GetProcess();
 }
 
-BluetoothDispatcherHost* WebBluetoothServiceImpl::GetBluetoothDispatcherHost() {
+BluetoothAdapterFactoryWrapper*
+WebBluetoothServiceImpl::GetBluetoothAdapterFactoryWrapper() {
   RenderProcessHostImpl* render_process_host_impl =
       static_cast<RenderProcessHostImpl*>(GetRenderProcessHost());
-  return render_process_host_impl->GetBluetoothDispatcherHost();
+  return render_process_host_impl->GetBluetoothAdapterFactoryWrapper();
 }
 
 void WebBluetoothServiceImpl::CrashRendererAndClosePipe(
@@ -832,6 +951,9 @@ void WebBluetoothServiceImpl::ClearState() {
   service_id_to_device_address_.clear();
   connected_devices_.reset(
       new FrameConnectedBluetoothDevices(render_frame_host_));
+  allowed_devices_map_ = BluetoothAllowedDevicesMap();
+  device_provider_.reset();
+  GetBluetoothAdapterFactoryWrapper()->ReleaseAdapter(this);
 }
 
 }  // namespace content