Increase Web Bluetooth device scanning duration

content/browser/bluetooth/bluetooth_device_chooser_controller.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.
 
 #include "content/browser/bluetooth/bluetooth_device_chooser_controller.h"
 
 #include <set>
 #include <string>
 #include <unordered_set>
 
 #include "base/bind.h"
 #include "base/bind_helpers.h"
+#include "base/metrics/histogram_macros.h"
 #include "base/strings/string_util.h"
 #include "base/strings/utf_string_conversions.h"
 #include "content/browser/bluetooth/bluetooth_blacklist.h"
 #include "content/browser/bluetooth/bluetooth_metrics.h"
 #include "content/browser/bluetooth/web_bluetooth_service_impl.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/content_browser_client.h"
 #include "content/public/browser/render_frame_host.h"
 #include "content/public/browser/web_contents.h"
 #include "content/public/browser/web_contents_delegate.h"
@@ skipping 24 matching lines @@
 
 namespace content {
 
 bool BluetoothDeviceChooserController::use_test_scan_duration_ = false;
 
 namespace {
 constexpr size_t kMaxLengthForDeviceName =
     29;  // max length of device name in filter.
 
 // The duration of a Bluetooth Scan in seconds.
-constexpr int kScanDuration = 10;
+constexpr int kScanDuration = 60;
 constexpr int kTestScanDuration = 0;
 
 void LogRequestDeviceOptions(
     const blink::mojom::WebBluetoothRequestDeviceOptionsPtr& options) {
   VLOG(1) << "requestDevice called with the following filters: ";
   int i = 0;
   for (const auto& filter : options->filters) {
     VLOG(1) << "Filter #" << ++i;
     if (!filter->name.is_null())
       VLOG(1) << "Name: " << filter->name;
@@ skipping 122 matching lines @@
       return UMARequestDeviceOutcome::ADAPTER_OFF_HELP_LINK_PRESSED;
     case BluetoothChooser::Event::SHOW_NEED_LOCATION_HELP:
       return UMARequestDeviceOutcome::NEED_LOCATION_HELP_LINK_PRESSED;
     case BluetoothChooser::Event::SELECTED:
       // We can't know if we are going to send a success message yet because
       // the device could have vanished. This event should be histogramed
       // manually after checking if the device is still around.
       NOTREACHED();
       return UMARequestDeviceOutcome::SUCCESS;
     case BluetoothChooser::Event::RESCAN:
-      // Rescanning doesn't result in a IPC message for the request being sent
-      // so no need to histogram it.
-      NOTREACHED();
-      return UMARequestDeviceOutcome::SUCCESS;
+      return UMARequestDeviceOutcome::BLUETOOTH_CHOOSER_RESCAN;
   }
   NOTREACHED();
   return UMARequestDeviceOutcome::SUCCESS;
 }
 
+void RecordScanningDuration(const base::TimeDelta& duration) {
+  UMA_HISTOGRAM_LONG_TIMES("Bluetooth.Web.RequestDevice.ScanningDuration",
+                           duration);
+}
+
 }  // namespace
 
 BluetoothDeviceChooserController::BluetoothDeviceChooserController(
     WebBluetoothServiceImpl* web_bluetooth_service,
     RenderFrameHost* render_frame_host,
     device::BluetoothAdapter* adapter)
     : adapter_(adapter),
       web_bluetooth_service_(web_bluetooth_service),
       render_frame_host_(render_frame_host),
       web_contents_(WebContents::FromRenderFrameHost(render_frame_host_)),
       discovery_session_timer_(
           FROM_HERE,
           // TODO(jyasskin): Add a way for tests to control the dialog
           // directly, and change this to a reasonable discovery timeout.
           base::TimeDelta::FromSeconds(
               use_test_scan_duration_ ? kTestScanDuration : kScanDuration),
           base::Bind(&BluetoothDeviceChooserController::StopDeviceDiscovery,
                      // base::Timer guarantees it won't call back after its
                      // destructor starts.
                      base::Unretained(this)),
           /*is_repeating=*/false),
       weak_ptr_factory_(this) {
   CHECK(adapter_);
 }
 
 BluetoothDeviceChooserController::~BluetoothDeviceChooserController() {
+  if (scanning_start_time_) {
+    RecordScanningDuration(base::TimeTicks::Now() -
+                           scanning_start_time_.value());
+    scanning_start_time_.reset();

[scheib, 2016/11/17 22:23:06]

Why do we need to clear the optional<timer> when destructing? 

[juncai, 2016/11/17 22:44:41]

Done.

+  }
+
   if (chooser_) {
     DCHECK(!error_callback_.is_null());
     error_callback_.Run(blink::mojom::WebBluetoothResult::CHOOSER_CANCELLED);
   }
 }
 
 void BluetoothDeviceChooserController::GetDevice(
     blink::mojom::WebBluetoothRequestDeviceOptionsPtr options,
     const SuccessCallback& success_callback,
     const ErrorCallback& error_callback) {
@@ skipping 187 matching lines @@
 
 void BluetoothDeviceChooserController::StartDeviceDiscovery() {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
 
   if (discovery_session_.get() && discovery_session_->IsActive()) {
     // Already running; just increase the timeout.
     discovery_session_timer_.Reset();
     return;
   }
 
+  scanning_start_time_ = base::TimeTicks::Now();
+
   chooser_->ShowDiscoveryState(BluetoothChooser::DiscoveryState::DISCOVERING);
   adapter_->StartDiscoverySessionWithFilter(
       ComputeScanFilter(options_->filters),
       base::Bind(
           &BluetoothDeviceChooserController::OnStartDiscoverySessionSuccess,
           weak_ptr_factory_.GetWeakPtr()),
       base::Bind(
           &BluetoothDeviceChooserController::OnStartDiscoverySessionFailed,
           weak_ptr_factory_.GetWeakPtr()));
 }
@@ skipping 27 matching lines @@
 
 void BluetoothDeviceChooserController::OnBluetoothChooserEvent(
     BluetoothChooser::Event event,
     const std::string& device_address) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
   // Shouldn't recieve an event from a closed chooser.
   DCHECK(chooser_.get());
 
   switch (event) {
     case BluetoothChooser::Event::RESCAN:
+      RecordRequestDeviceOutcome(OutcomeFromChooserEvent(event));
       PopulateConnectedDevices();
       DCHECK(chooser_);
       StartDeviceDiscovery();
       // No need to close the chooser so we return.
       return;
     case BluetoothChooser::Event::DENIED_PERMISSION:
       RecordRequestDeviceOutcome(OutcomeFromChooserEvent(event));
       PostErrorCallback(blink::mojom::WebBluetoothResult::
                             CHOOSER_NOT_SHOWN_USER_DENIED_PERMISSION_TO_SCAN);
       break;
@@ skipping 10 matching lines @@
       VLOG(1) << "Adapter Off Help link pressed.";
       RecordRequestDeviceOutcome(OutcomeFromChooserEvent(event));
       PostErrorCallback(blink::mojom::WebBluetoothResult::CHOOSER_CANCELLED);
       break;
     case BluetoothChooser::Event::SHOW_NEED_LOCATION_HELP:
       VLOG(1) << "Need Location Help link pressed.";
       RecordRequestDeviceOutcome(OutcomeFromChooserEvent(event));
       PostErrorCallback(blink::mojom::WebBluetoothResult::CHOOSER_CANCELLED);
       break;
     case BluetoothChooser::Event::SELECTED:
       PostSuccessCallback(device_address);

[scheib, 2016/11/17 22:23:06]

Add a comment here that RecordRequestDeviceOutcome is called in the callback,
because the device may have vanished.

[juncai, 2016/11/17 22:44:42]

Done.

       break;
   }
   // Close chooser.
   chooser_.reset();
 }
 
 void BluetoothDeviceChooserController::PostSuccessCallback(
     const std::string& device_address) {
   if (!base::ThreadTaskRunnerHandle::Get()->PostTask(
           FROM_HERE,
           base::Bind(success_callback_, base::Passed(std::move(options_)),
                      device_address))) {
     LOG(WARNING) << "No TaskRunner.";
   }
 }
 
 void BluetoothDeviceChooserController::PostErrorCallback(
     blink::mojom::WebBluetoothResult error) {
   if (!base::ThreadTaskRunnerHandle::Get()->PostTask(
           FROM_HERE, base::Bind(error_callback_, error))) {
     LOG(WARNING) << "No TaskRunner.";
   }
 }
 
 }  // namespace content