From 43341103394b63521cb0b4b5ab568d3ebd31c70e Mon Sep 17 00:00:00 2001
From: Samuel Maddock <samuel.maddock@gmail.com>
Date: Mon, 18 Jan 2021 21:37:21 -0500
Subject: [PATCH] fix: prevent crash when keyboard event immediately precedes
 calling BrowserWindow.close() (#27315)

* fix: prevent crash when destroyed widget receives keyboard event

Activating a key to close a window will cause a silent crash. Handling the keyboard
event will lead to a nullptr dereferenced in Chromium code if the window widget has
already been destroyed.

* test: ensure BrowserWindow doesn't crash from keyboard events during close
---
 shell/browser/native_window_views.cc | 7 +++++++
 shell/browser/native_window_views.h  | 2 ++
 spec-main/api-browser-window-spec.ts | 8 ++++++++
 3 files changed, 17 insertions(+)

diff --git a/shell/browser/native_window_views.cc b/shell/browser/native_window_views.cc
index d589363b66b6..cf804524627d 100644
--- a/shell/browser/native_window_views.cc
+++ b/shell/browser/native_window_views.cc
@@ -1417,6 +1417,10 @@ void NativeWindowViews::OnWidgetDestroying(views::Widget* widget) {
 #endif
 }
 
+void NativeWindowViews::OnWidgetDestroyed(views::Widget* changed_widget) {
+  widget_destroyed_ = true;
+}
+
 void NativeWindowViews::DeleteDelegate() {
   if (is_modal() && this->parent()) {
     auto* parent = this->parent();
@@ -1513,6 +1517,9 @@ void NativeWindowViews::OnWidgetMove() {
 void NativeWindowViews::HandleKeyboardEvent(
     content::WebContents*,
     const content::NativeWebKeyboardEvent& event) {
+  if (widget_destroyed_)
+    return;
+
 #if defined(OS_LINUX)
   if (event.windows_key_code == ui::VKEY_BROWSER_BACK)
     NotifyWindowExecuteAppCommand(kBrowserBackward);
diff --git a/shell/browser/native_window_views.h b/shell/browser/native_window_views.h
index 1024c43f6b8e..e1fff4a0d075 100644
--- a/shell/browser/native_window_views.h
+++ b/shell/browser/native_window_views.h
@@ -174,6 +174,7 @@ class NativeWindowViews : public NativeWindow,
   void OnWidgetBoundsChanged(views::Widget* widget,
                              const gfx::Rect& bounds) override;
   void OnWidgetDestroying(views::Widget* widget) override;
+  void OnWidgetDestroyed(views::Widget* widget) override;
 
   // views::WidgetDelegate:
   void DeleteDelegate() override;
@@ -313,6 +314,7 @@ class NativeWindowViews : public NativeWindow,
   std::string title_;
   gfx::Size widget_size_;
   double opacity_ = 1.0;
+  bool widget_destroyed_ = false;
 
   DISALLOW_COPY_AND_ASSIGN(NativeWindowViews);
 };
diff --git a/spec-main/api-browser-window-spec.ts b/spec-main/api-browser-window-spec.ts
index 7c6d58c310e0..d480f7b22712 100644
--- a/spec-main/api-browser-window-spec.ts
+++ b/spec-main/api-browser-window-spec.ts
@@ -110,6 +110,14 @@ describe('BrowserWindow module', () => {
       await emittedOnce(w.webContents, 'before-unload-fired');
     });
 
+    it('should not crash when keyboard event is sent before closing', async () => {
+      await w.loadURL('data:text/html,pls no crash');
+      const closed = emittedOnce(w, 'closed');
+      w.webContents.sendInputEvent({ type: 'keyDown', keyCode: 'Escape' });
+      w.close();
+      await closed;
+    });
+
     describe('when invoked synchronously inside navigation observer', () => {
       let server: http.Server = null as unknown as http.Server;
       let url: string = null as unknown as string;