Puts the new Material Design User Manager behind a flag

Puts the new Material Design User Manager behind a flag

Committed: https://crrev.com/4059f332355d7ba51b72e46c589c5c8c7efb65cc
Cr-Commit-Position: refs/heads/master@{#380451}

[Moe, 2016-03-09 21:12:48]

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

[Moe, 2016-03-09 21:12:49]

Hi Anthony, please review my changes.

[anthonyvd, 2016-03-10 16:01:45]

lgtm % a few nits.

Thanks!

https://codereview.chromium.org/1782583003/diff/1/chrome/browser/profiles/pro...
File chrome/browser/profiles/profile_window.cc (right):

https://codereview.chromium.org/1782583003/diff/1/chrome/browser/profiles/pro...
chrome/browser/profiles/profile_window.cc:9: #include <vector>
Do you need <vector>?

https://codereview.chromium.org/1782583003/diff/1/chrome/browser/profiles/pro...
chrome/browser/profiles/profile_window.cc:203: chrome::kChromeUIMdUserManagerUrl
: chrome::kChromeUIUserManagerURL;
Same comment as below w.r.t. "Md" abbreviation.

https://codereview.chromium.org/1782583003/diff/1/components/signin/core/comm...
File components/signin/core/common/profile_management_switches.cc (right):

https://codereview.chromium.org/1782583003/diff/1/components/signin/core/comm...
components/signin/core/common/profile_management_switches.cc:7: #include
<string>
Is this include necessary?

https://codereview.chromium.org/1782583003/diff/1/components/signin/core/comm...
File components/signin/core/common/profile_management_switches.h (right):

https://codereview.chromium.org/1782583003/diff/1/components/signin/core/comm...
components/signin/core/common/profile_management_switches.h:39: bool
IsMDUserManager();
I'd avoid the "MD" abbreviation and go with IsMaterialDesignUserManager() just
so that readers don't have to come and read this comment.

[Alexei Svitkine (slow), 2016-03-10 16:04:32]

asvitkine@chromium.org changed reviewers:
+ asvitkine@chromium.org

[Alexei Svitkine (slow), 2016-03-10 16:04:33]

https://codereview.chromium.org/1782583003/diff/1/components/signin/core/comm...
File components/signin/core/common/profile_management_switches.h (right):

https://codereview.chromium.org/1782583003/diff/1/components/signin/core/comm...
components/signin/core/common/profile_management_switches.h:39: bool
IsMDUserManager();
On 2016/03/10 16:01:45, anthonyvd wrote:
> I'd avoid the "MD" abbreviation and go with IsMaterialDesignUserManager() just
> so that readers don't have to come and read this comment.

Drive-by: Why not use the FeatureList::IsEnabled() switch directly instead of
having a wrapper?

[Moe, 2016-03-10 18:18:12]

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

[Moe, 2016-03-10 18:18:22]

https://codereview.chromium.org/1782583003/diff/1/chrome/browser/profiles/pro...
File chrome/browser/profiles/profile_window.cc (right):

https://codereview.chromium.org/1782583003/diff/1/chrome/browser/profiles/pro...
chrome/browser/profiles/profile_window.cc:9: #include <vector>
On 2016/03/10 16:01:44, anthonyvd wrote:
> Do you need <vector>?

Since i touched this file, git-cl lint complained about 'include what you use'.

https://codereview.chromium.org/1782583003/diff/1/chrome/browser/profiles/pro...
chrome/browser/profiles/profile_window.cc:203: chrome::kChromeUIMdUserManagerUrl
: chrome::kChromeUIUserManagerURL;
On 2016/03/10 16:01:44, anthonyvd wrote:
> Same comment as below w.r.t. "Md" abbreviation.

Done.

https://codereview.chromium.org/1782583003/diff/1/components/signin/core/comm...
File components/signin/core/common/profile_management_switches.cc (right):

https://codereview.chromium.org/1782583003/diff/1/components/signin/core/comm...
components/signin/core/common/profile_management_switches.cc:7: #include
<string>
On 2016/03/10 16:01:44, anthonyvd wrote:
> Is this include necessary?

to quiet git-cl lint

https://codereview.chromium.org/1782583003/diff/1/components/signin/core/comm...
File components/signin/core/common/profile_management_switches.h (right):

https://codereview.chromium.org/1782583003/diff/1/components/signin/core/comm...
components/signin/core/common/profile_management_switches.h:39: bool
IsMDUserManager();
On 2016/03/10 16:01:45, anthonyvd wrote:
> I'd avoid the "MD" abbreviation and go with IsMaterialDesignUserManager() just
> so that readers don't have to come and read this comment.

Done.

https://codereview.chromium.org/1782583003/diff/1/components/signin/core/comm...
components/signin/core/common/profile_management_switches.h:39: bool
IsMDUserManager();
On 2016/03/10 16:04:33, Alexei Svitkine (very slow) wrote:
> On 2016/03/10 16:01:45, anthonyvd wrote:
> > I'd avoid the "MD" abbreviation and go with IsMaterialDesignUserManager()
just
> > so that readers don't have to come and read this comment.
> 
> Drive-by: Why not use the FeatureList::IsEnabled() switch directly instead of
> having a wrapper?

simply following the existing standard in this file. I think it's not a bad
thing that this file is hiding implementation details of different
flags/switches

[Roger Tawa OOO till Jul 10th, 2016-03-10 18:23:17]

lgtm

https://codereview.chromium.org/1782583003/diff/20001/chrome/browser/profiles...
File chrome/browser/profiles/profile_window.cc (right):

https://codereview.chromium.org/1782583003/diff/20001/chrome/browser/profiles...
chrome/browser/profiles/profile_window.cc:203: chrome::kChromeUIMdUserManagerUrl
: chrome::kChromeUIUserManagerURL;
Not part of this CL, but I wonder if this constant should end with "URL" instead
of "Url"?

https://code.google.com/p/chromium/codesearch#chromium/src/chrome/common/url_...

[Moe, 2016-03-10 18:28:09]

The CQ bit was checked by mahmadi@chromium.org

[Moe, 2016-03-10 18:28:09]

The patchset sent to the CQ was uploaded after l-g-t-m from
anthonyvd@chromium.org
Link to the patchset: https://codereview.chromium.org/1782583003/#ps20001
(title: "Addressed comments")

[commit-bot: I haz the power, 2016-03-10 18:28:47]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1782583003/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1782583003/20001

[commit-bot: I haz the power, 2016-03-10 19:57:36]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-03-10 19:59:43]

Description was changed from

==========
Puts the new Material Design User Manager behind a flag
==========

to

==========
Puts the new Material Design User Manager behind a flag

Committed: https://crrev.com/4059f332355d7ba51b72e46c589c5c8c7efb65cc
Cr-Commit-Position: refs/heads/master@{#380451}
==========

[commit-bot: I haz the power, 2016-03-10 19:59:45]

Patchset 2 (id:??) landed as
https://crrev.com/4059f332355d7ba51b72e46c589c5c8c7efb65cc
Cr-Commit-Position: refs/heads/master@{#380451}