snap-group: Refresh visuals on window destruction

This cl implements the logic to refresh the visuals of the cycling
item on window destruction. If the destroying window belongs to
a snap group, the rounded corners of the cycling view that represents
window needs to be updated so that it is consistent with single
window cycle item in window cycle view.

Fixed: b/297079188
Demo: http://b/297079188#comment2
Test: Added unit tests + manual
Change-Id: I3a20eb5e74fb825b8e6f19840c077f640f33c6ec
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4811157
Reviewed-by: Sammie Quon <sammiequon@chromium.org>
Commit-Queue: Michele Fan <michelefan@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1188435}

ash/wm/snap_group/snap_group_unittest.cc

@@ -33,6 +33,7 @@
 #include "ash/wm/window_cycle/window_cycle_controller.h"
 #include "ash/wm/window_cycle/window_cycle_list.h"
 #include "ash/wm/window_cycle/window_cycle_view.h"
+#include "ash/wm/window_mini_view.h"
 #include "ash/wm/window_state.h"
 #include "ash/wm/window_util.h"
 #include "ash/wm/wm_event.h"
@@ -45,14 +46,17 @@
 #include "base/test/metrics/histogram_tester.h"
 #include "base/test/scoped_feature_list.h"
 #include "base/timer/timer.h"
+#include "chromeos/constants/chromeos_features.h"
 #include "chromeos/ui/base/window_state_type.h"
 #include "ui/aura/test/test_window_delegate.h"
 #include "ui/base/cursor/mojom/cursor_type.mojom-shared.h"
 #include "ui/base/hit_test.h"
 #include "ui/base/l10n/l10n_util.h"
+#include "ui/compositor/layer.h"
 #include "ui/events/keycodes/keyboard_codes_posix.h"
 #include "ui/gfx/geometry/point.h"
 #include "ui/gfx/geometry/rect.h"
+#include "ui/gfx/geometry/rounded_corners_f.h"
 #include "ui/gfx/geometry/vector2d.h"
 #include "ui/gfx/image/image_skia.h"
 #include "ui/gfx/image/image_unittest_util.h"
@@ -68,6 +72,8 @@ using ::ui::mojom::CursorType;
 
 using WindowCyclingDirection = WindowCycleController::WindowCyclingDirection;
 
+constexpr int kWindowMiniViewCornerRadius = 16;
+
 SplitViewController* split_view_controller() {
   return SplitViewController::Get(Shell::GetPrimaryRootWindow());
 }
@@ -164,8 +170,11 @@ class SnapGroupTest : public AshTestBase {
     // TODO(michelefan@): Change it back to
     // `scoped_feature_list_.InitAndEnableFeature(features::kSnapGroup)` when
     // do the refactor work for the snap group unit tests.
-    scoped_feature_list_.InitAndEnableFeatureWithParameters(
-        features::kSnapGroup, {{"AutomaticLockGroup", "false"}});
+    scoped_feature_list_.InitWithFeaturesAndParameters(
+        {{features::kSnapGroup, {{"AutomaticLockGroup", "false"}}},
+         {chromeos::features::kJellyroll, {}},
+         {chromeos::features::kJelly, {}}},
+        {});
   }
   SnapGroupTest(const SnapGroupTest&) = delete;
   SnapGroupTest& operator=(const SnapGroupTest&) = delete;
@@ -1271,6 +1280,45 @@ TEST_F(SnapGroupEntryPointArm1Test, SteppingInWindowCycleView) {
   EXPECT_TRUE(wm::IsActiveWindow(window2.get()));
 }
 
