Enable consistent identity on android platform by default.

Enable consistent identity on android platform by default.

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

[Roger Tawa OOO till Jul 10th, 2014-06-11 12:32:52]

Hi Hui,

Can you take a look please?  Also see corresponding change in cl/68954515.
Thanks.

[Roger Tawa OOO till Jul 10th, 2014-06-11 12:56:30]

Hi Monica,

Can you take a look profile changes?  Thanks.

[noms (inactive), 2014-06-11 14:14:12]

c/b/profiles lgtm, with super annoying nits :)

https://codereview.chromium.org/323133005/diff/80001/components/signin/core/c...
File components/signin/core/common/profile_management_switches.cc (right):

https://codereview.chromium.org/323133005/diff/80001/components/signin/core/c...
components/signin/core/common/profile_management_switches.cc:28: // is properly
initialize.
nit: s/initialize/initialized, here and below. sorry, i know it's not your
comment :(

https://codereview.chromium.org/323133005/diff/80001/components/signin/core/c...
File components/signin/core/common/signin_switches.cc (right):

https://codereview.chromium.org/323133005/diff/80001/components/signin/core/c...
components/signin/core/common/signin_switches.cc:5: #include
"build/build_config.h"
nit: i think this include should go below, because signin_switches.h is this
cc's file header?

https://codereview.chromium.org/323133005/diff/80001/components/signin/core/c...
components/signin/core/common/signin_switches.cc:39: // Disables new profile
management system, including new choosers.
nit: profile choosers? new profile chooser ui? (here and below)

https://codereview.chromium.org/323133005/diff/80001/components/signin/core/c...
File components/signin/core/common/signin_switches.h (right):

https://codereview.chromium.org/323133005/diff/80001/components/signin/core/c...
components/signin/core/common/signin_switches.h:26: // account consistency are
on by default.  User can choose to turn them off.
nit: s/user/users (here and below)

[guohui, 2014-06-11 15:53:01]

+asvitkine@ for the finch question below.

https://codereview.chromium.org/323133005/diff/80001/chrome/browser/about_fla...
File chrome/browser/about_flags.cc (right):

https://codereview.chromium.org/323133005/diff/80001/chrome/browser/about_fla...
chrome/browser/about_flags.cc:1447: #else
i think an alternative to creating two sets of switches is to use one set of
switches that flips the default state, e.g. "flip-new-profile-management", where
the default state is defined differently based on platforms.

However, this may confuse finch, so not sure if it is better, +aleksei.

https://codereview.chromium.org/323133005/diff/80001/components/signin/core/c...
File components/signin/core/common/profile_management_switches.cc (right):

https://codereview.chromium.org/323133005/diff/80001/components/signin/core/c...
components/signin/core/common/profile_management_switches.cc:26: State
GetProcessState() {
eh this method is essentially a copy/paste of the method below, with different
default states and different switch names, can we pass these variables as
parameters instead?

[Alexei Svitkine (slow), 2014-06-11 16:54:04]

https://codereview.chromium.org/323133005/diff/80001/chrome/browser/about_fla...
File chrome/browser/about_flags.cc (right):

https://codereview.chromium.org/323133005/diff/80001/chrome/browser/about_fla...
chrome/browser/about_flags.cc:1447: #else
On 2014/06/11 15:53:01, guohui wrote:
> i think an alternative to creating two sets of switches is to use one set of
> switches that flips the default state, e.g. "flip-new-profile-management",
where
> the default state is defined differently based on platforms.
> 
> However, this may confuse finch, so not sure if it is better, +aleksei.

I think it's better to use two sets of switches. That way, someone passing an
explicit --enable-new-profile-management or --disable-new-profile-management
will really be getting what they specify, instead of trying to guess the
default.

I also don't see the harm of just having a single block for these (i.e. no
ifdef) using the ENABLE_DISABLE_VALUE_TYPE() macro.

[Roger Tawa OOO till Jul 10th, 2014-06-13 02:17:19]

Having enable and disable flags on all platforms was not as hard as I thought. I
also cleaned up the way tests force specific mirror states. Please take another
look. Probably worth looking at patchset 5 as a new one and not compare against
previous ones. Thanks.

https://codereview.chromium.org/323133005/diff/80001/components/signin/core/c...
File components/signin/core/common/profile_management_switches.cc (right):

https://codereview.chromium.org/323133005/diff/80001/components/signin/core/c...
components/signin/core/common/profile_management_switches.cc:28: // is properly
initialize.
On 2014/06/11 14:14:12, Monica Dinculescu wrote:
> nit: s/initialize/initialized, here and below. sorry, i know it's not your
> comment :(

Done.

https://codereview.chromium.org/323133005/diff/80001/components/signin/core/c...
File components/signin/core/common/signin_switches.cc (right):

https://codereview.chromium.org/323133005/diff/80001/components/signin/core/c...
components/signin/core/common/signin_switches.cc:5: #include
"build/build_config.h"
On 2014/06/11 14:14:12, Monica Dinculescu wrote:
> nit: i think this include should go below, because signin_switches.h is this
> cc's file header?

Done.

https://codereview.chromium.org/323133005/diff/80001/components/signin/core/c...
components/signin/core/common/signin_switches.cc:39: // Disables new profile
management system, including new choosers.
On 2014/06/11 14:14:12, Monica Dinculescu wrote:
> nit: profile choosers? new profile chooser ui? (here and below)

Done.

https://codereview.chromium.org/323133005/diff/80001/components/signin/core/c...
File components/signin/core/common/signin_switches.h (right):

https://codereview.chromium.org/323133005/diff/80001/components/signin/core/c...
components/signin/core/common/signin_switches.h:26: // account consistency are
on by default.  User can choose to turn them off.
On 2014/06/11 14:14:12, Monica Dinculescu wrote:
> nit: s/user/users (here and below)

Done.

[guohui, 2014-06-13 17:56:12]

lgtm thanks!

https://codereview.chromium.org/323133005/diff/180001/components/signin/core/...
File components/signin/core/common/profile_management_switches.cc (right):

https://codereview.chromium.org/323133005/diff/180001/components/signin/core/...
components/signin/core/common/profile_management_switches.cc:27: // is properly
initialize.
nit:s/initialize/initialized

[Roger Tawa OOO till Jul 10th, 2014-06-16 17:23:35]

Hi Scott,

Can you do an owner review please?  Thanks.

https://codereview.chromium.org/323133005/diff/180001/components/signin/core/...
File components/signin/core/common/profile_management_switches.cc (right):

https://codereview.chromium.org/323133005/diff/180001/components/signin/core/...
components/signin/core/common/profile_management_switches.cc:27: // is properly
initialize.
On 2014/06/13 17:56:11, guohui wrote:
> nit:s/initialize/initialized

Done.

[Alexei Svitkine (slow), 2014-06-16 17:39:03]

lgtm

[sky, 2014-06-16 19:52:46]

On Mon, Jun 16, 2014 at 10:23 AM, <rogerta@chromium.org> wrote:

> Hi Scott,
>
> Can you do an owner review please?  Thanks.


Can you be more specific. What files do you need me to review?

  -Scott

>
>
>
> https://codereview.chromium.org/323133005/diff/180001/
> components/signin/core/common/profile_management_switches.cc
> File components/signin/core/common/profile_management_switches.cc
> (right):
>
> https://codereview.chromium.org/323133005/diff/180001/
> components/signin/core/common/profile_management_switches.cc#newcode27
> components/signin/core/common/profile_management_switches.cc:27: // is
> properly initialize.
> On 2014/06/13 17:56:11, guohui wrote:
>
>> nit:s/initialize/initialized
>>
>
> Done.
>
> https://codereview.chromium.org/323133005/
>

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

[Roger Tawa OOO till Jul 10th, 2014-06-16 21:33:06]

Sorry Scott.  The ui related files:

chrome/browser/ui/views/profiles/new_avatar_menu_button_browsertest.cc
chrome/browser/ui/views/frame/opaque_browser_frame_view_layout_unittest.cc
chrome/browser/ui/browser_command_controller_unittest.cc
chrome/browser/app_controller_mac_browsertest.mm

Thanks.

[sky, 2014-06-17 03:04:12]

Said files LGTM

[Roger Tawa OOO till Jul 10th, 2014-06-17 14:59:46]

Hi Xiyuan,

Can you take a look at profile_list_chromeos.cc please?  Thanks.

[xiyuan, 2014-06-17 16:13:23]

profile_list_chromeos.cc LGTM

[Roger Tawa OOO till Jul 10th, 2014-06-17 17:06:38]

The CQ bit was checked by rogerta@chromium.org

[commit-bot: I haz the power, 2014-06-17 17:07:04]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/rogerta@chromium.org/323133005/260001

[commit-bot: I haz the power, 2014-06-17 17:21:01]

Change committed as 277793