Moving Background App support on the Mac Dock menu into its own submenu

Moving Background App support on the Mac Dock menu into its own submenu

BUG=69448
TEST=manual

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

[John Gregg, 2011-01-20 19:58:26]

http://codereview.chromium.org/6241009/diff/2001/chrome/browser/app_controlle...
File chrome/browser/app_controller_mac.mm (right):

http://codereview.chromium.org/6241009/diff/2001/chrome/browser/app_controlle...
chrome/browser/app_controller_mac.mm:1203: int position = 0;
move declaration of position inside if block

http://codereview.chromium.org/6241009/diff/2001/chrome/browser/app_controlle...
chrome/browser/app_controller_mac.mm:1206: scoped_nsobject<NSMenu>
appMenu([[NSMenu alloc] initWithTitle: @"Background apps"]);
80 char.  How is that raw string okay for i18n?

http://codereview.chromium.org/6241009/diff/2001/chrome/browser/app_controlle...
chrome/browser/app_controller_mac.mm:1223: keyEquivalent:@""]);
formatting in this function is inconsistent with Obj-C style.  Check earlier in
the file for examples (basically, always align on ':' with multiple args to
function)

[Andrew T Wilson (Slow), 2011-01-20 20:02:33]

http://codereview.chromium.org/6241009/diff/2001/chrome/browser/app_controlle...
File chrome/browser/app_controller_mac.mm (right):

http://codereview.chromium.org/6241009/diff/2001/chrome/browser/app_controlle...
chrome/browser/app_controller_mac.mm:1206: scoped_nsobject<NSMenu>
appMenu([[NSMenu alloc] initWithTitle: @"Background apps"]);
On 2011/01/20 19:58:26, John Gregg wrote:
> 80 char.  How is that raw string okay for i18n?  
See below (GetNSStringXXXX) for examples of how to use a localized string here.

[The wrong rickcam account, 2011-01-20 22:13:37]

Please take another look.

In addition to the changes discussed here, my next revision moves the Background
Apps entry below New Incognito Window.

http://codereview.chromium.org/6241009/diff/2001/chrome/browser/app_controlle...
File chrome/browser/app_controller_mac.mm (right):

http://codereview.chromium.org/6241009/diff/2001/chrome/browser/app_controlle...
chrome/browser/app_controller_mac.mm:1203: int position = 0;
On 2011/01/20 19:58:26, John Gregg wrote:
> move declaration of position inside if block

Done.

http://codereview.chromium.org/6241009/diff/2001/chrome/browser/app_controlle...
chrome/browser/app_controller_mac.mm:1206: scoped_nsobject<NSMenu>
appMenu([[NSMenu alloc] initWithTitle: @"Background apps"]);
On 2011/01/20 19:58:26, John Gregg wrote:
> 80 char.  How is that raw string okay for i18n?  

Done.  It appears that content of the string is unused.  LIne 1221 (in the
reviewed revision) has string that is displayed in the dock menu.  Here I
considered using @"" which I've seen used elsewhere for an empty NSString.  What
I did instead was to introduce a localized string, in the manner suggested by
Drew, and use that for both the NSMenuItem and the NSMenu associated with the
Background Apps submenu.

http://codereview.chromium.org/6241009/diff/2001/chrome/browser/app_controlle...
chrome/browser/app_controller_mac.mm:1221:
initWithTitle:base::SysUTF16ToNSString(UTF8ToUTF16("Background apps"))
Replaced "Background apps" with a localized NSString for "Background Apps" (note
capitalized A in Apps)

http://codereview.chromium.org/6241009/diff/2001/chrome/browser/app_controlle...
chrome/browser/app_controller_mac.mm:1223: keyEquivalent:@""]);
On 2011/01/20 19:58:26, John Gregg wrote:
> formatting in this function is inconsistent with Obj-C style.  Check earlier
in
> the file for examples (basically, always align on ':' with multiple args to
> function)

Done.  Touched up a few other spots that had the same problem.

[John Gregg, 2011-01-20 23:00:21]

lgtm with style nit.

http://codereview.chromium.org/6241009/diff/11001/chrome/browser/app_controll...
File chrome/browser/app_controller_mac.mm (right):

http://codereview.chromium.org/6241009/diff/11001/chrome/browser/app_controll...
chrome/browser/app_controller_mac.mm:1202: keyEquivalent:@""]);
super nitpicky, but I don't think this is formatted correctly.

initWithTitle should be indented 4 and the rest match on ':' (like places below)
or line-break before [[NSMenuItem alloc indent 4, move initWithTitle up, and
work from there.

[Andrew T Wilson (Slow), 2011-01-20 23:03:04]

LGTM