diff --git a/shell/browser/api/electron_api_web_contents_view.cc b/shell/browser/api/electron_api_web_contents_view.cc index 8afe20ff34cf..6002ecb1ae76 100644 --- a/shell/browser/api/electron_api_web_contents_view.cc +++ b/shell/browser/api/electron_api_web_contents_view.cc @@ -67,9 +67,16 @@ WebContentsView::WebContentsView(v8::Isolate* isolate, WebContentsView::~WebContentsView() { if (api_web_contents_) { // destroy() is called - // Destroy WebContents asynchronously unless app is shutting down, - // because destroy() might be called inside WebContents's event handler. - api_web_contents_->DestroyWebContents(!Browser::Get()->is_shutting_down()); + // Destroy WebContents asynchronously, as we might be in GC currently and + // WebContents emits an event in its destructor. + base::PostTask(FROM_HERE, {content::BrowserThread::UI}, + base::BindOnce( + [](base::WeakPtr contents) { + if (contents) + contents->DestroyWebContents( + !Browser::Get()->is_shutting_down()); + }, + api_web_contents_->GetWeakPtr())); } } diff --git a/spec-main/api-web-contents-view-spec.ts b/spec-main/api-web-contents-view-spec.ts index 78942baabce9..3aeeb5da5b89 100644 --- a/spec-main/api-web-contents-view-spec.ts +++ b/spec-main/api-web-contents-view-spec.ts @@ -40,4 +40,29 @@ describe('WebContentsView', () => { expect(code).to.equal(0) }) }) + + function triggerGCByAllocation () { + const arr = [] + for (let i = 0; i < 1000000; i++) { + arr.push([]) + } + return arr + } + + it(`doesn't crash when GCed during allocation`, (done) => { + const web = (webContents as any).create({}) + // eslint-disable-next-line no-new + new WebContentsView(web) + setTimeout(() => { + // NB. the crash we're testing for is the lack of a current `v8::Context` + // when emitting an event in WebContents's destructor. V8 is inconsistent + // about whether or not there's a current context during garbage + // collection, and it seems that `v8Util.requestGarbageCollectionForTesting` + // causes a GC in which there _is_ a current context, so the crash isn't + // triggered. Thus, we force a GC by other means: namely, by allocating a + // bunch of stuff. + triggerGCByAllocation() + done() + }) + }) })