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

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

 // Copyright 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/ui/ash/launcher/chrome_launcher_controller_impl.h"
 
 #include <stddef.h>
 
 #include <vector>
 
@@ skipping 336 matching lines @@
 }
 
 void ChromeLauncherControllerImpl::CloseLauncherItem(ash::ShelfID id) {
   CHECK(id);
   if (IsPinned(id)) {
     // Create a new shortcut controller.
     IDToItemControllerMap::iterator iter = id_to_item_controller_map_.find(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);
   } else {
     LauncherItemClosed(id);
   }
 }
 
 void ChromeLauncherControllerImpl::Pin(ash::ShelfID id) {
   DCHECK(HasShelfIDToAppIDMapping(id));
@@ skipping 125 matching lines @@
   ash::ShelfID id = GetShelfIDForAppID(app_id);
   if (id) {
     LauncherItemController* controller = GetLauncherItemController(id);
     controller->Activate(source);
     return;
   }
 
   // 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
     LaunchApp(app_id, source, event_flags);
 }
 
 extensions::LaunchType ChromeLauncherControllerImpl::GetLaunchType(
     ash::ShelfID id) {
   const Extension* extension =
       GetExtensionForAppID(GetAppIDForShelfID(id), profile_);
@@ skipping 374 matching lines @@
 }
 
 void ChromeLauncherControllerImpl::OnShelfAutoHideStateChanged(
     ash::WmShelf* shelf) {}
 
 void ChromeLauncherControllerImpl::OnShelfVisibilityStateChanged(
     ash::WmShelf* shelf) {}
 
 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;
 }
 
 bool ChromeLauncherControllerImpl::HasShelfIDToAppIDMapping(
     ash::ShelfID id) const {
   return id_to_item_controller_map_.find(id) !=
          id_to_item_controller_map_.end();
 }
 
@@ skipping 139 matching lines @@
     }
   }
 }
 
 ash::ShelfID
 ChromeLauncherControllerImpl::CreateAppShortcutLauncherItemWithType(
     const std::string& app_id,
     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;
 }
 
 void ChromeLauncherControllerImpl::LauncherItemClosed(ash::ShelfID id) {
   IDToItemControllerMap::iterator iter = id_to_item_controller_map_.find(id);
   CHECK(iter != id_to_item_controller_map_.end());
   CHECK(iter->second);
   const std::string& app_id = iter->second->app_id();
@@ skipping 607 matching lines @@
     if (index == -1)
       continue;
     ash::ShelfItem item = model_->items()[index];
     item.image = image;
     if (arc_deferred_launcher_)
       arc_deferred_launcher_->MaybeApplySpinningEffect(id, &item.image);
     model_->Set(index, item);
     // It's possible we're waiting on more than one item, so don't break.
   }
 }