mash: Prerequisites for removing ShelfDelegate.

ash/wm/panels/panel_window_resizer_unittest.cc

Index: ash/wm/panels/panel_window_resizer_unittest.cc
diff --git a/ash/wm/panels/panel_window_resizer_unittest.cc b/ash/wm/panels/panel_window_resizer_unittest.cc
index 03636f7506b8b88e748c0baa0bc64820df1496ed..b0a50215069f679ab997602e859c2d08010839b0 100644
--- a/ash/wm/panels/panel_window_resizer_unittest.cc
+++ b/ash/wm/panels/panel_window_resizer_unittest.cc
@@ -17,7 +17,7 @@
 #include "ash/shell_port.h"
 #include "ash/test/ash_test_base.h"
 #include "ash/test/cursor_manager_test_api.h"
-#include "ash/test/test_shelf_delegate.h"
+#include "ash/test/shelf_view_test_api.h"
 #include "ash/wm/drag_window_resizer.h"
 #include "ash/wm/window_properties.h"
 #include "ash/wm/window_state.h"
@@ -42,7 +42,9 @@ class PanelWindowResizerTest : public test::AshTestBase {
   void SetUp() override {
     AshTestBase::SetUp();
     UpdateDisplay("600x400");
-    model_ = Shell::Get()->shelf_model();
+    shelf_view_test_.reset(new test::ShelfViewTestAPI(
+        GetPrimaryShelf()->GetShelfViewForTesting()));
+    shelf_view_test_->SetAnimationDuration(1);
   }
 
   void TearDown() override { AshTestBase::TearDown(); }
@@ -61,16 +63,14 @@ class PanelWindowResizerTest : public test::AshTestBase {
     gfx::Rect bounds(origin, gfx::Size(101, 101));
     aura::Window* window = CreateTestWindowInShellWithDelegateAndType(
         NULL, ui::wm::WINDOW_TYPE_PANEL, 0, bounds);
-    test::TestShelfDelegate::instance()->AddShelfItem(WmWindow::Get(window));
+    shelf_view_test_->RunMessageLoopUntilAnimationsDone();
     return window;
   }
 
   void DragStart(aura::Window* window) {
-    resizer_.reset(CreateWindowResizer(WmWindow::Get(window),
-                                       window->bounds().origin(), HTCAPTION,
-                                       aura::client::WINDOW_MOVE_SOURCE_MOUSE)
-                       .release());
-    ASSERT_TRUE(resizer_.get());
+    resizer_ =
+        CreateWindowResizer(WmWindow::Get(window), window->bounds().origin(),
+                            HTCAPTION, aura::client::WINDOW_MOVE_SOURCE_MOUSE);
   }
 
   void DragMove(int dx, int dy) {
@@ -87,7 +87,7 @@ class PanelWindowResizerTest : public test::AshTestBase {
     resizer_.reset();
   }
 
-  // Test dragging the panel slightly, then detaching, and then reattaching
+  // Test dragging the panel slightly, then detaching, and then reattaching,
   // dragging out by the vector (dx, dy).
   void DetachReattachTest(aura::Window* window, int dx, int dy) {
     EXPECT_TRUE(window->GetProperty(kPanelAttachedKey));
@@ -96,84 +96,79 @@ class PanelWindowResizerTest : public test::AshTestBase {
     DragStart(window);
     gfx::Rect initial_bounds = window->GetBoundsInScreen();
 
-    // Drag the panel slightly. The window should still be snapped to the
-    // launcher.
+    // Drag slightly, the panel window should remain attached to the shelf.
     DragMove(dx * 5, dy * 5);
     EXPECT_EQ(initial_bounds.x(), window->GetBoundsInScreen().x());
     EXPECT_EQ(initial_bounds.y(), window->GetBoundsInScreen().y());
 
-    // Drag further out and the window should now move to the cursor.
+    // Drag further out, the panel window should detach and move to the cursor.
     DragMove(dx * 100, dy * 100);
     EXPECT_EQ(initial_bounds.x() + dx * 100, window->GetBoundsInScreen().x());
     EXPECT_EQ(initial_bounds.y() + dy * 100, window->GetBoundsInScreen().y());
-
-    // The panel should be detached when the drag completes.
     DragEnd();
 
     EXPECT_FALSE(window->GetProperty(kPanelAttachedKey));
     EXPECT_EQ(kShellWindowId_DefaultContainer, window->parent()->id());
     EXPECT_EQ(root_window, window->GetRootWindow());
 
+    // Drag back towards the shelf, the panel window should re-attach.
     DragStart(window);
-    // Drag the panel down.
     DragMove(dx * -95, dy * -95);
-    // Release the mouse and the panel should be reattached.
     DragEnd();
 
-    // The panel should be reattached and have snapped to the launcher.
     EXPECT_TRUE(window->GetProperty(kPanelAttachedKey));
     EXPECT_EQ(initial_bounds.x(), window->GetBoundsInScreen().x());
     EXPECT_EQ(initial_bounds.y(), window->GetBoundsInScreen().y());
     EXPECT_EQ(kShellWindowId_PanelContainer, window->parent()->id());
   }
 
-  void TestWindowOrder(const std::vector<aura::Window*>& window_order) {
-    int panel_index = model_->FirstPanelIndex();
-    EXPECT_EQ((int)(panel_index + window_order.size()), model_->item_count());
-    for (std::vector<aura::Window *>::const_iterator
-             iter = window_order.begin();
-         iter != window_order.end(); ++iter, ++panel_index) {
-      ShelfID id = (*iter)->GetProperty(kShelfIDKey);
-      EXPECT_EQ(id, model_->items()[panel_index].id);
+  // Ensure |first| and its shelf item come before those of |second|.
+  // (eg. |first| is left of |second| in an LTR bottom-aligned shelf)
+  void CheckWindowAndItemPlacement(aura::Window* first, aura::Window* second) {
+    WmShelf* shelf = GetPrimaryShelf();
+    const gfx::Rect first_item_bounds =
+        shelf->GetScreenBoundsOfItemIconForWindow(WmWindow::Get(first));
+    const gfx::Rect second_item_bounds =
+        shelf->GetScreenBoundsOfItemIconForWindow(WmWindow::Get(second));
+    if (!base::i18n::IsRTL()) {
+      EXPECT_TRUE((first->bounds().x() < second->bounds().x()) ||
+                  (first->bounds().y() < second->bounds().y()));
+      EXPECT_TRUE((first_item_bounds.x() < second_item_bounds.x()) ||
+                  (first_item_bounds.y() < second_item_bounds.y()));

[James Cook, 2017/04/17 23:59:02]

Do you have to check Y for item icons? Shouldn't they all be in a horizontal
line?

[msw, 2017/04/18 01:07:05]

This function is also used for vertical shelf testing. (DragAlongShelfReorder is
called from PanelWindowResizerTest.DragReordersPanelsVertical)

[James Cook, 2017/04/18 15:10:36]

Can you either document that this is used for vertical shelf, or add a separate
function for testing vertical shelf that just does y?

[msw, 2017/04/18 16:43:45]

Done.

+    } else {
+      EXPECT_TRUE((first->bounds().x() > second->bounds().x()) ||
+                  (first->bounds().y() < second->bounds().y()));
+      EXPECT_TRUE((first_item_bounds.x() > second_item_bounds.x()) ||
+                  (first_item_bounds.y() < second_item_bounds.y()));
     }
   }
 
-  // Test dragging panel window along the shelf and verify that panel icons
-  // are reordered appropriately.
+  // Test dragging panel window along the shelf and verify that panel icons are
+  // reordered appropriately. New shelf items for panels are inserted before
+  // existing panel items (eg. to the left in an LTR bottom-aligned shelf).
   void DragAlongShelfReorder(int dx, int dy) {
-    gfx::Point origin(0, 0);
-    std::unique_ptr<aura::Window> w1(CreatePanelWindow(origin));
-    std::unique_ptr<aura::Window> w2(CreatePanelWindow(origin));
-    std::vector<aura::Window*> window_order_original;
-    std::vector<aura::Window*> window_order_swapped;
-    window_order_original.push_back(w1.get());
-    window_order_original.push_back(w2.get());
-    window_order_swapped.push_back(w2.get());
-    window_order_swapped.push_back(w1.get());
-    TestWindowOrder(window_order_original);
-
-    // Drag window #2 to the beginning of the shelf.
-    DragStart(w2.get());
+    std::unique_ptr<aura::Window> w1(CreatePanelWindow(gfx::Point()));
+    std::unique_ptr<aura::Window> w2(CreatePanelWindow(gfx::Point()));
+    CheckWindowAndItemPlacement(w2.get(), w1.get());
+
+    // Drag window #1 to the beginning of the shelf, the items should swap.
+    DragStart(w1.get());
     DragMove(400 * dx, 400 * dy);
-    TestWindowOrder(window_order_swapped);
+    CheckWindowAndItemPlacement(w1.get(), w2.get());
     DragEnd();
+    CheckWindowAndItemPlacement(w1.get(), w2.get());
 
-    // Expect swapped window order.
-    TestWindowOrder(window_order_swapped);
-
-    // Drag window #2 back to the end.
-    DragStart(w2.get());
+    // Drag window #1 back to the end, the items should swap back.
+    DragStart(w1.get());
     DragMove(-400 * dx, -400 * dy);
-    TestWindowOrder(window_order_original);
+    CheckWindowAndItemPlacement(w2.get(), w1.get());
     DragEnd();
-
-    // Expect original order.
-    TestWindowOrder(window_order_original);
+    CheckWindowAndItemPlacement(w2.get(), w1.get());
   }
 
  private:
   std::unique_ptr<WindowResizer> resizer_;
-  ShelfModel* model_;
+  std::unique_ptr<test::ShelfViewTestAPI> shelf_view_test_;
 
   DISALLOW_COPY_AND_ASSIGN(PanelWindowResizerTest);
 };
@@ -559,7 +554,6 @@ INSTANTIATE_TEST_CASE_P(LtrRtl,
 INSTANTIATE_TEST_CASE_P(NormalPanelPopup,
                         PanelWindowResizerTransientTest,
                         testing::Values(ui::wm::WINDOW_TYPE_NORMAL,
-                                        ui::wm::WINDOW_TYPE_PANEL,

[James Cook, 2017/04/17 23:59:02]

This goes away because a panel can't have a transient child that is a panel?

[msw, 2017/04/18 01:07:05]

That's the case as far as I know... Really, I removed this parameterized
instantiation of the test, because it fails 4 expectations for window bounds
while dragging (lines 513, 515, 525, and 527), likely related to the child panel
window attaching to the shelf in a different order (or perhaps at all...). I was
able to fix the parent panel window's bounds expectations by changing the order
of construction (creating the child before the parent, which requires explicit
destruction ordering by resetting the child's unique_ptr first at the bottom of
the test fixture), but I haven't been able to locally resolve the child panel's
bounds (I suspect it has to do with the child panel window actually attaching to
the shelf, whereas it didn't previously, but I'm not sure why...). I can dig
deeper or add a TODO and file a bug, if you think it's worthwhile.

[James Cook, 2017/04/18 15:10:36]

I think this is OK. Frankly, I don't really understand transient windows all
that well.

                                         ui::wm::WINDOW_TYPE_POPUP));
 
 }  // namespace ash