arc: Set custom icon in shelf for ARC apps.

chrome/browser/ui/ash/launcher/app_window_launcher_item_controller.h

Index: chrome/browser/ui/ash/launcher/app_window_launcher_item_controller.h
diff --git a/chrome/browser/ui/ash/launcher/app_window_launcher_item_controller.h b/chrome/browser/ui/ash/launcher/app_window_launcher_item_controller.h
index 0c7b1eabf060a9122ef3abb90b693cb2084aed3b..303b239dc3a696029cbb2eaafb29d149347bfa7b 100644
--- a/chrome/browser/ui/ash/launcher/app_window_launcher_item_controller.h
+++ b/chrome/browser/ui/ash/launcher/app_window_launcher_item_controller.h
@@ -65,8 +65,8 @@ class AppWindowLauncherItemController : public ash::ShelfItemDelegate,
  protected:
   explicit AppWindowLauncherItemController(const ash::ShelfID& shelf_id);
 
-  // Called when app window is removed from controller.
-  virtual void OnWindowRemoved(ui::BaseWindow* window) {}
+  // Called when active app window is changed.
+  virtual void OnActiveWindowChanged(ui::BaseWindow* active_window) {}

[xiyuan, 2017/05/18 16:22:48]

The name is confusing. I thought it meant something like |last_active_window_|
is changed. But it is not. It is more like the data about the active window
(such as launcher icon for the window) is changed.

How about call it UpdateLauncherIcon?

The "active" part is hard to enforce on the call sites. It is probably better to
let call sites to call with whatever window and we do the active window
filtering inside the function impl.

[khmel, 2017/05/18 17:29:34]

Done. Thanks for offline chat.

 
   // Returns the action performed. Should be one of SHELF_ACTION_NONE,
   // SHELF_ACTION_WINDOW_ACTIVATED, or SHELF_ACTION_WINDOW_MINIMIZED.
@@ -79,6 +79,9 @@ class AppWindowLauncherItemController : public ash::ShelfItemDelegate,
   ash::ShelfAction ActivateOrAdvanceToNextAppWindow(
       ui::BaseWindow* window_to_show);
 
+  // Returns last active window in the controller or top window.

[msw, 2017/05/18 18:26:47]

nit: s/top/first/? (to avoid confusion with MRU, z-order, or placement)
aside (for another cl): should something actually track MRU windows (maybe using
MruWindowTracker and checking against |windows_|) to avoid just returning 'any'
window if the |last_active_window_| is removed? Seems like we could maybe even
get rid of |last_active_window_| in that case.

[khmel, 2017/05/18 20:27:50]

Agree that last_active_window_ looks good target to refactor a bit. Comment
updated.

+  ui::BaseWindow* GetLastActiveWindow();

[msw, 2017/05/18 18:26:48]

nit: make this private if it's not used outside this class.

[khmel, 2017/05/18 20:27:50]

It is used from ArcAppWindowLauncherItemController.

+
  private:
   WindowList::iterator GetFromNativeWindow(aura::Window* window);