commit 156cecbc57b0cf55db5aa8d521292f3aaa7bd5ff
author Jeevan Shikaram <jshikaram@chromium.org> Wed May 22 04:16:07 2019
committer Commit Bot <commit-bot@chromium.org> Wed May 22 04:16:07 2019
tree 5da6f05aad340f0ddd0b66c8e0ed9d2037463fca
parent d0bd51908bf0a4e0c12f706ff243b53f67c1e265

[App Service] Get file path in a background thread.

ExtensionResource::GetFilePath will DCHECK if it's not called on a background
thread. Currently, the App Service calls this method on the UI thread, which
causes App Management to crash when DCHECKs are enabled.

This CL refactors apps::LoadIconFromExtension to ensure that this method is
only called on a background thread.

Bug: 916380
Change-Id: I3ba8757e884c6336b4b7b61fc46363c8273fcc86
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1619586
Reviewed-by: Nigel Tao <nigeltao@chromium.org>
Reviewed-by: Dominick Ng <dominickn@chromium.org>
Commit-Queue: Jeevan Shikaram <jshikaram@chromium.org>
Cr-Commit-Position: refs/heads/master@{#662061}

chrome/browser/apps/app_service/app_icon_factory.cc

diff --git a/chrome/browser/apps/app_service/app_icon_factory.cc b/chrome/browser/apps/app_service/app_icon_factory.cc
index 5cda01f..f2cf630 100644
--- a/chrome/browser/apps/app_service/app_icon_factory.cc
+++ b/chrome/browser/apps/app_service/app_icon_factory.cc
@@ -9,6 +9,7 @@
 
 #include "base/bind.h"
 #include "base/callback.h"
+#include "base/files/file_path.h"
 #include "base/files/file_util.h"
 #include "base/task/post_task.h"
 #include "base/task/task_traits.h"
@@ -67,8 +68,12 @@
 }
 
 std::vector<uint8_t> CompressedDataFromResource(
-    const extensions::ExtensionResource resource) {
-  return ReadFileAsCompressedData(resource.GetFilePath());
+    extensions::ExtensionResource resource) {
+  const base::FilePath& path = resource.GetFilePath();
+  if (path.empty()) {
+    return std::vector<uint8_t>();
+  }
+  return ReadFileAsCompressedData(path);
 }
 
 // Runs |callback| passing an IconValuePtr with a compressed image: a
@@ -284,18 +289,15 @@
           }
         }
 
-        // This is an "if", not an "else if", as some component extensions
-        // don't have resource-backed icons.
-        if (!ext_resource.GetFilePath().empty()) {
-          base::PostTaskWithTraitsAndReplyWithResult(
-              FROM_HERE, {base::MayBlock(), base::TaskPriority::USER_VISIBLE},
-              base::BindOnce(&CompressedDataFromResource,
-                             std::move(ext_resource)),
-              base::BindOnce(&RunCallbackWithCompressedData, size_hint_in_dip,
-                             default_icon_resource, is_placeholder_icon,
-                             icon_effects, std::move(callback)));
-          return;
-        }
+        // Try and load data from the resource file.
+        base::PostTaskWithTraitsAndReplyWithResult(
+            FROM_HERE, {base::MayBlock(), base::TaskPriority::USER_VISIBLE},
+            base::BindOnce(&CompressedDataFromResource,
+                           std::move(ext_resource)),
+            base::BindOnce(&RunCallbackWithCompressedData, size_hint_in_dip,
+                           default_icon_resource, is_placeholder_icon,
+                           icon_effects, std::move(callback)));
+        return;
       }
     }
   }