Mac: Use native folder icons for the bookmark bar.

Mac: Use native folder icons for the bookmark bar.

Based on http://codereview.chromium.org/342103 , but load the icon on a background thread. Check http://build.chromium.org/buildbot/perf/mac-release-10.5/startup/report.html?history=100 – this was checked in in r31647 and reverted in r31652. No startup performance impact can be seen on the normal startup test, but the theme startup time still regresses.

Hence, this can't be checked in.

BUG=26457
TEST=Look at bookmark bar's "Other bookmarks". Should have blue folder.

[TVL, 2009-11-11 14:16:17]

drive by (saw the cl on the waterfall and peeked)

http://codereview.chromium.org/384039/diff/1/3
File chrome/browser/cocoa/bookmark_bar_controller.mm (right):

http://codereview.chromium.org/384039/diff/1/3#newcode83
Line 83: [[NSWorkspace sharedWorkspace] iconForFileType:
nsworkspace isn't documented to be thread safe.  :(

http://codereview.chromium.org/384039/diff/1/3#newcode88
Line 88: withObject:[folder retain]
fyi - performSelector will retain for you (and release after your selector runs.

[TVL, 2009-11-11 14:16:36]

drive by (saw the cl on the waterfall and peeked)


http://codereview.chromium.org/384039/diff/1/3
File chrome/browser/cocoa/bookmark_bar_controller.mm (right):

http://codereview.chromium.org/384039/diff/1/3#newcode83
Line 83: [[NSWorkspace sharedWorkspace] iconForFileType:
nsworkspace isn't documented to be thread safe.  :(

http://codereview.chromium.org/384039/diff/1/3#newcode88
Line 88: withObject:[folder retain]
fyi - performSelector will retain for you (and release after your
selector runs.

http://codereview.chromium.org/384039

[Avi (use Gerrit), 2009-11-11 15:06:08]

http://codereview.chromium.org/384039/diff/1/3
File chrome/browser/cocoa/bookmark_bar_controller.mm (right):

http://codereview.chromium.org/384039/diff/1/3#newcode83
Line 83: [[NSWorkspace sharedWorkspace] iconForFileType:
On 2009/11/11 14:16:17, TVL wrote:
> nsworkspace isn't documented to be thread safe.  :(

You are correct that it's not documented, but it _is_ safe. From an email from
an Apple engineer:

"You're right to be cautious.  However, the method iconForFile: is indeed thread
safe on every version of Mac OS X, so it's fine to use in a background thread.

"This would ordinarily be a sound plan, but NSWorkspace does not ever expect to
have multiple instances of itself created.  You're better off just using
+[NSWorkspace sharedWorkspace], which is thread safe as well."

The proposed code is correct as written.

[Avi (use Gerrit), 2009-11-11 15:08:31]

http://codereview.chromium.org/384039/diff/1/3
File chrome/browser/cocoa/bookmark_bar_controller.mm (right):

http://codereview.chromium.org/384039/diff/1/3#newcode83
Line 83: [[NSWorkspace sharedWorkspace] iconForFileType:
On 2009/11/11 14:16:17, TVL wrote:
> nsworkspace isn't documented to be thread safe.  :(

You are correct that it's not documented, but it _is_ safe. From an
email from an Apple engineer:

"You're right to be cautious.  However, the method iconForFile: is
indeed thread safe on every version of Mac OS X, so it's fine to use in
a background thread.

"This would ordinarily be a sound plan, but NSWorkspace does not ever
expect to have multiple instances of itself created.  You're better off
just using +[NSWorkspace sharedWorkspace], which is thread safe as
well."

The proposed code is correct as written.

http://codereview.chromium.org/384039