From cd0792c6e8fdb7da33b3c0044218c131835ea4bd Mon Sep 17 00:00:00 2001
From: "trop[bot]" <37223003+trop[bot]@users.noreply.github.com>
Date: Tue, 28 Sep 2021 10:11:36 +0200
Subject: [PATCH] fix: avoid double free when destroying WebContents (#31131)

Co-authored-by: Cheng Zhao <zcbenz@gmail.com>
---
 shell/browser/api/electron_api_web_contents.cc | 17 +++++++++++++++--
 shell/browser/api/electron_api_web_contents.h  |  3 +++
 2 files changed, 18 insertions(+), 2 deletions(-)

diff --git a/shell/browser/api/electron_api_web_contents.cc b/shell/browser/api/electron_api_web_contents.cc
index bbb75b674a7f7..998b6dd66cefd 100644
--- a/shell/browser/api/electron_api_web_contents.cc
+++ b/shell/browser/api/electron_api_web_contents.cc
@@ -943,18 +943,31 @@ WebContents::~WebContents() {
   // InspectableWebContents will be automatically destroyed.
 }
 
+void WebContents::DeleteThisIfAlive() {
+  // It is possible that the FirstWeakCallback has been called but the
+  // SecondWeakCallback has not, in this case the garbage collection of
+  // WebContents has already started and we should not |delete this|.
+  // Calling |GetWrapper| can detect this corner case.
+  auto* isolate = JavascriptEnvironment::GetIsolate();
+  v8::HandleScope scope(isolate);
+  v8::Local<v8::Object> wrapper;
+  if (!GetWrapper(isolate).ToLocal(&wrapper))
+    return;
+  delete this;
+}
+
 void WebContents::Destroy() {
   // The content::WebContents should be destroyed asyncronously when possible
   // as user may choose to destroy WebContents during an event of it.
   if (Browser::Get()->is_shutting_down() || IsGuest() ||
       type_ == Type::kBrowserView) {
-    delete this;
+    DeleteThisIfAlive();
   } else {
     base::PostTask(FROM_HERE, {content::BrowserThread::UI},
                    base::BindOnce(
                        [](base::WeakPtr<WebContents> contents) {
                          if (contents)
-                           delete contents.get();
+                           contents->DeleteThisIfAlive();
                        },
                        GetWeakPtr()));
   }
diff --git a/shell/browser/api/electron_api_web_contents.h b/shell/browser/api/electron_api_web_contents.h
index c557e7e686965..a6a5ef6f5e367 100644
--- a/shell/browser/api/electron_api_web_contents.h
+++ b/shell/browser/api/electron_api_web_contents.h
@@ -432,6 +432,9 @@ class WebContents : public gin::Wrappable<WebContents>,
   WebContents(v8::Isolate* isolate, const gin_helper::Dictionary& options);
   ~WebContents() override;
 
+  // Delete this if garbage collection has not started.
+  void DeleteThisIfAlive();
+
   // Creates a InspectableWebContents object and takes ownership of
   // |web_contents|.
   void InitWithWebContents(std::unique_ptr<content::WebContents> web_contents,