Fix AmIBundled for command-line executions.

Fix AmIBundled for command-line executions.

BUG=304860, 312196
TEST=run from the command line with dots

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=231376

[Mark Mentovai, 2013-10-25 22:54:55]

https://codereview.chromium.org/45703003/diff/1/base/mac/foundation_util.mm
File base/mac/foundation_util.mm (right):

https://codereview.chromium.org/45703003/diff/1/base/mac/foundation_util.mm#n...
base/mac/foundation_util.mm:40: [[[bundle bundlePath] stringByStandardizingPath]
pathComponents];
This isn’t really correct. The tilde expansion it might do is wrong (in case a
directory begins with a ~ and you launch it as \~whatever/Google\ Chrome.app/… .

Which relative path invocations are you concerned about here?

https://developer.apple.com/library/mac/documentation/cocoa/Reference/Foundat...

says that this only resolves .. in absolute paths. And it says that in that
case, it uses stringByResolvingSymlinksInPath, which we don’t want to use here
because it does disk I/O.

[Avi (use Gerrit), 2013-10-25 23:07:55]

The issue was raised in
https://code.google.com/p/chromium/issues/detail?id=304860#c67 :

$ ../chromium/src/out/Release/Opera.app/Contents/MacOS/Opera
[1026/001233:INFO:foundation_util.mm(46)] depth_difference = 5
2013-10-26 00:12:33.342 Opera[87968:d0b] bundlePath =
/Users/jjgod/work/chromium/src/out/Release/Opera.app, executablePath =
/Users/jjgod/work/desktop/../chromium/src/out/Release/Opera.app/Contents/MacOS/Opera

I'm trying to think of a good way to deal with this. Count the number of ..s and
subtract two for each?

[Avi (use Gerrit), 2013-10-28 15:32:26]

Mark: better?

[Mark Mentovai, 2013-10-28 15:57:09]

Better.

https://codereview.chromium.org/45703003/diff/90001/base/mac/foundation_util.mm
File base/mac/foundation_util.mm (right):

https://codereview.chromium.org/45703003/diff/90001/base/mac/foundation_util....
base/mac/foundation_util.mm:31: NSUInteger CheapPathNormalizedCount(NSArray*
path_array) {
Don’t use an unsigned, because count could legitimately wind up negative.

[Avi (use Gerrit), 2013-10-28 16:03:49]

Done.

[Mark Mentovai, 2013-10-28 16:07:47]

LGTM otherwise

https://codereview.chromium.org/45703003/diff/140001/base/mac/foundation_util.mm
File base/mac/foundation_util.mm (right):

https://codereview.chromium.org/45703003/diff/140001/base/mac/foundation_util...
base/mac/foundation_util.mm:31: int CheapPathNormalizedCount(NSArray*
path_array) {
You can make this accept an NSString*, and perform the -[NSString
pathComponents] bit inside this function, so you don’t have to do it twice
outside this function at both of the call sites. No reason to keep this generic
and accept an NSArray* since you don’t have any callers that start out with an
NSArray*.

https://codereview.chromium.org/45703003/diff/140001/base/mac/foundation_util...
base/mac/foundation_util.mm:62: NSUInteger depth_difference = executable_count -
bundle_count;
Also make this an int.

[commit-bot: I haz the power, 2013-10-28 16:17:07]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/avi@chromium.org/45703003/130002

[Avi (use Gerrit), 2013-10-28 16:19:12]

https://codereview.chromium.org/45703003/diff/140001/base/mac/foundation_util.mm
File base/mac/foundation_util.mm (right):

https://codereview.chromium.org/45703003/diff/140001/base/mac/foundation_util...
base/mac/foundation_util.mm:31: int CheapPathNormalizedCount(NSArray*
path_array) {
On 2013/10/28 16:07:47, Mark Mentovai wrote:
> You can make this accept an NSString*, and perform the -[NSString
> pathComponents] bit inside this function, so you don’t have to do it twice
> outside this function at both of the call sites. No reason to keep this
generic
> and accept an NSArray* since you don’t have any callers that start out with an
> NSArray*.

Done.

https://codereview.chromium.org/45703003/diff/140001/base/mac/foundation_util...
base/mac/foundation_util.mm:62: NSUInteger depth_difference = executable_count -
bundle_count;
On 2013/10/28 16:07:47, Mark Mentovai wrote:
> Also make this an int.

Done.

[Mark Mentovai, 2013-10-28 16:20:53]

LGTM

[commit-bot: I haz the power, 2013-10-28 16:38:51]

Step "update" is always a major failure.
Look at the try server FAQ for more details.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win&number...

[commit-bot: I haz the power, 2013-10-28 16:41:55]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/avi@chromium.org/45703003/130002

[commit-bot: I haz the power, 2013-10-28 19:29:57]

Change committed as 231376