Retry icon fetching after intent_helper is ready

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

Index: chrome/browser/task_management/providers/arc/arc_process_task.cc
diff --git a/chrome/browser/task_management/providers/arc/arc_process_task.cc b/chrome/browser/task_management/providers/arc/arc_process_task.cc
index 3770482bf3f424a8e12bc78fcfda3b5ff701df35..0340744710b636d1edf5d3e56978ee7f8facfcd3 100644
--- a/chrome/browser/task_management/providers/arc/arc_process_task.cc
+++ b/chrome/browser/task_management/providers/arc/arc_process_task.cc
@@ -8,7 +8,6 @@
 #include "base/i18n/rtl.h"
 #include "base/strings/utf_string_conversions.h"
 #include "chrome/grit/generated_resources.h"
-#include "components/arc/arc_bridge_service.h"
 #include "components/arc/arc_service_manager.h"
 #include "components/arc/common/process.mojom.h"
 #include "content/public/browser/browser_thread.h"
@@ -51,32 +50,41 @@ ArcProcessTask::ArcProcessTask(base::ProcessId pid,
       nspid_(nspid),
       process_name_(process_name),
       process_state_(process_state),
+      // |packages| contains an alphabetically-sorted list of package names the
+      // process has. Since the Task class can hold only one icon per process,
+      // and there is no reliable way to pick the most important process from
+      // the |packages| list, just use the first item in the list.
+      package_name_(packages.empty() ? "" : packages.at(0)),
       weak_ptr_factory_(this) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
-  if (packages.empty())
-    return;
+  StartIconLoading();
+}
 
-  std::vector<arc::ActivityIconLoader::ActivityName> activity;
-  // |packages| contains an alphabetically-sorted list of package names the
-  // process has. Since the Task class can hold only one icon per process,
-  // and there is no reliable way to pick the most important process from
-  // the |packages| list, just use the first item in the list.
-  activity.emplace_back(packages.at(0), kEmptyActivityName);
+void ArcProcessTask::StartIconLoading() {
+  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
 
-  scoped_refptr<arc::ActivityIconLoader> icon_loader = GetIconLoader();
-  if (!icon_loader)
+  if (package_name_.empty())
     return;
 
-  // TODO(yusukes): ArcProcessTask might be created before intent_helper
-  // instance becomes ready. Because of the race, OnIconLoaded() might
-  // be called synchronously with no icon. Fix the race to ensure that
-  // ArcProcessTask can always fetch icons.
-  icon_loader->GetActivityIcons(activity,
-                                base::Bind(&ArcProcessTask::OnIconLoaded,
-                                           weak_ptr_factory_.GetWeakPtr()));
+  scoped_refptr<arc::ActivityIconLoader> icon_loader = GetIconLoader();
+  arc::ActivityIconLoader::GetResult result =
+      arc::ActivityIconLoader::GetResult::FAILED_ARC_NOT_READY;
+  if (icon_loader) {
+    std::vector<arc::ActivityIconLoader::ActivityName> activities = {
+        {package_name_, kEmptyActivityName}};
+    result = icon_loader->GetActivityIcons(
+        activities, base::Bind(&ArcProcessTask::OnIconLoaded,
+                               weak_ptr_factory_.GetWeakPtr()));
+  }
+
+  if (result == arc::ActivityIconLoader::GetResult::FAILED_ARC_NOT_READY) {
+    // Need to retry loading the icon.
+    arc::ArcBridgeService::Get()->AddObserver(this);
+  }
 }
 
 ArcProcessTask::~ArcProcessTask() {
+  arc::ArcBridgeService::Get()->RemoveObserver(this);
 }
 
 Task::Type ArcProcessTask::GetType() const {
@@ -108,6 +116,21 @@ void ArcProcessTask::Kill() {
       nspid_, "Killed manually from Task Manager");
 }
 
+void ArcProcessTask::OnIntentHelperInstanceReady() {
+  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+
+  VLOG(2) << "intent_helper instance is ready. Fetching the icon for "
+          << package_name_;
+  arc::ArcBridgeService::Get()->RemoveObserver(this);
+
+  // Instead of calling into StartIconLoading() directly, return to the main
+  // loop first to make sure other ArcBridgeService observers are notified.
+  // Otherwise, arc::ActivityIconLoader::GetActivityIcon() may fail again.
+  content::BrowserThread::PostTask(content::BrowserThread::UI, FROM_HERE,
+                                   base::Bind(&ArcProcessTask::StartIconLoading,
+                                              weak_ptr_factory_.GetWeakPtr()));
+}
+
 void ArcProcessTask::SetProcessState(arc::mojom::ProcessState process_state) {
   process_state_ = process_state;
 }