Get rid of RefCounted for ActivityIconLoader.

chrome/browser/task_manager/providers/arc/arc_process_task.cc

Index: chrome/browser/task_manager/providers/arc/arc_process_task.cc
diff --git a/chrome/browser/task_manager/providers/arc/arc_process_task.cc b/chrome/browser/task_manager/providers/arc/arc_process_task.cc
index 6b32713dd621529b5140de183f99e6bec2c4a1bd..56e1776a9767725314dbfedcd65e1d93854d1437 100644
--- a/chrome/browser/task_manager/providers/arc/arc_process_task.cc
+++ b/chrome/browser/task_manager/providers/arc/arc_process_task.cc
@@ -12,6 +12,7 @@
 #include "components/arc/arc_bridge_service.h"
 #include "components/arc/arc_service_manager.h"
 #include "components/arc/common/process.mojom.h"
+#include "components/arc/intent_helper/arc_intent_helper_bridge.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/common/child_process_host.h"
 #include "ui/base/l10n/l10n_util.h"
@@ -49,14 +50,6 @@ base::string16 MakeTitle(const std::string& process_name,
   return title;
 }
 
-scoped_refptr<arc::ActivityIconLoader> GetIconLoader() {

[afakhry, 2017/01/28 01:08:43]

You could still keep this function:

arc::ActivityIconLoader* GetIconLoader() {
  auto* intent_helper_bridge =
      arc::ArcServiceManager::
      GetGlobalService<arc::ArcIntentHelperBridge>();
  if (!intent_helper_bridge)
    return nullptr;

  return intent_helper_bridge->icon_loader();
}

And then the code in StartIconLoading() remains the same.

[hidehiko, 2017/01/30 17:04:03]

Hmm... Could you elaborate what's the goal to keep this utility?
IMHO, because very similar functions are defined in anonymous namespace of
multiple files, I do not think keeping it here is a good idea.

Providing a utility by arc namespace is another option, but it looks redundant
to me because icon_loader() is always alive if ArcIntentHelperBridge is alive.

(Also considering Yusuke's review comment about lifetime of icon_loader,)
I introduced a ArcIntentHelperBridge::GetActivityIcons, which is just a proxy to
ActivityIconLoader::GetActivityIcons, so that clients do not need worried about
its lifetime.

WDYT?

-  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
-  arc::ArcServiceManager* arc_service_manager = arc::ArcServiceManager::Get();
-  if (!arc_service_manager)
-    return nullptr;
-  return arc_service_manager->icon_loader();
-}
-
 // An activity name for retrieving the package's default icon without
 // specifying an activity name.
 constexpr char kEmptyActivityName[] = "";
@@ -88,17 +81,18 @@ ArcProcessTask::ArcProcessTask(base::ProcessId pid,
 void ArcProcessTask::StartIconLoading() {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
 
-  scoped_refptr<arc::ActivityIconLoader> icon_loader = GetIconLoader();
+  auto* intent_helper_bridge =
+      arc::ArcServiceManager::GetGlobalService<arc::ArcIntentHelperBridge>();
   arc::ActivityIconLoader::GetResult result =
       arc::ActivityIconLoader::GetResult::FAILED_ARC_NOT_READY;
-  if (icon_loader) {
+  if (intent_helper_bridge) {
     // In some case, the package_name_ does not exists, it would be expected to
     // get default process icon. For example, daemon processes in android
     // container such like surfaceflinger, debuggerd or installd. Each of them
     // would be shown on task manager but does not have a package name.
     std::vector<arc::ActivityIconLoader::ActivityName> activities = {
         {package_name_, kEmptyActivityName}};
-    result = icon_loader->GetActivityIcons(
+    result = intent_helper_bridge->icon_loader()->GetActivityIcons(
         activities, base::Bind(&ArcProcessTask::OnIconLoaded,
                                weak_ptr_factory_.GetWeakPtr()));
   }