Enable the MD User Menu by default on trunk.

Enable the MD User Menu by default on trunk.

BUG=616242
Committed: https://crrev.com/e3c600aa49b1e2bed95bfe9298a733bd72a0cb4e
Cr-Commit-Position: refs/heads/master@{#423787}

[anthonyvd, 2016-10-05 18:11:13]

anthonyvd@chromium.org changed reviewers:
+ rogerta@chromium.org

[anthonyvd, 2016-10-05 18:11:14]

Hi Roger, this change enables the new User Menu by default on ToT. Can you PTAL?

Thanks!

[Roger Tawa OOO till Jul 10th, 2016-10-05 18:56:22]

lgtm

[anthonyvd, 2016-10-05 18:58:14]

The CQ bit was checked by anthonyvd@chromium.org

[commit-bot: I haz the power, 2016-10-05 18:59:03]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-05 19:47:01]

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

[commit-bot: I haz the power, 2016-10-05 19:47:02]

Try jobs failed on following builders:
  linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[anthonyvd, 2016-10-05 20:20:26]

The CQ bit was checked by anthonyvd@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-10-05 20:20:55]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-05 21:14:22]

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

[commit-bot: I haz the power, 2016-10-05 21:14:22]

Dry run: Try jobs failed on following builders:
  linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[anthonyvd, 2016-10-05 22:03:59]

The CQ bit was checked by anthonyvd@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-10-05 22:04:49]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-05 22:49:57]

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

[commit-bot: I haz the power, 2016-10-05 22:49:58]

Dry run: Try jobs failed on following builders:
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[anthonyvd, 2016-10-06 14:17:20]

The CQ bit was checked by anthonyvd@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-10-06 14:17:31]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[anthonyvd, 2016-10-06 14:17:43]

The CQ bit was unchecked by anthonyvd@chromium.org

[anthonyvd, 2016-10-06 14:18:10]

The CQ bit was checked by anthonyvd@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-10-06 14:18:34]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[anthonyvd, 2016-10-06 16:28:30]

The CQ bit was checked by anthonyvd@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-10-06 16:28:50]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[anthonyvd, 2016-10-06 17:06:15]

The CQ bit was unchecked by anthonyvd@chromium.org

[anthonyvd, 2016-10-06 17:06:58]

The CQ bit was checked by anthonyvd@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-10-06 17:07:17]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[anthonyvd, 2016-10-06 17:47:24]

Patchset #6 (id:100001) has been deleted

[anthonyvd, 2016-10-06 17:47:30]

Patchset #5 (id:80001) has been deleted

[anthonyvd, 2016-10-06 17:47:36]

Patchset #4 (id:60001) has been deleted

[anthonyvd, 2016-10-06 17:47:44]

Patchset #3 (id:40001) has been deleted

[anthonyvd, 2016-10-06 17:47:49]

Patchset #2 (id:20001) has been deleted

[anthonyvd, 2016-10-06 18:09:25]

anthonyvd@chromium.org changed reviewers:
+ maxbogue@chromium.org, rsesek@chromium.org, sky@chromium.org

[anthonyvd, 2016-10-06 18:09:26]

Hi everyone,

We're enabling the Material Design User Menu on ToT and the flag being enabled
by default broke some tests. This CL enables the flag by default and fixes those
tests. Can you PTAL?

maxbogue@: c/b/sync/
sky@: c/b/ui/views/profiles/
rsesek@: c/b/ui/cocoa/profiles/

Thanks a lot!

[Robert Sesek, 2016-10-06 18:49:35]

https://codereview.chromium.org/2396893002/diff/120001/chrome/browser/ui/coco...
File chrome/browser/ui/cocoa/profiles/profile_chooser_controller_unittest.mm
(right):

https://codereview.chromium.org/2396893002/diff/120001/chrome/browser/ui/coco...
chrome/browser/ui/cocoa/profiles/profile_chooser_controller_unittest.mm:218:
NSUInteger last_subview_index = switches::IsMaterialDesignUserMenu() ? 4 : 2;
These tests are incorrectly using camelCase for variable names (when under_score
is appropriate for C++ functions). But consistency is the overriding style rule,
so this should probably be named lastSubviewIndex (and throughout the CL).

