Refactor extension app icon.

chrome/browser/extensions/extension_app_icon_service.cc

Index: chrome/browser/extensions/extension_app_icon_service.cc
diff --git a/chrome/browser/extensions/extension_app_icon_service.cc b/chrome/browser/extensions/extension_app_icon_service.cc
new file mode 100644
index 0000000000000000000000000000000000000000..eba5165640be4b77e7eabc954a135804cd1b7398
--- /dev/null
+++ b/chrome/browser/extensions/extension_app_icon_service.cc
@@ -0,0 +1,119 @@
+// 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.
+
+#include "chrome/browser/extensions/extension_app_icon_service.h"
+
+#include "base/bind.h"
+#include "base/memory/ptr_util.h"
+#include "chrome/browser/extensions/extension_app_icon.h"
+#include "chrome/browser/extensions/extension_app_icon_service_factory.h"
+#include "extensions/browser/extension_registry.h"
+
+#if defined(OS_CHROMEOS)
+#include "chrome/browser/ui/ash/launcher/launcher_extension_app_updater.h"
+#endif
+
+namespace extensions {
+
+#if defined(OS_CHROMEOS)
+
+class ExtensionAppIconService::Updater : public LauncherAppUpdater::Delegate {
+ public:
+  Updater(ExtensionAppIconService* host, content::BrowserContext* context)
+      : host_(host), extension_app_updater_(this, context) {}
+
+  ~Updater() override = default;
+
+ private:
+  // LauncherAppUpdater::Delegate:
+  void OnAppUpdated(content::BrowserContext* browser_context,
+                    const std::string& app_id) override {
+    host_->OnAppUpdated(app_id);
+  }
+
+  // Unowned pointer.
+  ExtensionAppIconService* const host_;
+
+  LauncherExtensionAppUpdater extension_app_updater_;
+
+  DISALLOW_COPY_AND_ASSIGN(Updater);
+};
+
+#endif
+
+// static
+ExtensionAppIconService* ExtensionAppIconService::Get(
+    content::BrowserContext* context) {
+  return ExtensionAppIconServiceFactory::GetInstance()->GetForBrowserContext(
+      context);
+}
+
+ExtensionAppIconService::ExtensionAppIconService(
+    content::BrowserContext* context)
+    : context_(context), observer_(this), weak_ptr_factory_(this) {
+#if defined(OS_CHROMEOS)
+  app_updater_ = base::MakeUnique<Updater>(this, context);
+#endif
+
+  observer_.Add(ExtensionRegistry::Get(context_));
+}
+
+ExtensionAppIconService::~ExtensionAppIconService() = default;
+
+void ExtensionAppIconService::Shutdown() {
+#if defined(OS_CHROMEOS)
+  app_updater_.reset();
+#endif
+  observer_.Remove(ExtensionRegistry::Get(context_));

[Devlin, 2017/05/01 14:51:11]

Given this depends on ExtensionRegistry(), we should just be able to let this
destruct and have the observer automatically remove itself.

[khmel, 2017/05/01 20:18:18]

Done.

+}
+
+std::unique_ptr<ExtensionAppIcon> ExtensionAppIconService::CreateIcon(
+    ExtensionAppIconDelegate* delegate,
+    const std::string& app_id,
+    int resource_size_in_dip) {
+  std::unique_ptr<ExtensionAppIcon> icon = base::MakeUnique<ExtensionAppIcon>(
+      delegate, context_,
+      base::Bind(&ExtensionAppIconService::OnIconDestroyed,
+                 weak_ptr_factory_.GetWeakPtr()),
+      app_id, resource_size_in_dip);
+
+  icon_map_[icon->app_id()].insert(icon.get());
+  return icon;
+}
+
+void ExtensionAppIconService::OnExtensionLoaded(
+    content::BrowserContext* browser_context,
+    const Extension* extension) {
+  OnAppUpdated(extension->id());
+}
+
+void ExtensionAppIconService::OnExtensionUnloaded(
+    content::BrowserContext* browser_context,
+    const Extension* extension,
+    UnloadedExtensionInfo::Reason reason) {
+  OnAppUpdated(extension->id());
+}
+
+void ExtensionAppIconService::OnAppUpdated(const std::string& app_id) {
+  IconMap::const_iterator it = icon_map_.find(app_id);
+  if (it == icon_map_.end())
+    return;
+  for (auto* icon : it->second)
+    icon->UpdateIcon();
+}
+
+void ExtensionAppIconService::OnIconDestroyed(ExtensionAppIcon* icon) {
+  DCHECK(icon);
+  IconMap::iterator it = icon_map_.find(icon->app_id());
+  if (it == icon_map_.end()) {
+    NOTREACHED();

[Devlin, 2017/05/01 14:51:11]

Why not a DCHECK?  The NOTREACHED(); return; pattern should only be used in
cases like IPCs where there's a chance that a compromised process could send
arbitrary values.  If we don't expect it to ever be the case, we shouldn't use
this pattern.

[khmel, 2017/05/01 20:18:18]

Changed to DCHECK

+    return;
+  }
+
+  it->second.erase(icon);
+  if (it->second.empty())
+    icon_map_.erase(it);
+}
+
+}  // namespace extensions