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

This commit is contained in:
Milan Burda
2019-06-14 18:18:38 +02:00
committed by Shelley Vohr
parent d3f7bdd0b3
commit 47827ed45c
4 changed files with 17 additions and 7 deletions

View File

@@ -67,7 +67,7 @@ BrowserWindow::BrowserWindow(v8::Isolate* isolate,
}
web_contents_.Reset(isolate, web_contents.ToV8());
api_web_contents_ = web_contents.get();
api_web_contents_ = web_contents->GetWeakPtr();
api_web_contents_->AddObserver(this);
Observe(api_web_contents_->web_contents());
@@ -94,7 +94,9 @@ BrowserWindow::BrowserWindow(v8::Isolate* isolate,
}
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,
// since the window object is managed by the TopLevelWindow class.
if (web_contents())

View File

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

View File

@@ -268,7 +268,9 @@ struct WebContents::FrameDispatchHelper {
WebContents::WebContents(v8::Isolate* isolate,
content::WebContents* web_contents)
: content::WebContentsObserver(web_contents), type_(REMOTE) {
: content::WebContentsObserver(web_contents),
type_(REMOTE),
weak_factory_(this) {
web_contents->SetUserAgentOverride(GetBrowserContext()->GetUserAgent(),
false);
Init(isolate);
@@ -283,7 +285,9 @@ WebContents::WebContents(v8::Isolate* isolate,
WebContents::WebContents(v8::Isolate* isolate,
std::unique_ptr<content::WebContents> web_contents,
Type type)
: content::WebContentsObserver(web_contents.get()), type_(type) {
: content::WebContentsObserver(web_contents.get()),
type_(type),
weak_factory_(this) {
DCHECK(type != REMOTE) << "Can't take ownership of a remote WebContents";
auto session = Session::CreateFrom(isolate, GetBrowserContext());
session_.Reset(isolate, session.ToV8());
@@ -291,8 +295,8 @@ WebContents::WebContents(v8::Isolate* isolate,
mate::Dictionary::CreateEmpty(isolate));
}
WebContents::WebContents(v8::Isolate* isolate,
const mate::Dictionary& options) {
WebContents::WebContents(v8::Isolate* isolate, const mate::Dictionary& options)
: weak_factory_(this) {
// Read options.
options.Get("backgroundThrottling", &background_throttling_);

View File

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