refactor: clean up Session with CleanedUpAtExit (electron#24603)

georgexu99 · Jul 28, 2020 · 7c3ae34 · 7c3ae34
1 parent fa1b532
commit 7c3ae34
Show file tree

Hide file tree

Showing 4 changed files with 17 additions and 11 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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;
   }
 }