Make the App Info Dialog appear modal

chrome/browser/ui/app_list/app_list_service_views_browsertest.cc

Index: chrome/browser/ui/app_list/app_list_service_views_browsertest.cc
diff --git a/chrome/browser/ui/app_list/app_list_service_views_browsertest.cc b/chrome/browser/ui/app_list/app_list_service_views_browsertest.cc
index 7bee1a8988873b191c3bf76423b50814fa465f7d..99c45792f26d1e92bcd96271ec6898b414a6b390 100644
--- a/chrome/browser/ui/app_list/app_list_service_views_browsertest.cc
+++ b/chrome/browser/ui/app_list/app_list_service_views_browsertest.cc
@@ -4,10 +4,15 @@
 
 #include "chrome/browser/ui/app_list/app_list_service_views.h"
 
+#include "base/path_service.h"
 #include "base/run_loop.h"
+#include "chrome/browser/extensions/extension_browsertest.h"
+#include "chrome/browser/ui/app_list/app_list_controller_delegate.h"
 #include "chrome/browser/ui/app_list/test/chrome_app_list_test_support.h"
 #include "chrome/browser/ui/browser.h"
+#include "chrome/common/chrome_paths.h"
 #include "chrome/test/base/in_process_browser_test.h"
+#include "ui/app_list/app_list_switches.h"
 #include "ui/views/widget/widget.h"
 
 // Browser Test for AppListService on Views platforms.
@@ -42,3 +47,60 @@ IN_PROC_BROWSER_TEST_F(AppListServiceViewsBrowserTest, NativeClose) {
   // Note: no need to sink message queue.
   EXPECT_FALSE(service->GetAppListWindow());
 }
+
+// Browser Test for AppListController that ensures the App Info dialog opens
+// correctly.
+typedef ExtensionBrowserTest AppListControllerAppInfoDialogBrowserTest;
+
+// Test the DoShowAppInfoFlow function of the controller delegate.
+IN_PROC_BROWSER_TEST_F(AppListControllerAppInfoDialogBrowserTest,
+                       DoShowAppInfoFlow) {
+  // Install an extension to open the dialog for.
+  test::SigninProfile(browser()->profile());
+  base::FilePath test_extension_path;
+  ASSERT_TRUE(PathService::Get(chrome::DIR_TEST_DATA, &test_extension_path));
+  test_extension_path = test_extension_path.AppendASCII("extensions")
+                            .AppendASCII("platform_apps")
+                            .AppendASCII("minimal");
+  const extensions::Extension* extension = InstallExtension(
+      test_extension_path, 1 /* expected_change: new install */);
+  ASSERT_TRUE(extension);
+
+  // Open the app list window.
+  AppListService* service = test::GetAppListService();
+  EXPECT_FALSE(service->GetAppListWindow());
+
+  service->ShowForProfile(browser()->profile());
+  gfx::NativeWindow window = service->GetAppListWindow();
+  EXPECT_TRUE(window);
+
+  views::Widget* widget = views::Widget::GetWidgetForNativeWindow(window);
+  ASSERT_TRUE(widget);
+
+  // Enable the 'App Info dialog' command switch so that the dialog is enabled.
+  CommandLine::ForCurrentProcess()->AppendSwitch(

[tapted, 2014/05/23 02:09:21]

kindaofanit: for browser tests, this should be done in 

AppListControllerAppInfoDialogBrowserTest::SetUpCommandLine(
    CommandLine* command_line) {

};

[sashab, 2014/05/23 06:10:21]

Done.

+      app_list::switches::kEnableAppInfo);
+
+  // Open the app info dialog.
+  views::Widget::Widgets owned_widgets;
+  widget->GetAllOwnedWidgets(window, &owned_widgets);
+  EXPECT_EQ(0U, owned_widgets.size());
+
+  AppListControllerDelegate* controller(service->GetControllerDelegate());

[tapted, 2014/05/23 02:09:21]

nit (for consistency): controller = service->GetControllerDelegate();

initializer syntax isn't usually used for primitive types like pointers.

[sashab, 2014/05/23 06:10:21]

Done.

+  ASSERT_TRUE(controller);
+  controller->DoShowAppInfoFlow(browser()->profile(), extension->id());

[tapted, 2014/05/23 02:09:21]

I think this test can be made a lot more compelling for this CL (in particular)
with a TestApi that exposes the overlay private member. You can then check that
it gets made visible, and invisible again when the child widget is closed.

apps_grid_view_unittest.cc has a good example of TestApi usage in a test (i.e.
src/ui/app_list/views/test/apps_grid_view_test_api.h). This one can probably be
a bit simpler, but there are some tricks to getting the view in the browser
tests (rather than a unit test). Let's huddle.

[sashab, 2014/05/23 06:10:21]

Done.

+  base::RunLoop().RunUntilIdle();
+
+  owned_widgets.clear();
+  widget->GetAllOwnedWidgets(window, &owned_widgets);
+  EXPECT_EQ(1U, owned_widgets.size());
+
+  // Close the app info dialog.
+  views::Widget* app_info_dialog = *owned_widgets.begin();
+  app_info_dialog->Close();
+  base::RunLoop().RunUntilIdle();
+
+  owned_widgets.clear();
+  widget->GetAllOwnedWidgets(window, &owned_widgets);
+  EXPECT_EQ(0U, owned_widgets.size());
+}