CrOS shelf: more fine grained forwarding of gestures from overflow

Currently, most gestures on the overflow shelf are completely ignored.
This causes (among other things) the two bugs linked from this CL
because gestures trying to reorder icons, or after a long press event,
are ignored.

The main reason for the special casing is that we don't want gestures
on the overflow shelf to cause showing the app list, or to drag the
shelf itself when it's an autohide shelf.

However, ignoring all gestures is overkill. This CL handles gestures
in a more fine grained way:

* The app list cannot be shown by swiping up from the shelf when the
  overflow shelf is shown. The change here is that one could initially
  swipe up from the main shelf even when the overflow shelf was shown.
  Now the overflow shelf needs to be hidden first. The previous
  behavior could arguably be seen as a bug because one could get a
  full screen app list while the overflow menu was still being shown,
  which looked quite strange.
* The shelf cannot be dragged up or down (in the case of an autohide
  shelf) when the overflow shelf is shown. Previously, one could drag
  the shelf while overflow was open (just not by performing the gesture
  on the overflow shelf itself). This resulted in behaviors where the
  whole "shelf + overflow" could be dragged up or down. Now, the
  overflow shelf will be dismissed first. One could argue either way,
  but I would consider the change a good change.
* All other gestures are handled in the overflow shelf in the same way
  as the main shelf.

Bug: 882991, 905121
Change-Id: I9269b9a0943f4aa39aa26d4d339cbbfa80b51135
Reviewed-on: https://chromium-review.googlesource.com/c/1343659
Commit-Queue: Manu Cornet <manucornet@chromium.org>
Reviewed-by: Mitsuru Oshima (OOO till 11/26) <oshima@chromium.org>
Cr-Commit-Position: refs/heads/master@{#610409}

ash/shelf/shelf_layout_manager.cc

@@ -1242,6 +1242,10 @@ bool ShelfLayoutManager::StartGestureDrag(
   if (is_app_list_visible_ && !IsHomeLauncherEnabledInTabletMode())
     return false;
 
+  // Also disable shelf drags until the overflow shelf is closed.
+  if (shelf_widget_->IsShowingOverflowBubble())
+    return false;
+
   gesture_drag_status_ = GESTURE_DRAG_IN_PROGRESS;
   gesture_drag_auto_hide_state_ = visibility_state() == SHELF_AUTO_HIDE
                                       ? auto_hide_state()
@@ -1390,6 +1394,11 @@ bool ShelfLayoutManager::CanStartFullscreenAppListDrag(
   if (!IsVisible())
     return false;
 
+  // Do not show the fullscreen app list until the overflow bubble has been
+  // closed.
+  if (shelf_widget_->IsShowingOverflowBubble())
+    return false;
+
   // If app list is already opened, swiping up on the shelf should keep the app
   // list opened.
   if (is_app_list_visible_)

ash/shelf/shelf_view.cc

@@ -1807,14 +1807,6 @@ void ShelfView::GetAccessibleNodeData(ui::AXNodeData* node_data) {
 }
 
 void ShelfView::OnGestureEvent(ui::GestureEvent* event) {
-  // Do not forward events to |shelf_| (which forwards events to the shelf
-  // layout manager) as we do not want gestures on the overflow to open the app
-  // list for example.
-  if (is_overflow_mode()) {
-    main_shelf_->overflow_bubble()->bubble_view()->ProcessGestureEvent(*event);
-    event->StopPropagation();
-    return;
-  }
 
   // Convert the event location from current view to screen, since swiping up on
   // the shelf can open the fullscreen app list. Updating the bounds of the app
@@ -1824,6 +1816,13 @@ void ShelfView::OnGestureEvent(ui::GestureEvent* event) {
   event->set_location(location_in_screen);
   if (shelf_->ProcessGestureEvent(*event))
     event->StopPropagation();
+  else if (is_overflow_mode()) {
+    // If the event hasn't been processed and the overflow shelf is showing,
+    // let the bubble process the event.
+    main_shelf_->overflow_bubble()->bubble_view()->ProcessGestureEvent(*event);
+    event->StopPropagation();
+    return;
+  }
 }
 
 bool ShelfView::OnMouseWheel(const ui::MouseWheelEvent& event) {

ash/shelf/shelf_view_unittest.cc

@@ -276,7 +276,7 @@ class ShelfViewTest : public AshTestBase {
       flag_warning->SetVisible(false);
     }
 
-    // The bounds should be big enough for 4 buttons + overflow chevron.
+    // The bounds should be big enough for 4 buttons + overflow button.
     ASSERT_GE(shelf_view_->width(), 500);
 
     test_api_.reset(new ShelfViewTestAPI(shelf_view_));
@@ -1759,6 +1759,53 @@ TEST_F(ShelfViewTest, CheckDragAndDropFromShelfToOtherShelf) {
                                           true /* cancel */);
 }
 
+// Checks drag-reorder items within the overflow shelf.
+TEST_F(ShelfViewTest, TestDragWithinOverflow) {
+  // Prepare the overflow and open it.
+  AddButtonsUntilOverflow();
+  // Add a couple more to make sure we have things to drag.
+  AddAppShortcut();
+  AddAppShortcut();
+  test_api_->ShowOverflowBubble();
+  ShelfView* overflow_shelf_view =
+      shelf_view_->overflow_bubble()->bubble_view()->shelf_view();
+  ASSERT_TRUE(test_api_->IsShowingOverflowBubble());
+
+  ShelfViewTestAPI overflow_api(overflow_shelf_view);
+
+  // We are going to drag the first item in the overflow (A) onto the last
+  // one (B).
+  int item_a_initial_index = overflow_api.GetFirstVisibleIndex();
+  int item_b_initial_index = overflow_api.GetLastVisibleIndex();
+  ShelfID item_a = GetItemId(item_a_initial_index);
+  ShelfID item_b = GetItemId(item_b_initial_index);
+  ShelfButton* item_a_button = overflow_api.GetButton(item_a_initial_index);
+  ShelfButton* item_b_button = overflow_api.GetButton(item_b_initial_index);
+  gfx::Point drag_point = GetButtonCenter(item_a_button);
+  gfx::Point drop_point = GetButtonCenter(item_b_button);
+
+  ui::test::EventGenerator* generator = GetEventGenerator();
+  generator->set_current_location(drag_point);
+  EXPECT_EQ(nullptr, overflow_shelf_view->drag_view());
+
+  // TODO(manucornet): Test the same thing with only touches.
+  generator->PressLeftButton();
+
+  generator->MoveMouseTo(drop_point);
+  EXPECT_NE(nullptr, overflow_shelf_view->drag_view());
+  generator->ReleaseLeftButton();
+  overflow_api.RunMessageLoopUntilAnimationsDone();
+
+  // Now, item A should be the last item, and item B should be just before it.
+  ShelfID new_first_visible_item =
+      GetItemId(overflow_api.GetFirstVisibleIndex());
+  EXPECT_NE(item_a, new_first_visible_item);
+  EXPECT_EQ(item_a, GetItemId(overflow_api.GetLastVisibleIndex()));
+  EXPECT_EQ(item_b, GetItemId(overflow_api.GetLastVisibleIndex() - 1));
+
+  test_api_->HideOverflowBubble();
+}
+
 // Checks creating app shortcut for an opened platform app in overflow bubble
 // should be invisible to the shelf. See crbug.com/605793.
 TEST_F(ShelfViewTest, CheckOverflowStatusPinOpenedAppToShelf) {