https://codereview.chromium.org/2396893002/diff/120001/chrome/browser/ui/coco...
chrome/browser/ui/cocoa/profiles/profile_chooser_controller_unittest.mm:298:
auto views_count = switches::IsMaterialDesignUserMenu() ? 5U : 3U;
Use NSUInteger instead of auto and drop the 'U's?

[anthonyvd, 2016-10-06 19:06:15]

https://codereview.chromium.org/2396893002/diff/120001/chrome/browser/ui/coco...
File chrome/browser/ui/cocoa/profiles/profile_chooser_controller_unittest.mm
(right):

https://codereview.chromium.org/2396893002/diff/120001/chrome/browser/ui/coco...
chrome/browser/ui/cocoa/profiles/profile_chooser_controller_unittest.mm:218:
NSUInteger last_subview_index = switches::IsMaterialDesignUserMenu() ? 4 : 2;
On 2016/10/06 at 18:49:35, Robert Sesek wrote:
> These tests are incorrectly using camelCase for variable names (when
under_score is appropriate for C++ functions). But consistency is the overriding
style rule, so this should probably be named lastSubviewIndex (and throughout
the CL).

Ahh, makes sense! Done everywhere.

https://codereview.chromium.org/2396893002/diff/120001/chrome/browser/ui/coco...
chrome/browser/ui/cocoa/profiles/profile_chooser_controller_unittest.mm:298:
auto views_count = switches::IsMaterialDesignUserMenu() ? 5U : 3U;
On 2016/10/06 at 18:49:35, Robert Sesek wrote:
> Use NSUInteger instead of auto and drop the 'U's?

Done.

[maxbogue, 2016-10-06 19:22:36]

lgtm w/ question

https://codereview.chromium.org/2396893002/diff/140001/chrome/browser/sync/sy...
File chrome/browser/sync/sync_global_error_unittest.cc (right):

https://codereview.chromium.org/2396893002/diff/140001/chrome/browser/sync/sy...
chrome/browser/sync/sync_global_error_unittest.cc:129: if
(switches::IsMaterialDesignUserMenu())
Presumably at some point this switch will be removed and then this entire test
would be removed as well?

[anthonyvd, 2016-10-06 19:25:44]

https://codereview.chromium.org/2396893002/diff/140001/chrome/browser/sync/sy...
File chrome/browser/sync/sync_global_error_unittest.cc (right):

https://codereview.chromium.org/2396893002/diff/140001/chrome/browser/sync/sy...
chrome/browser/sync/sync_global_error_unittest.cc:129: if
(switches::IsMaterialDesignUserMenu())
On 2016/10/06 at 19:22:36, maxbogue wrote:
> Presumably at some point this switch will be removed and then this entire test
would be removed as well?

Absolutely. There's going to be some clean up involved when removing that switch
and this is part of it.

[Robert Sesek, 2016-10-06 20:12:14]

LGTM

[sky, 2016-10-06 22:17:52]

LGTM

[anthonyvd, 2016-10-07 01:30:20]

The CQ bit was checked by anthonyvd@chromium.org

[anthonyvd, 2016-10-07 01:30:21]

The patchset sent to the CQ was uploaded after l-g-t-m from rogerta@chromium.org
Link to the patchset: https://codereview.chromium.org/2396893002/#ps140001
(title: "Address feedback on cocoa tests.")

[commit-bot: I haz the power, 2016-10-07 01:30:36]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-07 02:44:36]

Committed patchset #3 (id:140001)

[commit-bot: I haz the power, 2016-10-07 02:47:15]

Description was changed from

==========
Enable the MD User Menu by default on trunk.

BUG=616242
==========

to

==========
Enable the MD User Menu by default on trunk.

BUG=616242

Committed: https://crrev.com/e3c600aa49b1e2bed95bfe9298a733bd72a0cb4e
Cr-Commit-Position: refs/heads/master@{#423787}
==========

[commit-bot: I haz the power, 2016-10-07 02:47:17]

Patchset 3 (id:??) landed as
https://crrev.com/e3c600aa49b1e2bed95bfe9298a733bd72a0cb4e
Cr-Commit-Position: refs/heads/master@{#423787}