Fix the flaky test ArcAppModelBuilderTest.IconLoader/*.

The reason is AppServiceAppItem calls LoadIcon as well to load ARC
icon, which could cause the more updates then expected. Add a
ModelUpdater to check whether AppServiceAppItem has finished loading
icons. When AppServiceAppItem load icon is done, create
ArcAppIconLoader to valid the load icon function.

BUG=1030009

Change-Id: Id85f50624e1e365d82c120e1bfa28aa960bfa340
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1981409
Commit-Queue: Nancy Wang <nancylingwang@chromium.org>
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#728083}

chrome/browser/ui/app_list/arc/arc_app_unittest.cc

@@ -625,6 +625,8 @@ class ArcAppModelBuilderTest : public extensions::ExtensionServiceTestBase,
 
   arc::FakeAppInstance* app_instance() { return arc_test_.app_instance(); }
 
+  FakeAppListModelUpdater* model_updater() { return model_updater_.get(); }
+
  private:
   ArcAppTest arc_test_;
   std::unique_ptr<FakeAppListModelUpdater> model_updater_;
@@ -2140,16 +2142,23 @@ TEST_P(ArcAppModelBuilderTest, IconLoaderWithBadIcon) {
   EXPECT_EQ(delegate.update_image_count(), 1U);
 }
 
-// https://crbug.com/1030009
-TEST_P(ArcAppModelBuilderTest, DISABLED_IconLoader) {
+TEST_P(ArcAppModelBuilderTest, IconLoader) {
   const arc::mojom::AppInfo& app = fake_apps()[0];
   const std::string app_id = ArcAppTest::GetAppId(app);
 
   ArcAppListPrefs* prefs = ArcAppListPrefs::Get(profile_.get());
   ASSERT_NE(nullptr, prefs);
 
-  SendRefreshAppList(std::vector<arc::mojom::AppInfo>(fake_apps().begin(),
-                                                      fake_apps().begin() + 1));
+  const std::vector<ui::ScaleFactor>& scale_factors =
+      ui::GetSupportedScaleFactors();
+
+  app_instance()->SendRefreshAppList(std::vector<arc::mojom::AppInfo>(
+      fake_apps().begin(), fake_apps().begin() + 1));
+
+  // Wait AppServiceAppItem to finish loading icon, otherwise, the test result
+  // could be flaky, because the update image count could include the icon
+  // updates by AppServiceAppItem.
+  model_updater()->WaitForIconUpdates(1 + scale_factors.size());
 
   FakeAppIconLoaderDelegate delegate;
   ArcAppIconLoader icon_loader(
@@ -2163,8 +2172,6 @@ TEST_P(ArcAppModelBuilderTest, DISABLED_IconLoader) {
   // Validate default image.
   ValidateIcon(delegate.image());
 
-  const std::vector<ui::ScaleFactor>& scale_factors =
-      ui::GetSupportedScaleFactors();
   AppServiceAppItem* app_item = FindArcItem(app_id);
   for (auto& scale_factor : scale_factors) {
     // Force the icon to be loaded.

chrome/browser/ui/app_list/test/fake_app_list_model_updater.cc

@@ -7,6 +7,7 @@
 #include <utility>
 
 #include "base/containers/flat_map.h"
+#include "base/run_loop.h"
 #include "chrome/browser/ui/app_list/chrome_app_list_item.h"
 #include "extensions/common/constants.h"
 
@@ -64,6 +65,15 @@ void FakeAppListModelUpdater::MoveItemToFolder(const std::string& id,
   }
 }
 
+void FakeAppListModelUpdater::SetItemIcon(const std::string& id,
+                                          const gfx::ImageSkia& icon) {
+  ++update_image_count_;
+  if (update_image_count_ == expected_update_image_count_ &&
+      !icon_updated_callback_.is_null()) {
+    std::move(icon_updated_callback_).Run();
+  }
+}
+
 void FakeAppListModelUpdater::SetSearchEngineIsGoogle(bool is_google) {
   search_engine_is_google_ = is_google;
 }
@@ -233,3 +243,10 @@ void FakeAppListModelUpdater::RemoveObserver(
     AppListModelUpdaterObserver* observer) {
   observers_.RemoveObserver(observer);
 }
+
+void FakeAppListModelUpdater::WaitForIconUpdates(size_t expected_updates) {
+  base::RunLoop run_loop;
+  expected_update_image_count_ = expected_updates + update_image_count_;
+  icon_updated_callback_ = run_loop.QuitClosure();
+  run_loop.Run();
+}

chrome/browser/ui/app_list/test/fake_app_list_model_updater.h

@@ -38,6 +38,7 @@ class FakeAppListModelUpdater : public AppListModelUpdater {
   void RemoveUninstalledItem(const std::string& id) override;
   void MoveItemToFolder(const std::string& id,
                         const std::string& folder_id) override;
+  void SetItemIcon(const std::string& id, const gfx::ImageSkia& icon) override;
   // For SearchModel:
   void SetSearchEngineIsGoogle(bool is_google) override;
   void PublishSearchResults(
@@ -74,6 +75,8 @@ class FakeAppListModelUpdater : public AppListModelUpdater {
   void AddObserver(AppListModelUpdaterObserver* observer) override;
   void RemoveObserver(AppListModelUpdaterObserver* observer) override;
 
+  void WaitForIconUpdates(size_t expected_updates);
+
  private:
   bool search_engine_is_google_ = false;
   std::vector<std::unique_ptr<ChromeAppListItem>> items_;
@@ -81,6 +84,10 @@ class FakeAppListModelUpdater : public AppListModelUpdater {
   base::ObserverList<AppListModelUpdaterObserver> observers_;
   Profile* profile_;
 
+  size_t update_image_count_ = 0;
+  size_t expected_update_image_count_ = 0;
+  base::OnceClosure icon_updated_callback_;
+
   void FindOrCreateOemFolder(
       const std::string& oem_folder_name,
       const syncer::StringOrdinal& preferred_oem_position);