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 prevents this from happening by ignoring the clamshell options
if they are somehow executed in tablet mode.

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

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

@@ -38,6 +38,7 @@
 #include "ash/wm/tablet_mode/tablet_mode_controller.h"
 #include "base/auto_reset.h"
 #include "base/metrics/histogram_macros.h"
+#include "base/scoped_observer.h"
 #include "base/strings/utf_string_conversions.h"
 #include "chromeos/chromeos_switches.h"
 #include "ui/accessibility/ax_node_data.h"
@@ -320,16 +321,18 @@ 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)),
+      tablet_mode_observer_(this),
       shelf_item_background_color_(kShelfDefaultBaseColor),
       weak_factory_(this) {
   DCHECK(model_);
   DCHECK(shelf_);
   DCHECK(shelf_widget_);
-  bounds_animator_ = std::make_unique<views::BoundsAnimator>(this);
+  tablet_mode_observer_.Add(Shell::Get()->tablet_mode_controller());
   bounds_animator_->AddObserver(this);
   set_context_menu_controller(this);
-  focus_search_ = std::make_unique<ShelfFocusSearch>(this);
 }
 
 ShelfView::~ShelfView() {
@@ -586,6 +589,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

@@ -29,6 +29,10 @@
 #include "ui/views/view.h"
 #include "ui/views/view_model.h"
 
+namespace ash {
+class TabletModeController;
+}
+
 namespace ui {
 class SimpleMenuModel;
 }
@@ -110,7 +114,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 +184,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,
@@ -530,6 +539,10 @@ class ASH_EXPORT ShelfView : public views::View,
   // The view which gets replaced by our drag icon proxy.
   views::View* drag_replaced_view_ = nullptr;
 
+  // Observes tablet mode changing.
+  ScopedObserver<ash::TabletModeController, ash::TabletModeObserver>
+      tablet_mode_observer_;
+
   // True when the icon was dragged off the shelf.
   bool dragged_off_shelf_ = false;
 

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) {