Reintroduce OnWindowVisibilityChanging() hook in exo::ExtendedDragSource

It turns out that exo::ExtendedDragSource class can benefit from
overriding both OnWindowVisibilityChanging() and
OnWindowVisibilityChanged() hooks.

In [1], the use of OnWindowVisibilityChanging() hook was changed by
OnWindowVisibilityChanged(), since according to the existing Ash's
drag'n drop logic, by the time
ash::ToplevelWindowEventHandler::AttemptToStartDrag() is called
the window state should be up to date, something that occurs in
ash::TabletModeWindowManager::OnWindowVisibilityChanged().

However, it turns how that Lacros needs to set ash::kIsDraggingTabsKey
property as soon as possible, so that undesirable window resizes don't
take place during the drag'n drop beginning - causing the window to
flicker.

This CL reintroduces OnWindowVisibilityChanging() hook, and
moves some of the logic in OnWindowVisibilityChanged() into it,
In practice, it avoids Lacros from flicking when starting a drag'n
drop (see stack trace below)

[1] https://crrev.com/c/3254858

 #9 0x7f8f4dda154a ash::TabletModeWindowState::UpdateBounds() <-- calls aura::Window::SetBounds()
 #10 0x7f8f4dda0e0e ash::TabletModeWindowState::UpdateWindow()
 #11 0x7f8f4dda16ef ash::TabletModeWindowState::AttachState()
 #12 0x7f8f4ddcd304 ash::WindowState::SetStateObject()
 #13 0x7f8f4dda03b1 ash::TabletModeWindowState::TabletModeWindowState()
 #14 0x7f8f4dd98830 ash::TabletModeWindowManager::TrackWindow()
 #15 0x7f8f4dd995a7 ash::TabletModeWindowManager::OnWindowVisibilityChanged()
 #16 0x7f8f4ff664fc aura::Window::NotifyWindowVisibilityChangedAtReceiver()
 #17 0x7f8f4ff661c6 aura::Window::NotifyWindowVisibilityChangedDown()
 #18 0x7f8f4ff652d7 aura::Window::NotifyWindowVisibilityChanged()
 #19 0x7f8f4ff6002a aura::Window::SetVisibleInternal()
 #20 0x7f8f4ff5fcaa aura::Window::Show()
 #21 0x7f8f4f371b3b views::NativeWidgetAura::Show()
 #22 0x7f8f4f3260b8 views::Widget::Show()
 #23 0x55d01acb6fd6 exo::ShellSurfaceBase::CommitWidget()
 #24 0x55d01acb6b78 exo::ShellSurfaceBase::OnSurfaceCommit()
 #25 0x55d01ac7d90d exo::Surface::Commit()
 (..)

BUG=1252941
R=oshima@chromium.org

Change-Id: Ib6ba73d3bb2a90a3ac180c974f292bec39c1b3d4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3365554
Reviewed-by: Mitsuru Oshima <oshima@chromium.org>
Commit-Queue: Antonio Gomes <tonikitoo@igalia.com>
Cr-Commit-Position: refs/heads/main@{#955327}

components/exo/extended_drag_source.cc

@@ -86,6 +86,12 @@ class ExtendedDragSource::DraggedWindowHolder : public aura::WindowObserver {
     surface_->window()->RemoveObserver(this);
   }
 
+  void OnWindowVisibilityChanging(aura::Window* window, bool visible) override {
+    DCHECK(window);
+    if (window == toplevel_window_)
+      source_->OnDraggedWindowVisibilityChanging(visible);
+  }
+
   void OnWindowVisibilityChanged(aura::Window* window, bool visible) override {
     DCHECK(window);
     if (window == toplevel_window_)
@@ -279,9 +285,9 @@ void ExtendedDragSource::StartDrag(aura::Window* toplevel,
                                        /*grab_capture=*/false);
 }
 
-void ExtendedDragSource::OnDraggedWindowVisibilityChanged(bool visible) {
+void ExtendedDragSource::OnDraggedWindowVisibilityChanging(bool visible) {
   DCHECK(dragged_window_holder_);
-  DVLOG(1) << "Dragged window visibility changed. visible=" << visible;
+  DVLOG(1) << "Dragged window visibility changing. visible=" << visible;
 
   if (!visible) {
     dragged_window_holder_.reset();
@@ -297,6 +303,20 @@ void ExtendedDragSource::OnDraggedWindowVisibilityChanged(bool visible) {
     toplevel->SetProperty(ash::kTabDraggingSourceWindowKey,
                           drag_source_window_);
   }
+}
+
+void ExtendedDragSource::OnDraggedWindowVisibilityChanged(bool visible) {
+  DCHECK(dragged_window_holder_);
+  DVLOG(1) << "Dragged window visibility changed. visible=" << visible;
+
+  if (!visible) {
+    dragged_window_holder_.reset();
+    return;
+  }
+
+  aura::Window* toplevel = dragged_window_holder_->toplevel_window();
+  DCHECK(toplevel);
+  DCHECK(drag_source_window_);
 
   // The |toplevel| window for the dragged surface has just been created and
   // it's about to be mapped. Calculate and set its position based on

components/exo/extended_drag_source.h

@@ -94,6 +94,7 @@ class ExtendedDragSource : public DataSourceObserver,
   void UnlockCursor();
   void StartDrag(aura::Window* toplevel,
                  const gfx::PointF& pointer_location_in_screen);
+  void OnDraggedWindowVisibilityChanging(bool visible);
   void OnDraggedWindowVisibilityChanged(bool visible);
   gfx::Point CalculateOrigin(aura::Window* target) const;
   void Cleanup();

components/exo/extended_drag_source_unittest.cc

@@ -9,6 +9,7 @@
 
 #include "ash/drag_drop/drag_drop_controller.h"
 #include "ash/drag_drop/toplevel_window_drag_delegate.h"
+#include "ash/public/cpp/window_properties.h"
 #include "ash/shell.h"
 #include "ash/wm/toplevel_window_event_handler.h"
 #include "base/strings/utf_string_conversions.h"
@@ -40,7 +41,9 @@
 #include "ui/gfx/geometry/vector2d.h"
 
 using ::testing::_;
+using ::testing::DoAll;
 using ::testing::InvokeWithoutArgs;
+using ::testing::SaveArg;
 
 namespace exo {
 namespace {
@@ -308,14 +311,17 @@ TEST_F(ExtendedDragSourceTest, DragSurfaceNotMappedYet) {
 
   // Ensure drag 'n drop starts after
   // ExtendedDragSource::OnDraggedWindowVisibilityChanged()
+  aura::Window* toplevel_window;
   WindowObserverHookChecker checker(detached_surface->window());
   EXPECT_CALL(checker, OnWindowVisibilityChanging(_, _))
       .Times(1)
-      .WillOnce(InvokeWithoutArgs([]() {
-        auto* toplevel_handler =
-            ash::Shell::Get()->toplevel_window_event_handler();
-        EXPECT_FALSE(toplevel_handler->is_drag_in_progress());
-      }));
+      .WillOnce(DoAll(
+          SaveArg<0>(&toplevel_window), InvokeWithoutArgs([&]() {
+            auto* toplevel_handler =
+                ash::Shell::Get()->toplevel_window_event_handler();
+            EXPECT_FALSE(toplevel_handler->is_drag_in_progress());
+            EXPECT_TRUE(toplevel_window->GetProperty(ash::kIsDraggingTabsKey));
+          })));
   EXPECT_CALL(checker, OnWindowVisibilityChanged(_, _))
       .Times(1)
       .WillOnce(InvokeWithoutArgs([]() {