Enhance chrome.app.window API with better shelf integration

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

Index: chrome/browser/ui/ash/launcher/extension_app_window_launcher_controller.cc
diff --git a/chrome/browser/ui/ash/launcher/extension_app_window_launcher_controller.cc b/chrome/browser/ui/ash/launcher/extension_app_window_launcher_controller.cc
index d3d80af4098e70cf967eb808caea1a26930810fd..f4ce2b932c59b225cb921231b9b992291f662cf9 100644
--- a/chrome/browser/ui/ash/launcher/extension_app_window_launcher_controller.cc
+++ b/chrome/browser/ui/ash/launcher/extension_app_window_launcher_controller.cc
@@ -8,6 +8,7 @@
 #include "ash/shelf/shelf_util.h"
 #include "ash/shell.h"
 #include "ash/wm/window_util.h"
+#include "base/stl_util.h"
 #include "base/strings/stringprintf.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/ui/ash/launcher/chrome_launcher_controller.h"
@@ -24,9 +25,18 @@ using extensions::AppWindowRegistry;
 namespace {
 
 std::string GetAppShelfId(AppWindow* app_window) {
-  if (app_window->window_type_is_panel())
-    return base::StringPrintf("panel:%d", app_window->session_id().id());
-  return app_window->extension_id();
+  // Set app_shelf_id default value to extension_id. If showInShelf parameter
+  // is true or the window type is panel and the window key is not empty, its
+  // value is appended to the app_shelf_id. Otherwise, if the window key is
+  // empty, the session_id is used.
+  std::string app_shelf_id = app_window->extension_id();
+  if (app_window->show_in_shelf() || app_window->window_type_is_panel()) {
+    if (!app_window->window_key().empty())
+      app_shelf_id += app_window->window_key();
+    else
+      app_shelf_id += base::StringPrintf("%d", app_window->session_id().id());
+  }
+  return app_shelf_id;
 }
 
 }  // namespace
