Skip to content

Commit

Permalink
fix: avoid double free when destroying WebContents (#31131)
Browse files Browse the repository at this point in the history
Co-authored-by: Cheng Zhao <zcbenz@gmail.com>
  • Loading branch information
trop[bot] and zcbenz committed Sep 28, 2021
1 parent 6595bc7 commit cd0792c
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 2 deletions.
17 changes: 15 additions & 2 deletions shell/browser/api/electron_api_web_contents.cc
Expand Up @@ -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()));
}
Expand Down
3 changes: 3 additions & 0 deletions shell/browser/api/electron_api_web_contents.h
Expand Up @@ -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,
Expand Down

0 comments on commit cd0792c

Please sign in to comment.