Add "New Window" and "New Incognito Window" items to the Dock Menu....

Add "New Window" and "New Incognito Window" items to the Dock Menu.
Add a unit test module for AppController with a simple DockMenu test.
BUG=none
TEST=none

[sgk, 2009-09-04 22:08:25]

[Preliminary:  not cc'ed to chromium-reviews.]

I keep wanting to bring up a new Chrome window in an empty Space only to have
clicking the dock zip back to another Space with an already-open Chrome window.

I noticed Safari has a "New Window" item in its dock menu, so it seemed like an
opportunity to learn just enough Cocoa to add similar item(s) to ours.

Newbie disclaimers:  I have zero idea whether I'm conforming to style, did
something that has unintended consequences, etc.  I'm also unsure of how (or
whether) to add an appropriate test for this.  Let me know!

Thanks,

[sgk, 2009-09-09 22:48:50]

added a DockMenu unit test (per advice from jrg)

[John Grabowski, 2009-09-09 23:01:08]

LGTM with nits addressed
Perhaps get feedback from one of the official UI dudes (e.g. Ben) for the dock
menu as written.

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

http://codereview.chromium.org/199031/diff/2001/3001#newcode658
Line 658: - (NSMenu*)applicationDockMenu:(id)sender {
More thoughts: Safari has a much larger menu than just this (e.g. an item for
each window).  Add it now?  File a bug to add them later?

http://codereview.chromium.org/199031/diff/2001/3001#newcode668
Line 668: [item setTarget: self];
[object selector:arg], not [object selector:<space>arg]
A few times in here

http://codereview.chromium.org/199031/diff/2001/3002
File chrome/browser/app_controller_mac_unittest.mm (right):

http://codereview.chromium.org/199031/diff/2001/3002#newcode19
Line 19: EXPECT_NE(-1, [menu indexOfItemWithTag: IDC_NEW_WINDOW]);
Note [nil anySelector] returns 0.
So if menu is nil, this EXPECT_NE will falsely pass.
Add an EXPECT_TRUE(menu) right before this line.

http://codereview.chromium.org/199031/diff/2001/3002#newcode20
Line 20: EXPECT_NE(-1, [menu indexOfItemWithTag: IDC_NEW_INCOGNITO_WINDOW]);
Like other file make this [object selector:<no space>arg];
(many spots)

[sgk, 2009-09-09 23:11:41]

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

http://codereview.chromium.org/199031/diff/2001/3001#newcode658
Line 658: - (NSMenu*)applicationDockMenu:(id)sender {
On 2009/09/09 23:01:08, John Grabowski wrote:
> More thoughts: Safari has a much larger menu than just this (e.g. an item for
> each window).  Add it now?  File a bug to add them later?

Empirically, that seems to be taken care of by the system.  From the user's
point of view, this just adds the two new menu items in a section between the
standard items at the bottom (Keep in Dock, Open at Login, Show in Finder, Hide,
Quit) and the open-window items at the top.

http://codereview.chromium.org/199031/diff/2001/3001#newcode668
Line 668: [item setTarget: self];
On 2009/09/09 23:01:08, John Grabowski wrote:
> [object selector:arg], not [object selector:<space>arg]
> A few times in here

Done.

http://codereview.chromium.org/199031/diff/2001/3002
File chrome/browser/app_controller_mac_unittest.mm (right):

http://codereview.chromium.org/199031/diff/2001/3002#newcode19
Line 19: EXPECT_NE(-1, [menu indexOfItemWithTag: IDC_NEW_WINDOW]);
On 2009/09/09 23:01:08, John Grabowski wrote:
> Note [nil anySelector] returns 0.
> So if menu is nil, this EXPECT_NE will falsely pass.
> Add an EXPECT_TRUE(menu) right before this line.
> 

Done.

http://codereview.chromium.org/199031/diff/2001/3002#newcode20
Line 20: EXPECT_NE(-1, [menu indexOfItemWithTag: IDC_NEW_INCOGNITO_WINDOW]);
On 2009/09/09 23:01:08, John Grabowski wrote:
> Like other file make this [object selector:<no space>arg];
> (many spots)

Done.

[sgk, 2009-09-10 17:54:45]

Ben--

Is this okay with you from a UI perspective?

The practical upshot is that it makes our Dock Menu like Safari's, except that
where Safari has just a "New Window" item Chrome would have both "New Window"
and "New Incognito Window" items.

[Ben Goodger (Google), 2009-09-11 16:13:08]

SGTM
On 2009/09/10 17:54:45, sgk wrote:
> Ben--
> 
> Is this okay with you from a UI perspective?
> 
> The practical upshot is that it makes our Dock Menu like Safari's, except that
> where Safari has just a "New Window" item Chrome would have both "New Window"
> and "New Incognito Window" items.