Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: crash when restoring minimized hidden window #21820

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 3 additions & 0 deletions shell/browser/native_window_views.h
Expand Up @@ -271,6 +271,9 @@ class NativeWindowViews : public NativeWindow,

// Set to true if the window is always on top and behind the task bar.
bool behind_task_bar_ = false;

// Whether to block Chromium from handling window messages.
bool block_chromium_message_handler_ = false;
#endif

// Handles unhandled keyboard messages coming back from the renderer process.
Expand Down
19 changes: 19 additions & 0 deletions shell/browser/native_window_views_win.cc
Expand Up @@ -180,6 +180,14 @@ bool NativeWindowViews::PreHandleMSG(UINT message,
LRESULT* result) {
NotifyWindowMessage(message, w_param, l_param);

// See code below for why blocking Chromium from handling messages.
if (block_chromium_message_handler_) {
// Handle the message with default proc.
*result = DefWindowProc(GetAcceleratedWidget(), message, w_param, l_param);
// Tell Chromium to ignore this message.
return true;
}

switch (message) {
// Screen readers send WM_GETOBJECT in order to get the accessibility
// object, so take this opportunity to push Chromium into accessible
Expand Down Expand Up @@ -222,7 +230,18 @@ bool NativeWindowViews::PreHandleMSG(UINT message,
if (!last_normal_placement_bounds_.IsEmpty() &&
GetWindowPlacement(GetAcceleratedWidget(), &wp)) {
wp.rcNormalPosition = last_normal_placement_bounds_.ToRECT();

// When calling SetWindowPlacement, Chromium would do window messages
// handling. But since we are already in PreHandleMSG this would cause
// crash in Chromium under some cases.
//
// We work around the crash by prevent Chromium from handling window
// messages until the SetWindowPlacement call is done.
//
// See https://github.com/electron/electron/issues/21614 for more.
block_chromium_message_handler_ = true;
SetWindowPlacement(GetAcceleratedWidget(), &wp);
block_chromium_message_handler_ = false;

last_normal_placement_bounds_ = gfx::Rect();
}
Expand Down
7 changes: 7 additions & 0 deletions spec-main/api-browser-window-spec.ts
Expand Up @@ -2786,6 +2786,13 @@ describe('BrowserWindow module', () => {
w.restore()
expectBoundsEqual(w.getSize(), initialSize)
})

it('does not crash when restoring hidden minimized window', () => {
const w = new BrowserWindow({})
w.minimize()
w.hide()
w.show()
})
})

describe('BrowserWindow.unmaximize()', () => {
Expand Down