Do not touch ui::ResourceBundle on non-UI thread in web_app_mac.mm

chrome/browser/web_applications/web_app_mac.mm

Index: chrome/browser/web_applications/web_app_mac.mm
diff --git a/chrome/browser/web_applications/web_app_mac.mm b/chrome/browser/web_applications/web_app_mac.mm
index 6da77194e295239a9fa181c7918609165cfcc9a0..d39485ad23630300db88617d6eec02b3a8a1aae3 100644
--- a/chrome/browser/web_applications/web_app_mac.mm
+++ b/chrome/browser/web_applications/web_app_mac.mm
@@ -7,6 +7,7 @@
 #import <Cocoa/Cocoa.h>
 #include <stdint.h>
 
+#include <map>
 #include <utility>
 
 #include "base/command_line.h"
@@ -19,6 +20,7 @@
 #include "base/mac/scoped_cftyperef.h"
 #include "base/mac/scoped_nsobject.h"
 #include "base/macros.h"
+#include "base/memory/ptr_util.h"
 #include "base/memory/ref_counted.h"
 #include "base/metrics/histogram_macros.h"
 #include "base/path_service.h"
@@ -373,14 +375,79 @@ NSImageRep* OverlayImageRep(NSImage* background, NSImageRep* overlay) {
 
 // Helper function to extract the single NSImageRep held in a resource bundle
 // image.
-NSImageRep* ImageRepForResource(int resource_id) {
-  gfx::Image& image =
-      ResourceBundle::GetSharedInstance().GetNativeImageNamed(resource_id);
+NSImageRep* ImageRepForGFXImage(const gfx::Image& image) {
   NSArray* image_reps = [image.AsNSImage() representations];
   DCHECK_EQ(1u, [image_reps count]);
   return [image_reps objectAtIndex:0];
 }
 
+using ResourceIDToImage = std::map<int, gfx::Image>;
+
+// Returns a map of gfx::Image used by SetWorkspaceIconOnFILEThread.
+// Since ui::ResourceBundle can be call only on UI thread, this function also
+// needs to run on UI thread.
+std::unique_ptr<ResourceIDToImage> GetImageResourcesOnUIThread() {
+  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+
+  // This should match to the resource IDs used by SetWorkspaceIconOnFILEThread
+  // below.
+  int resource_ids[] = {

[tapted, 2017/05/01 08:43:45]

nit: constexpr int kResourceIDs

or inline the array literal in the for loop below

[tzik, 2017/05/02 17:58:36]

Done. Inlined it into the loop.

+      IDR_APPS_FOLDER_16, IDR_APPS_FOLDER_32, IDR_APPS_FOLDER_OVERLAY_128,
+      IDR_APPS_FOLDER_OVERLAY_512,
+  };
+
+  ui::ResourceBundle& resource_bundle = ui::ResourceBundle::GetSharedInstance();
+  std::unique_ptr<ResourceIDToImage> result =
+      base::MakeUnique<ResourceIDToImage>();
+  for (int id : resource_ids)
+    (*result)[id] = resource_bundle.GetNativeImageNamed(id);
+  return result;
+}
+
+void SetWorkspaceIconOnFILEThread(const base::FilePath& apps_directory,
+                                  std::unique_ptr<ResourceIDToImage> images) {
+  DCHECK_CURRENTLY_ON(content::BrowserThread::FILE);
+
+  base::scoped_nsobject<NSImage> folder_icon_image([[NSImage alloc] init]);
+  // Use complete assets for the small icon sizes. -[NSWorkspace setIcon:] has a
+  // bug when dealing with named NSImages where it incorrectly handles alpha
+  // premultiplication. This is most noticable with small assets since the 1px
+  // border is a much larger component of the small icons.
+  // See http://crbug.com/305373 for details.
+  auto found = images->find(IDR_APPS_FOLDER_16);

[tapted, 2017/05/01 08:43:45]

  for (int small_image_id : {IDR_APPS_FOLDER_16, IDR_APPS_FOLDER_32}) {
    ..

[tzik, 2017/05/02 17:58:36]

Done.

+  DCHECK(found != images->end());
+  [folder_icon_image addRepresentation:ImageRepForGFXImage(found->second)];
+
+  found = images->find(IDR_APPS_FOLDER_32);
+  DCHECK(found != images->end());
+  [folder_icon_image addRepresentation:ImageRepForGFXImage(found->second)];
+
+  // Brand larger folder assets with an embossed app launcher logo to
+  // conserve distro size and for better consistency with changing hue
+  // across OSX versions. The folder is textured, so compresses poorly
+  // without this.
+  const int kBrandResourceIds[] = {
+      IDR_APPS_FOLDER_OVERLAY_128, IDR_APPS_FOLDER_OVERLAY_512,
+  };
+  NSImage* base_image = [NSImage imageNamed:NSImageNameFolder];
+  for (size_t i = 0; i < arraysize(kBrandResourceIds); ++i) {

[tapted, 2017/05/01 08:43:45]

and I guess

  for (int brand_image_id : {IDR_APPS_FOLDER_OVERLAY_128,
IDR_APPS_FOLDER_OVERLAY_512}) {
    ..

so things are consistent

[tzik, 2017/05/02 17:58:36]

Done.

+    found = images->find(kBrandResourceIds[i]);
+    DCHECK(found != images->end());
+    NSImageRep* with_overlay =
+        OverlayImageRep(base_image, ImageRepForGFXImage(found->second));
+    DCHECK(with_overlay);
+    if (with_overlay)
+      [folder_icon_image addRepresentation:with_overlay];
+  }
+  [[NSWorkspace sharedWorkspace]
+      setIcon:folder_icon_image
+      forFile:base::mac::FilePathToNSString(apps_directory)
+      options:0];
+
+  content::BrowserThread::DeleteSoon(content::BrowserThread::UI, FROM_HERE,
+                                     images.release());
+}
+
 // Adds a localized strings file for the Chrome Apps directory using the current
 // locale. OSX will use this for the display name.
 // + Chrome Apps.localized (|apps_directory|)
@@ -409,35 +476,10 @@ void UpdateAppShortcutsSubdirLocalizedName(
   [strings_dict writeToFile:strings_path
                  atomically:YES];
 
-  base::scoped_nsobject<NSImage> folder_icon_image([[NSImage alloc] init]);
-
-  // Use complete assets for the small icon sizes. -[NSWorkspace setIcon:] has a
-  // bug when dealing with named NSImages where it incorrectly handles alpha
-  // premultiplication. This is most noticable with small assets since the 1px
-  // border is a much larger component of the small icons.
-  // See http://crbug.com/305373 for details.
-  [folder_icon_image addRepresentation:ImageRepForResource(IDR_APPS_FOLDER_16)];
-  [folder_icon_image addRepresentation:ImageRepForResource(IDR_APPS_FOLDER_32)];
-
-  // Brand larger folder assets with an embossed app launcher logo to conserve
-  // distro size and for better consistency with changing hue across OSX
-  // versions. The folder is textured, so compresses poorly without this.
-  const int kBrandResourceIds[] = {
-    IDR_APPS_FOLDER_OVERLAY_128,
-    IDR_APPS_FOLDER_OVERLAY_512,
-  };
-  NSImage* base_image = [NSImage imageNamed:NSImageNameFolder];
-  for (size_t i = 0; i < arraysize(kBrandResourceIds); ++i) {
-    NSImageRep* with_overlay =
-        OverlayImageRep(base_image, ImageRepForResource(kBrandResourceIds[i]));
-    DCHECK(with_overlay);
-    if (with_overlay)
-      [folder_icon_image addRepresentation:with_overlay];
-  }
-  [[NSWorkspace sharedWorkspace]
-      setIcon:folder_icon_image
-      forFile:base::mac::FilePathToNSString(apps_directory)
-      options:0];
+  content::BrowserThread::PostTaskAndReplyWithResult(
+      content::BrowserThread::UI, FROM_HERE,
+      base::BindOnce(&GetImageResourcesOnUIThread),
+      base::BindOnce(&SetWorkspaceIconOnFILEThread, apps_directory));
 }
 
 void DeletePathAndParentIfEmpty(const base::FilePath& app_path) {