Repository: electron/electron
Pull request: fix: bluetooth crash in `select-bluetooth-device` event

Backport of #46745

See that PR for details.


Notes: Fixed a possible crash when using `navigator.bluetooth.requestDevice` and the `select-bluetooth-device` event.


Changed files:
filename: shell/browser/lib/bluetooth_chooser.cc
patch: @@ -25,19 +25,6 @@ struct Converter<electron::BluetoothChooser::DeviceInfo> {
 
 namespace electron {
 
-namespace {
-
-void OnDeviceChosen(const content::BluetoothChooser::EventHandler& handler,
-                    const std::string& device_id) {
-  if (device_id.empty()) {
-    handler.Run(content::BluetoothChooserEvent::CANCELLED, device_id);
-  } else {
-    handler.Run(content::BluetoothChooserEvent::SELECTED, device_id);
-  }
-}
-
-}  // namespace
-
 BluetoothChooser::BluetoothChooser(api::WebContents* contents,
                                    const EventHandler& event_handler)
     : api_web_contents_(contents), event_handler_(event_handler) {}
@@ -49,14 +36,13 @@ BluetoothChooser::~BluetoothChooser() {
 void BluetoothChooser::SetAdapterPresence(AdapterPresence presence) {
   switch (presence) {
     case AdapterPresence::ABSENT:
+      NOTREACHED();
     case AdapterPresence::POWERED_OFF:
-    // Chrome currently directs the user to system preferences
-    // to grant bluetooth permission for this case, should we
-    // do something similar ?
-    // https://chromium-review.googlesource.com/c/chromium/src/+/2617129
-    case AdapterPresence::UNAUTHORIZED:
       event_handler_.Run(content::BluetoothChooserEvent::CANCELLED, "");
       break;
+    case AdapterPresence::UNAUTHORIZED:
+      event_handler_.Run(content::BluetoothChooserEvent::DENIED_PERMISSION, "");
+      break;
     case AdapterPresence::POWERED_ON:
       rescan_ = true;
       break;
@@ -74,25 +60,27 @@ void BluetoothChooser::ShowDiscoveryState(DiscoveryState state) {
       refreshing_ = false;
       idle_state = true;
       break;
+    // The first time this state fires is due to a rescan triggering so we
+    // set a flag to ignore devices - the second time this state fires
+    // we are now safe to pick a device.
     case DiscoveryState::DISCOVERING:
-      // The first time this state fires is due to a rescan triggering so set a
-      // flag to ignore devices
       if (rescan_ && !refreshing_) {
         refreshing_ = true;
       } else {
-        // The second time this state fires we are now safe to pick a device
         refreshing_ = false;
       }
       break;
   }
+
   bool prevent_default =
       api_web_contents_->Emit("select-bluetooth-device", GetDeviceList(),
-                              base::BindOnce(&OnDeviceChosen, event_handler_));
+                              base::BindOnce(&BluetoothChooser::OnDeviceChosen,
+                                             weak_ptr_factory_.GetWeakPtr()));
   if (!prevent_default && idle_state) {
-    if (device_map_.empty()) {
+    if (device_id_to_name_map_.empty()) {
       event_handler_.Run(content::BluetoothChooserEvent::CANCELLED, "");
     } else {
-      auto it = device_map_.begin();
+      auto it = device_id_to_name_map_.begin();
       auto device_id = it->first;
       event_handler_.Run(content::BluetoothChooserEvent::SELECTED, device_id);
     }
@@ -105,38 +93,47 @@ void BluetoothChooser::AddOrUpdateDevice(const std::string& device_id,
                                          bool is_gatt_connected,
                                          bool is_paired,
                                          int signal_strength_level) {
-  if (refreshing_) {
-    // If the list of bluetooth devices is currently being generated don't fire
-    // an event
+  // Don't fire an event during refresh.
+  if (refreshing_)
     return;
-  }
 
-  auto [iter, changed] = device_map_.try_emplace(device_id, device_name);
+  // Emit a select-bluetooth-device handler to allow for user to listen for
+  // bluetooth device found. If there's no listener in place, then select the
+  // first device that matches the filters provided.
+  auto [iter, changed] =
+      device_id_to_name_map_.try_emplace(device_id, device_name);
   if (!changed && should_update_name) {
     iter->second = device_name;
     changed = true;
   }
 
   if (changed) {
-    // Emit a select-bluetooth-device handler to allow for user to listen for
-    // bluetooth device found.
     bool prevent_default = api_web_contents_->Emit(
         "select-bluetooth-device", GetDeviceList(),
-        base::BindOnce(&OnDeviceChosen, event_handler_));
+        base::BindOnce(&BluetoothChooser::OnDeviceChosen,
+                       weak_ptr_factory_.GetWeakPtr()));
 
-    // If emit not implemented select first device that matches the filters
-    //  provided.
-    if (!prevent_default) {
+    if (!prevent_default)
       event_handler_.Run(content::BluetoothChooserEvent::SELECTED, device_id);
-    }
+  }
+}
+
+void BluetoothChooser::OnDeviceChosen(const std::string& device_id) {
+  if (event_handler_.is_null())
+    return;
+
+  if (device_id.empty()) {
+    event_handler_.Run(content::BluetoothChooserEvent::CANCELLED, device_id);
+  } else {
+    event_handler_.Run(content::BluetoothChooserEvent::SELECTED, device_id);
   }
 }
 
 std::vector<electron::BluetoothChooser::DeviceInfo>
 BluetoothChooser::GetDeviceList() {
   std::vector<electron::BluetoothChooser::DeviceInfo> vec;
-  vec.reserve(device_map_.size());
-  for (const auto& [device_id, device_name] : device_map_)
+  vec.reserve(device_id_to_name_map_.size());
+  for (const auto& [device_id, device_name] : device_id_to_name_map_)
     vec.emplace_back(device_id, device_name);
   return vec;
 }

filename: shell/browser/lib/bluetooth_chooser.h
patch: @@ -5,13 +5,14 @@
 #ifndef ELECTRON_SHELL_BROWSER_LIB_BLUETOOTH_CHOOSER_H_
 #define ELECTRON_SHELL_BROWSER_LIB_BLUETOOTH_CHOOSER_H_
 
-#include <map>
 #include <string>
 #include <vector>
 
 #include "base/memory/raw_ptr.h"
+#include "base/memory/weak_ptr.h"
 #include "content/public/browser/bluetooth_chooser.h"
 #include "shell/browser/api/electron_api_web_contents.h"
+#include "third_party/abseil-cpp/absl/container/flat_hash_map.h"
 
 namespace electron {
 
@@ -39,14 +40,18 @@ class BluetoothChooser : public content::BluetoothChooser {
                          bool is_gatt_connected,
                          bool is_paired,
                          int signal_strength_level) override;
+
+  void OnDeviceChosen(const std::string& device_id);
   std::vector<DeviceInfo> GetDeviceList();
 
  private:
-  std::map<std::string, std::u16string> device_map_;
+  absl::flat_hash_map<std::string, std::u16string> device_id_to_name_map_;
   raw_ptr<api::WebContents> api_web_contents_;
   EventHandler event_handler_;
   bool refreshing_ = false;
   bool rescan_ = false;
+
+  base::WeakPtrFactory<BluetoothChooser> weak_ptr_factory_{this};
 };
 
 }  // namespace electron