macviews: run menu closure animation for dismissals by clicking outside

This change:
1) Adds support to MenuClosureAnimationMac for animating menu closure without
   animating selecting an item;
2) Has MenuController use MenuClosureAnimationMac to animate cancels caused by
   outside clicks

Bug: 833856
Change-Id: I9743b396798e4b125c038b35decc9aceb083926d
Reviewed-on: https://chromium-review.googlesource.com/1231837
Commit-Queue: Elly Fong-Jones <ellyjones@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Cr-Commit-Position: refs/heads/master@{#594738}

chrome/browser/ui/views/menu_model_adapter_test.cc

@@ -21,6 +21,7 @@
 #include "ui/views/controls/menu/menu_item_view.h"
 #include "ui/views/controls/menu/menu_runner.h"
 #include "ui/views/controls/menu/submenu_view.h"
+#include "ui/views/test/menu_test_utils.h"
 #include "ui/views/widget/root_view.h"
 #include "ui/views/widget/widget.h"
 
@@ -211,6 +212,7 @@ class MenuModelAdapterTest : public ViewEventTestBase,
 
   // Open the submenu.
   void Step1() {
+    views::test::DisableMenuClosureAnimations();
     views::SubmenuView* topmenu = menu_->GetSubmenu();
     ASSERT_TRUE(topmenu);
     ASSERT_TRUE(topmenu->IsShowing());
@@ -251,6 +253,7 @@ class MenuModelAdapterTest : public ViewEventTestBase,
   // All done.
   void Step4() {
     views::SubmenuView* topmenu = menu_->GetSubmenu();
+    views::test::WaitForMenuClosureAnimation();
     ASSERT_TRUE(topmenu);
     ASSERT_FALSE(topmenu->IsShowing());
     ASSERT_FALSE(top_menu_model_.IsSubmenuShowing());

ui/views/controls/menu/menu_closure_animation_mac.h

@@ -14,6 +14,7 @@
 namespace views {
 
 class MenuItemView;
+class SubmenuView;
 
 // This class implements the Mac menu closure animation:
 //    1) For 100ms, the selected item is drawn as unselected
@@ -24,11 +25,19 @@ class MenuItemView;
 // will stop the timer or animation (if they are running), so the callback will
 // *not* be run - which is good, since the MenuController that would have
 // received it is being deleted.
+//
+// This class also supports animating a menu away without animating the
+// selection effect, which is achieved by passing nullptr for the item to
+// animate. In this case, the animation skips straight to step 3 above.
 class VIEWS_EXPORT MenuClosureAnimationMac : public gfx::AnimationDelegate {
  public:
-  // After this closure animation is done, |callback| is run to finally accept
-  // |item|.
-  MenuClosureAnimationMac(MenuItemView* item, base::OnceClosure callback);
+  // After this closure animation is done, |callback| is run to finish
+  // dismissing the menu. If |item| is given, this will animate the item being
+  // accepted before animating the menu closing; if |item| is nullptr, only the
+  // menu closure will be animated.
+  MenuClosureAnimationMac(MenuItemView* item,
+                          SubmenuView* menu,
+                          base::OnceClosure callback);
   ~MenuClosureAnimationMac() override;
 
   // Start the animation.
@@ -37,6 +46,9 @@ class VIEWS_EXPORT MenuClosureAnimationMac : public gfx::AnimationDelegate {
   // Returns the MenuItemView this animation targets.
   MenuItemView* item() { return item_; }
 
+  // Returns the SubmenuView this animation targets.
+  SubmenuView* menu() { return menu_; }
+
   // Causes animations to take no time for testing purposes. Note that this
   // still causes the completion callback to be run asynchronously, so test
   // situations have the same control flow as non-test situations.
@@ -51,7 +63,7 @@ class VIEWS_EXPORT MenuClosureAnimationMac : public gfx::AnimationDelegate {
     kFinish,
   };
 
-  static constexpr AnimationStep NextStepFor(AnimationStep step);
+  AnimationStep NextStepFor(AnimationStep step) const;
 
   void AdvanceAnimation();
 
@@ -64,6 +76,7 @@ class VIEWS_EXPORT MenuClosureAnimationMac : public gfx::AnimationDelegate {
   base::OneShotTimer timer_;
   std::unique_ptr<gfx::Animation> fade_animation_;
   MenuItemView* item_;
+  SubmenuView* menu_;
   AnimationStep step_;
 
   DISALLOW_COPY_AND_ASSIGN(MenuClosureAnimationMac);

ui/views/controls/menu/menu_closure_animation_mac.mm

@@ -10,6 +10,7 @@
 #include "base/threading/thread_task_runner_handle.h"
 #include "ui/gfx/animation/linear_animation.h"
 #include "ui/views/controls/menu/menu_item_view.h"
+#include "ui/views/controls/menu/submenu_view.h"
 #include "ui/views/widget/widget.h"
 
 namespace {
@@ -19,9 +20,11 @@ static bool g_disable_animations_for_testing = false;
 namespace views {
 
 MenuClosureAnimationMac::MenuClosureAnimationMac(MenuItemView* item,
+                                                 SubmenuView* menu,
                                                  base::OnceClosure callback)
     : callback_(std::move(callback)),
       item_(item),
+      menu_(menu),
       step_(AnimationStep::kStart) {}
 
 MenuClosureAnimationMac::~MenuClosureAnimationMac() {}
@@ -42,12 +45,11 @@ void MenuClosureAnimationMac::Start() {
 }
 
 // static
-constexpr MenuClosureAnimationMac::AnimationStep
-MenuClosureAnimationMac::NextStepFor(
-    MenuClosureAnimationMac::AnimationStep step) {
+MenuClosureAnimationMac::AnimationStep MenuClosureAnimationMac::NextStepFor(
+    MenuClosureAnimationMac::AnimationStep step) const {
   switch (step) {
     case AnimationStep::kStart:
-      return AnimationStep::kUnselected;
+      return item_ ? AnimationStep::kUnselected : AnimationStep::kFading;
     case AnimationStep::kUnselected:
       return AnimationStep::kSelected;
     case AnimationStep::kSelected:
@@ -84,7 +86,7 @@ void MenuClosureAnimationMac::DisableAnimationsForTesting() {
 
 void MenuClosureAnimationMac::AnimationProgressed(
     const gfx::Animation* animation) {
-  NSWindow* window = item_->GetWidget()->GetNativeWindow();
+  NSWindow* window = menu_->GetWidget()->GetNativeWindow();
   [window setAlphaValue:animation->CurrentValueBetween(1.0, 0.0)];
 }
 

ui/views/controls/menu/menu_controller.cc

@@ -1529,7 +1529,7 @@ void MenuController::UpdateInitialLocation(const gfx::Rect& bounds,
 void MenuController::Accept(MenuItemView* item, int event_flags) {
 #if defined(OS_MACOSX)
   menu_closure_animation_ = std::make_unique<MenuClosureAnimationMac>(
-      item,
+      item, item->GetParentMenuItem()->GetSubmenu(),
       base::BindOnce(&MenuController::ReallyAccept, base::Unretained(this),
                      base::Unretained(item), event_flags));
   menu_closure_animation_->Start();
@@ -2600,7 +2600,15 @@ void MenuController::RepostEventAndCancel(SubmenuView* source,
     if (last_part.type != MenuPart::NONE)
       exit_type = EXIT_OUTERMOST;
   }
+#if defined(OS_MACOSX)
+  menu_closure_animation_ = std::make_unique<MenuClosureAnimationMac>(
+      nullptr, source,
+      base::BindOnce(&MenuController::Cancel, base::Unretained(this),
+                     exit_type));
+  menu_closure_animation_->Start();
+#else
   Cancel(exit_type);
+#endif
 }
 
 void MenuController::SetDropMenuItem(MenuItemView* new_target,

ui/views/controls/menu/menu_controller_unittest.cc

@@ -1229,6 +1229,7 @@ TEST_F(MenuControllerTest, PreserveGestureForOwner) {
 // Tests that touch outside menu does not closes the menu when forwarding
 // gesture events to owner.
 TEST_F(MenuControllerTest, NoTouchCloseWhenSendingGesturesToOwner) {
+  views::test::DisableMenuClosureAnimations();
   MenuController* controller = menu_controller();
 
   // Owner wants the gesture events.
@@ -1256,6 +1257,7 @@ TEST_F(MenuControllerTest, NoTouchCloseWhenSendingGesturesToOwner) {
 
   // Touch outside again and menu should be closed.
   controller->OnTouchEvent(sub_menu, &touch_event);
+  views::test::WaitForMenuClosureAnimation();
   EXPECT_FALSE(IsShowing());
   EXPECT_EQ(MenuController::EXIT_ALL, controller->exit_type());
 }
@@ -1264,6 +1266,7 @@ TEST_F(MenuControllerTest, NoTouchCloseWhenSendingGesturesToOwner) {
 // occur outside of the bounds of the menu. Instead a proper shutdown should
 // occur.
 TEST_F(MenuControllerTest, AsynchronousRepostEvent) {
+  views::test::DisableMenuClosureAnimations();
   MenuController* controller = menu_controller();
   TestMenuControllerDelegate* delegate = menu_controller_delegate();
   std::unique_ptr<TestMenuControllerDelegate> nested_delegate(
@@ -1288,6 +1291,7 @@ TEST_F(MenuControllerTest, AsynchronousRepostEvent) {
   // When attempting to select outside of all menus this should lead to a
   // shutdown. This should not crash while attempting to repost the event.
   SetSelectionOnPointerDown(sub_menu, &event);
+  views::test::WaitForMenuClosureAnimation();
 
   EXPECT_EQ(delegate, GetCurrentDelegate());
   EXPECT_EQ(1, delegate->on_menu_closed_called());
@@ -1302,6 +1306,7 @@ TEST_F(MenuControllerTest, AsynchronousRepostEvent) {
 // Tests that an asynchronous menu reposts touch events that occur outside of
 // the bounds of the menu, and that the menu closes.
 TEST_F(MenuControllerTest, AsynchronousTouchEventRepostEvent) {
+  views::test::DisableMenuClosureAnimations();
   MenuController* controller = menu_controller();
   TestMenuControllerDelegate* delegate = menu_controller_delegate();
 
@@ -1316,6 +1321,7 @@ TEST_F(MenuControllerTest, AsynchronousTouchEventRepostEvent) {
       ui::ET_TOUCH_PRESSED, location, ui::EventTimeForNow(),
       ui::PointerDetails(ui::EventPointerType::POINTER_TYPE_TOUCH, 0));
   controller->OnTouchEvent(sub_menu, &event);
+  views::test::WaitForMenuClosureAnimation();
 
   EXPECT_FALSE(IsShowing());
   EXPECT_EQ(1, delegate->on_menu_closed_called());
@@ -1329,6 +1335,7 @@ TEST_F(MenuControllerTest, AsynchronousTouchEventRepostEvent) {
 // Tests that having the MenuController deleted during RepostEvent does not
 // cause a crash. ASAN bots should not detect use-after-free in MenuController.
 TEST_F(MenuControllerTest, AsynchronousRepostEventDeletesController) {
+  views::test::DisableMenuClosureAnimations();
   MenuController* controller = menu_controller();
   std::unique_ptr<TestMenuControllerDelegate> nested_delegate(
       new TestMenuControllerDelegate());
@@ -1355,6 +1362,7 @@ TEST_F(MenuControllerTest, AsynchronousRepostEventDeletesController) {
   // When attempting to select outside of all menus this should lead to a
   // shutdown. This should not crash while attempting to repost the event.
   SetSelectionOnPointerDown(sub_menu, &event);
+  views::test::WaitForMenuClosureAnimation();
 
   // Close to remove observers before test TearDown
   sub_menu->Close();