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

chrome/browser/web_applications/web_app_mac.mm

 // Copyright (c) 2012 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.
 
 #import "chrome/browser/web_applications/web_app_mac.h"
 
 #import <Cocoa/Cocoa.h>
 #include <stdint.h>
 
+#include <map>
 #include <utility>
 
 #include "base/command_line.h"
 #include "base/files/file_enumerator.h"
 #include "base/files/file_util.h"
 #include "base/files/scoped_temp_dir.h"
 #include "base/mac/foundation_util.h"
 #import "base/mac/launch_services_util.h"
 #include "base/mac/mac_util.h"
 #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"
 #include "base/process/process_handle.h"
 #include "base/strings/string16.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/string_split.h"
 #include "base/strings/string_util.h"
 #include "base/strings/sys_string_conversions.h"
 #include "base/strings/utf_string_conversions.h"
@@ skipping 334 matching lines @@
             operation:NSCompositeSourceOver
              fraction:1.0
        respectFlipped:NO
                 hints:0];
   [NSGraphicsContext restoreGraphicsState];
   return canvas.autorelease();
 }
 
 // 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|)
 // | + .localized
 // | | en.strings
 // | | de.strings
 void UpdateAppShortcutsSubdirLocalizedName(
     const base::FilePath& apps_directory) {
   base::FilePath localized = apps_directory.Append(".localized");
   if (!base::CreateDirectory(localized))
     return;
 
   base::FilePath directory_name = apps_directory.BaseName().RemoveExtension();
   base::string16 localized_name =
       shell_integration::GetAppShortcutsSubdirName();
   NSDictionary* strings_dict = @{
       base::mac::FilePathToNSString(directory_name) :
           base::SysUTF16ToNSString(localized_name)
   };
 
   std::string locale = l10n_util::NormalizeLocale(
       l10n_util::GetApplicationLocale(std::string()));
 
   NSString* strings_path = base::mac::FilePathToNSString(
       localized.Append(locale + ".strings"));
   [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) {
   DCHECK(!app_path.empty());
   base::DeleteFile(app_path, true);
   base::FilePath apps_folder = app_path.DirName();
   if (base::IsDirectoryEmpty(apps_folder))
     base::DeleteFile(apps_folder, false);
 }
 
@@ skipping 623 matching lines @@
     std::unique_ptr<web_app::ShortcutInfo> shortcut_info =
         BuildShortcutInfoFromBundle(*it);
     WebAppShortcutCreator shortcut_creator(it->DirName(), shortcut_info.get());
     shortcut_creator.DeleteShortcuts();
   }
 }
 
 }  // namespace internals
 
 }  // namespace web_app