Remove ShelfItemDelegate::IsDraggable; check for app list button.

ash/shelf/shelf_view_unittest.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "ash/common/shelf/shelf_view.h"
 
 #include <algorithm>
 #include <memory>
 #include <utility>
 #include <vector>
@@ skipping 421 matching lines @@
         gfx::Rect shelf_view_bounds = shelf_view_->GetLocalBounds();
         gfx::Rect item_bounds = test_api_->GetBoundsByIndex(i);
         EXPECT_GE(item_bounds.x(), 0);
         EXPECT_GE(item_bounds.y(), 0);
         EXPECT_LE(item_bounds.right(), shelf_view_bounds.width());
         EXPECT_LE(item_bounds.bottom(), shelf_view_bounds.height());
       }
     }
   }
 
+  // Simulate a mouse press event on the shelf's view at |view_index|.
+  views::View* SimulateViewPressed(ShelfView::Pointer pointer, int view_index) {

[James Cook, 2016/12/14 17:41:54]

Is |pointer| always ShelfView::MOUSE?

If so, it shouldn't need the parameter.

If not, it seems weird that this uses ET_MOUSE_PRESSED.

[msw, 2016/12/14 19:46:50]

ShelfView::TOUCH is used here via SimulateDrag, and this has worked as-is with
that. Your concern is valid, but out of scope of this CL (I would have to change
all the MouseEvents), I'll leave this alone for now if that's okay with you.

+    views::View* view = test_api_->GetViewAt(view_index);
+    ui::MouseEvent pressed_event(ui::ET_MOUSE_PRESSED, gfx::Point(),
+                                 view->GetBoundsInScreen().origin(),
+                                 ui::EventTimeForNow(), 0, 0);
+    shelf_view_->PointerPressedOnButton(view, pointer, pressed_event);
+    return view;
+  }
+
+  // Similar to SimulateViewPressed, but the index must not be for the app list,

[James Cook, 2016/12/14 17:41:54]

Can it DCHECK that button_index is not the app_list index?

[msw, 2016/12/14 19:46:50]

Done.

