bluetooth: Use BluetoothUUID instead of string when sending uuids

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 <unordered_set>
+
 #include "base/hash.h"
 #include "base/metrics/histogram_macros.h"
 #include "base/metrics/sparse_histogram.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.
-int HashUUID(const std::string& canonical_uuid) {
-  DCHECK(canonical_uuid == BluetoothUUID(canonical_uuid).canonical_value());
+int HashUUID(const std::unique_ptr<BluetoothUUID>& uuid) {
+  if (!uuid.get()) {
+    return 0;
+  }
 
-  // TODO(520284): Other than verifying that uuid is canonical, this logic
+  // TODO(520284): Other than verifying that uuid exists, this logic

[Jeffrey Yasskin, 2016/05/28 04:38:06]

If 'uuid' becomes a base::Optional, s/exists/contains a value/. Otherwise, "is
non-null".

[ortuno, 2016/05/31 17:30:47]

Done.

   // should be migrated to a dedicated histogram macro for hashed strings.
+  const std::string& canonical_uuid = uuid->canonical_value();
   uint32_t data =
       base::SuperFastHash(canonical_uuid.data(), canonical_uuid.size());
 
   // Strip off the sign bit because UMA doesn't support negative values,
   // but takes a signed int as input.
   return static_cast<int>(data & 0x7fffffff);
 }
 }  // namespace
 
 namespace content {
@@ skipping 14 matching lines @@
                             static_cast<int>(UMARequestDeviceOutcome::COUNT));
 }
 
 static void RecordRequestDeviceFilters(
     const mojo::Array<blink::mojom::WebBluetoothScanFilterPtr>& filters) {
   UMA_HISTOGRAM_COUNTS_100("Bluetooth.Web.RequestDevice.Filters.Count",
                            filters.size());
   for (const auto& filter : filters) {
     UMA_HISTOGRAM_COUNTS_100("Bluetooth.Web.RequestDevice.FilterSize",
                              filter->services.size());
-    for (const std::string& service : filter->services) {
+    for (const std::unique_ptr<BluetoothUUID>& 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));
     }
   }
 }
 
 static void RecordRequestDeviceOptionalServices(
-    const mojo::Array<mojo::String>& optional_services) {
+    const mojo::Array<std::unique_ptr<BluetoothUUID>>& optional_services) {
   UMA_HISTOGRAM_COUNTS_100("Bluetooth.Web.RequestDevice.OptionalServices.Count",
                            optional_services.size());
-  for (const std::string& service : optional_services) {
+  for (const std::unique_ptr<BluetoothUUID>& 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));
   }
 }
 
 static void RecordUnionOfServices(
     const blink::mojom::WebBluetoothRequestDeviceOptionsPtr& options) {
-  std::set<mojo::String> union_of_services(options->optional_services.begin(),
-                                           options->optional_services.end());
+  std::unordered_set<std::string> union_of_services;
+  for (const std::unique_ptr<BluetoothUUID>& service :
+       options->optional_services) {
+    union_of_services.insert(service->canonical_value());
+  }
 
-  for (const auto& filter : options->filters)
-    union_of_services.insert(filter->services.begin(), filter->services.end());
+  for (const auto& filter : options->filters) {
+    for (const std::unique_ptr<BluetoothUUID>& service : filter->services) {
+      union_of_services.insert(service->canonical_value());
+    }
+  }
 
   UMA_HISTOGRAM_COUNTS_100("Bluetooth.Web.RequestDevice.UnionOfServices.Count",
                            union_of_services.size());
 }
 
 void RecordRequestDeviceOptions(
     const blink::mojom::WebBluetoothRequestDeviceOptionsPtr& options) {
   RecordRequestDeviceFilters(options->filters);
   RecordRequestDeviceOptionalServices(options->optional_services);
   RecordUnionOfServices(options);
@@ skipping 15 matching lines @@
 void RecordConnectGATTTimeSuccess(const base::TimeDelta& duration) {
   UMA_HISTOGRAM_MEDIUM_TIMES("Bluetooth.Web.ConnectGATT.TimeSuccess", duration);
 }
 
 void RecordConnectGATTTimeFailed(const base::TimeDelta& duration) {
   UMA_HISTOGRAM_MEDIUM_TIMES("Bluetooth.Web.ConnectGATT.TimeFailed", duration);
 }
 
 // getPrimaryService
 
-void RecordGetPrimaryServiceService(const BluetoothUUID& service) {
+void RecordGetPrimaryServiceService(
+    const std::unique_ptr<BluetoothUUID>& service) {
   // TODO(ortuno): Use a macro to histogram strings.
   // http://crbug.com/520284
   UMA_HISTOGRAM_SPARSE_SLOWLY("Bluetooth.Web.GetPrimaryService.Services",
-                              HashUUID(service.canonical_value()));
+                              HashUUID(service));
 }
 
 void RecordGetPrimaryServiceOutcome(UMAGetPrimaryServiceOutcome outcome) {
   UMA_HISTOGRAM_ENUMERATION(
       "Bluetooth.Web.GetPrimaryService.Outcome", static_cast<int>(outcome),
       static_cast<int>(UMAGetPrimaryServiceOutcome::COUNT));
 }
 
 void RecordGetPrimaryServiceOutcome(CacheQueryOutcome outcome) {
   DCHECK(outcome == CacheQueryOutcome::NO_DEVICE);
@@ skipping 37 matching lines @@
                                       UMAGetCharacteristicOutcome::NO_SERVICE);
       return;
     case CacheQueryOutcome::NO_CHARACTERISTIC:
       NOTREACHED();
       return;
   }
 }
 
 void RecordGetCharacteristicsCharacteristic(
     blink::mojom::WebBluetoothGATTQueryQuantity quantity,
-    const std::string& characteristic) {
+    const std::unique_ptr<BluetoothUUID>& characteristic) {
   switch (quantity) {
     case blink::mojom::WebBluetoothGATTQueryQuantity::SINGLE:
       UMA_HISTOGRAM_SPARSE_SLOWLY(
           "Bluetooth.Web.GetCharacteristic.Characteristic",
           HashUUID(characteristic));
       return;
     case blink::mojom::WebBluetoothGATTQueryQuantity::MULTIPLE:
       UMA_HISTOGRAM_SPARSE_SLOWLY(
           "Bluetooth.Web.GetCharacteristics.Characteristic",
           HashUUID(characteristic));
@@ skipping 75 matching lines @@
       static_cast<int>(outcome),
       static_cast<int>(UMAGATTOperationOutcome::COUNT));
 }
 
 void RecordStartNotificationsOutcome(CacheQueryOutcome outcome) {
   RecordStartNotificationsOutcome(
       TranslateCacheQueryOutcomeToGATTOperationOutcome(outcome));
 }
 
 }  // namespace content