arc: Set custom icon in shelf for ARC apps.

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

 // Copyright 2014 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.
 
 #ifndef CHROME_BROWSER_UI_ASH_LAUNCHER_APP_WINDOW_LAUNCHER_ITEM_CONTROLLER_H_
 #define CHROME_BROWSER_UI_ASH_LAUNCHER_APP_WINDOW_LAUNCHER_ITEM_CONTROLLER_H_
 
 #include <list>
 #include <string>
 
@@ skipping 47 matching lines @@
   size_t window_count() const { return windows_.size(); }
 
   // Activates the window at position |index|.
   void ActivateIndexedApp(size_t index);
 
   const WindowList& windows() const { return windows_; }
 
  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.
   ash::ShelfAction ShowAndActivateOrMinimize(ui::BaseWindow* window);
 
   // Activate the given |window_to_show|, or - if already selected - advance to
   // the next window of similar type.
   // Returns the action performed. Should be one of SHELF_ACTION_NONE,
   // SHELF_ACTION_WINDOW_ACTIVATED, or SHELF_ACTION_WINDOW_MINIMIZED.
   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);
 
   // List of associated app windows
   WindowList windows_;
 
   // Pointer to the most recently active app window
   ui::BaseWindow* last_active_window_ = nullptr;
 
   // Scoped list of observed windows (for removal on destruction)
   ScopedObserver<aura::Window, aura::WindowObserver> observed_windows_;
 
   DISALLOW_COPY_AND_ASSIGN(AppWindowLauncherItemController);
 };
 
 #endif  // CHROME_BROWSER_UI_ASH_LAUNCHER_APP_WINDOW_LAUNCHER_ITEM_CONTROLLER_H_