+  // since the app list button is not a ShelfButton.
   ShelfButton* SimulateButtonPressed(ShelfView::Pointer pointer,
                                      int button_index) {
     ShelfButton* button = test_api_->GetButton(button_index);
-    ui::MouseEvent click_event(ui::ET_MOUSE_PRESSED, gfx::Point(),
-                               button->GetBoundsInScreen().origin(),
-                               ui::EventTimeForNow(), 0, 0);
-    shelf_view_->PointerPressedOnButton(button, pointer, click_event);
+    EXPECT_EQ(button, SimulateViewPressed(pointer, button_index));
     return button;
   }
 
   // Simulates a single mouse click.
   void SimulateClick(int button_index) {
     ShelfButton* button = SimulateButtonPressed(ShelfView::MOUSE, button_index);
     ui::MouseEvent release_event(ui::ET_MOUSE_RELEASED, gfx::Point(),
                                  button->GetBoundsInScreen().origin(),
                                  ui::EventTimeForNow(), 0, 0);
     test_api_->ButtonPressed(
@@ skipping 30 matching lines @@
    * Trigger ContinueDrag of the shelf
    * The argument progressively means whether to simulate the drag progress (a
    * series of changes of the posistion of dragged item), like the normal user
    * drag behavior.
    */
   void ContinueDrag(views::View* button,
                     ShelfView::Pointer pointer,
                     int from_index,
                     int to_index,
                     bool progressively) {
-    views::View* to = test_api_->GetButton(to_index);
-    views::View* from = test_api_->GetButton(from_index);
+    views::View* to = test_api_->GetViewAt(to_index);
+    views::View* from = test_api_->GetViewAt(from_index);
     int dist_x = to->x() - from->x();
     int dist_y = to->y() - from->y();
     if (progressively) {
       int sgn = dist_x > 0 ? 1 : -1;
       dist_x = abs(dist_x);
       for (; dist_x; dist_x -= std::min(10, dist_x))
         DoDrag(sgn * std::min(10, abs(dist_x)), 0, button, pointer, to);
     } else {
       DoDrag(dist_x, dist_y, button, pointer, to);
     }
   }
 
   /*
    * Simulate drag operation.
    * Argument progressively means whether to simulate the drag progress (a
    * series of changes of the posistion of dragged item) like the behavior of
    * user drags.
    */
   views::View* SimulateDrag(ShelfView::Pointer pointer,
                             int button_index,
                             int destination_index,
                             bool progressively) {
-    views::View* button = SimulateButtonPressed(pointer, button_index);
+    views::View* button = SimulateViewPressed(pointer, button_index);
 
     if (!progressively) {
       ContinueDrag(button, pointer, button_index, destination_index, false);
     } else if (button_index < destination_index) {
       for (int cur_index = button_index + 1; cur_index <= destination_index;
            cur_index++)
         ContinueDrag(button, pointer, cur_index - 1, cur_index, true);
     } else if (button_index > destination_index) {
       for (int cur_index = button_index - 1; cur_index >= destination_index;
            cur_index--)
@@ skipping 624 matching lines @@
   dragged_button_mouse = SimulateDrag(ShelfView::MOUSE, 1, 2, false);
 
   // Nothing changes since 2nd drag is ignored.
   ASSERT_NO_FATAL_FAILURE(CheckModelIDs(id_map));
 
   shelf_view_->PointerReleasedOnButton(dragged_button_touch, ShelfView::TOUCH,
                                        false);
   ASSERT_NO_FATAL_FAILURE(CheckModelIDs(id_map));
 }
 
-// Check that whether the ash behaves correctly if not draggable
-// item are in front of the shelf.
+// Ensure the app list button cannot be dragged and other items cannot be
+// dragged in front of the app list button.
 TEST_F(ShelfViewTest, DragWithNotDraggableItemInFront) {
+  // The expected id order is initialized as: 1, 2, 3, 4, 5, 6, 7
   std::vector<std::pair<ShelfID, views::View*>> id_map;
   SetupForDragTest(&id_map);
+  ASSERT_EQ(TYPE_APP_LIST, model_->items()[0].type);
 
-  (static_cast<TestShelfItemDelegate*>(
-       model_->GetShelfItemDelegate(id_map[1].first)))
-      ->set_is_draggable(false);
-  (static_cast<TestShelfItemDelegate*>(
-       model_->GetShelfItemDelegate(id_map[2].first)))
-      ->set_is_draggable(false);
+  // Ensure that the app list button cannot be dragged.
+  // The expected id order is unchanged: 1, 2, 3, 4, 5, 6, 7
+  ASSERT_NO_FATAL_FAILURE(DragAndVerify(0, 1, shelf_view_, id_map));
+  ASSERT_NO_FATAL_FAILURE(DragAndVerify(0, 2, shelf_view_, id_map));
+  ASSERT_NO_FATAL_FAILURE(DragAndVerify(0, 5, shelf_view_, id_map));
 
-  ASSERT_NO_FATAL_FAILURE(DragAndVerify(3, 1, shelf_view_, id_map));
-  ASSERT_NO_FATAL_FAILURE(DragAndVerify(3, 2, shelf_view_, id_map));
-
-  std::rotate(id_map.begin() + 3, id_map.begin() + 4, id_map.begin() + 5);
-  ASSERT_NO_FATAL_FAILURE(DragAndVerify(4, 1, shelf_view_, id_map));
-  std::rotate(id_map.begin() + 3, id_map.begin() + 5, id_map.begin() + 6);
-  ASSERT_NO_FATAL_FAILURE(DragAndVerify(5, 1, shelf_view_, id_map));
+  // Ensure that items cannot be dragged in front of the app list button.
+  // Attempting to do so will order buttons immediately after the app list.
+  // Dragging the second button in front should no-op: 1, 2, 3, 4, 5, 6, 7
+  ASSERT_NO_FATAL_FAILURE(DragAndVerify(1, 0, shelf_view_, id_map));
+  // Dragging the third button in front should yield: 1, 3, 2, 4, 5, 6, 7
+  std::rotate(id_map.begin() + 1, id_map.begin() + 2, id_map.begin() + 3);
+  ASSERT_NO_FATAL_FAILURE(DragAndVerify(2, 0, shelf_view_, id_map));
+  // Dragging the sixth button in front should yield: 1, 6, 3, 2, 4, 5, 7
+  std::rotate(id_map.begin() + 1, id_map.begin() + 5, id_map.begin() + 6);
+  ASSERT_NO_FATAL_FAILURE(DragAndVerify(5, 0, shelf_view_, id_map));

[James Cook, 2016/12/14 17:41:54]

Thanks for documenting all this.

[msw, 2016/12/14 19:46:50]

Acknowledged.

 }
 
 // Check that clicking first on one item and then dragging another works as
 // expected.
 TEST_F(ShelfViewTest, ClickOneDragAnother) {
   std::vector<std::pair<ShelfID, views::View*>> id_map;
   SetupForDragTest(&id_map);
 
   // A click on item 1 is simulated.
   SimulateClick(1);
@@ skipping 1916 matching lines @@
   EXPECT_THAT(overflow_button_ink_drop_->GetAndResetRequestedStates(),
               IsEmpty());
 
   ASSERT_TRUE(test_api_->IsShowingOverflowBubble());
 }
 
 #endif  // !defined(OS_WIN)
 
 }  // namespace test
 }  // namespace ash