NTP: Adds a context menu to the apps section...

NTP: Adds a context menu to the apps section

This uses the cr Menu system and replaces the old options menu with a cr Menu.

BUG=52446
TEST=Start chrome with enable-apps. Install some apps.
Right clicking on an app should show a context menu.
Clicking the wrench should show the same menu.



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

[arv (Not doing code reviews), 2010-09-03 21:55:46]

This is now ready for review.

[Aaron Boodman, 2010-09-03 23:46:15]

http://codereview.chromium.org/3315005/diff/32001/19006
File chrome/browser/resources/new_new_tab.js (right):

http://codereview.chromium.org/3315005/diff/32001/19006#newcode593
chrome/browser/resources/new_new_tab.js:593: while (p && p.tagName != 'H2' &&
p.tagName != 'BUTTON') {
I think this only works because getSectionMaxiview() below fails.

It would be better to just check the button case separately and return early
explicitly.

[Miranda Callahan, 2010-09-04 00:24:06]

What about the other context menu options for the nonpanel apps that Glen lists
in his mock -- the three options to open in different ways?

[arv1, 2010-09-04 00:25:54]

On 5:24 pm, mirandac <mirandac@chromium.org> wrote:
> What about the other context menu options for the nonpanel apps that
> Glen lists
> in his mock -- the three options to open in different ways?

I don't think we have the required infrasturcture for these at the moment.  
If we do I'd still want to do them in a separate patch.

[arv (Not doing code reviews), 2010-09-04 01:01:21]

Resynced, resolved, rebuilt an reteststed.

http://codereview.chromium.org/3315005/diff/32001/19006
File chrome/browser/resources/new_new_tab.js (right):

http://codereview.chromium.org/3315005/diff/32001/19006#newcode593
chrome/browser/resources/new_new_tab.js:593: while (p && p.tagName != 'H2' &&
p.tagName != 'BUTTON') {
On 2010/09/03 23:46:15, Aaron Boodman wrote:
> I think this only works because getSectionMaxiview() below fails.
> 
> It would be better to just check the button case separately and return early
> explicitly.

Yup. I'll make the suggested change since I agree that it is a bit obscure.

[Aaron Boodman, 2010-09-04 01:37:19]

On 2010/09/04 01:01:21, arv wrote:
> Yup. I'll make the suggested change since I agree that it is a bit obscure.

I still don't see this change.

LGTM after that.

[Miranda Callahan, 2010-09-04 02:59:38]

On 2010/09/04 00:25:54, arv1 wrote:
> On 5:24 pm, mirandac <mailto:mirandac@chromium.org> wrote:
> > What about the other context menu options for the nonpanel apps that
> > Glen lists
> > in his mock -- the three options to open in different ways?
> 
> I don't think we have the required infrasturcture for these at the moment.  
> If we do I'd still want to do them in a separate patch.
> 

Ok.  LGTM from me too.