+// Tests that the exposed rounded corners of the cycling items are rounded
+// corners. The visuals will be refreshed on window destruction that belongs to
+// a snap group.
+TEST_F(SnapGroupEntryPointArm1Test, WindowCycleRoundedCorners) {
+  std::unique_ptr<aura::Window> window0 =
+      CreateAppWindow(gfx::Rect(100, 200), AppType::BROWSER);
+  std::unique_ptr<aura::Window> window1 =
+      CreateAppWindow(gfx::Rect(200, 300), AppType::BROWSER);
+  std::unique_ptr<aura::Window> window2 =
+      CreateAppWindow(gfx::Rect(300, 400), AppType::BROWSER);
+  SnapTwoTestWindowsInArm1(window0.get(), window1.get());
+
+  WindowCycleController* window_cycle_controller =
+      Shell::Get()->window_cycle_controller();
+  CycleWindow(WindowCyclingDirection::kForward, /*steps=*/3);
+  EXPECT_TRUE(window_cycle_controller->IsCycling());
+  const auto* window_cycle_list = window_cycle_controller->window_cycle_list();
+  const auto* cycle_view = window_cycle_list->cycle_view();
+  auto& cycle_item_views = cycle_view->cycle_views_for_testing();
+  ASSERT_EQ(cycle_item_views.size(), 2u);
+  for (auto* cycle_item_view : cycle_item_views) {
+    EXPECT_EQ(cycle_item_view->GetRoundedCorners(),
+              gfx::RoundedCornersF(kWindowMiniViewCornerRadius));
+  }
+
+  // Destroy `window0` which belongs to a snap group.
+  window0.reset();
+  auto& new_cycle_item_views = cycle_view->cycle_views_for_testing();
+  EXPECT_EQ(new_cycle_item_views.size(), 2u);
+
+  // Verify that the visuals of the cycling items will be refreshed so that the
+  // exposed corners will be rounded corners.
+  for (auto* cycle_item_view : new_cycle_item_views) {
+    EXPECT_EQ(cycle_item_view->GetRoundedCorners(),
+              gfx::RoundedCornersF(kWindowMiniViewCornerRadius));
+  }
+  CompleteWindowCycling();
+}
+
 // Tests that after creating a snap group in clamshell, transition to tablet
 // mode won't crash (b/288179725).
 TEST_F(SnapGroupEntryPointArm1Test, NoCrashWhenRemovingGroupInTabletMode) {

ash/wm/window_cycle/window_cycle_item_view.cc

@@ -17,6 +17,7 @@
 #include "ui/base/metadata/metadata_impl_macros.h"
 #include "ui/compositor/layer.h"
 #include "ui/gfx/geometry/insets.h"
+#include "ui/gfx/geometry/rounded_corners_f.h"
 #include "ui/views/layout/box_layout.h"
 #include "ui/views/view.h"
 
@@ -134,10 +135,9 @@ bool WindowCycleItemView::HandleAccessibleAction(
 }
 
 void WindowCycleItemView::RefreshItemVisuals() {
-  CHECK(!preview_view());
-
   header_view()->UpdateIconView(source_window());
   SetShowPreview(/*show=*/true);
+  UpdateHeaderViewRoundedCorners();
   UpdatePreviewRoundedCorners(/*show=*/true);
 }
 
@@ -205,6 +205,8 @@ int GroupContainerCycleView::TryRemovingChildItem(
     }
   }
 
+  RefreshItemVisuals();
+
   return base::ranges::count_if(
       mini_views_ptrs,
       [](raw_ptr<WindowCycleItemView>* ptr) { return *ptr != nullptr; });
@@ -218,6 +220,32 @@ void GroupContainerCycleView::GetAccessibleNodeData(ui::AXNodeData* node_data) {
   node_data->SetName(u"Group container view");
 }
 
+gfx::RoundedCornersF GroupContainerCycleView::GetRoundedCorners() const {
+  if (!mini_view1_ && !mini_view2_) {
+    return gfx::RoundedCornersF();
+  }
+
+  // In normal use cases, the left corners (`upper_left` and `lower_left`) will
+  // depend on the primary snapped window, and likewise for the right corners.
+  // However, if one window gets destructed leaving only one mini view hosted by
+  // this. All the rounded corners have to be from the remaining mini view.
+  // TODO(b/294294344): for vertical split view, the upper corners will depend
+  // on the primary snapped window and likewise for the lower corners.
+  const float upper_left = mini_view1_
+                               ? mini_view1_->GetRoundedCorners().upper_left()
+                               : mini_view2_->GetRoundedCorners().upper_left();
+  const float upper_right =
+      mini_view2_ ? mini_view2_->GetRoundedCorners().upper_right()
+                  : mini_view1_->GetRoundedCorners().upper_right();
+  const float lower_right = mini_view2_
+                                ? mini_view2_->GetRoundedCorners().lower_right()
+                                : mini_view1_->GetRoundedCorners().lower_left();
+  const float lower_left = mini_view1_
+                               ? mini_view1_->GetRoundedCorners().lower_left()
+                               : mini_view2_->GetRoundedCorners().lower_right();
+  return gfx::RoundedCornersF(upper_left, upper_right, lower_right, lower_left);
+}
+
 BEGIN_METADATA(GroupContainerCycleView, WindowMiniViewBase)
 END_METADATA
 

ash/wm/window_cycle/window_cycle_item_view.h

