From 8364b62f681574bd5d94207a0f433aa83b6a7a74 Mon Sep 17 00:00:00 2001
From: Charles Kerr <charles@charleskerr.com>
Date: Tue, 27 Jan 2026 15:27:38 -0600
Subject: [PATCH] fix: potential dangling pointer in api::Screen (#49536)

fixes a regression from #49506
---
 shell/browser/api/electron_api_screen.cc | 53 ++++++++++++++++++++----
 shell/browser/api/electron_api_screen.h  | 26 ++++--------
 2 files changed, 52 insertions(+), 27 deletions(-)

diff --git a/shell/browser/api/electron_api_screen.cc b/shell/browser/api/electron_api_screen.cc
index 7652cb631d..d9ed7ea662 100644
--- a/shell/browser/api/electron_api_screen.cc
+++ b/shell/browser/api/electron_api_screen.cc
@@ -70,16 +70,26 @@ void DelayEmitWithMetrics(Screen* screen,
   screen->Emit(name, display, metrics);
 }
 
+// Calls the one-liner `display::Screen::Get()` to get ui's global screen.
+// NOTE: during shutdown, that screen can be destroyed before us. This means:
+// 1. Call this instead of keeping a possibly-dangling raw_ptr in api::Screen.
+// 2. Always check this function's return value for nullptr before use.
+[[nodiscard]] auto* GetDisplayScreen() {
+  return display::Screen::Get();
+}
+
+[[nodiscard]] auto GetFallbackDisplay() {
+  return display::Display::GetDefaultDisplay();
+}
 }  // namespace
 
-Screen::Screen(display::Screen* screen) : screen_{screen} {
-  screen_->AddObserver(this);
+Screen::Screen() {
+  if (auto* screen = GetDisplayScreen())
+    screen->AddObserver(this);
 }
 
 Screen::~Screen() {
-  // Use `display::Screen::Get()` here, not our cached `screen_`:
-  // during shutdown, it can get torn down before us.
-  if (auto* screen = display::Screen::Get())
+  if (auto* screen = GetDisplayScreen())
     screen->RemoveObserver(this);
 }
 
@@ -95,7 +105,33 @@ gfx::Point Screen::GetCursorScreenPoint(v8::Isolate* isolate) {
     return {};
   }
 #endif
-  return screen_->GetCursorScreenPoint();
+  auto* screen = GetDisplayScreen();
+  return screen ? screen->GetCursorScreenPoint() : gfx::Point{};
+}
+
+display::Display Screen::GetPrimaryDisplay() const {
+  const auto* screen = GetDisplayScreen();
+  return screen ? screen->GetPrimaryDisplay() : GetFallbackDisplay();
+}
+
+std::vector<display::Display> Screen::GetAllDisplays() const {
+  if (const auto* screen = GetDisplayScreen())
+    return screen->GetAllDisplays();
+
+  // Even though this is only reached during shutdown by Screen::Get() failing,
+  // display::Screen::GetAllDisplays() is guaranteed to return >= 1 display.
+  // For consistency with that API, let's return a nonempty vector here.
+  return {GetFallbackDisplay()};
+}
+
+display::Display Screen::GetDisplayNearestPoint(const gfx::Point& point) const {
+  const auto* screen = GetDisplayScreen();
+  return screen ? screen->GetDisplayNearestPoint(point) : GetFallbackDisplay();
+}
+
+display::Display Screen::GetDisplayMatching(const gfx::Rect& match_rect) const {
+  const auto* screen = GetDisplayScreen();
+  return screen ? screen->GetDisplayMatching(match_rect) : GetFallbackDisplay();
 }
 
 #if BUILDFLAG(IS_WIN)
@@ -182,14 +218,14 @@ Screen* Screen::Create(gin_helper::ErrorThrower error_thrower) {
     return {};
   }
 
-  display::Screen* screen = display::Screen::Get();
+  display::Screen* screen = GetDisplayScreen();
   if (!screen) {
     error_thrower.ThrowError("Failed to get screen information");
     return {};
   }
 
   return cppgc::MakeGarbageCollected<Screen>(
-      error_thrower.isolate()->GetCppHeap()->GetAllocationHandle(), screen);
+      error_thrower.isolate()->GetCppHeap()->GetAllocationHandle());
 }
 
 gin::ObjectTemplateBuilder Screen::GetObjectTemplateBuilder(
@@ -218,7 +254,6 @@ const gin::WrapperInfo* Screen::wrapper_info() const {
 const char* Screen::GetHumanReadableName() const {
   return "Electron / Screen";
 }
-
 }  // namespace electron::api
 
 namespace {
diff --git a/shell/browser/api/electron_api_screen.h b/shell/browser/api/electron_api_screen.h
index 19e910e05f..40c61aae7b 100644
--- a/shell/browser/api/electron_api_screen.h
+++ b/shell/browser/api/electron_api_screen.h
@@ -7,7 +7,6 @@
 
 #include <vector>
 
-#include "base/memory/raw_ptr.h"
 #include "gin/wrappable.h"
 #include "shell/browser/event_emitter_mixin.h"
 #include "ui/display/display_observer.h"
@@ -43,22 +42,16 @@ class Screen final : public gin::Wrappable<Screen>,
   Screen& operator=(const Screen&) = delete;
 
   // Make public for cppgc::MakeGarbageCollected.
-  explicit Screen(display::Screen* screen);
+  Screen();
   ~Screen() override;
 
-  gfx::Point GetCursorScreenPoint(v8::Isolate* isolate);
-  display::Display GetPrimaryDisplay() const {
-    return screen_->GetPrimaryDisplay();
-  }
-  const std::vector<display::Display>& GetAllDisplays() const {
-    return screen_->GetAllDisplays();
-  }
-  display::Display GetDisplayNearestPoint(const gfx::Point& point) const {
-    return screen_->GetDisplayNearestPoint(point);
-  }
-  display::Display GetDisplayMatching(const gfx::Rect& match_rect) const {
-    return screen_->GetDisplayMatching(match_rect);
-  }
+  [[nodiscard]] gfx::Point GetCursorScreenPoint(v8::Isolate* isolate);
+  [[nodiscard]] display::Display GetPrimaryDisplay() const;
+  [[nodiscard]] std::vector<display::Display> GetAllDisplays() const;
+  [[nodiscard]] display::Display GetDisplayNearestPoint(
+      const gfx::Point& point) const;
+  [[nodiscard]] display::Display GetDisplayMatching(
+      const gfx::Rect& match_rect) const;
 
   gfx::PointF ScreenToDIPPoint(const gfx::PointF& point_px);
   gfx::Point DIPToScreenPoint(const gfx::Point& point_dip);
@@ -68,9 +61,6 @@ class Screen final : public gin::Wrappable<Screen>,
   void OnDisplaysRemoved(const display::Displays& removed_displays) override;
   void OnDisplayMetricsChanged(const display::Display& display,
                                uint32_t changed_metrics) override;
-
- private:
-  raw_ptr<display::Screen> screen_;
 };
 
 }  // namespace electron::api