bluetooth: Move requestDevice to mojo

content/browser/bluetooth/bluetooth_metrics.cc

 // Copyright 2015 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.
 
 #include "content/browser/bluetooth/bluetooth_metrics.h"
 
 #include <stdint.h>
 
 #include <map>
 #include <set>
 #include "base/hash.h"
 #include "base/metrics/histogram_macros.h"
 #include "base/metrics/sparse_histogram.h"
-#include "content/common/bluetooth/bluetooth_scan_filter.h"
 #include "device/bluetooth/bluetooth_uuid.h"
 
 using device::BluetoothUUID;
 
 namespace {
 
 // Generates a hash from a canonical UUID string suitable for
 // UMA_HISTOGRAM_SPARSE_SLOWLY (positive int).
 //
 // Hash values can be produced manually using tool: bluetooth_metrics_hash.
@@ skipping 23 matching lines @@
 
 // requestDevice()
 
 void RecordRequestDeviceOutcome(UMARequestDeviceOutcome outcome) {
   UMA_HISTOGRAM_ENUMERATION("Bluetooth.Web.RequestDevice.Outcome",
                             static_cast<int>(outcome),
                             static_cast<int>(UMARequestDeviceOutcome::COUNT));
 }
 
 static void RecordRequestDeviceFilters(
-    const std::vector<content::BluetoothScanFilter>& filters) {
+    const mojo::Array<blink::mojom::WebBluetoothScanFilterPtr>& filters) {
   UMA_HISTOGRAM_COUNTS_100("Bluetooth.Web.RequestDevice.Filters.Count",
                            filters.size());
-  for (const content::BluetoothScanFilter& filter : filters) {
+  for (const auto& filter : filters) {
     UMA_HISTOGRAM_COUNTS_100("Bluetooth.Web.RequestDevice.FilterSize",
-                             filter.services.size());
-    for (const BluetoothUUID& service : filter.services) {
+                             filter->services.size());

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

I'm worried about these unchecked dereferences. It's true that they're not a
security concern since they'd just dereference null, but nothing in the mojom
interface guarantees that the renderer is always sending non-null values here.

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

Mojo actually guarantees that members won't be null if you marked the type as
non-nullable in the mojom file i.e. no question mark:

// .mojom
array<string?> nullable

array<string> non_nullable

For the first one mojo will construct an ArrayValidateParams[1] object with the
in_element_is_nullable param set to true and for the second one it will
construct an ArrayValidateParams object with is_element_is_nullable set to
false. Then it calls ValidateArray which ends up calling ValidateElement with
returns true or false depending on ArrayValidateParams, which causes the pipe to
close.

You can follow some of that code in [2], which is the generated code for
array<WebBluetoothRemoteGATTCharacteristic>.

[1]
https://code.google.com/p/chromium/codesearch#chromium/src/mojo/public/cpp/bi...
[2]
https://code.google.com/p/chromium/codesearch#chromium/src/out/Debug/gen/thir...

[Jeffrey Yasskin, 2016/05/13 21:36:55]

Oh, I see. It just doesn't change the type of the elements, so it's all a
runtime guarantee instead of a compile-time check. :( Oh well.

+    for (const std::string& service : filter->services) {
       // TODO(ortuno): Use a macro to histogram strings.
       // http://crbug.com/520284
       UMA_HISTOGRAM_SPARSE_SLOWLY(
-          "Bluetooth.Web.RequestDevice.Filters.Services",
-          HashUUID(service.canonical_value()));
+          "Bluetooth.Web.RequestDevice.Filters.Services", HashUUID(service));
     }
   }
 }
 
 static void RecordRequestDeviceOptionalServices(
-    const std::vector<BluetoothUUID>& optional_services) {
+    const mojo::Array<mojo::String>& optional_services) {
   UMA_HISTOGRAM_COUNTS_100("Bluetooth.Web.RequestDevice.OptionalServices.Count",
                            optional_services.size());
-  for (const BluetoothUUID& service : optional_services) {
+  for (const std::string& service : optional_services) {
     // TODO(ortuno): Use a macro to histogram strings.
     // http://crbug.com/520284
     UMA_HISTOGRAM_SPARSE_SLOWLY(
         "Bluetooth.Web.RequestDevice.OptionalServices.Services",
-        HashUUID(service.canonical_value()));
+        HashUUID(service));
   }
 }
 
 static void RecordUnionOfServices(
-    const std::vector<content::BluetoothScanFilter>& filters,
-    const std::vector<BluetoothUUID>& optional_services) {
-  std::set<BluetoothUUID> union_of_services(optional_services.begin(),
-                                            optional_services.end());
+    const blink::mojom::WebBluetoothRequestDeviceOptionsPtr& options) {
+  std::set<mojo::String> union_of_services(options->optional_services.begin(),
+                                           options->optional_services.end());
 
-  for (const content::BluetoothScanFilter& filter : filters)
-    union_of_services.insert(filter.services.begin(), filter.services.end());
+  for (const auto& filter : options->filters)
+    union_of_services.insert(filter->services.begin(), filter->services.end());
 
   UMA_HISTOGRAM_COUNTS_100("Bluetooth.Web.RequestDevice.UnionOfServices.Count",
                            union_of_services.size());
 }
 
-void RecordRequestDeviceArguments(
-    const std::vector<content::BluetoothScanFilter>& filters,
-    const std::vector<device::BluetoothUUID>& optional_services) {
-  RecordRequestDeviceFilters(filters);
-  RecordRequestDeviceOptionalServices(optional_services);
-  RecordUnionOfServices(filters, optional_services);
+void RecordRequestDeviceOptions(
+    const blink::mojom::WebBluetoothRequestDeviceOptionsPtr& options) {
+  RecordRequestDeviceFilters(options->filters);
+  RecordRequestDeviceOptionalServices(options->optional_services);
+  RecordUnionOfServices(options);
 }
 
 // GATTServer.Connect
 
 void RecordConnectGATTOutcome(UMAConnectGATTOutcome outcome) {
   UMA_HISTOGRAM_ENUMERATION("Bluetooth.Web.ConnectGATT.Outcome",
                             static_cast<int>(outcome),
                             static_cast<int>(UMAConnectGATTOutcome::COUNT));
 }
 
@@ skipping 163 matching lines @@
       static_cast<int>(outcome),
       static_cast<int>(UMAGATTOperationOutcome::COUNT));
 }
 
 void RecordStartNotificationsOutcome(CacheQueryOutcome outcome) {
   RecordStartNotificationsOutcome(
       TranslateCacheQueryOutcomeToGATTOperationOutcome(outcome));
 }
 
 }  // namespace content