bluetooth: Move GetCharacteristic(s) over to Mojo

content/browser/bluetooth/web_bluetooth_service_impl.cc

 // Copyright 2016 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+// ID Not In Map Note:
+// A service, characteristic, or descriptor ID not in the corresponding
+// BluetoothDispatcherHost map [service_to_device_, characteristic_to_service_,
+// descriptor_to_characteristic_] implies a hostile renderer because a renderer

[Jeffrey Yasskin, 2016/04/22 01:16:38]

This may not be true after all: if we revoke access to an attribute concurrently
with the renderer making a request about that attribute, then we should just
deny the request and not kill the renderer. No need to fix that while you're
migrating to Mojo, but we should fix it eventually.

[ortuno, 2016/04/25 15:29:45]

Yup. Opened issue so we don't forget: http://crbug.com/605962

+// obtains the corresponding ID from this class and it will be added to the map
+// at that time.
+
 #include "content/browser/bluetooth/web_bluetooth_service_impl.h"
 
 #include "base/thread_task_runner_handle.h"
 #include "content/browser/bluetooth/bluetooth_blacklist.h"
 #include "content/browser/bluetooth/bluetooth_dispatcher_host.h"
 #include "content/browser/renderer_host/render_process_host_impl.h"
 #include "content/public/browser/navigation_handle.h"
 #include "content/public/browser/render_frame_host.h"
 #include "content/public/browser/web_contents.h"
 #include "device/bluetooth/bluetooth_gatt_characteristic.h"
@@ skipping 36 matching lines @@
       return blink::mojom::WebBluetoothError::GATT_NOT_PAIRED;
     case BluetoothGattService::GATT_ERROR_NOT_SUPPORTED:
       RecordGATTOperationOutcome(operation,
                                  UMAGATTOperationOutcome::NOT_SUPPORTED);
       return blink::mojom::WebBluetoothError::GATT_NOT_SUPPORTED;
   }
   NOTREACHED();
   return blink::mojom::WebBluetoothError::GATT_UNTRANSLATED_ERROR_CODE;
 }
 
+// TODO(ortuno): This should really be a BluetoothDevice method.
+// Replace when implemented. http://crbug.com/552022
+std::vector<device::BluetoothGattCharacteristic*> GetCharacteristicsByUUID(
+    device::BluetoothGattService* service,
+    const std::string& characteristic_uuid) {
+  std::vector<device::BluetoothGattCharacteristic*> characteristics;
+  VLOG(1) << "Looking for characteristic: " << characteristic_uuid;
+  for (device::BluetoothGattCharacteristic* characteristic :
+       service->GetCharacteristics()) {
+    VLOG(1) << "Characteristic in cache: "
+            << characteristic->GetUUID().canonical_value();
+    if (characteristic->GetUUID().canonical_value() == characteristic_uuid) {
+      characteristics.push_back(characteristic);
+    }
+  }
+  return characteristics;
+}
+
 }  // namespace
 
