Reset Bluetooth EventHandler when it is invalid

When BluetoothChooser object is destroyed, its associated Bluetooth EventHandler becomes invalid too and should not be used any more. This CL resets the Bluetooth EventHandler when it becomes invalid so that it can't be used any more. BUG=629298 Review-Url: https://codereview.chromium.org/2203103003 Cr-Commit-Position: refs/heads/master@{#410690}
Denger-Network · Aug 9, 2016 · b583bbc · b583bbc
1 parent 59322e8
commit b583bbc
Show file tree

Hide file tree

Showing 6 changed files with 33 additions and 2 deletions.
diff --git a/chrome/browser/ui/bluetooth/DEPS b/chrome/browser/ui/bluetooth/DEPS
@@ -0,0 +1,3 @@
+include_rules = [
+  "+content/browser/bluetooth",
+]
diff --git a/chrome/browser/ui/bluetooth/bluetooth_chooser_controller.cc b/chrome/browser/ui/bluetooth/bluetooth_chooser_controller.cc
@@ -13,6 +13,7 @@
 #include "chrome/browser/ui/scoped_tabbed_browser_displayer.h"
 #include "chrome/common/url_constants.h"
 #include "chrome/grit/generated_resources.h"
+#include "content/browser/bluetooth/bluetooth_metrics.h"
 #include "ui/base/l10n/l10n_util.h"
 #include "url/gurl.h"
 
@@ -65,6 +66,8 @@ base::string16 BluetoothChooserController::GetOption(size_t index) const {
 }
 
 void BluetoothChooserController::RefreshOptions() {
+  if (event_handler_.is_null())
+    return;
   ClearAllDevices();
   event_handler_.Run(content::BluetoothChooser::Event::RESCAN, std::string());
 }
@@ -74,17 +77,27 @@ base::string16 BluetoothChooserController::GetStatus() const {
 }
 
 void BluetoothChooserController::Select(size_t index) {
+  if (event_handler_.is_null()) {
+    content::RecordRequestDeviceOutcome(
+        content::UMARequestDeviceOutcome::
+            BLUETOOTH_CHOOSER_EVENT_HANDLER_INVALID);
+    return;
+  }
   DCHECK_LT(index, device_names_and_ids_.size());
   event_handler_.Run(content::BluetoothChooser::Event::SELECTED,
                      device_names_and_ids_[index].second);
 }
 
 void BluetoothChooserController::Cancel() {
+  if (event_handler_.is_null())
+    return;
   event_handler_.Run(content::BluetoothChooser::Event::CANCELLED,
                      std::string());
 }
 
 void BluetoothChooserController::Close() {
+  if (event_handler_.is_null())
+    return;
   event_handler_.Run(content::BluetoothChooser::Event::CANCELLED,
                      std::string());
 }
@@ -172,6 +185,10 @@ void BluetoothChooserController::RemoveDevice(const std::string& device_id) {
   }
 }
 
+void BluetoothChooserController::ResetEventHandler() {
+  event_handler_.Reset();
+}
+
 void BluetoothChooserController::ClearAllDevices() {
   device_names_and_ids_.clear();
   device_name_map_.clear();

diff --git a/chrome/browser/ui/bluetooth/bluetooth_chooser_controller.h b/chrome/browser/ui/bluetooth/bluetooth_chooser_controller.h
@@ -54,6 +54,10 @@ class BluetoothChooserController : public ChooserController {
   // Tells the chooser that a device is no longer available.
   void RemoveDevice(const std::string& device_id);
 
+  // Called when |event_handler_| is no longer valid and should not be used
+  // any more.
+  void ResetEventHandler();
+
  private:
   // Clears |device_names_and_ids_| and |device_name_map_|. Called when
   // Bluetooth adapter is turned on or off, or when re-scan happens.

diff --git a/chrome/browser/ui/bluetooth/bluetooth_chooser_desktop.cc b/chrome/browser/ui/bluetooth/bluetooth_chooser_desktop.cc
@@ -13,7 +13,12 @@ BluetoothChooserDesktop::BluetoothChooserDesktop(
   DCHECK(bluetooth_chooser_controller_);
 }
 
-BluetoothChooserDesktop::~BluetoothChooserDesktop() {}
+BluetoothChooserDesktop::~BluetoothChooserDesktop() {
+  // This satisfies the WebContentsDelegate::RunBluetoothChooser() requirement
+  // that the EventHandler can be destroyed any time after the BluetoothChooser
+  // instance.
+  bluetooth_chooser_controller_->ResetEventHandler();
+}
 
 void BluetoothChooserDesktop::SetAdapterPresence(AdapterPresence presence) {
   bluetooth_chooser_controller_->OnAdapterPresenceChanged(presence);

diff --git a/content/browser/bluetooth/bluetooth_metrics.h b/content/browser/bluetooth/bluetooth_metrics.h
@@ -73,6 +73,7 @@ enum class UMARequestDeviceOutcome {
   NEED_LOCATION_HELP_LINK_PRESSED = 14,
   BLUETOOTH_CHOOSER_POLICY_DISABLED = 15,
   BLUETOOTH_GLOBALLY_DISABLED = 16,
+  BLUETOOTH_CHOOSER_EVENT_HANDLER_INVALID = 17,
   // NOTE: Add new requestDevice() outcomes immediately above this line. Make
   // sure to update the enum list in
   // tools/metrics/histograms/histograms.xml accordingly.
@@ -82,7 +83,7 @@ enum class UMARequestDeviceOutcome {
 // There should be a call to this function before every
 // Send(BluetoothMsg_RequestDeviceSuccess...) or
 // Send(BluetoothMsg_RequestDeviceError...).
-void RecordRequestDeviceOutcome(UMARequestDeviceOutcome outcome);
+CONTENT_EXPORT void RecordRequestDeviceOutcome(UMARequestDeviceOutcome outcome);
 
 // Records stats about the arguments used when calling requestDevice.
 //  - The number of filters used.

diff --git a/tools/metrics/histograms/histograms.xml b/tools/metrics/histograms/histograms.xml
@@ -95428,6 +95428,7 @@ To add a new entry, add it with any value and run test to compute valid value.
       label="Chooser insta-closed because user or enterprise policy has
              disabled it"/>
   <int value="16" label="Web Bluetooth kill switch enabled"/>
+  <int value="17" label="Web Bluetooth chooser event handler invalid"/>
 </enum>
 
 <enum name="WebCertVerifyAgreement" type="int">