Enhance chrome.app.window API for shelf integration with restore support

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

Index: chrome/browser/ui/ash/launcher/app_shortcut_launcher_item_controller.cc
diff --git a/chrome/browser/ui/ash/launcher/app_shortcut_launcher_item_controller.cc b/chrome/browser/ui/ash/launcher/app_shortcut_launcher_item_controller.cc
index 059ebcbb3512583fa764a9e146732539fa2961f9..f78b81291bf22c07c10c2c1fcef8a9a4358f73d2 100644
--- a/chrome/browser/ui/ash/launcher/app_shortcut_launcher_item_controller.cc
+++ b/chrome/browser/ui/ash/launcher/app_shortcut_launcher_item_controller.cc
@@ -109,7 +109,8 @@ bool AppShortcutLauncherItemController::IsVisible() const {
 
 void AppShortcutLauncherItemController::Launch(ash::LaunchSource source,
                                                int event_flags) {
-  launcher_controller()->LaunchApp(app_id(), source, event_flags);
+  launcher_controller()->LaunchAppWithLaunchId(app_id(), launch_id(), source,
+                                               event_flags);

[stevenjb, 2016/12/09 17:00:57]

Why use WithLaunchId with an empty launch_id() ?

[Andra Paraschiv, 2016/12/12 14:28:22]

Launch id should be non-empty if the window was created with showInShelf=true
and the id in CreateWindowOptions was set. When the window is pinned, then
closed, the launch_id is stored in the AppShorcutLauncherItem
(https://cs.chromium.org/chromium/src/chrome/browser/ui/ash/launcher/chrome_la...)

[stevenjb, 2016/12/13 19:31:14]

Let me be more clear:

Why not use launcher_controller()->LaunchApp()?

[Andra Paraschiv, 2016/12/14 14:59:38]

In this case, if we use LaunchApp instead of LaunchAppWithLaunchId, the
launch_id parameter that will be passed down the following call list will be
empty:

AppShorcutLauncherItemController::Launch
ChromeLauncherController::LaunchApp(app_id, ...)
LauncherControllerHelper::LaunchApp(app_id, ...)
LauncherControllerHelper::LaunchAppWithLaunchId(app_id, ""/*launch_id*/, ...) <-
in this method, the launch_id value will be used to set the launch_id
application parameter (params.launch_id). This parameter value will be used to
set the launchData.id. 

In order to restore a pinned window that was closed before, we should pass the
launch_id parameter value (that can be non-empty for showInShelf=true windows)
down through the call list to the onLaunched event launchData.id -
https://developer.chrome.com/apps/app_runtime#event-onLaunched. This way, if the
id is not empty, only the current pinned window will be restored and not the
whole set of windows that have the same app_id.

Thank you.

[stevenjb, 2016/12/14 18:40:21]

Ugh. My apologies. I was somehow reading that as LaunchId() not launch_id().
Sorry about that, too many code reviews.

[Andra Paraschiv, 2016/12/15 10:20:15]

There's no problem, Steven, it's good we have this discussions based on the
review in order to understand which it's the right way to include the launch_id
in the codebase.

Thanks, and I also added a unit test for the restore part in this CL -
https://codereview.chromium.org/2297633002/ .   

 }
 
 ash::ShelfItemDelegate::PerformedAction