@@ -66,6 +66,7 @@ class GroupContainerCycleView : public WindowMiniViewBase {
   aura::Window* GetWindowAtPoint(const gfx::Point& screen_point) const override;
   void RefreshItemVisuals() override;
   int TryRemovingChildItem(aura::Window* destroying_window) override;
+  gfx::RoundedCornersF GetRoundedCorners() const override;
 
   // views::View:
   void GetAccessibleNodeData(ui::AXNodeData* node_data) override;

ash/wm/window_cycle/window_cycle_view.h

@@ -137,6 +137,10 @@ class ASH_EXPORT WindowCycleView : public views::WidgetDelegateView,
     return mirror_container_;
   }
 
+  const std::vector<WindowMiniViewBase*>& cycle_views_for_testing() const {
+    return cycle_views_;
+  }
+
  private:
   friend class WindowCycleListTestApi;
 

ash/wm/window_mini_view.cc

@@ -198,6 +198,14 @@ void WindowMiniView::UpdatePreviewRoundedCorners(bool show) {
   layer->SetIsFastRoundedCorner(true);
 }
 
+void WindowMiniView::UpdateHeaderViewRoundedCorners() {
+  if (!header_view_) {
+    return;
+  }
+
+  header_view_->RefreshHeaderViewRoundedCorners();
+}
+
 bool WindowMiniView::Contains(aura::Window* window) const {
   return source_window_ == window;
 }
@@ -211,6 +219,20 @@ int WindowMiniView::TryRemovingChildItem(aura::Window* destroying_window) {
   return 0;
 }
 
