fix: crash when destroying WebContentsView during GC (#22674)
This commit is contained in:
		
					parent
					
						
							
								1b353d1ed3
							
						
					
				
			
			
				commit
				
					
						9c5874306d
					
				
			
		
					 2 changed files with 35 additions and 3 deletions
				
			
		| 
						 | 
					@ -67,9 +67,16 @@ WebContentsView::WebContentsView(v8::Isolate* isolate,
 | 
				
			||||||
 | 
					
 | 
				
			||||||
WebContentsView::~WebContentsView() {
 | 
					WebContentsView::~WebContentsView() {
 | 
				
			||||||
  if (api_web_contents_) {  // destroy() is called
 | 
					  if (api_web_contents_) {  // destroy() is called
 | 
				
			||||||
    // Destroy WebContents asynchronously unless app is shutting down,
 | 
					    // Destroy WebContents asynchronously, as we might be in GC currently and
 | 
				
			||||||
    // because destroy() might be called inside WebContents's event handler.
 | 
					    // WebContents emits an event in its destructor.
 | 
				
			||||||
    api_web_contents_->DestroyWebContents(!Browser::Get()->is_shutting_down());
 | 
					    base::PostTask(FROM_HERE, {content::BrowserThread::UI},
 | 
				
			||||||
 | 
					                   base::BindOnce(
 | 
				
			||||||
 | 
					                       [](base::WeakPtr<WebContents> contents) {
 | 
				
			||||||
 | 
					                         if (contents)
 | 
				
			||||||
 | 
					                           contents->DestroyWebContents(
 | 
				
			||||||
 | 
					                               !Browser::Get()->is_shutting_down());
 | 
				
			||||||
 | 
					                       },
 | 
				
			||||||
 | 
					                       api_web_contents_->GetWeakPtr()));
 | 
				
			||||||
  }
 | 
					  }
 | 
				
			||||||
}
 | 
					}
 | 
				
			||||||
 | 
					
 | 
				
			||||||
| 
						 | 
					
 | 
				
			||||||
| 
						 | 
					@ -40,4 +40,29 @@ describe('WebContentsView', () => {
 | 
				
			||||||
      expect(code).to.equal(0)
 | 
					      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()
 | 
				
			||||||
 | 
					    })
 | 
				
			||||||
 | 
					  })
 | 
				
			||||||
})
 | 
					})
 | 
				
			||||||
| 
						 | 
					
 | 
				
			||||||
		Loading…
	
	Add table
		Add a link
		
	
		Reference in a new issue