[Suggested files] Use alternative chip icons.

Some of the existing file icons look bad in the chips, because the
chips have a different background colour that results in them being
low-contrast. This CL refactors the file_icon_util.* files to allow
either the original or new icons to be retrieved.

The main change is to split
  GetIconResourceIdForLocalFilePath
into
  GetIconTypeForPath
  GetResourceIdForIconType
  GetChipResourceIdForIconType

The former method took a file path, found correct Icon enum, then found
the correct resource ID for the enum.

The new methods separate out the enum-finding step from the resource-
loading step, so that the Chip function can first try and return a
chip-specific icon, and then fall back to the original icons if needed.

Bug: 1034842
Change-Id: Ia7c3310c771993909f8d8b535229173bedb528d4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2089182
Commit-Queue: Tony Yeoman <tby@chromium.org>
Reviewed-by: Jenny Zhang <jennyz@chromium.org>
Reviewed-by: Thanh Nguyen <thanhdng@chromium.org>
Cr-Commit-Position: refs/heads/master@{#748424}

chrome/browser/ui/app_list/search/common/file_icon_util.h

@@ -12,11 +12,38 @@
 namespace app_list {
 namespace internal {
 
-int GetIconResourceIdForLocalFilePath(const base::FilePath& filepath);
-
-}
-
-gfx::ImageSkia GetIconForLocalFilePath(const base::FilePath& filepath);
+enum class IconType {
+  AUDIO,
+  ARCHIVE,
+  CHART,
+  EXCEL,
+  FOLDER,
+  FORM,
+  GDOC,
+  GDRAW,
+  GENERIC,
+  GSHEET,
+  GSITE,
+  GSLIDES,
+  GTABLE,
+  IMAGE,
+  MAP,
+  PDF,
+  PPT,
+  SCRIPT,
+  SITES,
+  TINI,
+  VIDEO,
+  WORD,
+};
+
+IconType GetIconTypeForPath(const base::FilePath& filepath);
+int GetResourceIdForIconType(IconType icon);
+int GetChipResourceIdForIconType(IconType icon);
+}  // namespace internal
+
+gfx::ImageSkia GetIconForPath(const base::FilePath& filepath);
+gfx::ImageSkia GetChipIconForPath(const base::FilePath& filepath);
 
 }  // namespace app_list
 

chrome/browser/ui/app_list/search/common/file_icon_util_unittest.cc

@@ -6,6 +6,7 @@
 
 #include <string>
 #include <utility>
+#include <vector>
 
 #include "base/files/file_path.h"
 #include "base/files/file_util.h"
@@ -14,32 +15,64 @@
 #include "chrome/browser/profiles/profile.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