+gfx::RoundedCornersF WindowMiniView::GetRoundedCorners() const {
+  if (!header_view_ || !preview_view_) {
+    return gfx::RoundedCornersF();
+  }
+
+  auto header_rounded_corners =
+      header_view_->GetHeaderRoundedCorners(source_window_);
+  auto preview_rounded_corners = preview_view_->layer()->rounded_corner_radii();
+  return gfx::RoundedCornersF(header_rounded_corners.upper_left(),
+                              header_rounded_corners.upper_right(),
+                              preview_rounded_corners.lower_right(),
+                              preview_rounded_corners.lower_left());
+}
+
 gfx::Rect WindowMiniView::GetHeaderBounds() const {
   gfx::Rect header_bounds = GetContentsBounds();
   header_bounds.set_height(kHeaderHeightDp);

ash/wm/window_mini_view.h

@@ -8,13 +8,17 @@
 #include "ash/ash_export.h"
 #include "base/memory/raw_ptr.h"
 #include "base/scoped_observation.h"
-#include "ui/aura/window.h"
 #include "ui/aura/window_observer.h"
 #include "ui/base/metadata/metadata_header_macros.h"
 #include "ui/views/view.h"
 
+namespace aura {
+class Window;
+}  // namespace aura
+
 namespace gfx {
 class Point;
+class RoundedCornersF;
 }  // namespace gfx
 
 namespace views {
@@ -36,6 +40,9 @@ class WindowMiniViewBase : public views::View {
   WindowMiniViewBase& operator=(const WindowMiniViewBase&) = delete;
   ~WindowMiniViewBase() override;
 
+  // Shows or hides a focus ring around this.
+  void UpdateFocusState(bool focus);
+
   // Returns true if a preview of the given `window` is contained in `this`.
   virtual bool Contains(aura::Window* window) const = 0;
 
@@ -55,8 +62,8 @@ class WindowMiniViewBase : public views::View {
   // rather than a child item.
   virtual int TryRemovingChildItem(aura::Window* destroying_window) = 0;
 
-  // Shows or hides a focus ring around this.
-  void UpdateFocusState(bool focus);
+  // Returns the exposed rounded corners.
+  virtual gfx::RoundedCornersF GetRoundedCorners() const = 0;
 
  protected:
   WindowMiniViewBase();
@@ -106,13 +113,17 @@ class ASH_EXPORT WindowMiniView : public WindowMiniViewBase,
   // Creates or deletes |preview_view_| as needed.
   void SetShowPreview(bool show);
 
-  // Sets or hides rounded corners on |preview_view_|, if it exists.
+  // Sets or hides rounded corners on `preview_view_`, if it exists.
   void UpdatePreviewRoundedCorners(bool show);
 
+  // Updates the rounded corners on `header_view_`, if it exists.
+  void UpdateHeaderViewRoundedCorners();
+
   // WindowMiniViewBase:
   bool Contains(aura::Window* window) const override;
   aura::Window* GetWindowAtPoint(const gfx::Point& screen_point) const override;
   int TryRemovingChildItem(aura::Window* destroying_window) override;
+  gfx::RoundedCornersF GetRoundedCorners() const override;
 
  protected:
   explicit WindowMiniView(aura::Window* source_window);

ash/wm/window_mini_view_header_view.cc

@@ -53,29 +53,6 @@ std::u16string GetWindowTitle(aura::Window* window) {
              : transient_root->GetTitle();
 }
 
-gfx::RoundedCornersF GetHeaderRoundedCorners(aura::Window* window) {
-  const float scale = window->layer()->GetTargetTransform().To2dScale().x();
-  const float scaled_corner_radius = kHeaderTopCornerRadius / scale;
-  SnapGroupController* snap_group_controller =
-      Shell::Get()->snap_group_controller();
-  if (snap_group_controller) {
-    SnapGroup* snap_group =
-        snap_group_controller->GetSnapGroupForGivenWindow(window);
-    if (snap_group) {
-      auto* window1 = snap_group->window1();
-      auto* window2 = snap_group->window2();
-      CHECK(window == window1 || window == window2);
-      // `window1` is guaranteed to be the primary snapped window in a snap
-      // group and `window2` is guaranteed to be the secondary snapped window in
-      // a snap group.
-      return window == window1
-                 ? gfx::RoundedCornersF(scaled_corner_radius, 0, 0, 0)
-                 : gfx::RoundedCornersF(0, scaled_corner_radius, 0, 0);
-    }
-  }
-  return gfx::RoundedCornersF(scaled_corner_radius, scaled_corner_radius, 0, 0);
-}
-
 }  // namespace
 
 WindowMiniViewHeaderView::~WindowMiniViewHeaderView() = default;
@@ -108,12 +85,7 @@ WindowMiniViewHeaderView::WindowMiniViewHeaderView(
   icon_label_view_->SetFlexForView(title_label_, 1);
 
   if (is_jellyroll_enabled) {
-    SetBackground(views::CreateThemedRoundedRectBackground(
-        chromeos::features::IsJellyrollEnabled()
-            ? cros_tokens::kCrosSysHeader
-            : static_cast<ui::ColorId>(kColorAshShieldAndBase80),
-        GetHeaderRoundedCorners(window_mini_view_->source_window()),
-        /*for_border_thickness=*/0));
+    RefreshHeaderViewRoundedCorners();
 
     views::Separator* separator =
         AddChildView(std::make_unique<views::Separator>());
@@ -147,6 +119,39 @@ void WindowMiniViewHeaderView::UpdateTitleLabel(aura::Window* window) {
   title_label_->SetText(GetWindowTitle(window));
 }
 
+void WindowMiniViewHeaderView::RefreshHeaderViewRoundedCorners() {
+  SetBackground(views::CreateThemedRoundedRectBackground(
+      chromeos::features::IsJellyrollEnabled()
+          ? cros_tokens::kCrosSysHeader
+          : static_cast<ui::ColorId>(kColorAshShieldAndBase80),
+      GetHeaderRoundedCorners(window_mini_view_->source_window()),
+      /*for_border_thickness=*/0));
+}
+
+gfx::RoundedCornersF WindowMiniViewHeaderView::GetHeaderRoundedCorners(
+    aura::Window* window) const {
+  const float scale = window->layer()->GetTargetTransform().To2dScale().x();
+  const float scaled_corner_radius = kHeaderTopCornerRadius / scale;
+  SnapGroupController* snap_group_controller =
+      Shell::Get()->snap_group_controller();
+  if (snap_group_controller) {
+    SnapGroup* snap_group =
+        snap_group_controller->GetSnapGroupForGivenWindow(window);
+    if (snap_group) {
+      auto* window1 = snap_group->window1();
+      auto* window2 = snap_group->window2();
+      CHECK(window == window1 || window == window2);
+      // `window1` is guaranteed to be the primary snapped window in a snap
+      // group and `window2` is guaranteed to be the secondary snapped window in
+      // a snap group.
+      return window == window1
+                 ? gfx::RoundedCornersF(scaled_corner_radius, 0, 0, 0)
+                 : gfx::RoundedCornersF(0, scaled_corner_radius, 0, 0);
+    }
+  }
+  return gfx::RoundedCornersF(scaled_corner_radius, scaled_corner_radius, 0, 0);
+}
+
 BEGIN_METADATA(WindowMiniViewHeaderView, views::View)
 END_METADATA