arc: Stabilize ARC Icon test

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

 // Copyright 2015 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 <stddef.h>
 #include <stdint.h>
 
 #include <algorithm>
 #include <map>
 #include <memory>
@@ skipping 50 matching lines @@
 class FakeAppIconLoaderDelegate : public AppIconLoaderDelegate {
  public:
   FakeAppIconLoaderDelegate() = default;
   ~FakeAppIconLoaderDelegate() override = default;
 
   void OnAppImageUpdated(const std::string& app_id,
                          const gfx::ImageSkia& image) override {
     app_id_ = app_id;
     image_ = image;
     ++update_image_cnt_;
+    if (update_image_cnt_ == expected_update_image_cnt_ &&
+        !icon_updated_callback_.is_null()) {
+      base::ResetAndReturn(&icon_updated_callback_).Run();
+    }
+  }
+
+  void WaitForIconUpdates(size_t expected_updates) {
+    base::RunLoop run_loop;
+    expected_update_image_cnt_ = expected_updates + update_image_cnt_;
+    icon_updated_callback_ = run_loop.QuitClosure();
+    run_loop.Run();
   }
 
   size_t update_image_cnt() const { return update_image_cnt_; }
 
   const std::string& app_id() const { return app_id_; }
 
   const gfx::ImageSkia& image() { return image_; }
 
  private:
   size_t update_image_cnt_ = 0;
+  size_t expected_update_image_cnt_ = 0;
   std::string app_id_;
   gfx::ImageSkia image_;
+  base::OnceClosure icon_updated_callback_;
 
   DISALLOW_COPY_AND_ASSIGN(FakeAppIconLoaderDelegate);
 };
 
