Implementation of a full screen app list and re-alphabetized switches

ash/app_list/app_list_presenter_delegate_unittest.cc

Index: ash/app_list/app_list_presenter_delegate_unittest.cc
diff --git a/ash/app_list/app_list_presenter_delegate_unittest.cc b/ash/app_list/app_list_presenter_delegate_unittest.cc
index d554816c4b30abc599c2ef45436ad97f7e2baf9f..882656e0c9d2991d88bc1743046a209eb4f76688 100644
--- a/ash/app_list/app_list_presenter_delegate_unittest.cc
+++ b/ash/app_list/app_list_presenter_delegate_unittest.cc
@@ -4,6 +4,7 @@
 
 #include <memory>
 
+#include "ash/ash_switches.h"
 #include "ash/public/cpp/shell_window_ids.h"
 #include "ash/shell.h"
 #include "ash/shell_port.h"
@@ -11,7 +12,9 @@
 #include "ash/test/test_app_list_view_presenter_impl.h"
 #include "ash/wm/window_util.h"
 #include "ash/wm_window.h"
+#include "base/command_line.h"
 #include "base/macros.h"
+#include "ui/app_list/app_list_switches.h"
 #include "ui/app_list/views/app_list_view.h"
 #include "ui/aura/test/test_windows.h"
 #include "ui/aura/window.h"
@@ -26,9 +29,16 @@ int64_t GetPrimaryDisplayId() {
   return display::Screen::GetScreen()->GetPrimaryDisplay().id();
 }
 
+void SetFullscreenAppListSwitch() {

[vadimt, 2017/04/19 00:57:15]

You can define it in app_list_switches.h and use everywhere.

[newcomer, 2017/04/20 19:59:01]

Done.

[vadimt, 2017/04/20 20:21:34]

Thanks, but I made a mistake :(, I meant the method that *checks* the flag...

[newcomer, 2017/04/21 19:51:22]

 Roger and Done! I moved both functions into app_list_switches.cc. I'll talk
with you to confirm if you want both in app_list_switches or just the function
that checks the flag.

+  base::CommandLine::ForCurrentProcess()->AppendSwitch(
+      app_list::switches::kEnableFullscreenAppList);
+}
+
 }  // namespace
 
-class AppListPresenterDelegateTest : public test::AshTestBase {
+class AppListPresenterDelegateTest
+    : public test::AshTestBase,
+      public ::testing::WithParamInterface<bool> {

[vadimt, 2017/04/19 00:57:15]

Do we need that '::' before 'testing'?

[newcomer, 2017/04/20 19:59:01]

Done.

  public:
   AppListPresenterDelegateTest() {}
   ~AppListPresenterDelegateTest() override {}
@@ -51,8 +61,16 @@ class AppListPresenterDelegateTest : public test::AshTestBase {
   DISALLOW_COPY_AND_ASSIGN(AppListPresenterDelegateTest);
 };
 
+// Instantiate the Boolean which is used to toggle the Fullscreen app list in
+// the parameterized tests.
+INSTANTIATE_TEST_CASE_P(, AppListPresenterDelegateTest, testing::Bool());
+
 // Tests that app launcher hides when focus moves to a normal window.
-TEST_F(AppListPresenterDelegateTest, HideOnFocusOut) {
+TEST_P(AppListPresenterDelegateTest, HideOnFocusOut) {
+  bool testWithFullscreen = GetParam();

[sky, 2017/04/19 15:18:51]

test_with_fullscreen That said, why don't you configre this in SetUp() so you
don't need it in each test?

[newcomer, 2017/04/20 19:59:01]

Done! Check SetUp().

+  if (testWithFullscreen)
+    SetFullscreenAppListSwitch();
+
   app_list_presenter_impl()->Show(GetPrimaryDisplayId());
   EXPECT_TRUE(app_list_presenter_impl()->GetTargetVisibility());
 
@@ -64,8 +82,12 @@ TEST_F(AppListPresenterDelegateTest, HideOnFocusOut) {
 
 // Tests that app launcher remains visible when focus is moved to a different
 // window in kShellWindowId_AppListContainer.
-TEST_F(AppListPresenterDelegateTest,
+TEST_P(AppListPresenterDelegateTest,
        RemainVisibleWhenFocusingToApplistContainer) {
+  bool testWithFullscreen = GetParam();
+  if (testWithFullscreen)
+    SetFullscreenAppListSwitch();
+
   app_list_presenter_impl()->Show(GetPrimaryDisplayId());
   EXPECT_TRUE(app_list_presenter_impl()->GetTargetVisibility());
 
@@ -79,7 +101,10 @@ TEST_F(AppListPresenterDelegateTest,
 }
 
 // Tests that clicking outside the app-list bubble closes it.
-TEST_F(AppListPresenterDelegateTest, ClickOutsideBubbleClosesBubble) {
+TEST_P(AppListPresenterDelegateTest, ClickOutsideBubbleClosesBubble) {
+  bool testWithFullscreen = GetParam();
+  if (testWithFullscreen)
+    SetFullscreenAppListSwitch();
   app_list_presenter_impl()->Show(GetPrimaryDisplayId());
   aura::Window* app_window = app_list_presenter_impl()->GetWindow();
   ASSERT_TRUE(app_window);
@@ -100,7 +125,10 @@ TEST_F(AppListPresenterDelegateTest, ClickOutsideBubbleClosesBubble) {
 }
 
 // Tests that clicking outside the app-list bubble closes it.
-TEST_F(AppListPresenterDelegateTest, TapOutsideBubbleClosesBubble) {
+TEST_P(AppListPresenterDelegateTest, TapOutsideBubbleClosesBubble) {
+  bool testWithFullscreen = GetParam();
+  if (testWithFullscreen)
+    SetFullscreenAppListSwitch();
   app_list_presenter_impl()->Show(GetPrimaryDisplayId());
 
   aura::Window* app_window = app_list_presenter_impl()->GetWindow();
@@ -122,8 +150,12 @@ TEST_F(AppListPresenterDelegateTest, TapOutsideBubbleClosesBubble) {
 
 // Tests opening the app launcher on a non-primary display, then deleting the
 // display.
-TEST_F(AppListPresenterDelegateTest, NonPrimaryDisplay) {
+TEST_P(AppListPresenterDelegateTest, NonPrimaryDisplay) {
   // Set up a screen with two displays (horizontally adjacent).
+  bool testWithFullscreen = GetParam();
+  if (testWithFullscreen)
+    SetFullscreenAppListSwitch();
+
   UpdateDisplay("1024x768,1024x768");
 
   std::vector<WmWindow*> root_windows = ShellPort::Get()->GetAllRootWindows();
@@ -156,10 +188,12 @@ TEST_F(AppListPresenterDelegateTest, TinyDisplay) {
   // from the anchor (center) and height. There isn't a bounds rect that gives
   // the actual app list position (the widget bounds include the bubble border
   // which is much bigger than the actual app list size).
+
   app_list::AppListView* app_list = app_list_presenter_impl()->GetView();
   int app_list_view_top =
       app_list->anchor_rect().y() - app_list->bounds().height() / 2;
   const int kMinimalAppListMargin = 10;
+
   EXPECT_GE(app_list_view_top, kMinimalAppListMargin);
 }