mirrors/electron
2
0
Fork 0
Code Issues Pull requests Projects Releases Packages 1 Wiki Activity Actions

fix: crash in BrowserWindow destructor after win.webContents.destroy() (#18686)

Browse source
This commit is contained in:
Milan Burda 2019-06-14 04:44:36 +02:00 committed by Cheng Zhao
parent da58ac7c20
commit 6e327184bd
4 changed files with 17 additions and 7 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

6
atom/browser/api/atom_api_browser_window.cc
View file

@ -66,7 +66,7 @@ BrowserWindow::BrowserWindow(v8::Isolate* isolate,
} }
web_contents_.Reset(isolate, web_contents.ToV8()); web_contents_.Reset(isolate, web_contents.ToV8());
api_web_contents_ = web_contents.get(); api_web_contents_ = web_contents->GetWeakPtr();
api_web_contents_->AddObserver(this); api_web_contents_->AddObserver(this);
Observe(api_web_contents_->web_contents()); Observe(api_web_contents_->web_contents());
@ -93,7 +93,9 @@ BrowserWindow::BrowserWindow(v8::Isolate* isolate,
} }
BrowserWindow::~BrowserWindow() { BrowserWindow::~BrowserWindow() {
api_web_contents_->RemoveObserver(this); // FIXME This is a hack rather than a proper fix preventing shutdown crashes.
if (api_web_contents_)
api_web_contents_->RemoveObserver(this);
// Note that the OnWindowClosed will not be called after the destructor runs, // Note that the OnWindowClosed will not be called after the destructor runs,
// since the window object is managed by the TopLevelWindow class. // since the window object is managed by the TopLevelWindow class.
if (web_contents()) if (web_contents())

2
atom/browser/api/atom_api_browser_window.h
View file

@ -116,7 +116,7 @@ class BrowserWindow : public TopLevelWindow,
#endif #endif
v8::Global<v8::Value> web_contents_; v8::Global<v8::Value> web_contents_;
api::WebContents* api_web_contents_; base::WeakPtr<api::WebContents> api_web_contents_;
base::WeakPtrFactory<BrowserWindow> weak_factory_; base::WeakPtrFactory<BrowserWindow> weak_factory_;

12
atom/browser/api/atom_api_web_contents.cc
View file

@ -253,7 +253,9 @@ void OnCapturePageDone(util::Promise promise, const SkBitmap& bitmap) {
WebContents::WebContents(v8::Isolate* isolate, WebContents::WebContents(v8::Isolate* isolate,
content::WebContents* web_contents) content::WebContents* web_contents)
: content::WebContentsObserver(web_contents), type_(Type::REMOTE) { : content::WebContentsObserver(web_contents),
type_(Type::REMOTE),
weak_factory_(this) {
web_contents->SetUserAgentOverride(GetBrowserContext()->GetUserAgent(), web_contents->SetUserAgentOverride(GetBrowserContext()->GetUserAgent(),
false); false);
Init(isolate); Init(isolate);
@ -268,7 +270,9 @@ WebContents::WebContents(v8::Isolate* isolate,
WebContents::WebContents(v8::Isolate* isolate, WebContents::WebContents(v8::Isolate* isolate,
std::unique_ptr<content::WebContents> web_contents, std::unique_ptr<content::WebContents> web_contents,
Type type) Type type)
: content::WebContentsObserver(web_contents.get()), type_(type) { : content::WebContentsObserver(web_contents.get()),
type_(type),
weak_factory_(this) {
DCHECK(type != Type::REMOTE) DCHECK(type != Type::REMOTE)
<< "Can't take ownership of a remote WebContents"; << "Can't take ownership of a remote WebContents";
auto session = Session::CreateFrom(isolate, GetBrowserContext()); auto session = Session::CreateFrom(isolate, GetBrowserContext());
@ -277,8 +281,8 @@ WebContents::WebContents(v8::Isolate* isolate,
mate::Dictionary::CreateEmpty(isolate)); mate::Dictionary::CreateEmpty(isolate));
} }
WebContents::WebContents(v8::Isolate* isolate, WebContents::WebContents(v8::Isolate* isolate, const mate::Dictionary& options)
const mate::Dictionary& options) { : weak_factory_(this) {
// Read options. // Read options.
options.Get("backgroundThrottling", &background_throttling_); options.Get("backgroundThrottling", &background_throttling_);

4
atom/browser/api/atom_api_web_contents.h
View file

@ -118,6 +118,8 @@ class WebContents : public mate::TrackableObject<WebContents>,
static void BuildPrototype(v8::Isolate* isolate, static void BuildPrototype(v8::Isolate* isolate,
v8::Local<v8::FunctionTemplate> prototype); v8::Local<v8::FunctionTemplate> prototype);
base::WeakPtr<WebContents> GetWeakPtr() { return weak_factory_.GetWeakPtr(); }
// Destroy the managed content::WebContents instance. // Destroy the managed content::WebContents instance.
// //
// Note: The |async| should only be |true| when users are expecting to use the // Note: The |async| should only be |true| when users are expecting to use the
@ -564,6 +566,8 @@ class WebContents : public mate::TrackableObject<WebContents>,
std::map<content::RenderFrameHost*, std::vector<mojo::BindingId>> std::map<content::RenderFrameHost*, std::vector<mojo::BindingId>>
frame_to_bindings_map_; frame_to_bindings_map_;
base::WeakPtrFactory<WebContents> weak_factory_;
DISALLOW_COPY_AND_ASSIGN(WebContents); DISALLOW_COPY_AND_ASSIGN(WebContents);
}; };