-void WaitForIconReady(ArcAppListPrefs* prefs,
-                      const std::string& app_id,
-                      ui::ScaleFactor scale_factor) {
+bool IsIconCreated(ArcAppListPrefs* prefs,
+                   const std::string& app_id,
+                   ui::ScaleFactor scale_factor) {
+  return base::PathExists(prefs->GetIconPath(app_id, scale_factor));
+}
+
+void WaitForIconCreation(ArcAppListPrefs* prefs,
+                         const std::string& app_id,
+                         ui::ScaleFactor scale_factor) {
   const base::FilePath icon_path = prefs->GetIconPath(app_id, scale_factor);
   // Process pending tasks. This performs multiple thread hops, so we need
   // to run it continuously until it is resolved.
   do {
     content::RunAllBlockingPoolTasksUntilIdle();
   } while (!base::PathExists(icon_path));
 }
 
+void WaitForIconUpdates(Profile* profile,
+                        const std::string& app_id,
+                        size_t expected_updates) {
+  FakeAppIconLoaderDelegate delegate;
+  ArcAppIconLoader icon_loader(profile, app_list::kListIconSize, &delegate);
+  icon_loader.FetchImage(app_id);
+  delegate.WaitForIconUpdates(expected_updates);
+}
+
 enum class ArcState {
   // By default, ARC is non-persistent and Play Store is unmanaged.
   ARC_PLAY_STORE_UNMANAGED,
   // ARC is persistent and Play Store is unmanaged
   ARC_PERSISTENT_PLAY_STORE_UNMANAGED,
   // ARC is non-persistent and Play Store is managed and enabled.
   ARC_PLAY_STORE_MANAGED_AND_ENABLED,
   // ARC is non-persistent and Play Store is managed and disabled.
   ARC_PLAY_STORE_MANAGED_AND_DISABLED,
   // ARC is persistent and Play Store is managed and enabled.
@@ skipping 731 matching lines @@
   const std::vector<ui::ScaleFactor>& scale_factors =
       ui::GetSupportedScaleFactors();
   for (auto& scale_factor : scale_factors) {
     expected_mask |= 1 <<  scale_factor;
     for (auto& app : fake_apps()) {
       ArcAppItem* app_item = FindArcItem(ArcAppTest::GetAppId(app));
       ASSERT_NE(nullptr, app_item);
       const float scale = ui::GetScaleForScaleFactor(scale_factor);
       app_item->icon().GetRepresentation(scale);
 
-      // This does not result in an icon being loaded, so WaitForIconReady
+      // This does not result in an icon being loaded, so WaitForIconUpdates
       // cannot be used.
       content::RunAllBlockingPoolTasksUntilIdle();
     }
   }
 
   const size_t expected_size = scale_factors.size() * fake_apps().size();
 
   // At this moment we should receive all requests for icon loading.
   const std::vector<std::unique_ptr<arc::FakeAppInstance::IconRequest>>&
       icon_requests = app_instance()->icon_requests();
@@ skipping 31 matching lines @@
   ASSERT_NE(nullptr, prefs);
 
   // Icons representations loading is done asynchronously and is started once
   // the ArcAppItem is created. Wait for icons for all supported scales to be
   // loaded.
   uint32_t expected_mask = 0;
   ArcAppItem* app_item = FindArcItem(ArcAppTest::GetAppId(shortcut));
   ASSERT_NE(nullptr, app_item);
   const std::vector<ui::ScaleFactor>& scale_factors =
       ui::GetSupportedScaleFactors();
+  WaitForIconUpdates(profile_.get(), app_item->id(), scale_factors.size());
   for (auto& scale_factor : scale_factors) {
     expected_mask |= 1 << scale_factor;
-    const base::FilePath icon_path =
-        prefs->GetIconPath(ArcAppTest::GetAppId(shortcut), scale_factor);
-    WaitForIconReady(prefs, ArcAppTest::GetAppId(shortcut), scale_factor);
+    EXPECT_TRUE(
+        IsIconCreated(prefs, ArcAppTest::GetAppId(shortcut), scale_factor));
   }
 
   // At this moment we should receive all requests for icon loading.
   const size_t expected_size = scale_factors.size();
   const std::vector<std::unique_ptr<arc::FakeAppInstance::ShortcutIconRequest>>&
       icon_requests = app_instance()->shortcut_icon_requests();
   EXPECT_EQ(expected_size, icon_requests.size());
   uint32_t app_mask = 0;
   for (size_t i = 0; i < icon_requests.size(); ++i) {
     const arc::FakeAppInstance::ShortcutIconRequest* icon_request =
@@ skipping 24 matching lines @@
   app_instance()->RefreshAppList();
   app_instance()->SendRefreshAppList(std::vector<arc::mojom::AppInfo>(
       fake_apps().begin(), fake_apps().begin() + 1));
   const arc::mojom::AppInfo& app = fake_apps()[0];
 
   ArcAppListPrefs* prefs = ArcAppListPrefs::Get(profile_.get());
   ASSERT_NE(nullptr, prefs);
 
   const ui::ScaleFactor scale_factor = ui::GetSupportedScaleFactors()[0];
   const float scale = ui::GetScaleForScaleFactor(scale_factor);
-  const base::FilePath icon_path = prefs->GetIconPath(ArcAppTest::GetAppId(app),
-                                                      scale_factor);
-  EXPECT_FALSE(base::PathExists(icon_path));
+  const std::string app_id = ArcAppTest::GetAppId(app);
+  const base::FilePath icon_path = prefs->GetIconPath(app_id, scale_factor);
+  EXPECT_FALSE(IsIconCreated(prefs, app_id, scale_factor));
 
-  const ArcAppItem* app_item = FindArcItem(ArcAppTest::GetAppId(app));
+  const ArcAppItem* app_item = FindArcItem(app_id);
   EXPECT_NE(nullptr, app_item);
   // This initiates async loading.
   app_item->icon().GetRepresentation(scale);
 
   // Now send generated icon for the app.
   std::string png_data;
   EXPECT_TRUE(app_instance()->GenerateAndSendIcon(
       app, static_cast<arc::mojom::ScaleFactor>(scale_factor), &png_data));
-  WaitForIconReady(prefs, ArcAppTest::GetAppId(app), scale_factor);
+  WaitForIconUpdates(profile_.get(), app_id, 1);
 
   // Validate that icons are installed, have right content and icon is
   // refreshed for ARC app item.
-  EXPECT_TRUE(base::PathExists(icon_path));
+  EXPECT_TRUE(IsIconCreated(prefs, app_id, scale_factor));
 
   std::string icon_data;
   // Read the file from disk and compare with reference data.
   EXPECT_TRUE(base::ReadFileToString(icon_path, &icon_data));
   ASSERT_EQ(icon_data, png_data);
 }
 
 TEST_P(ArcAppModelBuilderTest, RemoveAppCleanUpFolder) {
   // Make sure we are on UI thread.
   ASSERT_TRUE(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI));
 
   app_instance()->RefreshAppList();
   app_instance()->SendRefreshAppList(std::vector<arc::mojom::AppInfo>(
       fake_apps().begin(), fake_apps().begin() + 1));
   const arc::mojom::AppInfo& app = fake_apps()[0];
 
   ArcAppListPrefs* prefs = ArcAppListPrefs::Get(profile_.get());
   ASSERT_NE(nullptr, prefs);
 
   const std::string app_id = ArcAppTest::GetAppId(app);
   const base::FilePath app_path = prefs->GetAppPath(app_id);
   const ui::ScaleFactor scale_factor = ui::GetSupportedScaleFactors()[0];
 
   // No app folder by default.
   base::RunLoop().RunUntilIdle();
-  EXPECT_FALSE(base::PathExists(app_path));
+  EXPECT_FALSE(IsIconCreated(prefs, app_id, scale_factor));
 
   // Now send generated icon for the app.
   std::string png_data;
   EXPECT_TRUE(app_instance()->GenerateAndSendIcon(
       app, static_cast<arc::mojom::ScaleFactor>(scale_factor), &png_data));
-  WaitForIconReady(prefs, app_id, scale_factor);
-  EXPECT_TRUE(base::PathExists(app_path));
+  WaitForIconUpdates(profile_.get(), app_id, 1);
+  EXPECT_TRUE(IsIconCreated(prefs, app_id, scale_factor));
 
   // Send empty app list. This will delete app and its folder.
   app_instance()->SendRefreshAppList(std::vector<arc::mojom::AppInfo>());
-  // This cannot be WaitForIconReady since it needs to wait until the icon is
-  // removed, not added.
   // Process pending tasks. This performs multiple thread hops, so we need
   // to run it continuously until it is resolved.
   do {
     content::RunAllBlockingPoolTasksUntilIdle();
-  } while (base::PathExists(app_path));
-  EXPECT_FALSE(base::PathExists(app_path));
+  } while (IsIconCreated(prefs, app_id, scale_factor));
 }
 
 TEST_P(ArcAppModelBuilderTest, LastLaunchTime) {
   // Make sure we are on UI thread.
   ASSERT_TRUE(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI));
   ASSERT_GE(fake_apps().size(), 3U);
   app_instance()->RefreshAppList();
   app_instance()->SendRefreshAppList(std::vector<arc::mojom::AppInfo>(
       fake_apps().begin(), fake_apps().begin() + 3));
   const arc::mojom::AppInfo& app1 = fake_apps()[0];
