Make all views platforms use 'new' (cros-style) wrench menu.

Make all views platforms use 'new' (cros-style) wrench menu.

On ChromeOS, this patch should have no effect on the menu's appearance.

On Windows, this patch will give the menu the new style buttons, item placement, and item strings. The rows with buttons should be the same height before and after the patch (which is less than the height for ChromeOS).

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

[Evan Stade, 2014-07-30 00:13:09]

see comments on bug from sgabriel

[sky, 2014-07-30 01:06:53]

I added a comment to the bug. Ping this review when the issue is resolved.

On Tue, Jul 29, 2014 at 5:13 PM,  <estade@chromium.org> wrote:
> Reviewers: sky,
>
> Message:
> see comments on bug from sgabriel
>
> Description:
> Make all views platforms use 'new' (cros-style) wrench menu.
>
> BUG=365418
>
> Please review this at https://codereview.chromium.org/431473003/
>
> SVN Base: svn://svn.chromium.org/chrome/trunk/src
>
> Affected files (+67, -203 lines):
>   M chrome/browser/ui/toolbar/wrench_menu_model.h
>   M chrome/browser/ui/toolbar/wrench_menu_model.cc
>   M chrome/browser/ui/views/toolbar/toolbar_view.cc
>   M chrome/browser/ui/views/toolbar/wrench_menu.h
>   M chrome/browser/ui/views/toolbar/wrench_menu.cc
>
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[Evan Stade, 2014-07-30 21:51:57]

sgabriel responded on the bug. Is that response sufficient, or would you like me
to make sure that moving the page zoom control is desired as well?

On 2014/07/30 01:06:53, sky wrote:
> I added a comment to the bug. Ping this review when the issue is resolved.
> 
> On Tue, Jul 29, 2014 at 5:13 PM,  <mailto:estade@chromium.org> wrote:
> > Reviewers: sky,
> >
> > Message:
> > see comments on bug from sgabriel
> >
> > Description:
> > Make all views platforms use 'new' (cros-style) wrench menu.
> >
> > BUG=365418
> >
> > Please review this at https://codereview.chromium.org/431473003/
> >
> > SVN Base: svn://svn.chromium.org/chrome/trunk/src
> >
> > Affected files (+67, -203 lines):
> >   M chrome/browser/ui/toolbar/wrench_menu_model.h
> >   M chrome/browser/ui/toolbar/wrench_menu_model.cc
> >   M chrome/browser/ui/views/toolbar/toolbar_view.cc
> >   M chrome/browser/ui/views/toolbar/wrench_menu.h
> >   M chrome/browser/ui/views/toolbar/wrench_menu.cc
> >
> >
> 
> To unsubscribe from this group and stop receiving emails from it, send an
email
> to mailto:chromium-reviews+unsubscribe@chromium.org.

[Peter Kasting, 2014-08-03 09:28:19]

Not LGTM (see comments on bug)

[Evan Stade, 2014-08-04 19:24:40]

patch updated; screenshots on bug. PTAL

[Peter Kasting, 2014-08-04 21:27:44]

On 2014/08/03 09:28:19, Peter Kasting wrote:
> Not LGTM (see comments on bug)

Withdrawing this -- LGTM on appearance

[sky, 2014-08-04 21:52:43]

LGTM

[Evan Stade, 2014-08-04 21:53:25]

thanks for review.

[Evan Stade, 2014-08-04 21:54:08]

The CQ bit was checked by estade@chromium.org

[commit-bot: I haz the power, 2014-08-04 21:56:09]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/estade@chromium.org/431473003/150001

[commit-bot: I haz the power, 2014-08-04 23:06:23]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  mac_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/mac_gpu/builds/37237)
  android_aosp on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_aosp/bu...)
  linux_chromium_chromeos_rel_swarming on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_rel_swarming on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  mac_chromium_compile_dbg on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
  mac_chromium_rel on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel/...)
  win8_chromium_rel on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win8_chromium_rel...)
  win_chromium_compile_dbg on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_comp...)
  win_chromium_rel on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel/...)
  win_chromium_x64_rel on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[commit-bot: I haz the power, 2014-08-04 23:12:43]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-08-04 23:12:44]

Try jobs failed on following builders:
  mac_chromium_compile_dbg on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)

[Evan Stade, 2014-08-05 00:55:28]

The CQ bit was checked by estade@chromium.org

[commit-bot: I haz the power, 2014-08-05 00:57:34]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/estade@chromium.org/431473003/170001

[commit-bot: I haz the power, 2014-08-05 02:50:59]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  linux_chromium_chromeos_rel_swarming on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  win8_chromium_rel on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win8_chromium_rel...)

[commit-bot: I haz the power, 2014-08-05 04:19:38]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-08-05 04:19:39]

Try jobs failed on following builders:
  linux_chromium_chromeos_rel_swarming on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Evan Stade, 2014-08-05 17:31:06]

The CQ bit was checked by estade@chromium.org

[commit-bot: I haz the power, 2014-08-05 17:32:55]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/estade@chromium.org/431473003/170001

[commit-bot: I haz the power, 2014-08-05 18:20:32]

Change committed as 287570