-using CacheQueryResult = BluetoothDispatcherHost::CacheQueryResult;
-
 WebBluetoothServiceImpl::WebBluetoothServiceImpl(
     RenderFrameHost* render_frame_host,
     blink::mojom::WebBluetoothServiceRequest request)
     : WebContentsObserver(WebContents::FromRenderFrameHost(render_frame_host)),
       render_frame_host_(render_frame_host),
       binding_(this, std::move(request)),
       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);
 }
 
 void WebBluetoothServiceImpl::SetClientConnectionErrorHandler(
     base::Closure closure) {
   binding_.set_connection_error_handler(closure);
 }
 
 void WebBluetoothServiceImpl::DidFinishNavigation(
     NavigationHandle* navigation_handle) {
   if (navigation_handle->HasCommitted() &&
       navigation_handle->GetRenderFrameHost() == render_frame_host_ &&
       !navigation_handle->IsSamePage()) {
-    // After navigation we need to clear the frame's state.
     ClearState();
   }
 }
 
 void WebBluetoothServiceImpl::AdapterPresentChanged(
     device::BluetoothAdapter* adapter,
     bool present) {
   if (!present) {
     ClearState();
   }
 }
 
 void WebBluetoothServiceImpl::GattCharacteristicValueChanged(
     device::BluetoothAdapter* adapter,
     device::BluetoothGattCharacteristic* characteristic,
     const std::vector<uint8_t>& value) {
-  // TODO(ortuno): Only send characteristic value changed events for
-  // characteristics that we've returned in the past. We can't yet do
-  // this because WebBluetoothServiceImpl doesn't have direct access
-  // to the list of returned characteristics.
-  // https://crbug.com/508771
-  if (BluetoothBlacklist::Get().IsExcluded(characteristic->GetUUID())) {
+  // Don't notify of characteristics that we haven't returned.
+  if (!ContainsKey(characteristic_to_service_,
+                   characteristic->GetIdentifier())) {
     return;
   }
 
   // On Chrome OS and Linux, GattCharacteristicValueChanged is called before the
   // success callback for ReadRemoteCharacteristic is called, which could result
   // in an event being fired before the readValue promise is resolved.
   if (!base::ThreadTaskRunnerHandle::Get()->PostTask(
           FROM_HERE,
           base::Bind(&WebBluetoothServiceImpl::NotifyCharacteristicValueChanged,
                      weak_ptr_factory_.GetWeakPtr(),
@@ skipping 10 matching lines @@
         characteristic_instance_id, mojo::Array<uint8_t>(std::move(value)));
   }
 }
 
 void WebBluetoothServiceImpl::SetClient(
     blink::mojom::WebBluetoothServiceClientAssociatedPtrInfo client) {
   DCHECK(!client_.get());
   client_.Bind(std::move(client));
 }
 
+void WebBluetoothServiceImpl::RemoteServiceGetCharacteristics(
+    const mojo::String& service_instance_id,
+    bool single_characteristic,
+    const mojo::String& characteristics_uuid,
+    const RemoteServiceGetCharacteristicsCallback& callback) {
+  DCHECK_CURRENTLY_ON(BrowserThread::UI);
+  RecordWebBluetoothFunctionCall(
+      single_characteristic
+          ? UMAWebBluetoothFunction::SERVICE_GET_CHARACTERISTIC
+          : UMAWebBluetoothFunction::SERVICE_GET_CHARACTERISTICS);
+  RecordGetCharacteristicsCharacteristic(single_characteristic,
+                                         characteristics_uuid);
+
+  if (!characteristics_uuid.is_null() &&
+      BluetoothBlacklist::Get().IsExcluded(
+          device::BluetoothUUID(characteristics_uuid))) {
+    RecordGetCharacteristicsOutcome(single_characteristic,
+                                    UMAGetCharacteristicOutcome::BLACKLISTED);
+    callback.Run(
+        blink::mojom::WebBluetoothError::BLACKLISTED_CHARACTERISTIC_UUID,
+        nullptr /* characteristics */);
+    return;
+  }
+
+  const CacheQueryResult query_result =
+      GetBluetoothDispatcherHost()->QueryCacheForService(GetOrigin(),
+                                                         service_instance_id);
+
+  if (query_result.outcome == CacheQueryOutcome::BAD_RENDERER) {
+    binding_.Close();
+    return;
+  }
+
+  if (query_result.outcome != CacheQueryOutcome::SUCCESS) {
+    RecordGetCharacteristicsOutcome(single_characteristic,
+                                    query_result.outcome);
+    callback.Run(query_result.GetWebError(), nullptr /* characteristics */);
+    return;
+  }
+
+  std::vector<device::BluetoothGattCharacteristic*> characteristics =
+      characteristics_uuid.is_null()
+          ? query_result.service->GetCharacteristics()
+          : GetCharacteristicsByUUID(query_result.service,
+                                     characteristics_uuid);
+
+  mojo::Array<blink::mojom::WebBluetoothRemoteGATTCharacteristicPtr>
+      response_characteristics;
+  for (device::BluetoothGattCharacteristic* characteristic : characteristics) {
+    if (BluetoothBlacklist::Get().IsExcluded(characteristic->GetUUID())) {
+      continue;
+    }
+    std::string characteristic_instance_id = characteristic->GetIdentifier();
+    auto insert_result = characteristic_to_service_.insert(
+        std::make_pair(characteristic_instance_id, service_instance_id));
+    // If value is already in map, DCHECK it's valid.
+    if (!insert_result.second)
+      DCHECK(insert_result.first->second == service_instance_id);
+
+    blink::mojom::WebBluetoothRemoteGATTCharacteristicPtr characteristic_ptr =
+        blink::mojom::WebBluetoothRemoteGATTCharacteristic::New();
+    characteristic_ptr->instance_id =
+        mojo::String::From(characteristic_instance_id);
+    characteristic_ptr->uuid =
+        characteristics_uuid.is_null()
+            ? mojo::String::From(characteristic->GetUUID().canonical_value())
+            : nullptr;

[Jeffrey Yasskin, 2016/04/22 01:16:38]

The characteristic has a UUID even if the user isn't filtering by UUID. We
should include it unconditionally, right?

[ortuno, 2016/04/25 15:29:45]

If the user is filtering by UUID then the renderer knows the UUID already so
there is no need to send it over. This optimization existed in Dispatcher Host
before and I tried to keep it here. It definitely makes the code harder to read
(here and on the renderer) and I don't expect users to be calling
GetCharacteristic a lot so I wouldn't be opposed to removing it.

[Jeffrey Yasskin, 2016/04/25 17:26:10]

Yeah, we might DCHECK that they're the same on the renderer side, but I like
having simpler code. Sending an extra 36 bytes isn't likely to be significant,
and we could optimize UUIDs in general if it is.

[ortuno, 2016/04/25 20:37:41]

Done.

+    characteristic_ptr->properties =
+        static_cast<uint32_t>(characteristic->GetProperties());
+    response_characteristics.push_back(std::move(characteristic_ptr));
+
+    if (single_characteristic) {
+      break;
+    }
+  }
+
+  if (!response_characteristics.empty()) {
+    RecordGetCharacteristicsOutcome(single_characteristic,
+                                    UMAGetCharacteristicOutcome::SUCCESS);
+    callback.Run(blink::mojom::WebBluetoothError::SUCCESS,
+                 std::move(response_characteristics));
+    return;
+  }
+  RecordGetCharacteristicsOutcome(
+      single_characteristic,
+      characteristics_uuid.is_null()
+          ? UMAGetCharacteristicOutcome::NO_CHARACTERISTICS
+          : UMAGetCharacteristicOutcome::NOT_FOUND);
+  callback.Run(characteristics_uuid.is_null()
+                   ? blink::mojom::WebBluetoothError::NO_CHARACTERISTICS_FOUND
+                   : blink::mojom::WebBluetoothError::CHARACTERISTIC_NOT_FOUND,
+               nullptr /* characteristics */);
+  return;
+}
+
 void WebBluetoothServiceImpl::RemoteCharacteristicReadValue(
     const mojo::String& characteristic_instance_id,
     const RemoteCharacteristicReadValueCallback& callback) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
   RecordWebBluetoothFunctionCall(
       UMAWebBluetoothFunction::CHARACTERISTIC_READ_VALUE);
 
   const CacheQueryResult query_result =
-      GetBluetoothDispatcherHost()->QueryCacheForCharacteristic(
-          GetOrigin(), characteristic_instance_id);
+      QueryCacheForCharacteristic(characteristic_instance_id);
 
   if (query_result.outcome == CacheQueryOutcome::BAD_RENDERER) {
     return;
   }
 
   if (query_result.outcome != CacheQueryOutcome::SUCCESS) {
     RecordCharacteristicReadValueOutcome(query_result.outcome);
     callback.Run(query_result.GetWebError(), nullptr /* value */);
     return;
   }
@@ skipping 23 matching lines @@
 
   // We perform the length check on the renderer side. So if we
   // get a value with length > 512, we can assume it's a hostile
   // renderer and kill it.
   if (value.size() > 512) {
     CrashRendererAndClosePipe(bad_message::BDH_INVALID_WRITE_VALUE_LENGTH);
     return;
   }
 
   const CacheQueryResult query_result =
-      GetBluetoothDispatcherHost()->QueryCacheForCharacteristic(
-          GetOrigin(), characteristic_instance_id);
+      QueryCacheForCharacteristic(characteristic_instance_id);
 
   if (query_result.outcome == CacheQueryOutcome::BAD_RENDERER) {
-    binding_.Close();
     return;
   }
 
   if (query_result.outcome != CacheQueryOutcome::SUCCESS) {
     RecordCharacteristicWriteValueOutcome(query_result.outcome);
     callback.Run(query_result.GetWebError());
     return;
   }
 
   if (BluetoothBlacklist::Get().IsExcludedFromWrites(
@@ skipping 22 matching lines @@
       characteristic_id_to_notify_session_.find(characteristic_instance_id);
   if (iter != characteristic_id_to_notify_session_.end() &&
       iter->second->IsActive()) {
     // If the frame has already started notifications and the notifications
     // are active we return SUCCESS.
     callback.Run(blink::mojom::WebBluetoothError::SUCCESS);
     return;
   }
 
   const CacheQueryResult query_result =
-      GetBluetoothDispatcherHost()->QueryCacheForCharacteristic(
-          GetOrigin(), characteristic_instance_id);
+      QueryCacheForCharacteristic(characteristic_instance_id);
 
   if (query_result.outcome == CacheQueryOutcome::BAD_RENDERER) {
-    binding_.Close();
     return;
   }
 
   if (query_result.outcome != CacheQueryOutcome::SUCCESS) {
     RecordStartNotificationsOutcome(query_result.outcome);
     callback.Run(query_result.GetWebError());
     return;
   }
 
   device::BluetoothGattCharacteristic::Properties notify_or_indicate =
@@ skipping 13 matching lines @@
 }
 
 void WebBluetoothServiceImpl::RemoteCharacteristicStopNotifications(
     const mojo::String& characteristic_instance_id,
     const RemoteCharacteristicStopNotificationsCallback& callback) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
   RecordWebBluetoothFunctionCall(
       UMAWebBluetoothFunction::CHARACTERISTIC_STOP_NOTIFICATIONS);
 
   const CacheQueryResult query_result =
-      GetBluetoothDispatcherHost()->QueryCacheForCharacteristic(
-          GetOrigin(), characteristic_instance_id);
+      QueryCacheForCharacteristic(characteristic_instance_id);
 
   if (query_result.outcome == CacheQueryOutcome::BAD_RENDERER) {
-    binding_.Close();
     return;
   }
 
   auto notify_session_iter =
       characteristic_id_to_notify_session_.find(characteristic_instance_id);
   if (notify_session_iter == characteristic_id_to_notify_session_.end()) {
     // If the frame hasn't subscribed to notifications before we just
     // run the callback.
     callback.Run();
     return;
@@ skipping 58 matching lines @@
       error_code, UMAGATTOperation::START_NOTIFICATIONS));
 }
 
 void WebBluetoothServiceImpl::OnStopNotifySessionComplete(
     const std::string& characteristic_instance_id,
     const RemoteCharacteristicStopNotificationsCallback& callback) {
   characteristic_id_to_notify_session_.erase(characteristic_instance_id);
   callback.Run();
 }
 
+CacheQueryResult WebBluetoothServiceImpl::QueryCacheForCharacteristic(
+    const std::string& characteristic_instance_id) {
+  auto characteristic_iter =
+      characteristic_to_service_.find(characteristic_instance_id);
+
+  // Kill the render, see "ID Not in Map Note" above.
+  if (characteristic_iter == characteristic_to_service_.end()) {
+    CrashRendererAndClosePipe(bad_message::BDH_INVALID_CHARACTERISTIC_ID);
+    return CacheQueryResult(CacheQueryOutcome::BAD_RENDERER);
+  }
+
+  CacheQueryResult result = GetBluetoothDispatcherHost()->QueryCacheForService(

[Jeffrey Yasskin, 2016/04/22 01:16:38]

Do you need to close the binding_ if this returns BAD_RENDERER?

[ortuno, 2016/04/25 15:29:45]

Done.

+      GetOrigin(), characteristic_iter->second);
+  if (result.outcome != CacheQueryOutcome::SUCCESS) {
+    return result;
+  }
+
+  result.characteristic =
+      result.service->GetCharacteristic(characteristic_instance_id);
+
+  if (result.characteristic == nullptr) {
+    result.outcome = CacheQueryOutcome::NO_CHARACTERISTIC;
+  }
+
+  return result;
+}
+
 RenderProcessHost* WebBluetoothServiceImpl::GetRenderProcessHost() {
   return render_frame_host_->GetProcess();
 }
 
 BluetoothDispatcherHost* WebBluetoothServiceImpl::GetBluetoothDispatcherHost() {
   RenderProcessHostImpl* render_process_host_impl =
       static_cast<RenderProcessHostImpl*>(GetRenderProcessHost());
   return render_process_host_impl->GetBluetoothDispatcherHost();
 }
 
 void WebBluetoothServiceImpl::CrashRendererAndClosePipe(
     bad_message::BadMessageReason reason) {
   bad_message::ReceivedBadMessage(GetRenderProcessHost(), reason);
   binding_.Close();
 }
 
 url::Origin WebBluetoothServiceImpl::GetOrigin() {
   return render_frame_host_->GetLastCommittedOrigin();
 }
 
 void WebBluetoothServiceImpl::ClearState() {
   characteristic_id_to_notify_session_.clear();
+  characteristic_to_service_.clear();
 }
 
 }  // namespace content