@@ skipping 204 matching lines @@
   // Store number of requests generated during the App List item creation. Same
   // request will not be re-sent without clearing the request record in
   // ArcAppListPrefs.
   const size_t initial_icon_request_count =
       app_instance()->icon_requests().size();
 
   FakeAppIconLoaderDelegate delegate;
   ArcAppIconLoader icon_loader(profile(), app_list::kListIconSize, &delegate);
   icon_loader.FetchImage(app_id);
 
-  content::RunAllBlockingPoolTasksUntilIdle();
+  // So far one updated of default icon is expected.
+  EXPECT_EQ(delegate.update_image_cnt(), 1U);
+
   // Although icon file is still missing, expect no new request sent to ARC as
   // them are recorded in IconRequestRecord in ArcAppListPrefs.
   EXPECT_EQ(app_instance()->icon_requests().size(), initial_icon_request_count);
   // Validate default image.
   ValidateIcon(delegate.image());
 
   MaybeRemoveIconRequestRecord(app_id);
 
   // Install Bad image.
   const std::vector<ui::ScaleFactor>& scale_factors =
       ui::GetSupportedScaleFactors();
   ArcAppItem* app_item = FindArcItem(app_id);
   for (auto& scale_factor : scale_factors) {
     app_instance()->GenerateAndSendBadIcon(
         app, static_cast<arc::mojom::ScaleFactor>(scale_factor));
     const float scale = ui::GetScaleForScaleFactor(scale_factor);
     // Force the icon to be loaded.
     app_item->icon().GetRepresentation(scale);
-    WaitForIconReady(prefs, app_id, scale_factor);
+    WaitForIconCreation(prefs, app_id, scale_factor);
   }
