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

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

Index: chrome/browser/ui/ash/launcher/chrome_launcher_controller_impl.cc
diff --git a/chrome/browser/ui/ash/launcher/chrome_launcher_controller_impl.cc b/chrome/browser/ui/ash/launcher/chrome_launcher_controller_impl.cc
index 5eaee8fdf60c61d4d1b4ce6f51700dfdd90f9dc0..f7932bc4800d31cb85a5d26bc34ec315d3d8c7b5 100644
--- a/chrome/browser/ui/ash/launcher/chrome_launcher_controller_impl.cc
+++ b/chrome/browser/ui/ash/launcher/chrome_launcher_controller_impl.cc
@@ -354,7 +354,14 @@ void ChromeLauncherControllerImpl::CloseLauncherItem(ash::ShelfID id) {
     CHECK(iter != id_to_item_controller_map_.end());
     SetItemStatus(id, ash::STATUS_CLOSED);
     std::string app_id = iter->second->app_id();
-    iter->second = AppShortcutLauncherItemController::Create(app_id, this);
+    std::string launch_id = "";

[stevenjb, 2016/08/30 16:12:51]

' = ""' unnecessary.

[Andra Paraschiv, 2016/08/31 10:57:18]

Done.

+    if (iter->second->type() == LauncherItemController::TYPE_APP) {
+      AppWindowLauncherItemController* app_window_controller =
+          static_cast<AppWindowLauncherItemController*>(iter->second);
+      launch_id = app_window_controller->launch_id();
+    }
+    iter->second =
+        AppShortcutLauncherItemController::Create(app_id, launch_id, this);
     iter->second->set_shelf_id(id);
     // Existing controller is destroyed and replaced by registering again.
     SetShelfItemDelegate(id, iter->second);
@@ -500,7 +507,7 @@ void ChromeLauncherControllerImpl::ActivateApp(const std::string& app_id,
   // Create a temporary application launcher item and use it to see if there are
   // running instances.
   std::unique_ptr<AppShortcutLauncherItemController> app_controller(
-      AppShortcutLauncherItemController::Create(app_id, this));
+      AppShortcutLauncherItemController::Create(app_id, "", this));
   if (!app_controller->GetRunningApplications().empty())
     app_controller->Activate(source);
   else
@@ -895,13 +902,43 @@ void ChromeLauncherControllerImpl::OnShelfVisibilityStateChanged(
 
 ash::ShelfID ChromeLauncherControllerImpl::GetShelfIDForAppID(
     const std::string& app_id) {
-  for (IDToItemControllerMap::const_iterator i =
-           id_to_item_controller_map_.begin();
-       i != id_to_item_controller_map_.end(); ++i) {
-    if (i->second->type() == LauncherItemController::TYPE_APP_PANEL)
+  // Get shelf id for app_id and empty launch_id.
+  ash::ShelfID shelf_id = GetShelfIDForAppIDAndLaunchID(app_id, "");

[stevenjb, 2016/08/30 16:12:52]

No need for intermediate |shelf_id|

[Andra Paraschiv, 2016/08/31 10:57:18]

Done.

+  return shelf_id;
+}
+
+ash::ShelfID ChromeLauncherControllerImpl::GetShelfIDForAppIDAndLaunchID(
+    const std::string& app_id,
+    const std::string& launch_id) {
+  for (const auto& id_to_item_controller_pair : id_to_item_controller_map_) {
+    if (id_to_item_controller_pair.second->type() ==
+        LauncherItemController::TYPE_APP_PANEL)
       continue;  // Don't include panels
-    if (i->second->app_id() == app_id)
-      return i->first;
+    // If app window controller, check app_id and launch_id.

[stevenjb, 2016/08/30 16:12:52]

Move these comments inside the if clauses

[Andra Paraschiv, 2016/08/31 10:57:18]

Done.

+    if (id_to_item_controller_pair.second->type() ==
+        LauncherItemController::TYPE_APP) {
+      AppWindowLauncherItemController* app_window_controller =
+          static_cast<AppWindowLauncherItemController*>(
+              id_to_item_controller_pair.second);
+      if ((app_window_controller->app_id() == app_id) &&
+          (app_window_controller->launch_id() == launch_id)) {
+        return id_to_item_controller_pair.first;
+      }
+      // If not browser shortcut controller, check app_id and launch_id.
+    } else if ((id_to_item_controller_pair.second->type() ==
+                LauncherItemController::TYPE_SHORTCUT) &&
+               (id_to_item_controller_pair.second->app_id() !=
+                extension_misc::kChromeAppId)) {
+      AppShortcutLauncherItemController* app_shortcut_controller =
+          static_cast<AppShortcutLauncherItemController*>(
+              id_to_item_controller_pair.second);
+      if ((app_shortcut_controller->app_id() == app_id) &&
+          (app_shortcut_controller->launch_id() == launch_id)) {
+        return id_to_item_controller_pair.first;
+      }

[stevenjb, 2016/08/30 16:12:51]

These tests are the same, why cast them? e.g.:

if (type == APP || (type == SHORTCUT && app_id != kChrome) {
  if (second->app_id() == app_id && second->launc_id() == launc_id)
    return first;
} else ...

[Andra Paraschiv, 2016/08/31 10:57:18]

Yes, they are the same, but we should include the launch_id in the
LauncherItemController general class to do this checks without specific casts. 

Updated this class to include the launch_id field.

[stevenjb, 2016/08/31 18:24:27]

Ahh, I didn't realize that launch_id() didn't exist in LauncherItemController.
We could make GetLaunchId() a viertual function instead, but this way seems
fine.

+    } else if (id_to_item_controller_pair.second->app_id() == app_id) {
+      return id_to_item_controller_pair.first;
+    }
   }
   return 0;
 }
@@ -1061,7 +1098,7 @@ ChromeLauncherControllerImpl::CreateAppShortcutLauncherItemWithType(
     int index,
     ash::ShelfItemType shelf_item_type) {
   AppShortcutLauncherItemController* controller =
-      AppShortcutLauncherItemController::Create(app_id, this);
+      AppShortcutLauncherItemController::Create(app_id, "", this);
   ash::ShelfID shelf_id = InsertAppLauncherItem(
       controller, app_id, ash::STATUS_CLOSED, index, shelf_item_type);
   return shelf_id;