From 372cdb5deec889251e1b0b40e2440ff9fa8341e3 Mon Sep 17 00:00:00 2001 From: Shelley Vohr Date: Fri, 25 Apr 2025 17:26:19 +0200 Subject: [PATCH] fix: bluetooth crash in `select-bluetooth-device` event (#46745) fix: bluetooth crash on bluetooth off --- shell/browser/lib/bluetooth_chooser.cc | 73 ++++++++++++-------------- shell/browser/lib/bluetooth_chooser.h | 9 +++- 2 files changed, 42 insertions(+), 40 deletions(-) diff --git a/shell/browser/lib/bluetooth_chooser.cc b/shell/browser/lib/bluetooth_chooser.cc index 0c8708a91a48..23f39e1a3160 100644 --- a/shell/browser/lib/bluetooth_chooser.cc +++ b/shell/browser/lib/bluetooth_chooser.cc @@ -25,19 +25,6 @@ struct Converter { 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 BluetoothChooser::GetDeviceList() { std::vector 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; } diff --git a/shell/browser/lib/bluetooth_chooser.h b/shell/browser/lib/bluetooth_chooser.h index 19f7eac83891..5b377e5e1b69 100644 --- a/shell/browser/lib/bluetooth_chooser.h +++ b/shell/browser/lib/bluetooth_chooser.h @@ -5,13 +5,14 @@ #ifndef ELECTRON_SHELL_BROWSER_LIB_BLUETOOTH_CHOOSER_H_ #define ELECTRON_SHELL_BROWSER_LIB_BLUETOOTH_CHOOSER_H_ -#include #include #include #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 GetDeviceList(); private: - std::map device_map_; + absl::flat_hash_map device_id_to_name_map_; raw_ptr api_web_contents_; EventHandler event_handler_; bool refreshing_ = false; bool rescan_ = false; + + base::WeakPtrFactory weak_ptr_factory_{this}; }; } // namespace electron