Skip to content

Commit

Permalink
X11: Fix leaving fullscreen regression.
Browse files Browse the repository at this point in the history
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 <[email protected]>
Commit-Queue: Thomas Anderson <[email protected]>
Cr-Commit-Position: refs/heads/master@{#727224}
  • Loading branch information
msisov authored and Commit Bot committed Dec 23, 2019
1 parent 3501fb8 commit 8b85759
Showing 1 changed file with 21 additions and 6 deletions.
27 changes: 21 additions & 6 deletions ui/platform_window/x11/x11_window.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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
Expand All @@ -308,10 +311,7 @@ void X11Window::Minimize() {
}

void X11Window::Restore() {
if (XWindow::IsFullscreen())
ToggleFullscreen();
if (XWindow::IsMaximized())
XWindow::Unmaximize();
XWindow::Unmaximize();
XWindow::Unhide();
}

Expand Down Expand Up @@ -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_);
}
Expand Down

0 comments on commit 8b85759

Please sign in to comment.