Browse Source

fix: prevent crash when keyboard event immediately precedes calling BrowserWindow.close() (#27357)

* 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

Co-authored-by: samuelmaddock <[email protected]>
trop[bot] 4 years ago
parent
commit
c95eda8420

+ 7 - 0
shell/browser/native_window_views.cc

@@ -1366,6 +1366,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();
@@ -1462,6 +1466,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);

+ 2 - 0
shell/browser/native_window_views.h

@@ -173,6 +173,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;
@@ -312,6 +313,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);
 };

+ 8 - 0
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;