Enhance chrome.app.window API with better shelf integration

chrome/browser/ui/ash/launcher/chrome_launcher_controller_browsertest.cc

Index: chrome/browser/ui/ash/launcher/chrome_launcher_controller_browsertest.cc
diff --git a/chrome/browser/ui/ash/launcher/chrome_launcher_controller_browsertest.cc b/chrome/browser/ui/ash/launcher/chrome_launcher_controller_browsertest.cc
index af5321e305de9196353e895b18ca213b8d4ea03a..2a8340b67ca757ca99d31419bdea161ce0bba0de 100644
--- a/chrome/browser/ui/ash/launcher/chrome_launcher_controller_browsertest.cc
+++ b/chrome/browser/ui/ash/launcher/chrome_launcher_controller_browsertest.cc
@@ -1492,6 +1492,89 @@ IN_PROC_BROWSER_TEST_F(LauncherPlatformAppBrowserTest, WindowAttentionStatus) {
   EXPECT_EQ(ash::STATUS_ACTIVE, item.status);
 }
 
+IN_PROC_BROWSER_TEST_F(LauncherPlatformAppBrowserTest, ShowInShelfWindows) {
+  ash::ShelfModel* shelf_model = ash::Shell::GetInstance()->shelf_model();
+
+  // Test 1

[stevenjb, 2016/04/26 16:30:00]

Describe what each part of this test is testing for instead of 'Test 1', etc.

[Valentin Ilie, 2016/04/28 09:40:09]

Done.

+  int item_count = shelf_model->item_count();
+  const Extension* extension = LoadAndLaunchPlatformApp("launch", "Launched");
+  AppWindow::CreateParams params;
+  params.show_in_shelf = true;
+  AppWindow* window1 =
+      CreateAppWindowFromParams(browser()->profile(), extension, params);
+  // There should be only 1 item added to the shelf.
+  item_count++;

[stevenjb, 2016/04/26 16:30:00]

nit: instead of incrementing item_count here and below, this would be a little
more readable as: EXPECT_EQ(item_count + 1, ...)

[Valentin Ilie, 2016/04/28 09:40:09]

Done.

+  EXPECT_EQ(item_count, shelf_model->item_count());
+  CloseAppWindow(window1);
+
+  // Test 2
+  item_count = shelf_model->item_count();
+  extension = LoadAndLaunchPlatformApp("launch", "Launched");
+
+  params.show_in_shelf = false;
+  window1 = CreateAppWindowFromParams(browser()->profile(), extension, params);
+  params.show_in_shelf = true;
+  AppWindow* window2 =
+      CreateAppWindowFromParams(browser()->profile(), extension, params);
+  // There should be 2 items added to the shelf: although window1 has
+  // show_in_shelf set to false, it's the first window created so its icon must
+  // show up in shelf.
+  item_count += 2;
+  EXPECT_EQ(item_count, shelf_model->item_count());
+  CloseAppWindow(window1);
+  CloseAppWindow(window2);
+
+  // Test 3
+  item_count = shelf_model->item_count();
+  extension = LoadAndLaunchPlatformApp("launch", "Launched");
+
+  params.show_in_shelf = false;
+  window1 = CreateAppWindowFromParams(browser()->profile(), extension, params);
+  // There should be 1 item added to the shelf: although show_in_shelf is false,
+  // this is the first window created.
+  item_count++;
+  EXPECT_EQ(item_count, shelf_model->item_count());
+  CloseAppWindow(window1);
+
+  // Test 4
+  item_count = shelf_model->item_count();
+
+  extension = LoadAndLaunchPlatformApp("launch", "Launched");
+
+  params.show_in_shelf = true;
+  window1 = CreateAppWindowFromParams(browser()->profile(), extension, params);
+  params.show_in_shelf = false;
+  window2 = CreateAppWindowFromParams(browser()->profile(), extension, params);
+  // There should be 1 item added to the shelf.
+  item_count += 1;

[stevenjb, 2016/04/26 16:30:00]

This seems incorrect. In the second test, 2 items are created. The creation
order shouldn't matter. window 2 should be associated with the "default" icon
and window 1 should have a separate item.

[Valentin Ilie, 2016/04/28 09:40:09]

We should decide the appropriate behavior here.

+  EXPECT_EQ(item_count, shelf_model->item_count());
+  CloseAppWindow(window1);

[stevenjb, 2016/04/26 16:30:00]

We should also test that the shelf item(s) go away when they are supposed to.
e.g. after every CloseAppWindow we should test item_count. 

[Valentin Ilie, 2016/04/28 09:40:09]

Done.

+  CloseAppWindow(window2);
+
+  // Test 5
+  item_count = shelf_model->item_count();
+
+  extension = LoadAndLaunchPlatformApp("launch", "Launched");
+  params.show_in_shelf = false;
+  window1 = CreateAppWindowFromParams(browser()->profile(), extension, params);
+  params.show_in_shelf = false;
+  window2 = CreateAppWindowFromParams(browser()->profile(), extension, params);
+  params.show_in_shelf = true;
+  AppWindow* window3 =
+      CreateAppWindowFromParams(browser()->profile(), extension, params);
+  params.show_in_shelf = true;
+  AppWindow* window4 =
+      CreateAppWindowFromParams(browser()->profile(), extension, params);
+  // There should be 3 item added to the shelf.

[stevenjb, 2016/04/26 16:30:00]

s/item/items/

[Valentin Ilie, 2016/04/28 09:40:09]

Done.

+  item_count += 3;
+  EXPECT_EQ(item_count, shelf_model->item_count());
+  // Any window close order should be valid
+  CloseAppWindow(window4);
+  CloseAppWindow(window1);
+  CloseAppWindow(window3);
+  CloseAppWindow(window2);
+}
+
 // Checks that the browser Alt "tabbing" is properly done.
 IN_PROC_BROWSER_TEST_F(ShelfAppBrowserTestNoDefaultBrowser,
                        AltNumberBrowserTabbing) {