[ozone/wayland] Relax condition to call PlatformWindowDelegate::OnWindowStateChanged()

The way the logic is currently implemented prevents the delegate from
being notified of state changes most of the time. This CL relaxes the
sanity checks in  WaylandToplevelWindow::HandleSurfaceConfigure(),
causing DesktopWTHPlatform::OnWindowStateChanged() to get called
more frequently, but holding now the correct state.

This will allow DesktopWTHPlatform to react on state changes more
reliably (eg lacros' immersive fullscreen behavior).

Last, this CL changes the logic that sets and unsets fullscreen.
Previously, UnSetFullscreen() was being called
for all operations different than "enter fullscreen".
This is needless and might mess compositors up.

TEST=ozone_unittests adapted accordingly.

BUG=1113900
R=msisov@igalia.com, nickdiego@igalia.com

Change-Id: Iddffaf5d4f0e20898db0016320935904b9816e45
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2601219
Auto-Submit: Antonio Gomes <tonikitoo@igalia.com>
Reviewed-by: Maksim Sisov <msisov@igalia.com>
Reviewed-by: Nick Yamane <nickdiego@igalia.com>
Commit-Queue: Antonio Gomes <tonikitoo@igalia.com>
Cr-Commit-Position: refs/heads/master@{#840125}

ui/ozone/platform/wayland/host/wayland_toplevel_window.cc

@@ -267,7 +267,6 @@ void WaylandToplevelWindow::HandleSurfaceConfigure(int32_t width,
                                                    bool is_activated) {
   // Store the old state to propagte state changes if Wayland decides to change
   // the state to something else.
-  PlatformWindowState old_state = state_;
   if (state_ == PlatformWindowState::kMinimized && !is_activated) {
     state_ = PlatformWindowState::kMinimized;
   } else if (is_fullscreen) {
@@ -278,7 +277,6 @@ void WaylandToplevelWindow::HandleSurfaceConfigure(int32_t width,
     state_ = PlatformWindowState::kNormal;
   }
 
-  const bool state_changed = old_state != state_;
   const bool is_normal = state_ == PlatformWindowState::kNormal;
 
   // Update state before notifying delegate.
@@ -316,8 +314,8 @@ void WaylandToplevelWindow::HandleSurfaceConfigure(int32_t width,
   SetOrResetRestoredBounds();
   ApplyPendingBounds();
 
-  if (state_changed)
-    delegate()->OnWindowStateChanged(state_);
+  // Notify the delegate about the latest state set.
+  delegate()->OnWindowStateChanged(state_);
 
   if (did_active_change)
     delegate()->OnActivationChanged(is_active_);
@@ -426,22 +424,21 @@ void WaylandToplevelWindow::TriggerStateChanges() {
   if (!shell_surface_)
     return;
 
-  if (state_ == PlatformWindowState::kFullScreen)
-    shell_surface_->SetFullscreen();
-  else
-    shell_surface_->UnSetFullscreen();
-
   // Call UnSetMaximized only if current state is normal. Otherwise, if the
   // current state is fullscreen and the previous is maximized, calling
   // UnSetMaximized may result in wrong restored window position that clients
   // are not allowed to know about.
-  if (state_ == PlatformWindowState::kMaximized)
+  if (state_ == PlatformWindowState::kMinimized) {
+    shell_surface_->SetMinimized();
+  } else if (state_ == PlatformWindowState::kFullScreen) {
+    shell_surface_->SetFullscreen();
+  } else if (previous_state_ == PlatformWindowState::kFullScreen) {
+    shell_surface_->UnSetFullscreen();
+  } else if (state_ == PlatformWindowState::kMaximized) {
     shell_surface_->SetMaximized();
-  else if (state_ == PlatformWindowState::kNormal)
+  } else if (state_ == PlatformWindowState::kNormal) {
     shell_surface_->UnSetMaximized();
-
-  if (state_ == PlatformWindowState::kMinimized)
-    shell_surface_->SetMinimized();
+  }
 
   connection()->ScheduleFlush();
 }

ui/ozone/platform/wayland/host/wayland_window_unittest.cc

@@ -46,6 +46,7 @@
 
 using ::testing::_;
 using ::testing::Eq;
+using ::testing::InvokeWithoutArgs;
 using ::testing::Mock;
 using ::testing::Return;
 using ::testing::SaveArg;
@@ -296,7 +297,7 @@ TEST_P(WaylandWindowTest, MaximizeAndRestore) {
                                                kMaximizedBounds.height()));
   EXPECT_CALL(delegate_, OnActivationChanged(Eq(true)));
   EXPECT_CALL(delegate_, OnBoundsChanged(kMaximizedBounds));
-  EXPECT_CALL(delegate_, OnWindowStateChanged(_)).Times(0);
+  EXPECT_CALL(delegate_, OnWindowStateChanged(_)).Times(1);
   window_->Maximize();
   SendConfigureEvent(xdg_surface_, kMaximizedBounds.width(),
                      kMaximizedBounds.height(), ++serial,
@@ -327,7 +328,7 @@ TEST_P(WaylandWindowTest, MaximizeAndRestore) {
 
   EXPECT_CALL(*xdg_surface_, SetWindowGeometry(0, 0, kNormalBounds.width(),
                                                kNormalBounds.height()));
-  EXPECT_CALL(delegate_, OnWindowStateChanged(_)).Times(0);
+  EXPECT_CALL(delegate_, OnWindowStateChanged(_)).Times(1);
   EXPECT_CALL(delegate_, OnActivationChanged(_)).Times(0);
   EXPECT_CALL(delegate_, OnBoundsChanged(kNormalBounds));
   EXPECT_CALL(*GetXdgToplevel(), UnsetMaximized());
@@ -347,7 +348,7 @@ TEST_P(WaylandWindowTest, Minimize) {
   Sync();
 
   EXPECT_CALL(*GetXdgToplevel(), SetMinimized());
-  EXPECT_CALL(delegate_, OnWindowStateChanged(_)).Times(0);
+  EXPECT_CALL(delegate_, OnWindowStateChanged(_)).Times(1);
   window_->Minimize();
   EXPECT_EQ(window_->GetPlatformWindowState(), PlatformWindowState::kMinimized);
 
@@ -364,8 +365,14 @@ TEST_P(WaylandWindowTest, Minimize) {
 
   // Send one additional empty configuration event (which means the surface is
   // not maximized, fullscreen or activated) to ensure, WaylandWindow stays in
-  // the same minimized state and doesn't notify its delegate.
-  EXPECT_CALL(delegate_, OnWindowStateChanged(_)).Times(0);
+  // the same minimized state, but the delegate is always notified.
+  //
+  // TODO(tonikito): Improve filtering of delegate notification here.
+  ui::PlatformWindowState state;
+  EXPECT_CALL(delegate_, OnWindowStateChanged(_))
+      .WillRepeatedly(DoAll(SaveArg<0>(&state), InvokeWithoutArgs([&]() {
+                              EXPECT_EQ(state, PlatformWindowState::kMinimized);
+                            })));
   SendConfigureEvent(xdg_surface_, 0, 0, 3, states.get());
   Sync();
 
@@ -385,7 +392,7 @@ TEST_P(WaylandWindowTest, SetFullscreenAndRestore) {
   AddStateToWlArray(XDG_TOPLEVEL_STATE_FULLSCREEN, states.get());
 
   EXPECT_CALL(*GetXdgToplevel(), SetFullscreen());
-  EXPECT_CALL(delegate_, OnWindowStateChanged(_)).Times(0);
+  EXPECT_CALL(delegate_, OnWindowStateChanged(_)).Times(1);
   window_->ToggleFullscreen();
   // Make sure than WaylandWindow manually handles fullscreen states. Check the
   // comment in the WaylandWindow::ToggleFullscreen.
@@ -395,7 +402,7 @@ TEST_P(WaylandWindowTest, SetFullscreenAndRestore) {
   Sync();
 
   EXPECT_CALL(*GetXdgToplevel(), UnsetFullscreen());
-  EXPECT_CALL(delegate_, OnWindowStateChanged(_)).Times(0);
+  EXPECT_CALL(delegate_, OnWindowStateChanged(_)).Times(1);
   window_->Restore();
   EXPECT_EQ(window_->GetPlatformWindowState(), PlatformWindowState::kNormal);
   // Reinitialize wl_array, which removes previous old states.
@@ -481,9 +488,8 @@ TEST_P(WaylandWindowTest, StartMaximized) {
 
   Sync();
 
-  // Once the surface will be activated, the window state mustn't be changed
-  // and retain the same.
-  EXPECT_CALL(delegate_, OnWindowStateChanged(_)).Times(0);
+  // Once the surface will be activated, the window state gets updated.
+  EXPECT_CALL(delegate_, OnWindowStateChanged(_)).Times(1);
   EXPECT_EQ(window_->GetPlatformWindowState(), PlatformWindowState::kMaximized);
 
   // Activate the surface.
@@ -599,7 +605,7 @@ TEST_P(WaylandWindowTest, SetMaximizedFullscreenAndRestore) {
                                                kMaximizedBounds.height()));
   EXPECT_CALL(delegate_, OnActivationChanged(Eq(true)));
   EXPECT_CALL(delegate_, OnBoundsChanged(kMaximizedBounds));
-  EXPECT_CALL(delegate_, OnWindowStateChanged(_)).Times(0);
+  EXPECT_CALL(delegate_, OnWindowStateChanged(_)).Times(1);
   window_->Maximize();
   // State changes are synchronous.
   EXPECT_EQ(PlatformWindowState::kMaximized, window_->GetPlatformWindowState());
@@ -615,7 +621,7 @@ TEST_P(WaylandWindowTest, SetMaximizedFullscreenAndRestore) {
   EXPECT_CALL(*xdg_surface_, SetWindowGeometry(0, 0, kMaximizedBounds.width(),
                                                kMaximizedBounds.height()));
   EXPECT_CALL(delegate_, OnBoundsChanged(_)).Times(0);
-  EXPECT_CALL(delegate_, OnWindowStateChanged(_)).Times(0);
+  EXPECT_CALL(delegate_, OnWindowStateChanged(_)).Times(1);
   window_->ToggleFullscreen();
   // State changes are synchronous.
   EXPECT_EQ(PlatformWindowState::kFullScreen,
@@ -633,9 +639,8 @@ TEST_P(WaylandWindowTest, SetMaximizedFullscreenAndRestore) {
   EXPECT_CALL(*xdg_surface_, SetWindowGeometry(0, 0, kNormalBounds.width(),
                                                kNormalBounds.height()));
   EXPECT_CALL(*GetXdgToplevel(), UnsetFullscreen());
-  EXPECT_CALL(*GetXdgToplevel(), UnsetMaximized());
   EXPECT_CALL(delegate_, OnBoundsChanged(kNormalBounds));
-  EXPECT_CALL(delegate_, OnWindowStateChanged(_)).Times(0);
+  EXPECT_CALL(delegate_, OnWindowStateChanged(_)).Times(1);
   window_->Restore();
   EXPECT_EQ(PlatformWindowState::kNormal, window_->GetPlatformWindowState());
   // Reinitialize wl_array, which removes previous old states.