Skip callback once ShellSurface is being destroyed

Explicitly skip EndDrag if the end reason is "window is being destroyed".

Bug: 1151072
Test: covered by unittest

Change-Id: I4310f21993d938ee560d13f7a51755a41c375824
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2570315
Commit-Queue: Mitsuru Oshima <oshima@chromium.org>
Reviewed-by: Malay Keshav <malaykeshav@chromium.org>
Cr-Commit-Position: refs/heads/master@{#834463}

components/exo/shell_surface.cc

@@ -106,6 +106,8 @@ ShellSurface::ShellSurface(Surface* surface)
 
 ShellSurface::~ShellSurface() {
   DCHECK(!scoped_configure_);
+  // Client is gone by now, so don't call callbask.
+  configure_callback_.Reset();
   if (widget_)
     ash::WindowState::Get(widget_->GetNativeWindow())->RemoveObserver(this);
 }
@@ -551,8 +553,8 @@ void ShellSurface::Configure(bool ends_drag) {
   // If surface is being resized, save the resize direction.
   if (window_state && window_state->is_dragged() && !ends_drag)
     resize_component = window_state->drag_details()->window_component;
-
   uint32_t serial = 0;
+
   if (!configure_callback_.is_null()) {
     if (window_state) {
       serial = configure_callback_.Run(
@@ -604,7 +606,8 @@ void ShellSurface::AttemptToStartDrag(int component) {
   }
   auto end_drag = [](ShellSurface* shell_surface,
                      ash::ToplevelWindowEventHandler::DragResult result) {
-    shell_surface->EndDrag();
+    if (result != ash::ToplevelWindowEventHandler::DragResult::WINDOW_DESTROYED)
+      shell_surface->EndDrag();
   };
 
   if (gesture_target) {

components/exo/shell_surface_base.cc

@@ -329,6 +329,12 @@ ShellSurfaceBase::~ShellSurfaceBase() {
   // Remove activation observer before hiding widget to prevent it from
   // casuing the configure callback to be called.
   WMHelper::GetInstance()->RemoveActivationObserver(this);
+
+  // Client is gone by now, so don't call callbacks.
+  close_callback_.Reset();
+  pre_close_callback_.Reset();
+  surface_destroyed_callback_.Reset();
+
   if (widget_) {
     widget_->GetNativeWindow()->RemoveObserver(this);
     widget_->RemoveObserver(this);

components/exo/shell_surface_unittest.cc

@@ -17,6 +17,7 @@
 #include "base/callback.h"
 #include "base/strings/stringprintf.h"
 #include "base/strings/utf_string_conversions.h"
+#include "base/test/bind.h"
 #include "components/exo/buffer.h"
 #include "components/exo/permission.h"
 #include "components/exo/shell_surface_util.h"
@@ -472,11 +473,17 @@ TEST_F(ShellSurfaceTest, SetStartupId) {
 }
 
 TEST_F(ShellSurfaceTest, StartMove) {
+  // TODO: Ractor out the shell surface creation.
+  gfx::Size buffer_size(64, 64);
+  std::unique_ptr<Buffer> buffer(
+      new Buffer(exo_test_helper()->CreateGpuMemoryBuffer(buffer_size)));
   std::unique_ptr<Surface> surface(new Surface);
   std::unique_ptr<ShellSurface> shell_surface(new ShellSurface(surface.get()));
 
   // Map shell surface.
+  surface->Attach(buffer.get());
   surface->Commit();
+  ASSERT_TRUE(shell_surface->GetWidget());
 
   // The interactive move should end when surface is destroyed.
   shell_surface->StartMove();
@@ -486,11 +493,16 @@ TEST_F(ShellSurfaceTest, StartMove) {
 }
 
 TEST_F(ShellSurfaceTest, StartResize) {
+  gfx::Size buffer_size(64, 64);
+  std::unique_ptr<Buffer> buffer(
+      new Buffer(exo_test_helper()->CreateGpuMemoryBuffer(buffer_size)));
   std::unique_ptr<Surface> surface(new Surface);
   std::unique_ptr<ShellSurface> shell_surface(new ShellSurface(surface.get()));
 
   // Map shell surface.
+  surface->Attach(buffer.get());
   surface->Commit();
+  ASSERT_TRUE(shell_surface->GetWidget());
 
   // The interactive resize should end when surface is destroyed.
   shell_surface->StartResize(HTBOTTOMRIGHT);
@@ -499,6 +511,45 @@ TEST_F(ShellSurfaceTest, StartResize) {
   surface.reset();
 }
 
+TEST_F(ShellSurfaceTest, StartResizeAndDestroyShell) {
+  gfx::Size buffer_size(64, 64);
+  std::unique_ptr<Buffer> buffer(
+      new Buffer(exo_test_helper()->CreateGpuMemoryBuffer(buffer_size)));
+  std::unique_ptr<Surface> surface(new Surface);
+  std::unique_ptr<ShellSurface> shell_surface(new ShellSurface(surface.get()));
+
+  uint32_t serial = 0;
+  auto configure_callback = base::BindRepeating(
+      [](uint32_t* const serial_ptr, const gfx::Size& size,
+         chromeos::WindowStateType state_type, bool resizing, bool activated,
+         const gfx::Vector2d& origin_offset) { return ++(*serial_ptr); },
+      &serial);
+
+  // Map shell surface.
+  surface->Attach(buffer.get());
+  shell_surface->set_configure_callback(configure_callback);
+
+  surface->Commit();
+  ASSERT_TRUE(shell_surface->GetWidget());
+
+  // The interactive resize should end when surface is destroyed.
+  shell_surface->StartResize(HTBOTTOMRIGHT);
+
+  // Go through configure/commit stage to update the resize component.
+  shell_surface->AcknowledgeConfigure(serial);
+  surface->Commit();
+
+  shell_surface->set_configure_callback(base::BindRepeating(
+      [](const gfx::Size& size, chromeos::WindowStateType state_type,
+         bool resizing, bool activated, const gfx::Vector2d& origin_offset) {
+        ADD_FAILURE() << "Configure Should not be called";
+        return uint32_t{0};
+      }));
+
+  // Test that destroying the surface before resize ends is OK.
+  shell_surface.reset();
+}
+
 TEST_F(ShellSurfaceTest, SetGeometry) {
   gfx::Size buffer_size(64, 64);
   std::unique_ptr<Buffer> buffer(