Tablet WM : Swiping on system tray bubble.

ash/system/tray/system_tray_unittest.cc

Index: ash/system/tray/system_tray_unittest.cc
diff --git a/ash/system/tray/system_tray_unittest.cc b/ash/system/tray/system_tray_unittest.cc
index c8dc570c4f0103ed481221f980abfc7c284330cf..c8f45eae2058b652accd6eee6f7ebc79384533df 100644
--- a/ash/system/tray/system_tray_unittest.cc
+++ b/ash/system/tray/system_tray_unittest.cc
@@ -16,11 +16,13 @@
 #include "ash/system/status_area_widget.h"
 #include "ash/system/tray/system_tray_bubble.h"
 #include "ash/system/tray/system_tray_item.h"
+#include "ash/system/tray/system_tray_test_api.h"

[minch1, 2017/06/20 16:45:35]

Will remove this in next PS.

[tdanderson, 2017/06/20 22:42:31]

Acknowledged.

 #include "ash/system/tray/tray_constants.h"
 #include "ash/system/web_notification/web_notification_tray.h"
 #include "ash/test/ash_test_base.h"
 #include "ash/test/status_area_widget_test_helper.h"
 #include "ash/test/test_system_tray_item.h"
+#include "ash/wm/maximize_mode/maximize_mode_controller.h"
 #include "ash/wm/window_util.h"
 #include "base/memory/ptr_util.h"
 #include "base/run_loop.h"
@@ -56,7 +58,210 @@ class ModalWidgetDelegate : public views::WidgetDelegateView {
 
 }  // namespace
 
