Add a macutil for the main app bundle and override...

Add a macutil for the main app bundle and override
- provide apis to get and override the app bundle
- w/in the core code, use this api for fetching the bundle
  - render sandbox config
  - resource bundles
  - test shell font
- w/in the unittest boot straps, use the mac util to override the bundle so resources can be found.

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=10798

[TVL, 2009-03-03 15:27:10]

ok, new version up that seems to work.

[Mark Mentovai, 2009-03-03 15:42:17]

LG with a few minor nits.

Fix the changelist description, it refers to "the above" but there's nothing
above.

http://codereview.chromium.org/28214/diff/3010/2009
File base/mac_util.h (right):

http://codereview.chromium.org/28214/diff/3010/2009#newcode27
Line 27: // Returns the main bundle or the override, used for code code that
needs
code code

http://codereview.chromium.org/28214/diff/3010/2009#newcode28
Line 28: // to fetch resources from bundles, but work w/in a unittest where we
within

http://codereview.chromium.org/28214/diff/3010/2009#newcode32
Line 32: // Set the bundle to return for MainAppBundle to over the default
value.
Comments shouldn't be harder to read than code.

Set the bundle that MainAppBundle will return, overriding the default value.

I'd also say that you can restore the default by calling
SetOverrideAppBundle(nil).

http://codereview.chromium.org/28214/diff/3010/2009#newcode34
Line 34: void SetOverrideAppBundle(const FilePath& file_path);
Overloading is forbidden.  You need SetOverrideAppBundle and
SetOverrideAppBundlePath, or some other suitable names.

http://codereview.chromium.org/28214/diff/3010/2008
File base/mac_util.mm (right):

http://codereview.chromium.org/28214/diff/3010/2008#newcode46
Line 46: // Not threading worries since NSBundle isn't thread safe.
No threading worries, I guess?

http://codereview.chromium.org/28214/diff/3010/2008#newcode63
Line 63: DCHECK(bundle) << "failed to load the bundle";
Either do just DCHECK(bundle) or do DCHECK(bundle) << "blah blah " <<
file_path.value().

http://codereview.chromium.org/28214/diff/3010/2011
File chrome/common/resource_bundle_mac.mm (right):

http://codereview.chromium.org/28214/diff/3010/2011#newcode39
Line 39: NSString *resource_path = [mac_util::MainAppBundle()
pathForResource:name
I like how this worked out.

http://codereview.chromium.org/28214/diff/3010/2012
File chrome/renderer/renderer_main_platform_delegate_mac.mm (right):

http://codereview.chromium.org/28214/diff/3010/2012#newcode102
Line 102: // possible, but still be able to draw.
Why are you taking out the "pure computation sandbox" thing for unit tests?

http://codereview.chromium.org/28214/diff/3010/2006
File webkit/tools/test_shell/test_shell_mac.mm (right):

http://codereview.chromium.org/28214/diff/3010/2006#newcode186
Line 186: 
This blank line no longer serves a purpose either.

[TVL, 2009-03-03 16:15:09]

http://codereview.chromium.org/28214/diff/3010/2009
File base/mac_util.h (right):

http://codereview.chromium.org/28214/diff/3010/2009#newcode27
Line 27: // Returns the main bundle or the override, used for code code that
needs
On 2009/03/03 15:42:17, Mark Mentovai wrote:
> code code

Done.

http://codereview.chromium.org/28214/diff/3010/2009#newcode28
Line 28: // to fetch resources from bundles, but work w/in a unittest where we
On 2009/03/03 15:42:17, Mark Mentovai wrote:
> within

Done.

http://codereview.chromium.org/28214/diff/3010/2009#newcode32
Line 32: // Set the bundle to return for MainAppBundle to over the default
value.
On 2009/03/03 15:42:17, Mark Mentovai wrote:
> Comments shouldn't be harder to read than code.
> 
> Set the bundle that MainAppBundle will return, overriding the default value.
> 
> I'd also say that you can restore the default by calling
> SetOverrideAppBundle(nil).

Done.

http://codereview.chromium.org/28214/diff/3010/2009#newcode34
Line 34: void SetOverrideAppBundle(const FilePath& file_path);
On 2009/03/03 15:42:17, Mark Mentovai wrote:
> Overloading is forbidden.  You need SetOverrideAppBundle and
> SetOverrideAppBundlePath, or some other suitable names.

Done.

http://codereview.chromium.org/28214/diff/3010/2008
File base/mac_util.mm (right):

http://codereview.chromium.org/28214/diff/3010/2008#newcode46
Line 46: // Not threading worries since NSBundle isn't thread safe.
On 2009/03/03 15:42:17, Mark Mentovai wrote:
> No threading worries, I guess?

Done.

http://codereview.chromium.org/28214/diff/3010/2008#newcode63
Line 63: DCHECK(bundle) << "failed to load the bundle";
On 2009/03/03 15:42:17, Mark Mentovai wrote:
> Either do just DCHECK(bundle) or do DCHECK(bundle) << "blah blah " <<
> file_path.value().

Done.

http://codereview.chromium.org/28214/diff/3010/2012
File chrome/renderer/renderer_main_platform_delegate_mac.mm (right):

http://codereview.chromium.org/28214/diff/3010/2012#newcode102
Line 102: // possible, but still be able to draw.
On 2009/03/03 15:42:17, Mark Mentovai wrote:
> Why are you taking out the "pure computation sandbox" thing for unit tests?

it was only here because we couldn't get to the bundle, now that we can, might
was well run the renderer tests in the same env the renderers run in.

http://codereview.chromium.org/28214/diff/3010/2006
File webkit/tools/test_shell/test_shell_mac.mm (right):

http://codereview.chromium.org/28214/diff/3010/2006#newcode186
Line 186: 
On 2009/03/03 15:42:17, Mark Mentovai wrote:
> This blank line no longer serves a purpose either.

Done.

[Mark Mentovai, 2009-03-03 16:21:45]

Very good, LG™.

Edit that CL description too.

http://codereview.chromium.org/28214/diff/3014/2021
File base/mac_util.mm (right):

http://codereview.chromium.org/28214/diff/3014/2021#newcode9
Line 9: #include "base/file_path.h"
Leave the blank line between system and chrome headers.