Load the old ARC icon file to resolve the icon lag issue when migration.

When the system is migrated from the adaptive icon disable to enabled,
all ARC apps request the foreground and background icon files, to
generate the adaptive icons. And it might take long time to load ARC
icons.

Modify ArcAppIcon to temporarily load the adaptive icon from the old
file path if the foreground file doesn't exist, and still call arc
prefs to request the adaptive icon files.

Add the test case and the default app(test app3), which has the old icon
file only, to verify the icon can be loaded.

BUG=1083331

Change-Id: Icb848c53f9ba36d1af62e55cfcb7b1d9565b092b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2344437Reviewed-by: Long Cheng <lgcheng@google.com>
Commit-Queue: Nancy Wang <nancylingwang@chromium.org>
Cr-Commit-Position: refs/heads/master@{#797057}

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

@@ -330,20 +330,6 @@ void ArcAppIcon::LoadForScaleFactor(ui::ScaleFactor scale_factor) {
   std::vector<base::FilePath> paths;
   std::vector<base::FilePath> default_app_paths;
   switch (icon_type_) {
-    case IconType::kUncompressed: {
-      // Deliberately fall through to IconType::kCompressed to add |path| to
-      // |paths|.
-      FALLTHROUGH;
-    }
-    case IconType::kCompressed: {
-      base::FilePath path = prefs->GetIconPath(mapped_app_id_, descriptor);
-      if (path.empty())
-        return;
-      paths.emplace_back(path);
-      default_app_paths.emplace_back(
-          prefs->MaybeGetIconPathForDefaultApp(mapped_app_id_, descriptor));
-      break;
-    }
     case IconType::kAdaptive: {
       base::FilePath foreground_path =
           prefs->GetForegroundIconPath(mapped_app_id_, descriptor);
@@ -360,6 +346,24 @@ void ArcAppIcon::LoadForScaleFactor(ui::ScaleFactor scale_factor) {
       default_app_paths.emplace_back(
           prefs->MaybeGetBackgroundIconPathForDefaultApp(mapped_app_id_,
                                                          descriptor));
+      // Deliberately fall through to IconType::kCompressed to add |path| to
+      // |paths|. For the migration scenario, when the foreground icon file
+      // doesn't exist, load the original icon file to resolve the icon lag
+      // issue.
+      FALLTHROUGH;
+    }
+    case IconType::kUncompressed: {
+      // Deliberately fall through to IconType::kCompressed to add |path| to
+      // |paths|.
+      FALLTHROUGH;
+    }
+    case IconType::kCompressed: {
+      base::FilePath path = prefs->GetIconPath(mapped_app_id_, descriptor);
+      if (path.empty())
+        return;
+      paths.emplace_back(path);
+      default_app_paths.emplace_back(
+          prefs->MaybeGetIconPathForDefaultApp(mapped_app_id_, descriptor));
       break;
     }
   }
@@ -441,16 +445,57 @@ std::unique_ptr<ArcAppIcon::ReadResult> ArcAppIcon::ReadAdaptiveIconFiles(
     ui::ScaleFactor scale_factor,
     const std::vector<base::FilePath>& paths,
     const std::vector<base::FilePath>& default_app_paths) {
-  DCHECK_EQ(2u, paths.size());
+  DCHECK_EQ(3u, paths.size());
 
   const base::FilePath& foreground_path = paths[0];
   const base::FilePath& background_path = paths[1];
   if (!base::PathExists(foreground_path)) {
-    DCHECK_EQ(2u, default_app_paths.size());
+    // For the migration scenario, when the foreground icon file doesn't
+    // exist, load the original icon file to resolve the icon lag issue.
+    if (!paths[2].empty() && base::PathExists(paths[2])) {
+      return ArcAppIcon::ReadFile(true /* request_to_install */, scale_factor,
+                                  false /* resize_allowed */, paths[2]);
+    }
+
+    // Check and read the default app icon path for the default app if the files
+    // exist.
+    return ReadDefaultAppAdaptiveIconFiles(scale_factor, default_app_paths);
+  }
+
+  if (!base::PathExists(background_path)) {
+    // For non-adaptive icon, there could be a |foreground_icon_path| file
+    // only without a |background_icon_path| file.
+    return ArcAppIcon::ReadFile(false /* request_to_install */, scale_factor,
+                                false /* resize_allowed */, foreground_path);
+  }
+
+  return ArcAppIcon::ReadFiles(false /* request_to_install */, scale_factor,
+                               false /* resize_allowed */, foreground_path,
+                               background_path);
+}
+
+// static
+std::unique_ptr<ArcAppIcon::ReadResult>
+ArcAppIcon::ReadDefaultAppAdaptiveIconFiles(
+    ui::ScaleFactor scale_factor,
+    const std::vector<base::FilePath>& default_app_paths) {
+  // Check the default app icon path, and read the icon files for the default
+  // app if the files exist.
+  DCHECK_EQ(3u, default_app_paths.size());
   const base::FilePath& default_app_foreground_path = default_app_paths[0];
   const base::FilePath& default_app_background_path = default_app_paths[1];
   if (default_app_foreground_path.empty() ||
       !base::PathExists(default_app_foreground_path)) {
+    // For the migration scenario, when the foreground icon file doesn't
+    // exist, load the original icon file to resolve the icon lag issue for
+    // the default app.
+    if (default_app_paths.size() == 3u && !default_app_paths[2].empty() &&
+        base::PathExists(default_app_paths[2])) {
+      return ArcAppIcon::ReadFile(true /* request_to_install */, scale_factor,
+                                  true /* resize_allowed */,
+                                  default_app_paths[2]);
+    }
+
     return std::make_unique<ArcAppIcon::ReadResult>(
         false /* error */, true /* request_to_install */, scale_factor,
         false /* resize_allowed */,
@@ -469,18 +514,6 @@ std::unique_ptr<ArcAppIcon::ReadResult> ArcAppIcon::ReadAdaptiveIconFiles(
   return ArcAppIcon::ReadFiles(
       true /* request_to_install */, scale_factor, true /* resize_allowed */,
       default_app_foreground_path, default_app_background_path);
-  }
-
-  if (!base::PathExists(background_path)) {
-    // For non-adaptive icon, there could be a |foreground_icon_path| file
-    // only without a |background_icon_path| file.
-    return ArcAppIcon::ReadFile(false /* request_to_install */, scale_factor,
-                                false /* resize_allowed */, foreground_path);
-  }
-
-  return ArcAppIcon::ReadFiles(false /* request_to_install */, scale_factor,
-                               false /* resize_allowed */, foreground_path,
-                               background_path);
 }
 
 // static

chrome/browser/ui/app_list/arc/arc_app_icon.h

@@ -150,10 +150,44 @@ class ArcAppIcon {
       ui::ScaleFactor scale_factor,
       const base::FilePath& path,
       const base::FilePath& default_app_path);
+
+  // For the adaptive icon, currently, there are 3 images returned from the ARC
+  // side:
+  // (1) Icon_png_data, the adaptive icon generated by the ARC side, for
+  // backward compatibility.
+  // (2) Foreground_icon_png_data, the foreground image for
+  // the adaptive icon. Some icons are not adaptive icons, and don’t have the
+  // background images, then the foreground image is the app icon.
+  // (3) Background_icon_png_data, the background image for the adaptive icon.
+  // Some icons are not adaptive icons, and don’t have the background images.
+  //
+  // There are a few scenarios for the adaptive icon feature:
+  // A. For the adaptive icon, the foreground image and the background image are
+  // merged by the Chromium side, and applied with the mask, to generate the
+  // adaptive icon.
+  // B. For the non adaptive icon, the Chromium side adds a white background to
+  // the foreground image, then applies the mask to generate the adaptive icon.
+  // C. For the migration scenario (from the adaptive icon feature disable to
+  // enable), since neither foreground images and background images present on
+  // the system, the Chromium side sends requests to the ARC side to load the
+  // foreground and background images. However, it might take a few seconds to
+  // get the images files, so for users, it has a long lag for the ARC icon
+  // loading. To resolve the ARC icon lag issue, the old icon_png_data on the
+  // system is used to generate the icon, the same as the previous
+  // implementation, and at the same time, sending the request to the ARC side
+  // to request the new foreground and background images.
+  //
+  // TODO(crbug.com/1083331): Remove the migration handling code, which reads
+  // the old icon_png_data, when the adaptive icon feature is enabled in the
+  // stable release, and the adaptive icon flag is removed.
   static std::unique_ptr<ArcAppIcon::ReadResult> ReadAdaptiveIconFiles(
       ui::ScaleFactor scale_factor,
       const std::vector<base::FilePath>& paths,
       const std::vector<base::FilePath>& default_app_paths);
+  static std::unique_ptr<ArcAppIcon::ReadResult>
+  ReadDefaultAppAdaptiveIconFiles(
+      ui::ScaleFactor scale_factor,
+      const std::vector<base::FilePath>& default_app_paths);
   static std::unique_ptr<ArcAppIcon::ReadResult> ReadFile(
       bool request_to_install,
       ui::ScaleFactor scale_factor,

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

@@ -169,6 +169,12 @@ void ArcAppTest::CreateFakeAppsAndPackages() {
   app.sticky = true;
   fake_default_apps_.push_back(app);
 
+  app.name = "TestApp3";
+  app.package_name = "test.app3";
+  app.activity = "test.app3.activity";
+  app.sticky = true;
+  fake_default_apps_.push_back(app);
+
   base::flat_map<arc::mojom::AppPermission, arc::mojom::PermissionStatePtr>
       permissions1;
   permissions1.emplace(arc::mojom::AppPermission::CAMERA,

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

@@ -2872,6 +2872,27 @@ TEST_P(ArcAppLauncherForDefaultAppTest, AppIconNonDefaultDip) {
   icon_loader.reset();
 }
 
+// Validates that default app icon can be loaded for the single icon file
+// generated when the adaptive icon feature is disabled.
+TEST_P(ArcAppLauncherForDefaultAppTest, AppIconMigration) {
+  ArcAppListPrefs* prefs = ArcAppListPrefs::Get(profile_.get());
+  ASSERT_NE(nullptr, prefs);
+
+  ASSERT_EQ(3u, fake_default_apps().size());
+  const arc::mojom::AppInfo& app = fake_default_apps()[2];
+  const std::string app_id = ArcAppTest::GetAppId(app);
+
+  // Icon can be only fetched after app is registered in the system.
+  arc_test()->WaitForDefaultApps();
+
+  FakeAppIconLoaderDelegate icon_delegate;
+  std::unique_ptr<AppServiceAppIconLoader> icon_loader =
+      std::make_unique<AppServiceAppIconLoader>(profile(), 64, &icon_delegate);
+  icon_loader->FetchImage(app_id);
+  icon_delegate.WaitForIconUpdates(1);
+  icon_loader.reset();
+}
+
 TEST_P(ArcAppLauncherForDefaultAppTest, AppLauncherForDefaultApps) {
   ArcAppListPrefs* prefs = ArcAppListPrefs::Get(profile_.get());
   ASSERT_NE(nullptr, prefs);

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

@@ -47,6 +47,8 @@ const base::FilePath::CharType kArcTestDirectory[] =
     FILE_PATH_LITERAL("arc_default_apps");
 const base::FilePath::CharType kArcTestBoardDirectory[] =
     FILE_PATH_LITERAL("arc_board_default_apps");
+const base::FilePath::CharType kArcTestNonAdaptiveDirectory[] =
+    FILE_PATH_LITERAL("arc_non_adaptive_default_apps");
 
 bool use_test_apps_directory = false;
 
@@ -229,6 +231,7 @@ void ArcDefaultAppList::LoadDefaultApps(std::string board_name) {
     DCHECK(valid_path);
     sources.push_back(base_path.Append(kArcTestDirectory));
     sources.push_back(base_path.Append(kArcTestBoardDirectory));
+    sources.push_back(base_path.Append(kArcTestNonAdaptiveDirectory));
   }
 
   // Using base::Unretained(this) here is safe since we own barrier_closure_.

chrome/test/data/arc_non_adaptive_default_apps/test_app3.json

+{
+  "name": "TestApp3",
+  "package_name": "test.app3",
+  "activity": "test.app3.activity",
+  "oem": true,
+  "app_path": "test_app3"
+}