+
   // After clear request record related to |app_id|, when bad icon is installed,
   // decoding failure will trigger re-sending new icon request to ARC.
   EXPECT_TRUE(app_instance()->icon_requests().size() >
               initial_icon_request_count);
   for (size_t i = initial_icon_request_count;
        i < app_instance()->icon_requests().size(); ++i) {
     const auto& request = app_instance()->icon_requests()[i];
     EXPECT_TRUE(request->IsForApp(app));
   }
+
+  // Icon update is not expected because of bad icon.
+  EXPECT_EQ(delegate.update_image_cnt(), 1U);
 }
 
-// TODO(crbug.com/628425) -- reenable once this test is less flaky.
-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);
 
   app_instance()->RefreshAppList();
   app_instance()->SendRefreshAppList(std::vector<arc::mojom::AppInfo>(
       fake_apps().begin(), fake_apps().begin() + 1));
 
@@ skipping 12 matching lines @@
   const std::vector<ui::ScaleFactor>& scale_factors =
       ui::GetSupportedScaleFactors();
   ArcAppItem* app_item = FindArcItem(app_id);
   for (auto& scale_factor : scale_factors) {
     std::string png_data;
     EXPECT_TRUE(app_instance()->GenerateAndSendIcon(
         app, static_cast<arc::mojom::ScaleFactor>(scale_factor), &png_data));
     const float scale = ui::GetScaleForScaleFactor(scale_factor);
     // Force the icon to be loaded.
     app_item->icon().GetRepresentation(scale);
-    WaitForIconReady(prefs, app_id, scale_factor);
   }
 
+  delegate.WaitForIconUpdates(scale_factors.size());
+
   // Validate loaded image.
   EXPECT_EQ(1 + scale_factors.size(), delegate.update_image_cnt());
   EXPECT_EQ(app_id, delegate.app_id());
   ValidateIcon(delegate.image());
+
+  // No more updates are expected.
+  base::RunLoop().RunUntilIdle();
+  EXPECT_EQ(1 + scale_factors.size(), delegate.update_image_cnt());
 }
 
 TEST_P(ArcAppModelBuilderTest, AppLauncher) {
   ArcAppListPrefs* prefs = ArcAppListPrefs::Get(profile());
   ASSERT_NE(nullptr, prefs);
 
   // App1 is called in deferred mode, after refreshing apps.
   // App2 is never called since app is not avaialble.
   // App3 is never called immediately because app is available already.
   const arc::mojom::AppInfo& app1 = fake_apps()[0];
@@ skipping 313 matching lines @@
                         ::testing::ValuesIn(kUnmanagedArcStates));
 INSTANTIATE_TEST_CASE_P(,
                         ArcDefaulAppForManagedUserTest,
                         ::testing::ValuesIn(kManagedArcStates));
 INSTANTIATE_TEST_CASE_P(,
                         ArcPlayStoreAppTest,
                         ::testing::ValuesIn(kUnmanagedArcStates));
 INSTANTIATE_TEST_CASE_P(,
                         ArcAppModelBuilderRecreate,
                         ::testing::ValuesIn(kUnmanagedArcStates));