Refactor extension app icon.

chrome/browser/extensions/chrome_app_icon_service.h

+// Copyright 2017 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_EXTENSIONS_CHROME_APP_ICON_SERVICE_H_
+#define CHROME_BROWSER_EXTENSIONS_CHROME_APP_ICON_SERVICE_H_
+
+#include <map>
+#include <memory>
+#include <set>
+
+#include "base/macros.h"
+#include "base/memory/weak_ptr.h"
+#include "base/scoped_observer.h"
+#include "components/keyed_service/core/keyed_service.h"
+#include "extensions/browser/extension_registry_observer.h"
+
+namespace content {
+class BrowserContext;
+}
+
+namespace extensions {
+
+class ChromeAppIcon;
+class ChromeAppIconDelegate;
+
+// Factory for ChromeAppIcon. Each created icon is tracked by this service.
+// Once some condition that affects how extension app icon should look is
+// changed then corresponded app icons are automatically updated. This service
+// is bound to content::BrowserContext.
+// Usage: ChromeAppIconService::Get(context)->CreateIcon().
+class ChromeAppIconService : public KeyedService,

[msw, 2017/05/08 19:59:53]

It would be very nice if this functionality (updating icon with extension state
changes) could be folded into ChromeAppIconLoader (or vice versa?), instead of
adding another class and splitting up the functionality.

[khmel, 2017/05/09 00:05:04]

I described in another comments. ChromeAppIconLoader is optional class. Please
read more details at another comment

+                             public ExtensionRegistryObserver {
+ public:
+  explicit ChromeAppIconService(content::BrowserContext* context);
+
+  ~ChromeAppIconService() override;
+
+  // Convenience function to get the ChromeAppIconService for a
+  // BrowserContext.
+  static ChromeAppIconService* Get(content::BrowserContext* context);
+
+  // Creates extension app icon for requested app and size. Icon updates are
+  // dispatched via |delegate|.
+  std::unique_ptr<ChromeAppIcon> CreateIcon(ChromeAppIconDelegate* delegate,
+                                            const std::string& app_id,
+                                            int resource_size_in_dip);
+
+  // KeyedService:
+  void Shutdown() override;
+
+ private:
+  class Updater;
+  using IconMap = std::map<std::string, std::set<ChromeAppIcon*>>;

[msw, 2017/05/08 19:59:53]

nit: comment on why each app would have a set of icons, rather than just one
icon per app.

[khmel, 2017/05/09 00:05:05]

Done.

+
+  // Called from ChromeAppIcon DTOR.
+  void OnIconDestroyed(ChromeAppIcon* icon);
+
+  // Called from Updater when corresponded app icons need to be updated.
+  void OnAppUpdated(const std::string& app_id);
+
+  // ExtensionRegistryObserver:
+  void OnExtensionLoaded(content::BrowserContext* browser_context,
+                         const Extension* extension) override;
+

[msw, 2017/05/08 19:59:54]

nit: remove blank line

[khmel, 2017/05/09 00:05:05]

Done.

+  void OnExtensionUnloaded(content::BrowserContext* browser_context,
+                           const Extension* extension,
+                           UnloadedExtensionReason reason) override;
+
+  // Unowned pointer.
+  content::BrowserContext* context_;
+
+#if defined(OS_CHROMEOS)
+  std::unique_ptr<Updater> app_updater_;

[msw, 2017/05/08 19:59:53]

nit: add a comment

[khmel, 2017/05/09 00:05:05]

Done.

+#endif
+
+  IconMap icon_map_;
+
+  ScopedObserver<ExtensionRegistry, ExtensionRegistryObserver> observer_;
+
+  base::WeakPtrFactory<ChromeAppIconService> weak_ptr_factory_;
+
+  DISALLOW_COPY_AND_ASSIGN(ChromeAppIconService);
+};
+
+}  // namespace extensions
+
+#endif  // CHROME_BROWSER_EXTENSIONS_CHROME_APP_ICON_SERVICE_H_