@@ -121,10 +131,13 @@ void ExtensionAppWindowLauncherController::RegisterApp(AppWindow* app_window) {
   ash::ShelfItemStatus status = ash::wm::IsActiveWindow(window)
                                     ? ash::STATUS_ACTIVE
                                     : ash::STATUS_RUNNING;
-  AppControllerMap::iterator iter = app_controller_map_.find(app_shelf_id);
+  AppControllerMap::iterator app_controller_iter =
+      app_controller_map_.find(app_shelf_id);
   ash::ShelfID shelf_id = 0;
-  if (iter != app_controller_map_.end()) {
-    ExtensionAppWindowLauncherItemController* controller = iter->second;
+
+  if (app_controller_iter != app_controller_map_.end()) {
+    ExtensionAppWindowLauncherItemController* controller =
+        app_controller_iter->second;
     DCHECK(controller->app_id() == app_id);
     shelf_id = controller->shelf_id();
     controller->AddAppWindow(app_window);
@@ -137,13 +150,34 @@ void ExtensionAppWindowLauncherController::RegisterApp(AppWindow* app_window) {
         new ExtensionAppWindowLauncherItemController(type, app_shelf_id, app_id,
                                                      owner());
     controller->AddAppWindow(app_window);
-    // If the app shelf id is not unique, and there is already a shelf
-    // item for this app id (e.g. pinned), use that shelf item.
-    if (app_shelf_id == app_id) {
+    // If there is already a shelf id mapped to this app_shelf_id (e.g. pinned),
+    // use that shelf item.
+    AppShelfIdToShelfIdMap::iterator app_shelf_id_iter =
+        app_shelf_id_to_shelf_id_map_.find(app_shelf_id);
+    if (app_shelf_id_iter != app_shelf_id_to_shelf_id_map_.end()) {
+      if (owner()->IsPinned(app_shelf_id_iter->second)) {
+        shelf_id = app_shelf_id_iter->second;
+      } else {
+        app_shelf_id_to_shelf_id_map_.erase(app_shelf_id);
+      }
+    } else if (app_shelf_id == app_id) {

[stevenjb, 2016/07/15 16:58:31]

Add a comment:
// show_in_shelf is false and not a panel.

[Valentin Ilie, 2016/07/18 11:05:20]

Done.

       shelf_id =
-          ash::Shell::GetInstance()->GetShelfDelegate()->GetShelfIDForAppID(
-              app_id);
+         ash::Shell::GetInstance()->GetShelfDelegate()->GetShelfIDForAppID(
+             app_id);
+      // Check if the shelf_id corresponds to an already opened
+      // showInShelf=true window that has the same app_id. The current
+      // showInShelf=false window should not fold under this shelf item,
+      // so the shelf_id is set to 0 to get a new shelf_id.
+      for (AppShelfIdToShelfIdMap::const_iterator i =
+           app_shelf_id_to_shelf_id_map_.begin();
+           i != app_shelf_id_to_shelf_id_map_.end(); ++i) {
+        if (shelf_id == i->second) {
+           shelf_id = 0;
+           break;
+        }
+      }

[stevenjb, 2016/07/15 16:58:31]

We should be able to do this with find + lambda, e.g. something like:

  const auto& id_map = app_shelf_id_to_shelf_id_map_;
  if (std::find_if(id_map.begin(), id_map.end(), [shelf_id](iter) {
        return iter->second == shelf_id; }) != id_map.end()) {
    shelf_id = 0;
  }

[Valentin Ilie, 2016/07/18 11:05:20]

Done.

     }
+
     if (shelf_id == 0) {
       shelf_id = owner()->CreateAppLauncherItem(controller, app_id, status);
       // Restore any existing app icon and flag as set.
@@ -155,31 +189,39 @@ void ExtensionAppWindowLauncherController::RegisterApp(AppWindow* app_window) {
     } else {
       owner()->SetItemController(shelf_id, controller);
     }
-    const std::string app_shelf_id = GetAppShelfId(app_window);
+
+    // We need to change the controller associated with app_shelf_id.
     app_controller_map_[app_shelf_id] = controller;
+    app_shelf_id_to_shelf_id_map_[app_shelf_id] = shelf_id;
   }
   owner()->SetItemStatus(shelf_id, status);
   ash::SetShelfIDForWindow(shelf_id, window);
 }
 
 void ExtensionAppWindowLauncherController::UnregisterApp(aura::Window* window) {
-  WindowToAppShelfIdMap::iterator iter1 =
+  WindowToAppShelfIdMap::iterator window_iter =
       window_to_app_shelf_id_map_.find(window);
-  DCHECK(iter1 != window_to_app_shelf_id_map_.end());
-  std::string app_shelf_id = iter1->second;
-  window_to_app_shelf_id_map_.erase(iter1);
+  DCHECK(window_iter != window_to_app_shelf_id_map_.end());
+  std::string app_shelf_id = window_iter->second;
+  window_to_app_shelf_id_map_.erase(window_iter);
   window->RemoveObserver(this);
 
-  AppControllerMap::iterator iter2 = app_controller_map_.find(app_shelf_id);
-  DCHECK(iter2 != app_controller_map_.end());
-  ExtensionAppWindowLauncherItemController* controller = iter2->second;
+  AppControllerMap::iterator app_controller_iter =
+      app_controller_map_.find(app_shelf_id);
+  DCHECK(app_controller_iter != app_controller_map_.end());
+  ExtensionAppWindowLauncherItemController* controller;
+  controller = app_controller_iter->second;
+
   controller->RemoveWindow(controller->GetAppWindow(window));
   if (controller->window_count() == 0) {
-    // If this is the last window associated with the app shelf id, close the
-    // shelf item.
+    // If this is the last window associated with the app window shelf id,
+    // close the shelf item.
     ash::ShelfID shelf_id = controller->shelf_id();
+    if (!owner()->IsPinned(shelf_id)) {
+      app_shelf_id_to_shelf_id_map_.erase(app_shelf_id);
+    }
     owner()->CloseLauncherItem(shelf_id);
-    app_controller_map_.erase(iter2);
+    app_controller_map_.erase(app_controller_iter);
   }
 }
 
@@ -194,13 +236,13 @@ bool ExtensionAppWindowLauncherController::IsRegisteredApp(
 AppWindowLauncherItemController*
 ExtensionAppWindowLauncherController::ControllerForWindow(
     aura::Window* window) {
-  WindowToAppShelfIdMap::iterator iter1 =
+  WindowToAppShelfIdMap::iterator window_iter =
       window_to_app_shelf_id_map_.find(window);
-  if (iter1 == window_to_app_shelf_id_map_.end())
+  if (window_iter == window_to_app_shelf_id_map_.end())
     return nullptr;
-  std::string app_shelf_id = iter1->second;
-  AppControllerMap::iterator iter2 = app_controller_map_.find(app_shelf_id);
-  if (iter2 == app_controller_map_.end())
+  AppControllerMap::iterator app_controller_iter =
+      app_controller_map_.find(window_iter->second);
+  if (app_controller_iter == app_controller_map_.end())
     return nullptr;
-  return iter2->second;
+  return app_controller_iter->second;
 }