+#include "ui/chromeos/resources/grit/ui_chromeos_resources.h"
 #include "ui/file_manager/grit/file_manager_resources.h"
 
 namespace app_list {
 
-class AppListFileIconUtilTest
-    : public testing::TestWithParam<std::pair<std::string, int>> {};
-INSTANTIATE_TEST_SUITE_P(
-    All,
-    AppListFileIconUtilTest,
-    testing::ValuesIn((std::pair<std::string, int>[]){
-        {"/my/test/file.pdf", IDR_FILE_MANAGER_IMG_LAUNCHER_FILETYPE_2X_PDF},
-        {"/my/test/file.Pdf", IDR_FILE_MANAGER_IMG_LAUNCHER_FILETYPE_2X_PDF},
-        {"/my/test/file.tar.gz",
-         IDR_FILE_MANAGER_IMG_LAUNCHER_FILETYPE_2X_ARCHIVE},
-        {"/my/test/.gslides",
-         IDR_FILE_MANAGER_IMG_LAUNCHER_FILETYPE_2X_GSLIDES},
-        {"/my/test/noextension",
-         IDR_FILE_MANAGER_IMG_LAUNCHER_FILETYPE_2X_GENERIC},
-        {"/my/test/file.missing",
-         IDR_FILE_MANAGER_IMG_LAUNCHER_FILETYPE_2X_GENERIC}}));
-
-TEST_P(AppListFileIconUtilTest, IconResourceIdForFilepath) {
-  const auto& arg = GetParam();
-  EXPECT_EQ(::app_list::internal::GetIconResourceIdForLocalFilePath(
-                base::FilePath(arg.first)),
-            arg.second);
+TEST(AppListFileIconUtilTest, GetIconTypeForPath) {
+  const std::vector<std::pair<std::string, internal::IconType>>
+      file_path_to_icon_type = {
+          {"/my/test/file.pdf", internal::IconType::PDF},
+          {"/my/test/file.Pdf", internal::IconType::PDF},
+          {"/my/test/file.tar.gz", internal::IconType::ARCHIVE},
+          {"/my/test/.gslides", internal::IconType::GSLIDES},
+          {"/my/test/noextension", internal::IconType::GENERIC},
+          {"/my/test/file.missing", internal::IconType::GENERIC}};
+
+  for (const auto& pair : file_path_to_icon_type) {
+    EXPECT_EQ(internal::GetIconTypeForPath(base::FilePath(pair.first)),
+              pair.second);
+  }
+}
+
+TEST(AppListFileIconUtilTest, GetResourceIdForIconType) {
+  const std::vector<std::pair<internal::IconType, int>>
+      icon_type_to_resource_id = {
+          {internal::IconType::PDF,
+           IDR_FILE_MANAGER_IMG_LAUNCHER_FILETYPE_2X_PDF},
+          {internal::IconType::PDF,
+           IDR_FILE_MANAGER_IMG_LAUNCHER_FILETYPE_2X_PDF},
+          {internal::IconType::ARCHIVE,
+           IDR_FILE_MANAGER_IMG_LAUNCHER_FILETYPE_2X_ARCHIVE},
+          {internal::IconType::GSLIDES,
+           IDR_FILE_MANAGER_IMG_LAUNCHER_FILETYPE_2X_GSLIDES},
+          {internal::IconType::GENERIC,
+           IDR_FILE_MANAGER_IMG_LAUNCHER_FILETYPE_2X_GENERIC}};
+
+  for (const auto& pair : icon_type_to_resource_id) {
+    EXPECT_EQ(::app_list::internal::GetResourceIdForIconType(pair.first),
+              pair.second);
+  }
+}
+
+TEST(AppListFileIconUtilTest, GetChipResourceIdForIconType) {
+  const std::vector<std::pair<internal::IconType, int>>
+      icon_type_to_resource_id = {
+          {internal::IconType::PDF,
+           IDR_FILE_MANAGER_IMG_LAUNCHER_FILETYPE_2X_PDF},
+          {internal::IconType::PDF,
+           IDR_FILE_MANAGER_IMG_LAUNCHER_FILETYPE_2X_PDF},
+          {internal::IconType::ARCHIVE,
+           IDR_FILE_MANAGER_IMG_LAUNCHER_FILETYPE_2X_ARCHIVE},
+          {internal::IconType::IMAGE, IDR_LAUNCHER_CHIP_ICON_IMAGE},
+          {internal::IconType::GSLIDES, IDR_LAUNCHER_CHIP_ICON_GSLIDES},
+          {internal::IconType::GENERIC, IDR_LAUNCHER_CHIP_ICON_GENERIC}};
+
+  for (const auto& pair : icon_type_to_resource_id) {
+    EXPECT_EQ(::app_list::internal::GetChipResourceIdForIconType(pair.first),
+              pair.second);
+  }
 }
 
 }  // namespace app_list

chrome/browser/ui/app_list/search/zero_state_file_result.cc

@@ -61,8 +61,8 @@ ZeroStateFileResult::ZeroStateFileResult(const base::FilePath& filepath,
       l10n_util::GetStringUTF16(IDS_FILEMANAGER_APP_NAME), true);
   base::i18n::SanitizeUserSuppliedString(&sanitized_name);
   SetDetails(sanitized_name);
-  SetIcon(GetIconForLocalFilePath(filepath));
-  SetChipIcon(GetIconForLocalFilePath(filepath));
+  SetIcon(GetIconForPath(filepath));
+  SetChipIcon(GetChipIconForPath(filepath));
 }
 
 ZeroStateFileResult::~ZeroStateFileResult() = default;