Browse Source

refactor: add SerialChooserController::web_contents_ (#43823)

Add a `base::WeakPtr<WebContents>` field to SerialChooserController and
stop subclassing from WebContentsObserver. This follows the Observer docs:

> don't create a `WebContentsObserver` just to be able to check
> for a null `WebContentsObserver::web_contents()`.
> Use a `base::WeakPtr<WebContents>` instead.
Charles Kerr 6 months ago
parent
commit
79627deea3

+ 11 - 13
shell/browser/serial/serial_chooser_controller.cc

@@ -110,17 +110,18 @@ SerialChooserController::SerialChooserController(
     content::SerialChooser::Callback callback,
     content::WebContents* web_contents,
     base::WeakPtr<ElectronSerialDelegate> serial_delegate)
-    : WebContentsObserver(web_contents),
+    : web_contents_{web_contents ? web_contents->GetWeakPtr()
+                                 : base::WeakPtr<content::WebContents>()},
       filters_(std::move(filters)),
       allowed_bluetooth_service_class_ids_(
           std::move(allowed_bluetooth_service_class_ids)),
       callback_(std::move(callback)),
       serial_delegate_(serial_delegate),
       render_frame_host_id_(render_frame_host->GetGlobalId()) {
-  origin_ = web_contents->GetPrimaryMainFrame()->GetLastCommittedOrigin();
+  origin_ = web_contents_->GetPrimaryMainFrame()->GetLastCommittedOrigin();
 
   chooser_context_ = SerialChooserContextFactory::GetForBrowserContext(
-                         web_contents->GetBrowserContext())
+                         web_contents_->GetBrowserContext())
                          ->AsWeakPtr();
   DCHECK(chooser_context_);
   chooser_context_->GetPortManager()->GetDevices(base::BindOnce(
@@ -133,10 +134,10 @@ SerialChooserController::~SerialChooserController() {
 }
 
 api::Session* SerialChooserController::GetSession() {
-  if (!web_contents()) {
+  if (!web_contents_) {
     return nullptr;
   }
-  return api::Session::FromBrowserContext(web_contents()->GetBrowserContext());
+  return api::Session::FromBrowserContext(web_contents_->GetBrowserContext());
 }
 
 void SerialChooserController::OnPortAdded(
@@ -148,7 +149,7 @@ void SerialChooserController::OnPortAdded(
 
   api::Session* session = GetSession();
   if (session) {
-    session->Emit("serial-port-added", port.Clone(), web_contents());
+    session->Emit("serial-port-added", port.Clone(), web_contents_.get());
   }
 }
 
@@ -157,10 +158,8 @@ void SerialChooserController::OnPortRemoved(
   const auto it = std::ranges::find(ports_, port.token,
                                     &device::mojom::SerialPortInfo::token);
   if (it != ports_.end()) {
-    api::Session* session = GetSession();
-    if (session) {
-      session->Emit("serial-port-removed", port.Clone(), web_contents());
-    }
+    if (api::Session* session = GetSession())
+      session->Emit("serial-port-removed", port.Clone(), web_contents_.get());
     ports_.erase(it);
   }
 }
@@ -203,10 +202,9 @@ void SerialChooserController::OnGetDevices(
   }
 
   bool prevent_default = false;
-  api::Session* session = GetSession();
-  if (session) {
+  if (api::Session* session = GetSession()) {
     prevent_default = session->Emit(
-        "select-serial-port", ports_, web_contents(),
+        "select-serial-port", ports_, web_contents_.get(),
         base::BindRepeating(&SerialChooserController::OnDeviceChosen,
                             weak_factory_.GetWeakPtr()));
   }

+ 3 - 3
shell/browser/serial/serial_chooser_controller.h

@@ -12,7 +12,6 @@
 #include "base/scoped_observation.h"
 #include "content/public/browser/global_routing_id.h"
 #include "content/public/browser/serial_chooser.h"
-#include "content/public/browser/web_contents_observer.h"
 #include "services/device/public/mojom/serial.mojom-forward.h"
 #include "shell/browser/serial/serial_chooser_context.h"
 #include "third_party/blink/public/mojom/serial/serial.mojom-forward.h"
@@ -32,8 +31,7 @@ class ElectronSerialDelegate;
 
 // SerialChooserController provides data for the Serial API permission prompt.
 class SerialChooserController final
-    : private SerialChooserContext::PortObserver,
-      private content::WebContentsObserver {
+    : private SerialChooserContext::PortObserver {
  public:
   SerialChooserController(
       content::RenderFrameHost* render_frame_host,
@@ -64,6 +62,8 @@ class SerialChooserController final
   void RunCallback(device::mojom::SerialPortInfoPtr port);
   void OnDeviceChosen(const std::string& port_id);
 
+  base::WeakPtr<content::WebContents> web_contents_;
+
   std::vector<blink::mojom::SerialPortFilterPtr> filters_;
   std::vector<::device::BluetoothUUID> allowed_bluetooth_service_class_ids_;
   content::SerialChooser::Callback callback_;