From 2e9cc7ca075ed29d49825a47f25e4d5a513d8b47 Mon Sep 17 00:00:00 2001
From: Milan Burda <milan.burda@gmail.com>
Date: Fri, 14 Jun 2019 04:44:36 +0200
Subject: [PATCH] fix: crash in BrowserWindow destructor after
 win.webContents.destroy() (#18686)

---
 atom/browser/api/atom_api_browser_window.cc | 6 ++++--
 atom/browser/api/atom_api_browser_window.h  | 2 +-
 atom/browser/api/atom_api_web_contents.cc   | 8 +++++---
 atom/browser/api/atom_api_web_contents.h    | 4 ++++
 4 files changed, 14 insertions(+), 6 deletions(-)

diff --git a/atom/browser/api/atom_api_browser_window.cc b/atom/browser/api/atom_api_browser_window.cc
index b54e7d42b87b6..5ed118c39aba0 100644
--- a/atom/browser/api/atom_api_browser_window.cc
+++ b/atom/browser/api/atom_api_browser_window.cc
@@ -66,7 +66,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());
 
@@ -97,7 +97,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())
diff --git a/atom/browser/api/atom_api_browser_window.h b/atom/browser/api/atom_api_browser_window.h
index 79a0737d7704b..ddc71a0572b25 100644
--- a/atom/browser/api/atom_api_browser_window.h
+++ b/atom/browser/api/atom_api_browser_window.h
@@ -113,7 +113,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_;
 
diff --git a/atom/browser/api/atom_api_web_contents.cc b/atom/browser/api/atom_api_web_contents.cc
index bcb84de93dbfc..6b78b62de2929 100644
--- a/atom/browser/api/atom_api_web_contents.cc
+++ b/atom/browser/api/atom_api_web_contents.cc
@@ -289,7 +289,9 @@ struct WebContents::FrameDispatchHelper {
 WebContents::WebContents(v8::Isolate* isolate,
                          content::WebContents* web_contents,
                          Type type)
-    : content::WebContentsObserver(web_contents), type_(type) {
+    : content::WebContentsObserver(web_contents),
+      type_(type),
+      weak_factory_(this) {
   const mate::Dictionary options = mate::Dictionary::CreateEmpty(isolate);
   if (type == REMOTE) {
     web_contents->SetUserAgentOverride(GetBrowserContext()->GetUserAgent(),
@@ -304,8 +306,8 @@ WebContents::WebContents(v8::Isolate* 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_);
 
diff --git a/atom/browser/api/atom_api_web_contents.h b/atom/browser/api/atom_api_web_contents.h
index ec0a9a381a6ec..af2303349d39c 100644
--- a/atom/browser/api/atom_api_web_contents.h
+++ b/atom/browser/api/atom_api_web_contents.h
@@ -99,6 +99,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
@@ -515,6 +517,8 @@ class WebContents : public mate::TrackableObject<WebContents>,
   // Observers of this WebContents.
   base::ObserverList<ExtendedWebContentsObserver> observers_;
 
+  base::WeakPtrFactory<WebContents> weak_factory_;
+
   DISALLOW_COPY_AND_ASSIGN(WebContents);
 };