fix: memory leak in BrowserWindow (#27640)

Co-authored-by: Jeremy Rose <[email protected]>

shell/browser/electron_browser_main_parts.cc

@@ -277,14 +277,6 @@ int ElectronBrowserMainParts::GetExitCode() {
   return exit_code_ != nullptr ? *exit_code_ : 0;
 }
 
-void ElectronBrowserMainParts::RegisterDestructionCallback(
-    base::OnceClosure callback) {
-  // The destructors should be called in reversed order, so dependencies between
-  // JavaScript objects can be correctly resolved.
-  // For example WebContentsView => WebContents => Session.
-  destructors_.insert(destructors_.begin(), std::move(callback));
-}
-
 int ElectronBrowserMainParts::PreEarlyInitialization() {
   field_trial_list_ = std::make_unique<base::FieldTrialList>(nullptr);
 #if defined(USE_X11)
@@ -577,18 +569,6 @@ void ElectronBrowserMainParts::PostMainMessageLoopRun() {
     }
   }
 
-  // Make sure destruction callbacks are called before message loop is
-  // destroyed, otherwise some objects that need to be deleted on IO thread
-  // won't be freed.
-  // We don't use ranged for loop because iterators are getting invalided when
-  // the callback runs.
-  for (auto iter = destructors_.begin(); iter != destructors_.end();) {
-    base::OnceClosure callback = std::move(*iter);
-    if (!callback.is_null())
-      std::move(callback).Run();
-    ++iter;
-  }
-
   // Destroy node platform after all destructors_ are executed, as they may
   // invoke Node/V8 APIs inside them.
   node_debugger_->Stop();

shell/browser/electron_browser_main_parts.h

@@ -76,11 +76,6 @@ class ElectronBrowserMainParts : public content::BrowserMainParts {
   // Gets the exit code
   int GetExitCode();
 
-  // Register a callback that should be destroyed before JavaScript environment
-  // gets destroyed.
-  // Returns a closure that can be used to remove |callback| from the list.
-  void RegisterDestructionCallback(base::OnceClosure callback);
-
   // Returns the connection to GeolocationControl which can be
   // used to enable the location services once per client.
   device::mojom::GeolocationControl* GetGeolocationControl();
@@ -159,9 +154,6 @@ class ElectronBrowserMainParts : public content::BrowserMainParts {
   std::unique_ptr<ElectronExtensionsBrowserClient> extensions_browser_client_;
 #endif
 
-  // List of callbacks should be executed before destroying JS env.
-  std::list<base::OnceClosure> destructors_;
-
   mojo::Remote<device::mojom::GeolocationControl> geolocation_control_;
 
   static ElectronBrowserMainParts* self_;

shell/common/gin_helper/trackable_object.cc

@@ -35,9 +35,6 @@ TrackableObjectBase::TrackableObjectBase() : weak_factory_(this) {
   // TODO(zcbenz): Make TrackedObject work in renderer process.
   DCHECK(gin_helper::Locker::IsBrowserProcess())
       << "This class only works for browser process";
-
-  electron::ElectronBrowserMainParts::Get()->RegisterDestructionCallback(
-      GetDestroyClosure());
 }
 
 TrackableObjectBase::~TrackableObjectBase() = default;

shell/common/gin_helper/trackable_object.h

@@ -9,6 +9,7 @@
 
 #include "base/bind.h"
 #include "base/memory/weak_ptr.h"
+#include "shell/common/gin_helper/cleaned_up_at_exit.h"
 #include "shell/common/gin_helper/event_emitter.h"
 #include "shell/common/key_weak_map.h"
 
@@ -19,7 +20,7 @@ class SupportsUserData;
 namespace gin_helper {
 
 // Users should use TrackableObject instead.
-class TrackableObjectBase {
+class TrackableObjectBase : public CleanedUpAtExit {
  public:
   TrackableObjectBase();
 
@@ -33,7 +34,7 @@ class TrackableObjectBase {
   static int32_t GetIDFromWrappedClass(base::SupportsUserData* wrapped);
 
  protected:
-  virtual ~TrackableObjectBase();
+  ~TrackableObjectBase() override;
 
   // Returns a closure that can destroy the native class.
   base::OnceClosure GetDestroyClosure();