[ozone/wayland] Fixed wrong behaviour on maximize/restore.

The handler of configure surface events didn't take whether the window
had normal state when responding to zero width and height passed by the
compositor.  This resulted in unnecessary restoring of window size
when it should remain maximized of expanded to full screen.

This CL fixes the behaviour and improves the test to cover the scenario
explained above.

R=msisov@igalia.com

Bug: 934686
Change-Id: I4dd7e01484ccea663f0e0012288d6de70e48ad06
Reviewed-on: https://chromium-review.googlesource.com/c/1489195
Commit-Queue: Alexander Dunaev <adunaev@igalia.com>
Reviewed-by: Maksim Sisov <msisov@igalia.com>
Cr-Commit-Position: refs/heads/master@{#636757}

ui/ozone/platform/wayland/wayland_window.cc

@@ -369,14 +369,6 @@ void WaylandWindow::ToggleFullscreen() {
     // DesktopWindowTreeHostPlatform::IsFullscreen, for example, and media
     // files can never be set to fullscreen.
     state_ = PlatformWindowState::PLATFORM_WINDOW_STATE_FULLSCREEN;
-    // Client might have requested a fullscreen state while the window was in
-    // a maximized state. Thus, |restored_bounds_| can contain the bounds of a
-    // "normal" state before the window was maximized. We don't override them
-    // unless they are empty, because |bounds_| can contain bounds of a
-    // maximized window instead.
-    if (restored_bounds_.IsEmpty())
-      SetRestoredBoundsInPixels(bounds_);
-
     xdg_surface_->SetFullscreen();
   } else {
     // Check the comment above. If it's not handled synchronously, media files
@@ -394,15 +386,6 @@ void WaylandWindow::Maximize() {
   if (IsFullscreen())
     ToggleFullscreen();
 
-  // Keeps track of the previous bounds, which are used to restore a window
-  // after unmaximize call. We don't override |restored_bounds_| if they have
-  // already had value, which means the previous state has been a fullscreen
-  // state. That is, the bounds can be stored during a change from a normal
-  // state to a maximize state, and then preserved to be the same, when changing
-  // from maximized to fullscreen and back to a maximized state.
-  if (restored_bounds_.IsEmpty())
-    SetRestoredBoundsInPixels(bounds_);
-
   xdg_surface_->SetMaximized();
   connection_->ScheduleFlush();
 }
@@ -557,6 +540,8 @@ void WaylandWindow::HandleSurfaceConfigure(int32_t width,
   } else {
     state_ = PlatformWindowState::PLATFORM_WINDOW_STATE_NORMAL;
   }
+  const bool state_changed = old_state != state_;
+  const bool is_normal = !IsFullscreen() && !IsMaximized();
 
   // Update state before notifying delegate.
   const bool did_active_change = is_active_ != is_activated;
@@ -569,27 +554,36 @@ void WaylandWindow::HandleSurfaceConfigure(int32_t width,
   //
   // Width or height set to 0 means that we should decide on width and height by
   // ourselves, but we don't want to set them to anything else. Use restored
-  // bounds size or the current bounds.
+  // bounds size or the current bounds iff the current state is normal (neither
+  // maximized nor fullscreen).
   //
   // Note: if the browser was started with --start-fullscreen and a user exits
   // the fullscreen mode, wayland may set the width and height to be 1. Instead,
   // explicitly set the bounds to the current desired ones or the previous
   // bounds.
-  if (width <= 1 || height <= 1) {
+  if (width > 1 && height > 1) {
+    pending_bounds_ = gfx::Rect(0, 0, width, height);
+  } else if (is_normal) {
     pending_bounds_.set_size(restored_bounds_.IsEmpty()
                                  ? GetBounds().size()
                                  : restored_bounds_.size());
-  } else {
-    pending_bounds_ = gfx::Rect(0, 0, width, height);
   }
 
-  const bool is_normal = !IsFullscreen() && !IsMaximized();
-  const bool state_changed = old_state != state_;
-  if (is_normal && state_changed)
-    restored_bounds_ = gfx::Rect();
+  if (state_changed) {
+    // The |restored_bounds_| are used when the window gets back to normal
+    // state after it went maximized or fullscreen.  So we reset these if the
+    // window has just become normal and store the current bounds if it is
+    // either going out of normal state or simply changes the state and we don't
+    // have any meaningful value stored.
+    if (is_normal) {
+      SetRestoredBoundsInPixels({});
+    } else if (old_state == PlatformWindowState::PLATFORM_WINDOW_STATE_NORMAL ||
+               restored_bounds_.IsEmpty()) {
+      SetRestoredBoundsInPixels(bounds_);
+    }
 
-  if (state_changed)
     delegate_->OnWindowStateChanged(state_);
+  }
 
   if (did_active_change)
     delegate_->OnActivationChanged(is_active_);

ui/ozone/platform/wayland/wayland_window.h

@@ -204,7 +204,7 @@ class WaylandWindow : public PlatformWindow,
 
   gfx::Rect bounds_;
   gfx::Rect pending_bounds_;
-  // The bounds of our window before we were maximized or fullscreen.
+  // The bounds of the window before it went maximized or fullscreen.
   gfx::Rect restored_bounds_;
   bool has_pointer_focus_ = false;
   bool has_keyboard_focus_ = false;

ui/ozone/platform/wayland/wayland_window_unittest.cc

@@ -24,11 +24,11 @@
 #include "ui/ozone/test/mock_platform_window_delegate.h"
 #include "ui/platform_window/platform_window_init_properties.h"
 
+using ::testing::_;
 using ::testing::Eq;
 using ::testing::Mock;
 using ::testing::SaveArg;
 using ::testing::StrEq;
-using ::testing::_;
 
 namespace ui {
 
@@ -118,6 +118,13 @@ class WaylandWindowTest : public WaylandTest {
     return states;
   }
 
+  ScopedWlArray MakeStateArray(const std::vector<int32_t> states) {
+    ScopedWlArray result;
+    for (const auto state : states)
+      AddStateToWlArray(state, result.get());
+    return result;
+  }
+
   std::unique_ptr<WaylandWindow> CreateWaylandWindowWithParams(
       PlatformWindowType type,
       gfx::AcceleratedWidget parent_widget,
@@ -147,6 +154,11 @@ class WaylandWindowTest : public WaylandTest {
     hit_tests->push_back(static_cast<int>(HTTOPRIGHT));
   }
 
+  void VerifyAndClearExpectations() {
+    Mock::VerifyAndClearExpectations(xdg_surface_);
+    Mock::VerifyAndClearExpectations(&delegate_);
+  }
+
   wl::MockXdgSurface* xdg_surface_;
 
   MouseEvent test_mouse_event_;
@@ -161,24 +173,62 @@ TEST_P(WaylandWindowTest, SetTitle) {
 }
 
 TEST_P(WaylandWindowTest, MaximizeAndRestore) {
-  ScopedWlArray states = InitializeWlArrayWithActivatedState();
+  const auto kNormalBounds = gfx::Rect{0, 0, 500, 300};
+  const auto kMaximizedBounds = gfx::Rect{0, 0, 800, 600};
 
-  EXPECT_CALL(delegate_,
-              OnWindowStateChanged(Eq(PLATFORM_WINDOW_STATE_MAXIMIZED)));
-  AddStateToWlArray(XDG_SURFACE_STATE_MAXIMIZED, states.get());
+  // Make sure the window has normal state initially.
+  EXPECT_CALL(delegate_, OnBoundsChanged(kNormalBounds));
+  window_->SetBounds(kNormalBounds);
+  EXPECT_EQ(PLATFORM_WINDOW_STATE_NORMAL, window_->GetPlatformWindowState());
+  VerifyAndClearExpectations();
 
+  auto active_maximized = MakeStateArray(
+      {XDG_SURFACE_STATE_ACTIVATED, XDG_SURFACE_STATE_MAXIMIZED});
   EXPECT_CALL(*GetXdgSurface(), SetMaximized());
+  EXPECT_CALL(*xdg_surface_, SetWindowGeometry(0, 0, kMaximizedBounds.width(),
+                                               kMaximizedBounds.height()));
+  EXPECT_CALL(delegate_, OnActivationChanged(Eq(true)));
+  EXPECT_CALL(delegate_, OnBoundsChanged(kMaximizedBounds));
+  EXPECT_CALL(delegate_,
+              OnWindowStateChanged(Eq(PLATFORM_WINDOW_STATE_MAXIMIZED)));
   window_->Maximize();
-  SendConfigureEvent(0, 0, 1, states.get());
+  SendConfigureEvent(kMaximizedBounds.width(), kMaximizedBounds.height(), 1,
+                     active_maximized.get());
   Sync();
+  VerifyAndClearExpectations();
 
+  auto inactive_maximized = MakeStateArray({XDG_SURFACE_STATE_MAXIMIZED});
+  EXPECT_CALL(*xdg_surface_, SetWindowGeometry(0, 0, kMaximizedBounds.width(),
+                                               kMaximizedBounds.height()));
+  EXPECT_CALL(delegate_, OnActivationChanged(Eq(false)));
+  EXPECT_CALL(delegate_, OnBoundsChanged(_)).Times(0);
+  SendConfigureEvent(kMaximizedBounds.width(), kMaximizedBounds.height(), 2,
+                     inactive_maximized.get());
+  Sync();
+  EXPECT_FALSE(window_->is_active());
+  VerifyAndClearExpectations();
+
+  EXPECT_CALL(*xdg_surface_, SetWindowGeometry(0, 0, kMaximizedBounds.width(),
+                                               kMaximizedBounds.height()));
+  EXPECT_CALL(delegate_, OnActivationChanged(Eq(true)));
+  EXPECT_CALL(delegate_, OnBoundsChanged(_)).Times(0);
+  SendConfigureEvent(kMaximizedBounds.width(), kMaximizedBounds.height(), 3,
+                     active_maximized.get());
+  Sync();
+  EXPECT_TRUE(window_->is_active());
+  VerifyAndClearExpectations();
+
+  EXPECT_CALL(*xdg_surface_, SetWindowGeometry(0, 0, kNormalBounds.width(),
+                                               kNormalBounds.height()));
   EXPECT_CALL(delegate_,
               OnWindowStateChanged(Eq(PLATFORM_WINDOW_STATE_NORMAL)));
+  EXPECT_CALL(delegate_, OnActivationChanged(_)).Times(0);
+  EXPECT_CALL(delegate_, OnBoundsChanged(kNormalBounds));
   EXPECT_CALL(*GetXdgSurface(), UnsetMaximized());
   window_->Restore();
   // Reinitialize wl_array, which removes previous old states.
-  states = InitializeWlArrayWithActivatedState();
-  SendConfigureEvent(0, 0, 2, states.get());
+  auto active = InitializeWlArrayWithActivatedState();
+  SendConfigureEvent(0, 0, 4, active.get());
   Sync();
 }
 
@@ -287,32 +337,54 @@ TEST_P(WaylandWindowTest, StartWithFullscreen) {
 }
 
 TEST_P(WaylandWindowTest, SetMaximizedFullscreenAndRestore) {
-  ScopedWlArray states = InitializeWlArrayWithActivatedState();
+  const auto kNormalBounds = gfx::Rect{0, 0, 500, 300};
+  const auto kMaximizedBounds = gfx::Rect{0, 0, 800, 600};
 
+  // Make sure the window has normal state initially.
+  EXPECT_CALL(delegate_, OnBoundsChanged(kNormalBounds));
+  window_->SetBounds(kNormalBounds);
+  EXPECT_EQ(PLATFORM_WINDOW_STATE_NORMAL, window_->GetPlatformWindowState());
+  VerifyAndClearExpectations();
+
+  auto active_maximized = MakeStateArray(
+      {XDG_SURFACE_STATE_ACTIVATED, XDG_SURFACE_STATE_MAXIMIZED});
   EXPECT_CALL(*GetXdgSurface(), SetMaximized());
+  EXPECT_CALL(*xdg_surface_, SetWindowGeometry(0, 0, kMaximizedBounds.width(),
+                                               kMaximizedBounds.height()));
+  EXPECT_CALL(delegate_, OnActivationChanged(Eq(true)));
+  EXPECT_CALL(delegate_, OnBoundsChanged(kMaximizedBounds));
   EXPECT_CALL(delegate_,
               OnWindowStateChanged(Eq(PLATFORM_WINDOW_STATE_MAXIMIZED)));
   window_->Maximize();
-  AddStateToWlArray(XDG_SURFACE_STATE_MAXIMIZED, states.get());
-  SendConfigureEvent(0, 0, 2, states.get());
+  SendConfigureEvent(kMaximizedBounds.width(), kMaximizedBounds.height(), 2,
+                     active_maximized.get());
   Sync();
+  VerifyAndClearExpectations();
 
   EXPECT_CALL(*GetXdgSurface(), SetFullscreen());
+  EXPECT_CALL(*xdg_surface_, SetWindowGeometry(0, 0, kMaximizedBounds.width(),
+                                               kMaximizedBounds.height()));
+  EXPECT_CALL(delegate_, OnBoundsChanged(_)).Times(0);
   EXPECT_CALL(delegate_,
               OnWindowStateChanged(Eq(PLATFORM_WINDOW_STATE_FULLSCREEN)));
   window_->ToggleFullscreen();
-  AddStateToWlArray(XDG_SURFACE_STATE_FULLSCREEN, states.get());
-  SendConfigureEvent(0, 0, 3, states.get());
+  AddStateToWlArray(XDG_SURFACE_STATE_FULLSCREEN, active_maximized.get());
+  SendConfigureEvent(kMaximizedBounds.width(), kMaximizedBounds.height(), 3,
+                     active_maximized.get());
   Sync();
+  VerifyAndClearExpectations();
 
+  EXPECT_CALL(*xdg_surface_, SetWindowGeometry(0, 0, kNormalBounds.width(),
+                                               kNormalBounds.height()));
   EXPECT_CALL(*GetXdgSurface(), UnsetFullscreen());
   EXPECT_CALL(*GetXdgSurface(), UnsetMaximized());
+  EXPECT_CALL(delegate_, OnBoundsChanged(kNormalBounds));
   EXPECT_CALL(delegate_,
               OnWindowStateChanged(Eq(PLATFORM_WINDOW_STATE_NORMAL)));
   window_->Restore();
   // Reinitialize wl_array, which removes previous old states.
-  states = InitializeWlArrayWithActivatedState();
-  SendConfigureEvent(0, 0, 4, states.get());
+  auto active = InitializeWlArrayWithActivatedState();
+  SendConfigureEvent(0, 0, 4, active.get());
   Sync();
 }