From 8b85759ba0d1189caa9f212fc4e63802290ef615 Mon Sep 17 00:00:00 2001 From: Maksim Sisov Date: Mon, 23 Dec 2019 19:54:27 +0000 Subject: [PATCH] X11: Fix leaving fullscreen regression. For Metacity, we have a workaround which fixes unfullscreening issue. That is, we restore and maximize the window if the previous state before the fullscreen state was maximized. By that, we force metacity to leave the fullscreen. If we don't do it, it may end up in a fullscreen again. However, during our previous refactorings, a regression was introduced. Even though, we followed the same approach, the X11Window::Maximize method didn't directly call XWindow::SetFullScreen, but rather called X11Window::ToggleFullscreen, which resulted in recursive calls to the X11Window::ToggleFullscreen and crashes on X11 level. Also, this CL forces to use previous state when fullscreen is left as long as X server might not send a configure event and change our state. Thus, if it was maximized, set it to maximized. Otherwise, set it to normal as opposed to Unknown state set before. Also, we forgot to move the code that deals with restored bounds when a state change was triggered by X server. This CL restores that behaviour. PS: In a follow-up CL, I will make all state changes to be synchronous as we are doing now in Wayland. TBR=sky Bug: 1034791 Change-Id: I0725625148acf6602e7818823cb96b1d82ef9b38 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1978637 Reviewed-by: Thomas Anderson Commit-Queue: Thomas Anderson Cr-Commit-Position: refs/heads/master@{#727224} --- ui/platform_window/x11/x11_window.cc | 27 +++++++++++++++++++++------ 1 file changed, 21 insertions(+), 6 deletions(-) diff --git a/ui/platform_window/x11/x11_window.cc b/ui/platform_window/x11/x11_window.cc index d6e17cde3bafca..3bfbbdc08a73c4 100644 --- a/ui/platform_window/x11/x11_window.cc +++ b/ui/platform_window/x11/x11_window.cc @@ -250,8 +250,11 @@ void X11Window::ToggleFullscreen() { // files can never be set to fullscreen. Wayland does the same. if (fullscreen) state_ = PlatformWindowState::kFullScreen; + else if (IsMaximized()) + state_ = PlatformWindowState::kMaximized; else - state_ = PlatformWindowState::kUnknown; + state_ = PlatformWindowState::kNormal; + SetFullscreen(fullscreen); if (unmaximize_and_remaximize) @@ -282,7 +285,7 @@ void X11Window::ToggleFullscreen() { void X11Window::Maximize() { if (IsFullscreen()) { // Unfullscreen the window if it is fullscreen. - ToggleFullscreen(); + SetFullscreen(false); // Resize the window so that it does not have the same size as a monitor. // (Otherwise, some window managers immediately put the window back in @@ -308,10 +311,7 @@ void X11Window::Minimize() { } void X11Window::Restore() { - if (XWindow::IsFullscreen()) - ToggleFullscreen(); - if (XWindow::IsMaximized()) - XWindow::Unmaximize(); + XWindow::Unmaximize(); XWindow::Unhide(); } @@ -539,6 +539,21 @@ void X11Window::OnXWindowStateChanged() { state_ = PlatformWindowState::kNormal; } + if (restored_bounds_in_pixels_.IsEmpty()) { + if (IsMaximized()) { + // The request that we become maximized originated from a different + // process. |bounds_in_pixels_| already contains our maximized bounds. Do + // a best effort attempt to get restored bounds by setting it to our + // previously set bounds (and if we get this wrong, we aren't any worse + // off since we'd otherwise be returning our maximized bounds). + SetRestoredBoundsInPixels(previous_bounds()); + } + } else if (!IsMaximized() && !IsFullscreen()) { + // If we have restored bounds, but WM_STATE no longer claims to be + // maximized or fullscreen, we should clear our restored bounds. + SetRestoredBoundsInPixels(gfx::Rect()); + } + if (old_state != state_) platform_window_delegate_->OnWindowStateChanged(state_); }