Browse Source

fix: avoid double free when destroying WebContents (#31131)

Co-authored-by: Cheng Zhao <[email protected]>
trop[bot] 3 years ago
parent
commit
cd0792c6e8

+ 15 - 2
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()));
   }

+ 3 - 0
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,