Refactor tray bubble code to make bubble activation generic

Removed the `show_by_click` init param for tray bubbles.
Moved generic PerformAction logic to TrayBackgroundView to reduce
code duplication.

Bug: 1177679
Change-Id: I6ffed1b53a8e3a66429347077b47e91de5b7f8f2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2711007
Commit-Queue: Ahmed Mehfooz <amehfooz@chromium.org>
Reviewed-by: Steven Holte <holte@chromium.org>
Reviewed-by: Toni Baržić <tbarzic@chromium.org>
Reviewed-by: Mitsuru Oshima <oshima@chromium.org>
Cr-Commit-Position: refs/heads/master@{#859204}

ash/accelerators/accelerator_controller_impl.cc

@@ -829,7 +829,7 @@ void HandleToggleSystemTrayBubbleInternal(bool focus_message_center) {
   if (tray->IsBubbleShown()) {
     tray->CloseBubble();
   } else {
-    tray->ShowBubble(false /* show_by_click */);
+    tray->ShowBubble();
     tray->ActivateBubble();
 
     if (focus_message_center)
@@ -1013,7 +1013,7 @@ void HandleShowImeMenuBubble() {
     ImeMenuTray* ime_menu_tray = status_area_widget->ime_menu_tray();
     if (ime_menu_tray && ime_menu_tray->GetVisible() &&
         !ime_menu_tray->GetBubbleView()) {
-      ime_menu_tray->ShowBubble(false /* show_by_click */);
+      ime_menu_tray->ShowBubble();
     }
   }
 }
@@ -1069,7 +1069,7 @@ PaletteTray* GetPaletteTray() {
 
 void HandleShowStylusTools() {
   base::RecordAction(UserMetricsAction("Accel_Show_Stylus_Tools"));
-  GetPaletteTray()->ShowBubble(false /* show_by_click */);
+  GetPaletteTray()->ShowBubble();
 }
 
 bool CanHandleShowStylusTools() {

ash/autoclick/autoclick_unittest.cc

@@ -1045,7 +1045,7 @@ TEST_F(AutoclickTest, AvoidsShelfBubble) {
     ASSERT_TRUE(menu);
     gfx::Rect menu_bounds = menu->GetBoundsInScreen();
 
-    unified_system_tray->ShowBubble(true /* show_by_click */);
+    unified_system_tray->ShowBubble();
     gfx::Rect new_menu_bounds = menu->GetBoundsInScreen();
     // Y is unchanged when the bubble shows.
     EXPECT_TRUE(abs(menu_bounds.y() - new_menu_bounds.y()) < 5);

ash/public/cpp/external_arc/message_center/arc_notification_content_view_unittest.cc

@@ -369,7 +369,7 @@ TEST_F(ArcNotificationContentViewTest, CloseButtonInMessageCenterView) {
   // Show MessageCenterView and activate its widget.
   auto* unified_system_tray =
       StatusAreaWidgetTestHelper::GetStatusAreaWidget()->unified_system_tray();
-  unified_system_tray->ShowBubble(false /* show_by_click */);
+  unified_system_tray->ShowBubble();
   unified_system_tray->ActivateBubble();
 
   auto notification_item =

ash/public/cpp/system_tray.h

@@ -109,9 +109,8 @@ class ASH_PUBLIC_EXPORT SystemTray {
   virtual void ShowVolumeSliderBubble() = 0;
 
   // Shows the network detailed view bubble at the right bottom of the primary
-  // display. Set |show_by_click| to true if bubble is shown by mouse or gesture
-  // click (it is used e.g. for timing histograms).
-  virtual void ShowNetworkDetailedViewBubble(bool show_by_click) = 0;
+  // display.
+  virtual void ShowNetworkDetailedViewBubble() = 0;
 
   // Provides Phone Hub functionality to the system tray.
   virtual void SetPhoneHubManager(

ash/shelf/shelf_layout_manager_unittest.cc

@@ -1876,6 +1876,7 @@ TEST_F(ShelfLayoutManagerTest, ShelfFlickerOnTrayActivation) {
   // Show the status menu. That should make the shelf visible again.
   Shell::Get()->accelerator_controller()->PerformActionIfEnabled(
       TOGGLE_SYSTEM_TRAY_BUBBLE, {});
+  GetAppListTestHelper()->WaitUntilIdle();
   EXPECT_EQ(SHELF_AUTO_HIDE, shelf->GetVisibilityState());
   EXPECT_EQ(SHELF_AUTO_HIDE_SHOWN, shelf->GetAutoHideState());
   EXPECT_TRUE(GetPrimaryUnifiedSystemTray()->IsBubbleShown());
@@ -2490,6 +2491,7 @@ TEST_F(ShelfLayoutManagerTest, ShelfItemRespondToGestureEvent) {
   // after triggering the accelerator.
   generator->PressKey(ui::VKEY_S, ui::EF_SHIFT_DOWN | ui::EF_ALT_DOWN);
   generator->ReleaseKey(ui::VKEY_S, ui::EF_SHIFT_DOWN | ui::EF_ALT_DOWN);
+  GetAppListTestHelper()->WaitUntilIdle();
   EXPECT_EQ(SHELF_AUTO_HIDE_SHOWN, shelf->GetAutoHideState());
 
   // Tap on the shelf button. Expect that the shelf button responds to gesture

ash/system/accessibility/floating_accessibility_detailed_controller.cc

@@ -79,7 +79,6 @@ void FloatingAccessibilityDetailedController::Show(
   init_params.corner_radius = kUnifiedTrayCornerRadius;
   init_params.has_shadow = false;
   init_params.translucent = true;
-  init_params.show_by_click = true;
 
   bubble_view_ = new DetailedBubbleView(init_params);
   bubble_view_->SetArrowWithoutResizing(alignment);

ash/system/accessibility/switch_access_menu_bubble_controller_unittest.cc

@@ -132,7 +132,7 @@ TEST_F(SwitchAccessMenuBubbleControllerTest, AvoidsShelfBubble) {
     GetSessionControllerClient()->SetSessionState(test.session_state);
     // Set up the switch access menu and the shelf
     auto* system_tray = GetPrimaryUnifiedSystemTray();
-    system_tray->ShowBubble(true /* show_by_click */);
+    system_tray->ShowBubble();
     gfx::Rect tray_bubble_bounds = system_tray->GetBubbleBoundsInScreen();
 
     // Try to show the menu in the center of the tray bubble.

ash/system/holding_space/holding_space_tray.cc

@@ -231,26 +231,6 @@ void HoldingSpaceTray::UpdateAfterLoginStatusChange() {
   UpdateVisibility();
 }
 
-bool HoldingSpaceTray::PerformAction(const ui::Event& event) {
-  if (bubble_) {
-    CloseBubble();
-    return true;
-  }
-
-  ShowBubble(event.IsMouseEvent() || event.IsGestureEvent());
-
-  // Activate the bubble for a11y or if it was shown via keypress. Otherwise
-  // focus will remain on the tray when it should enter the bubble.
-  if (event.IsKeyEvent() ||
-      Shell::Get()->accessibility_controller()->spoken_feedback().enabled()) {
-    DCHECK(bubble_ && bubble_->GetBubbleWidget());
-    bubble_->GetBubbleWidget()->widget_delegate()->SetCanActivate(true);
-    bubble_->GetBubbleWidget()->Activate();
-  }
-
-  return true;
-}
-
 void HoldingSpaceTray::CloseBubble() {
   if (!bubble_)
     return;
@@ -264,7 +244,7 @@ void HoldingSpaceTray::CloseBubble() {
   SetIsActive(false);
 }
 
-void HoldingSpaceTray::ShowBubble(bool show_by_click) {
+void HoldingSpaceTray::ShowBubble() {
   if (bubble_)
     return;
 
@@ -273,7 +253,7 @@ void HoldingSpaceTray::ShowBubble(bool show_by_click) {
 
   DCHECK(tray_container());
 
-  bubble_ = std::make_unique<HoldingSpaceTrayBubble>(this, show_by_click);
+  bubble_ = std::make_unique<HoldingSpaceTrayBubble>(this);
 
   // Observe the bubble widget so that we can do proper clean up when it is
   // being destroyed. If destruction is due to a call to `CloseBubble()` we will
@@ -288,6 +268,10 @@ TrayBubbleView* HoldingSpaceTray::GetBubbleView() {
   return bubble_ ? bubble_->GetBubbleView() : nullptr;
 }
 
+views::Widget* HoldingSpaceTray::GetBubbleWidget() const {
+  return bubble_ ? bubble_->GetBubbleWidget() : nullptr;
+}
+
 void HoldingSpaceTray::SetVisiblePreferred(bool visible_preferred) {
   if (this->visible_preferred() == visible_preferred)
     return;

ash/system/holding_space/holding_space_tray.h

@@ -65,10 +65,10 @@ class ASH_EXPORT HoldingSpaceTray : public TrayBackgroundView,
   void HideBubbleWithView(const TrayBubbleView* bubble_view) override;
   void AnchorUpdated() override;
   void UpdateAfterLoginStatusChange() override;
-  bool PerformAction(const ui::Event& event) override;
   void CloseBubble() override;
-  void ShowBubble(bool show_by_click) override;
+  void ShowBubble() override;
   TrayBubbleView* GetBubbleView() override;
+  views::Widget* GetBubbleWidget() const override;
   void SetVisiblePreferred(bool visible_preferred) override;
   bool GetDropFormats(int* formats,
                       std::set<ui::ClipboardFormatType>* format_types) override;

ash/system/holding_space/holding_space_tray_bubble.cc

@@ -301,8 +301,7 @@ class HoldingSpaceTrayBubble::ChildBubbleContainer
 // HoldingSpaceTrayBubble ------------------------------------------------------
 
 HoldingSpaceTrayBubble::HoldingSpaceTrayBubble(
-    HoldingSpaceTray* holding_space_tray,
-    bool show_by_click)
+    HoldingSpaceTray* holding_space_tray)
     : holding_space_tray_(holding_space_tray) {
   TrayBubbleView::InitParams init_params;
   init_params.delegate = holding_space_tray;
@@ -315,8 +314,8 @@ HoldingSpaceTrayBubble::HoldingSpaceTrayBubble(
   init_params.shelf_alignment = holding_space_tray->shelf()->alignment();
   init_params.preferred_width = kHoldingSpaceBubbleWidth;
   init_params.close_on_deactivate = true;
-  init_params.show_by_click = show_by_click;
   init_params.has_shadow = false;
+  init_params.reroute_event_handler = true;
 
   // Create and customize bubble view.
   TrayBubbleView* bubble_view = new TrayBubbleView(init_params);

ash/system/holding_space/holding_space_tray_bubble.h

@@ -28,8 +28,7 @@ class ASH_EXPORT HoldingSpaceTrayBubble : public ScreenLayoutObserver,
                                           public ShelfObserver,
                                           public TabletModeObserver {
  public:
-  HoldingSpaceTrayBubble(HoldingSpaceTray* holding_space_tray,
-                         bool show_by_click);
+  explicit HoldingSpaceTrayBubble(HoldingSpaceTray* holding_space_tray);
   HoldingSpaceTrayBubble(const HoldingSpaceTrayBubble&) = delete;
   HoldingSpaceTrayBubble& operator=(const HoldingSpaceTrayBubble&) = delete;
   ~HoldingSpaceTrayBubble() override;

ash/system/ime_menu/ime_menu_tray.cc

@@ -23,6 +23,7 @@
 #include "ash/system/tray/detailed_view_delegate.h"
 #include "ash/system/tray/system_menu_button.h"
 #include "ash/system/tray/system_tray_notifier.h"
+#include "ash/system/tray/tray_background_view.h"
 #include "ash/system/tray/tray_constants.h"
 #include "ash/system/tray/tray_container.h"
 #include "ash/system/tray/tray_popup_utils.h"
@@ -331,7 +332,7 @@ ImeMenuTray::~ImeMenuTray() {
     keyboard_controller->RemoveObserver(this);
 }
 
-void ImeMenuTray::ShowImeMenuBubbleInternal(bool show_by_click) {
+void ImeMenuTray::ShowImeMenuBubbleInternal() {
   TrayBubbleView::InitParams init_params;
   init_params.delegate = this;
   init_params.parent_window = GetBubbleWindowContainer();
@@ -342,7 +343,7 @@ void ImeMenuTray::ShowImeMenuBubbleInternal(bool show_by_click) {
   init_params.has_shadow = false;
   init_params.translucent = true;
   init_params.corner_radius = kTrayItemCornerRadius;
-  init_params.show_by_click = show_by_click;
+  init_params.reroute_event_handler = true;
 
   auto setup_layered_view = [](views::View* view) {
     view->SetPaintToLayer();
@@ -445,13 +446,11 @@ void ImeMenuTray::ClickedOutsideBubble() {
 }
 
 bool ImeMenuTray::PerformAction(const ui::Event& event) {
-  UserMetricsRecorder::RecordUserClickOnTray(
-      LoginMetricsRecorder::TrayClickTarget::kImeTray);
-  if (bubble_)
-    CloseBubble();
-  else
-    ShowBubble(event.IsMouseEvent() || event.IsGestureEvent());
-  return true;
+  if (event.IsMouseEvent() || event.IsGestureEvent()) {
+    UserMetricsRecorder::RecordUserClickOnTray(
+        LoginMetricsRecorder::TrayClickTarget::kImeTray);
+  }
+  return TrayBackgroundView::PerformAction(event);
 }
 
 void ImeMenuTray::CloseBubble() {
@@ -461,20 +460,24 @@ void ImeMenuTray::CloseBubble() {
   shelf()->UpdateAutoHideState();
 }
 
-void ImeMenuTray::ShowBubble(bool show_by_click) {
+void ImeMenuTray::ShowBubble() {
   auto* keyboard_controller = keyboard::KeyboardUIController::Get();
   if (keyboard_controller->IsKeyboardVisible()) {
     show_bubble_after_keyboard_hidden_ = true;
     keyboard_controller->AddObserver(this);
     keyboard_controller->HideKeyboardExplicitlyBySystem();
   } else {
     base::RecordAction(base::UserMetricsAction("Tray_ImeMenu_Opened"));
-    ShowImeMenuBubbleInternal(show_by_click);
+    ShowImeMenuBubbleInternal();
   }
 }
 
 TrayBubbleView* ImeMenuTray::GetBubbleView() {
-  return bubble_ ? bubble_->bubble_view() : nullptr;
+  return bubble_ ? bubble_->GetBubbleView() : nullptr;
+}
+
+views::Widget* ImeMenuTray::GetBubbleWidget() const {
+  return bubble_ ? bubble_->GetBubbleWidget() : nullptr;
 }
 
 void ImeMenuTray::OnIMERefresh() {
@@ -513,7 +516,7 @@ void ImeMenuTray::OnKeyboardHidden(bool is_temporary_hide) {
     auto* keyboard_controller = keyboard::KeyboardUIController::Get();
     keyboard_controller->RemoveObserver(this);
 
-    ShowImeMenuBubbleInternal(false /* show_by_click */);
+    ShowImeMenuBubbleInternal();
     return;
   }
 }

ash/system/ime_menu/ime_menu_tray.h

@@ -62,8 +62,9 @@ class ASH_EXPORT ImeMenuTray : public TrayBackgroundView,
   void ClickedOutsideBubble() override;
   bool PerformAction(const ui::Event& event) override;
   void CloseBubble() override;
-  void ShowBubble(bool show_by_click) override;
+  void ShowBubble() override;
   TrayBubbleView* GetBubbleView() override;
+  views::Widget* GetBubbleWidget() const override;
 
   // IMEObserver:
   void OnIMERefresh() override;
@@ -83,9 +84,8 @@ class ASH_EXPORT ImeMenuTray : public TrayBackgroundView,
  private:
   friend class ImeMenuTrayTest;
 
-  // Show the IME menu bubble immediately. Set |show_by_click| to true if bubble
-  // is shown by mouse or gesture click.
-  void ShowImeMenuBubbleInternal(bool show_by_click);
+  // Show the IME menu bubble immediately.
+  void ShowImeMenuBubbleInternal();
 
   // Updates the text of the label on the tray.
   void UpdateTrayLabel();

ash/system/media/media_tray.cc

@@ -251,11 +251,11 @@ bool MediaTray::PerformAction(const ui::Event& event) {
   if (bubble_)
     CloseBubble();
   else
-    ShowBubble(event.IsMouseEvent() || event.IsGestureEvent());
+    ShowBubble();
   return true;
 }
 
-void MediaTray::ShowBubble(bool show_by_click) {
+void MediaTray::ShowBubble() {
   DCHECK(MediaNotificationProvider::Get());
   SetNotificationColorTheme();
 
@@ -272,7 +272,7 @@ void MediaTray::ShowBubble(bool show_by_click) {
   init_params.has_shadow = false;
   init_params.translucent = true;
   init_params.corner_radius = kTrayItemCornerRadius;
-  init_params.show_by_click = show_by_click;
+  init_params.reroute_event_handler = true;
 
   TrayBubbleView* bubble_view = new TrayBubbleView(init_params);
 
@@ -290,13 +290,6 @@ void MediaTray::ShowBubble(bool show_by_click) {
                                                 false /*is_persistent*/);
   SetIsActive(true);
 
-  // Only focus the widget if it's opened by the keyboard.
-  if (!show_by_click) {
-    views::Widget* widget = bubble_->GetBubbleWidget();
-    widget->widget_delegate()->SetCanActivate(true);
-    Shell::Get()->focus_cycler()->FocusWidget(widget);
-  }
-
   base::UmaHistogramBoolean("Media.CrosGlobalMediaControls.RepeatUsageOnShelf",
                             bubble_has_shown_);
   bubble_has_shown_ = true;

ash/system/media/media_tray.h

@@ -60,7 +60,7 @@ class ASH_EXPORT MediaTray : public MediaNotificationProviderObserver,
   void UpdateAfterLoginStatusChange() override;
   void HandleLocaleChange() override;
   bool PerformAction(const ui::Event& event) override;
-  void ShowBubble(bool show_by_click) override;
+  void ShowBubble() override;
   void CloseBubble() override;
   void HideBubbleWithView(const TrayBubbleView* bubble_view) override;
   void ClickedOutsideBubble() override;

ash/system/media/media_tray_unittest.cc

@@ -6,6 +6,7 @@
 
 #include "ash/public/cpp/media_notification_provider.h"
 #include "ash/shelf/shelf.h"
+#include "ash/shell.h"
 #include "ash/system/status_area_widget.h"
 #include "ash/system/status_area_widget_test_helper.h"
 #include "ash/system/tray/tray_bubble_wrapper.h"
@@ -290,7 +291,12 @@ TEST_F(MediaTrayTest, PinToShelfDefaultBehavior) {
 }
 
 TEST_F(MediaTrayTest, BubbleGetsFocusWhenOpenWithKeyboard) {
-  media_tray()->ShowBubble(false /* show_by_click */);
+  media_tray()->ShowBubble();
+
+  // Generate a tab key press.
+  ui::test::EventGenerator generator(Shell::GetPrimaryRootWindow());
+  generator.PressKey(ui::KeyboardCode::VKEY_TAB, ui::EventFlags::EF_NONE);
+
   EXPECT_TRUE(GetBubbleWrapper()->GetBubbleWidget()->IsActive());
 }
 

ash/system/media/unified_media_controls_container_unittest.cc

@@ -31,7 +31,7 @@ class UnifiedMediaControlsContainerTest : public AshTestBase {
 
     StatusAreaWidgetTestHelper::GetStatusAreaWidget()
         ->unified_system_tray()
-        ->ShowBubble(false /* show_by_click */);
+        ->ShowBubble();
   }
 
   UnifiedSystemTrayView* system_tray_view() {

ash/system/media/unified_media_controls_detailed_view_controller_unittest.cc

@@ -67,7 +67,7 @@ class UnifiedMediaControlsDetailedViewControllerTest : public AshTestBase {
 
     StatusAreaWidgetTestHelper::GetStatusAreaWidget()
         ->unified_system_tray()
-        ->ShowBubble(false /* show_by_click */);
+        ->ShowBubble();
   }
 
   void TearDown() override {

ash/system/message_center/arc_notification_manager_delegate_impl.cc

@@ -32,7 +32,7 @@ void ArcNotificationManagerDelegateImpl::ShowMessageCenter() {
       ->GetPrimaryRootWindowController()
       ->GetStatusAreaWidget()
       ->unified_system_tray()
-      ->ShowBubble(false /* show_by_click */);
+      ->ShowBubble();
 }
 
 void ArcNotificationManagerDelegateImpl::HideMessageCenter() {