Reland:cros: Ignore clamshell only requests in tablet mode

There is a bug where users can keep their clamshell mode context menu
open by detaching the keyboard. This allows users to have a side shelf
in tablet mode, and they may get stuck in this state.

This CL fixes the bug by explicitly closing menus when tablet mode state
changes.

Bug: 870469
Change-Id: I72bb42733000077947219f5868d32615738ab80d
Reviewed-on: https://chromium-review.googlesource.com/1165699
Commit-Queue: Alex Newcomer <newcomer@chromium.org>
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#581362}

ash/shelf/shelf_context_menu_model.cc

@@ -191,8 +191,13 @@ void ShelfContextMenuModel::ExecuteCommand(int command_id, int event_flags) {
     return;
 
   UserMetricsRecorder* metrics = Shell::Get()->metrics();
+  // Clamshell mode only options should not activate in tablet mode.
+  const bool is_tablet_mode = Shell::Get()
+                                  ->tablet_mode_controller()
+                                  ->IsTabletModeWindowManagerEnabled();
   switch (command_id) {
     case MENU_AUTO_HIDE:
+      DCHECK(!is_tablet_mode);
       SetShelfAutoHideBehaviorPref(
           prefs, display_id_,
           GetShelfAutoHideBehaviorPref(prefs, display_id_) ==
@@ -201,14 +206,17 @@ void ShelfContextMenuModel::ExecuteCommand(int command_id, int event_flags) {
               : SHELF_AUTO_HIDE_BEHAVIOR_ALWAYS);
       break;
     case MENU_ALIGNMENT_LEFT:
+      DCHECK(!is_tablet_mode);
       metrics->RecordUserMetricsAction(UMA_SHELF_ALIGNMENT_SET_LEFT);
       SetShelfAlignmentPref(prefs, display_id_, SHELF_ALIGNMENT_LEFT);
       break;
     case MENU_ALIGNMENT_RIGHT:
+      DCHECK(!is_tablet_mode);
       metrics->RecordUserMetricsAction(UMA_SHELF_ALIGNMENT_SET_RIGHT);
       SetShelfAlignmentPref(prefs, display_id_, SHELF_ALIGNMENT_RIGHT);
       break;
     case MENU_ALIGNMENT_BOTTOM:
+      DCHECK(!is_tablet_mode);
       metrics->RecordUserMetricsAction(UMA_SHELF_ALIGNMENT_SET_BOTTOM);
       SetShelfAlignmentPref(prefs, display_id_, SHELF_ALIGNMENT_BOTTOM);
       break;

ash/shelf/shelf_view.cc

@@ -320,19 +320,23 @@ ShelfView::ShelfView(ShelfModel* model, Shelf* shelf, ShelfWidget* shelf_widget)
       shelf_(shelf),
       shelf_widget_(shelf_widget),
       view_model_(new views::ViewModel),
+      bounds_animator_(std::make_unique<views::BoundsAnimator>(this)),
       tooltip_(this),
+      focus_search_(std::make_unique<ShelfFocusSearch>(this)),
       shelf_item_background_color_(kShelfDefaultBaseColor),
       weak_factory_(this) {
   DCHECK(model_);
   DCHECK(shelf_);
   DCHECK(shelf_widget_);
-  bounds_animator_ = std::make_unique<views::BoundsAnimator>(this);
+  Shell::Get()->tablet_mode_controller()->AddObserver(this);
   bounds_animator_->AddObserver(this);
   set_context_menu_controller(this);
-  focus_search_ = std::make_unique<ShelfFocusSearch>(this);
 }
 
 ShelfView::~ShelfView() {
+  // Shell destroys the TabletModeController before destroying all root windows.
+  if (Shell::Get()->tablet_mode_controller())
+    Shell::Get()->tablet_mode_controller()->RemoveObserver(this);
   bounds_animator_->RemoveObserver(this);
   model_->RemoveObserver(this);
 }
@@ -586,6 +590,20 @@ void ShelfView::CreateDragIconProxy(
   drag_image_->SetWidgetVisible(true);
 }
 
+void ShelfView::OnTabletModeStarted() {
+  // Close all menus when tablet mode starts to ensure that the clamshell only
+  // context menu options are not available in tablet mode.
+  if (shelf_menu_model_adapter_)
+    shelf_menu_model_adapter_->Cancel();
+}
+
+void ShelfView::OnTabletModeEnded() {
+  // Close all menus when tablet mode ends so that menu options are kept
+  // consistent with device state.
+  if (shelf_menu_model_adapter_)
+    shelf_menu_model_adapter_->Cancel();
+}
+
 void ShelfView::CreateDragIconProxyByLocationWithNoAnimation(
     const gfx::Point& origin_in_screen_coordinates,
     const gfx::ImageSkia& icon,

ash/shelf/shelf_view.h

@@ -110,7 +110,8 @@ class ASH_EXPORT ShelfView : public views::View,
                              public views::ContextMenuController,
                              public views::FocusTraversable,
                              public views::BoundsAnimatorObserver,
-                             public app_list::ApplicationDragAndDropHost {
+                             public app_list::ApplicationDragAndDropHost,
+                             public ash::TabletModeObserver {
  public:
   ShelfView(ShelfModel* model, Shelf* shelf, ShelfWidget* shelf_widget);
   ~ShelfView() override;
@@ -179,6 +180,10 @@ class ASH_EXPORT ShelfView : public views::View,
                            const gfx::Vector2d& cursor_offset_from_center,
                            float scale_factor) override;
 
+  // Overridden from ash::TabletModeObserver:
+  void OnTabletModeStarted() override;
+  void OnTabletModeEnded() override;
+
   void CreateDragIconProxyByLocationWithNoAnimation(
       const gfx::Point& origin_in_screen_coordinates,
       const gfx::ImageSkia& icon,

ash/shelf/shelf_view_unittest.cc

@@ -1966,9 +1966,33 @@ TEST_F(ShelfViewTest, ShelfViewShowsContextMenu) {
   generator->MoveMouseTo(shelf_view_->GetBoundsInScreen().CenterPoint());
   generator->PressRightButton();
   generator->ReleaseRightButton();
+
   EXPECT_TRUE(test_api_->CloseMenu());
 }
 
+TEST_F(ShelfViewTest, TabletModeStartAndEndClosesContextMenu) {
+  // Show a context menu on the shelf
+  ui::test::EventGenerator* generator = GetEventGenerator();
+  generator->MoveMouseTo(shelf_view_->GetBoundsInScreen().CenterPoint());
+  generator->PressRightButton();
+
+  // Start tablet mode, which should close the menu.
+  shelf_view_->OnTabletModeStarted();
+
+  // Attempt to close the menu, which should already be closed.
+  EXPECT_FALSE(test_api_->CloseMenu());
+
+  // Show another context menu on the shelf.
+  generator->MoveMouseTo(shelf_view_->GetBoundsInScreen().CenterPoint());
+  generator->PressRightButton();
+
+  // End tablet mode, which should close the menu.
+  shelf_view_->OnTabletModeEnded();
+
+  // Attempt to close the menu, which should already be closed.
+  EXPECT_FALSE(test_api_->CloseMenu());
+}
+
 // Tests that the app list button shows a context menu on right click when
 // touchable app context menus are not enabled.
 TEST_F(ShelfViewTest, AppListButtonShowsContextMenu) {