refactor: simplify BaseWindow parent<->child association code (#49537)

* refactor: simplify BaseWindow parent<->child association code

* refactor: remove unnecessary local variable from BaseWindow::GetParentWindow()

shell/browser/api/electron_api_base_window.cc

@@ -148,15 +148,6 @@ BaseWindow::~BaseWindow() {
 void BaseWindow::InitWith(v8::Isolate* isolate, v8::Local<v8::Object> wrapper) {
   gin_helper::TrackableObject<BaseWindow>::InitWith(isolate, wrapper);
 
-  // We can only append this window to parent window's child windows after this
-  // window's JS wrapper gets initialized.
-  if (!parent_window_.IsEmpty()) {
-    gin_helper::Handle<BaseWindow> parent;
-    gin::ConvertFromV8(isolate, GetParentWindow(), &parent);
-    DCHECK(!parent.IsEmpty());
-    parent->child_windows_.Set(isolate, weak_map_id(), wrapper);
-  }
-
   // Reference this object in case it got garbage collected.
   self_ref_.Reset(isolate, wrapper);
 }
@@ -182,7 +173,7 @@ void BaseWindow::OnWindowClosed() {
 
   Emit("closed");
 
-  RemoveFromParentChildWindows();
+  parent_window_.Reset();
 
   // Destroy the native class when window is closed.
   base::SingleThreadTaskRunner::GetCurrentDefault()->PostTask(
@@ -770,14 +761,11 @@ void BaseWindow::SetParentWindow(v8::Local<v8::Value> value,
 
   gin_helper::Handle<BaseWindow> parent;
   if (value->IsNull() || value->IsUndefined()) {
-    RemoveFromParentChildWindows();
     parent_window_.Reset();
     window_->SetParentWindow(nullptr);
   } else if (gin::ConvertFromV8(isolate(), value, &parent)) {
-    RemoveFromParentChildWindows();
-    parent_window_.Reset(isolate(), value);
-    window_->SetParentWindow(parent->window_.get());
-    parent->child_windows_.Set(isolate(), weak_map_id(), GetWrapper());
+    parent_window_.Reset(isolate(), parent.ToV8());
+    window_->SetParentWindow(parent->window());
   } else {
     args->ThrowTypeError("Must pass BaseWindow instance or null");
   }
@@ -989,15 +977,28 @@ v8::Local<v8::Value> BaseWindow::GetContentView() const {
     return v8::Local<v8::Value>::New(isolate(), content_view_);
 }
 
-v8::Local<v8::Value> BaseWindow::GetParentWindow() const {
+BaseWindow* BaseWindow::GetParentWindow() const {
   if (parent_window_.IsEmpty())
-    return v8::Null(isolate());
-  else
-    return v8::Local<v8::Value>::New(isolate(), parent_window_);
+    return nullptr;
+
+  v8::HandleScope scope{isolate()};
+  auto local = v8::Local<v8::Value>::New(isolate(), parent_window_);
+  BaseWindow* parent = nullptr;
+  gin::ConvertFromV8(isolate(), local, &parent);
+  return parent;
 }
 
-std::vector<v8::Local<v8::Object>> BaseWindow::GetChildWindows() const {
-  return child_windows_.Values(isolate());
+std::vector<BaseWindow*> BaseWindow::GetChildWindows() const {
+  std::vector<BaseWindow*> children;
+  auto* const isolate = this->isolate();
+  v8::HandleScope scope{isolate};
+  for (auto wrapper : BaseWindow::GetAll(isolate)) {
+    BaseWindow* win = nullptr;
+    gin::ConvertFromV8(isolate, wrapper, &win);
+    if (win && win->GetParentWindow() == this)
+      children.emplace_back(win);
+  }
+  return children;
 }
 
 bool BaseWindow::IsModal() const {
@@ -1145,21 +1146,6 @@ void BaseWindow::SetTitleBarOverlay(const gin_helper::Dictionary& options,
 }
 #endif
 
-void BaseWindow::RemoveFromParentChildWindows() {
-  if (parent_window_.IsEmpty())
-    return;
-
-  v8::Isolate* isolate = JavascriptEnvironment::GetIsolate();
-  v8::HandleScope handle_scope(isolate);
-  gin_helper::Handle<BaseWindow> parent;
-  if (!gin::ConvertFromV8(isolate, GetParentWindow(), &parent) ||
-      parent.IsEmpty()) {
-    return;
-  }
-
-  parent->child_windows_.Remove(weak_map_id());
-}
-
 // static
 gin_helper::WrappableBase* BaseWindow::New(gin::Arguments* const args) {
   auto options = gin_helper::Dictionary::CreateEmpty(args->isolate());

shell/browser/api/electron_api_base_window.h

@@ -8,7 +8,6 @@
 #include <array>
 #include <map>
 #include <memory>
-#include <optional>
 #include <string>
 #include <string_view>
 #include <vector>
@@ -233,8 +232,8 @@ class BaseWindow : public gin_helper::TrackableObject<BaseWindow>,
 
   // Public getters of NativeWindow.
   v8::Local<v8::Value> GetContentView() const;
-  v8::Local<v8::Value> GetParentWindow() const;
-  std::vector<v8::Local<v8::Object>> GetChildWindows() const;
+  BaseWindow* GetParentWindow() const;
+  std::vector<BaseWindow*> GetChildWindows() const;
   bool IsModal() const;
 
   // Extra APIs added in JS.
@@ -270,9 +269,6 @@ class BaseWindow : public gin_helper::TrackableObject<BaseWindow>,
  private:
   // Helpers.
 
-  // Remove this window from parent window's |child_windows_|.
-  void RemoveFromParentChildWindows();
-
   template <typename... Args>
   void EmitEventSoon(std::string_view eventName) {
     content::GetUIThreadTaskRunner({})->PostTask(
@@ -289,7 +285,6 @@ class BaseWindow : public gin_helper::TrackableObject<BaseWindow>,
   v8::Global<v8::Value> content_view_;
   v8::Global<v8::Value> menu_;
   v8::Global<v8::Value> parent_window_;
-  KeyWeakMap<int> child_windows_;
 
   std::unique_ptr<NativeWindow> window_;
 

shell/browser/api/electron_api_browser_window.cc

@@ -209,12 +209,8 @@ void BrowserWindow::UpdateWindowControlsOverlay(
 
 void BrowserWindow::CloseImmediately() {
   // Close all child windows before closing current window.
-  v8::HandleScope handle_scope(isolate());
-  for (v8::Local<v8::Value> value : GetChildWindows()) {
-    gin_helper::Handle<BrowserWindow> child;
-    if (gin::ConvertFromV8(isolate(), value, &child) && !child.IsEmpty())
-      child->window()->CloseImmediately();
-  }
+  for (BaseWindow* child : GetChildWindows())
+    child->window()->CloseImmediately();
 
   BaseWindow::CloseImmediately();
 }