From f0953902db523e05a3d91579d766979f5b7daa36 Mon Sep 17 00:00:00 2001
From: Jeremy Rose <jeremya@chromium.org>
Date: Mon, 20 Jul 2020 12:13:33 -0700
Subject: [PATCH] refactor: clean up Session with CleanedUpAtExit (#24603)

---
 shell/browser/api/electron_api_session.cc     |  2 --
 shell/browser/api/electron_api_session.h      |  2 ++
 shell/browser/javascript_environment.cc       |  6 +++++-
 shell/common/gin_helper/cleaned_up_at_exit.cc | 18 ++++++++++--------
 4 files changed, 17 insertions(+), 11 deletions(-)

diff --git a/shell/browser/api/electron_api_session.cc b/shell/browser/api/electron_api_session.cc
index dac3f1a2e8..7cee6468a9 100644
--- a/shell/browser/api/electron_api_session.cc
+++ b/shell/browser/api/electron_api_session.cc
@@ -942,8 +942,6 @@ gin::Handle<Session> Session::CreateFrom(
   // The Sessions should never be garbage collected, since the common pattern is
   // to use partition strings, instead of using the Session object directly.
   handle->Pin(isolate);
-  ElectronBrowserMainParts::Get()->RegisterDestructionCallback(
-      base::BindOnce([](Session* session) { delete session; }, handle.get()));
 
   App::Get()->EmitCustomEvent("session-created",
                               handle.ToV8().As<v8::Object>());
diff --git a/shell/browser/api/electron_api_session.h b/shell/browser/api/electron_api_session.h
index 31d22622ee..f74f148fd5 100644
--- a/shell/browser/api/electron_api_session.h
+++ b/shell/browser/api/electron_api_session.h
@@ -15,6 +15,7 @@
 #include "gin/wrappable.h"
 #include "shell/browser/event_emitter_mixin.h"
 #include "shell/browser/net/resolve_proxy_helper.h"
+#include "shell/common/gin_helper/cleaned_up_at_exit.h"
 #include "shell/common/gin_helper/error_thrower.h"
 #include "shell/common/gin_helper/function_template_extensions.h"
 #include "shell/common/gin_helper/pinnable.h"
@@ -51,6 +52,7 @@ namespace api {
 class Session : public gin::Wrappable<Session>,
                 public gin_helper::Pinnable<Session>,
                 public gin_helper::EventEmitterMixin<Session>,
+                public gin_helper::CleanedUpAtExit,
 #if BUILDFLAG(ENABLE_BUILTIN_SPELLCHECKER)
                 public SpellcheckHunspellDictionary::Observer,
 #endif
diff --git a/shell/browser/javascript_environment.cc b/shell/browser/javascript_environment.cc
index 910d900873..ab6f8dc79c 100644
--- a/shell/browser/javascript_environment.cc
+++ b/shell/browser/javascript_environment.cc
@@ -46,7 +46,6 @@ JavascriptEnvironment::~JavascriptEnvironment() {
   {
     v8::Locker locker(isolate_);
     v8::HandleScope scope(isolate_);
-    gin_helper::CleanedUpAtExit::DoCleanup();
     context_.Get(isolate_)->Exit();
   }
   isolate_->Exit();
@@ -97,6 +96,11 @@ void JavascriptEnvironment::OnMessageLoopCreated() {
 
 void JavascriptEnvironment::OnMessageLoopDestroying() {
   DCHECK(microtasks_runner_);
+  {
+    v8::Locker locker(isolate_);
+    v8::HandleScope scope(isolate_);
+    gin_helper::CleanedUpAtExit::DoCleanup();
+  }
   base::MessageLoopCurrent::Get()->RemoveTaskObserver(microtasks_runner_.get());
   platform_->DrainTasks(isolate_);
   platform_->UnregisterIsolate(isolate_);
diff --git a/shell/common/gin_helper/cleaned_up_at_exit.cc b/shell/common/gin_helper/cleaned_up_at_exit.cc
index 9255aa3fca..1ed74675e7 100644
--- a/shell/common/gin_helper/cleaned_up_at_exit.cc
+++ b/shell/common/gin_helper/cleaned_up_at_exit.cc
@@ -4,29 +4,31 @@
 
 #include "shell/common/gin_helper/cleaned_up_at_exit.h"
 
-#include "base/containers/flat_set.h"
+#include <algorithm>
+#include <vector>
+
 #include "base/no_destructor.h"
 
 namespace gin_helper {
 
-base::flat_set<CleanedUpAtExit*>& GetDoomed() {
-  static base::NoDestructor<base::flat_set<CleanedUpAtExit*>> doomed;
+std::vector<CleanedUpAtExit*>& GetDoomed() {
+  static base::NoDestructor<std::vector<CleanedUpAtExit*>> doomed;
   return *doomed;
 }
 CleanedUpAtExit::CleanedUpAtExit() {
-  GetDoomed().insert(this);
+  GetDoomed().emplace_back(this);
 }
 CleanedUpAtExit::~CleanedUpAtExit() {
-  GetDoomed().erase(this);
+  auto& doomed = GetDoomed();
+  doomed.erase(std::remove(doomed.begin(), doomed.end(), this), doomed.end());
 }
 
 // static
 void CleanedUpAtExit::DoCleanup() {
   auto& doomed = GetDoomed();
   while (!doomed.empty()) {
-    auto iter = doomed.begin();
-    delete *iter;
-    // It removed itself from the list.
+    CleanedUpAtExit* next = doomed.back();
+    delete next;
   }
 }