-typedef AshTestBase SystemTrayTest;
+class SystemTrayTest : public AshTestBase {
+ public:
+  SystemTrayTest() {}
+  ~SystemTrayTest() override {}
+
+  void SendGestureEvent(gfx::Point& start,

[tdanderson, 2017/06/20 22:42:31]

nit: can you please add brief documentation for each of the helper functions you
are introducing here (above lines 66, 88, 100).

[minch1, 2017/06/22 17:42:11]

Done.

+                        float delta,
+                        bool is_fling,
+                        float velocity_y) {
+    SystemTray* system_tray = GetPrimarySystemTray();
+    base::TimeTicks timestamp = base::TimeTicks::Now();
+    SendScrollStartAndUpdate(start, delta, timestamp);
+
+    if (is_fling) {

[tdanderson, 2017/06/20 22:42:31]

To simplify this, you could replace lines 74-85 with just the three lines:

ui::GestureEventDetails details = is_fling ? ... : ...;
ui::GestureEvent event = ui::GestureEvent(..., details);
system_tray->OnGestureEvent(&event);

[minch1, 2017/06/22 17:42:11]

Done.

+      ui::GestureEventDetails fling_details(ui::ET_SCROLL_FLING_START, 0,
+                                            velocity_y);
+      ui::GestureEvent fling_event = ui::GestureEvent(
+          start.x(), start.y() + delta, ui::EF_NONE, timestamp, fling_details);
+      system_tray->OnGestureEvent(&fling_event);
+    } else {
+      ui::GestureEventDetails end_details(ui::ET_GESTURE_SCROLL_END);
+      ui::GestureEvent end_event = ui::GestureEvent(
+          start.x(), start.y() + delta, ui::EF_NONE, timestamp, end_details);
+      system_tray->OnGestureEvent(&end_event);
+    }
+  }
+
+  float GetSystemBubbleHeight() {

[tdanderson, 2017/06/20 22:42:29]

Does this function always return the same value? You call this many times in
your tests, and it seems like a lot of unnecessary overhead to show the view,
capture the bounds, and then close the view every time.

[minch1, 2017/06/22 17:42:11]

It is not always the same value when you open the bubble. So I think it is
necessary to get the height of the bubble for each action.

[tdanderson, 2017/06/22 20:21:42]

Acknowledged.

+    SystemTray* system_tray = GetPrimarySystemTray();
+    system_tray->ShowDefaultView(BUBBLE_CREATE_NEW);
+    gfx::Rect bounds = system_tray->GetSystemBubble()
+                           ->bubble_view()
+                           ->GetWidget()
+                           ->GetWindowBoundsInScreen();
+    system_tray->CloseSystemBubble();
+
+    return bounds.height();
+  }
+
+  void SendScrollStartAndUpdate(gfx::Point& start,
+                                float delta,
+                                base::TimeTicks& timestamp) {
+    SystemTray* system_tray = GetPrimarySystemTray();
+    ui::GestureEventDetails begin_details(ui::ET_GESTURE_SCROLL_BEGIN);
+    ui::GestureEvent begin_event = ui::GestureEvent(
+        start.x(), start.y(), ui::EF_NONE, timestamp, begin_details);
+    system_tray->OnGestureEvent(&begin_event);
+
+    ui::GestureEventDetails update_details(ui::ET_GESTURE_SCROLL_UPDATE, 0,
+                                           delta);
+    timestamp += base::TimeDelta::FromMilliseconds(100);
+    ui::GestureEvent update_event = ui::GestureEvent(
+        start.x(), start.y(), ui::EF_NONE, timestamp, update_details);
+    system_tray->OnGestureEvent(&update_event);
+  }
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(SystemTrayTest);
+};
+
+TEST_F(SystemTrayTest, FlingOnSystemTray) {

[tdanderson, 2017/06/20 22:42:31]

nit: can you please add brief documentation above each of the tests you are
adding so that readers can get a general sense of what the test does without
needing to look at its implementation. See for instance what is done on line
266.

[minch1, 2017/06/22 17:42:11]

Done.

+  Shelf* shelf = GetPrimaryShelf();
+  SystemTray* system_tray = GetPrimarySystemTray();
+  gfx::Point start = system_tray->GetBoundsInScreen().CenterPoint();
+  shelf->SetAlignment(SHELF_ALIGNMENT_BOTTOM);
+  float kFlingVelocity = 100.0f;

[tdanderson, 2017/06/20 22:42:30]

I think it would be a good idea to define this constant in a single place
(system_tray.h/cc is appropriate) and then reference it from both
SystemTray::ShouldShowSystemBubbleForDragging() and from this line. That way if
the constant is ever changed, your test gets updated for free.

[minch1, 2017/06/22 17:42:11]

Done.

+  Shell::Get()->maximize_mode_controller()->EnableMaximizeModeWindowManager(
+      true);
+
+  // Fling up on the system tray should show the bubble if the |velocity_y| is
+  // larger than kFlingVelocity and the dragging amount is larger than one third
+  // of the height of the bubble.
+  float delta = -GetSystemBubbleHeight();
+  SendGestureEvent(start, delta, true, -(kFlingVelocity + 1));
+  EXPECT_TRUE(system_tray->HasSystemBubble());
+  system_tray->CloseSystemBubble();
+
+  // Fling up on the system tray should show the bubble if the |velocity_y| is
+  // larger than kFlingVelocity even the dragging amount is less than one third
+  // of the height of the bubble.
+  delta /= 4;
+  SendGestureEvent(start, delta, true, -(kFlingVelocity + 1));
+  EXPECT_TRUE(system_tray->HasSystemBubble());
+  system_tray->CloseSystemBubble();
+
+  // Fling up on the system tray should show the bubble if the |velocity_y| is
+  // less than kFlingVelocity but the dragging amount if larger than one third
+  // of the height of the bubble.
+  delta = -GetSystemBubbleHeight();
+  SendGestureEvent(start, delta, true, -(kFlingVelocity - 1));
+  EXPECT_TRUE(system_tray->HasSystemBubble());
+  system_tray->CloseSystemBubble();
+
+  // Fling up on the system tray should close the bubble if the |velocity_y|
+  // is less than kFlingVelocity and the dragging amount is less than one third
+  // of the height of the bubble.
+  delta /= 4;
+  SendGestureEvent(start, delta, true, -(kFlingVelocity - 1));
+  EXPECT_FALSE(system_tray->HasSystemBubble());
+
+  // Fling down on the system tray should close the bubble if the |velocity_y|
+  // is larger than kFLingVelocity.
+  SendGestureEvent(start, delta, true, kFlingVelocity + 1);
+  EXPECT_FALSE(system_tray->HasSystemBubble());
+
+  // Fling down on the system tray should close the bubble if the |velocity_y|
+  // is larger than kFlingVelocity even the dragging amount is larger than one
+  // third of the height of the bubble.
+  delta = -GetSystemBubbleHeight();
+  SendGestureEvent(start, delta, true, kFlingVelocity + 1);
+  EXPECT_FALSE(system_tray->HasSystemBubble());
+
+  // Fling down on the system tray should open the bubble if the |velocity_y| is
+  // less than kFlingVelocity but the dragging amount exceed one third of the
+  // height of the bubble.
+  SendGestureEvent(start, delta, true, kFlingVelocity - 1);
+  EXPECT_TRUE(system_tray->HasSystemBubble());
+  system_tray->CloseSystemBubble();
+
+  // Fling down on the system tray should close the bubble if the |velocity_y|
+  // is less than kFlingVelocity and the dragging amount is less than one third
+  // of the height of the bubble.
+  delta /= 4;
+  SendGestureEvent(start, delta, true, kFlingVelocity - 1);
+  EXPECT_FALSE(system_tray->HasSystemBubble());
+}
+
+TEST_F(SystemTrayTest, TapOutsideCloseBubble) {

[tdanderson, 2017/06/20 22:42:31]

Thanks for adding this test after fixing the crash! Very helpful in guarding
against regressions.

+  Shelf* shelf = GetPrimaryShelf();
+  SystemTray* system_tray = GetPrimarySystemTray();
+  gfx::Point start = system_tray->GetBoundsInScreen().CenterPoint();
+  shelf->SetAlignment(SHELF_ALIGNMENT_BOTTOM);
+
+  float delta = -GetSystemBubbleHeight();
+  Shell::Get()->maximize_mode_controller()->EnableMaximizeModeWindowManager(
+      true);
+  base::TimeTicks timestamp = base::TimeTicks::Now();
+  SendScrollStartAndUpdate(start, delta, timestamp);
+  EXPECT_TRUE(system_tray->HasSystemBubble());
+
+  ui::test::EventGenerator& generator = GetEventGenerator();
+  gfx::Rect bounds = system_tray->GetSystemBubble()
+                         ->bubble_view()
+                         ->GetWidget()
+                         ->GetWindowBoundsInScreen();
+  gfx::Point point_outside = gfx::Point(bounds.x() - 5, bounds.y() - 5);
+  generator.GestureTapAt(point_outside);
+  EXPECT_FALSE(system_tray->HasSystemBubble());
+}
+
+TEST_F(SystemTrayTest, SwipingOnSystemTray) {
+  Shelf* shelf = GetPrimaryShelf();
+  SystemTray* system_tray = GetPrimarySystemTray();
+  gfx::Point start = system_tray->GetBoundsInScreen().CenterPoint();
+  shelf->SetAlignment(SHELF_ALIGNMENT_BOTTOM);
+
+  // Swiping up on the system tray has no effect if it is not in maximize mode.
+  float delta = -GetSystemBubbleHeight();
+  Shell::Get()->maximize_mode_controller()->EnableMaximizeModeWindowManager(
+      false);
+  EXPECT_FALSE(system_tray->HasSystemBubble());
+  SendGestureEvent(start, delta, false, 0);
+  EXPECT_FALSE(system_tray->HasSystemBubble());
+
+  // Swiping up on the system tray should show the system tray bubble if it is
+  // in maximize mode.
+  Shell::Get()->maximize_mode_controller()->EnableMaximizeModeWindowManager(
+      true);
+  SendGestureEvent(start, delta, false, 0);
+  EXPECT_TRUE(system_tray->HasSystemBubble());
+  system_tray->CloseSystemBubble();
+
+  // Swiping up less than one third of the bubble's height should not show the
+  // bubble.
+  delta /= 4;
+  SendGestureEvent(start, delta, false, 0);
+  EXPECT_FALSE(system_tray->HasSystemBubble());
+
+  // Swiping up more than one third of the bubble's height should show the
+  // bubble.
+  delta = -GetSystemBubbleHeight() / 2;
+  SendGestureEvent(start, delta, false, 0);
+  EXPECT_TRUE(system_tray->HasSystemBubble());
+  system_tray->CloseSystemBubble();
+
+  // Swiping up on system tray should not show the system tray bubble if the
+  // shelf is left alignment.
+  delta = -GetSystemBubbleHeight();
+  shelf->SetAlignment(SHELF_ALIGNMENT_LEFT);
+  SendGestureEvent(start, delta, false, 0);
+  EXPECT_FALSE(system_tray->HasSystemBubble());
+
+  // Swiping up on system tray should not show the system tray bubble if the
+  // shelf is right alignment.
+  shelf->SetAlignment(SHELF_ALIGNMENT_RIGHT);
+  SendGestureEvent(start, delta, false, 0);
+  EXPECT_FALSE(system_tray->HasSystemBubble());
+
+  // Swiping down on the shelf should not show the system tray bubble.
+  shelf->SetAlignment(SHELF_ALIGNMENT_BOTTOM);
+  delta = -delta;
+  SendGestureEvent(start, delta, false, 0);
+  EXPECT_FALSE(system_tray->HasSystemBubble());
+}
 
 // Verifies only the visible default views are recorded in the
 // "Ash.SystemMenu.DefaultView.VisibleItems" histogram.