CrOS: Move force UI mode switch from switches to ash switches.

CrOS: Move force UI mode switch from switches to ash switches.

We will need this switch in ash switches to use with ash. See dependent patchset for usage.

TEST=unit_tests --gtest_filter="AboutFlagsHistogramTest.*"
BUG=701135
Review-Url: https://codereview.chromium.org/2765373002
Cr-Commit-Position: refs/heads/master@{#460391}
Committed: https://chromium.googlesource.com/chromium/src/+/df9bf7c32b4361a7b66a241317ca3571f58c28b3

[sammiequon, 2017-03-28 19:02:02]

Description was changed from

==========
CrOS: Move force UI mode switch from switches to ash switches.

We will need this switch in ash switches to use with ash.

TEST=unit_tests --gtest_filter="AboutFlagsHistogramTest.*"
BUG=701135
==========

to

==========
CrOS: Move force UI mode switch from switches to ash switches.

We will need this switch in ash switches to use with ash. See dependent patchset
for usage.

TEST=unit_tests --gtest_filter="AboutFlagsHistogramTest.*"
BUG=701135
==========

[sammiequon, 2017-03-28 19:02:02]

sammiequon@chromium.org changed reviewers:
+ derat@chromium.org

[sammiequon, 2017-03-28 19:02:38]

derat@ - Please take a look. Thanks!

[Daniel Erat, 2017-03-28 19:30:13]

https://codereview.chromium.org/2765373002/diff/20001/ash/common/ash_switches.cc
File ash/common/ash_switches.cc (right):

https://codereview.chromium.org/2765373002/diff/20001/ash/common/ash_switches...
ash/common/ash_switches.cc:73: // chromebook is currently in the selected UI
mode.
nit: Chromebook

https://codereview.chromium.org/2765373002/diff/20001/chrome/browser/about_fl...
File chrome/browser/about_flags.cc (right):

https://codereview.chromium.org/2765373002/diff/20001/chrome/browser/about_fl...
chrome/browser/about_flags.cc:2399: {"ash-force-tablet-mode",
IDS_FLAGS_FORCE_TABLET_MODE_NAME,
just to make sure i understand this: changing this will drop the old value (if
anyone has set it) and possibly leave an orphaned pref, right? maybe that's not
a big deal, but components/flags_ui/feature_entry.h mentions that you shouldn't
change it.

[sammiequon, 2017-03-28 20:32:00]

https://codereview.chromium.org/2765373002/diff/20001/ash/common/ash_switches.cc
File ash/common/ash_switches.cc (right):

https://codereview.chromium.org/2765373002/diff/20001/ash/common/ash_switches...
ash/common/ash_switches.cc:73: // chromebook is currently in the selected UI
mode.
On 2017/03/28 19:30:12, Daniel Erat wrote:
> nit: Chromebook

Done.

https://codereview.chromium.org/2765373002/diff/20001/chrome/browser/about_fl...
File chrome/browser/about_flags.cc (right):

https://codereview.chromium.org/2765373002/diff/20001/chrome/browser/about_fl...
chrome/browser/about_flags.cc:2399: {"ash-force-tablet-mode",
IDS_FLAGS_FORCE_TABLET_MODE_NAME,
On 2017/03/28 19:30:12, Daniel Erat wrote:
> just to make sure i understand this: changing this will drop the old value (if
> anyone has set it) and possibly leave an orphaned pref, right? maybe that's
not
> a big deal, but components/flags_ui/feature_entry.h mentions that you
shouldn't
> change it.

I see. Would this work? It wouldn't have "ash-" but then we wouldn't have to
have two flags showing up with one doing nothing.

[Daniel Erat, 2017-03-28 20:35:33]

lgtm

https://codereview.chromium.org/2765373002/diff/20001/chrome/browser/about_fl...
File chrome/browser/about_flags.cc (right):

https://codereview.chromium.org/2765373002/diff/20001/chrome/browser/about_fl...
chrome/browser/about_flags.cc:2399: {"ash-force-tablet-mode",
IDS_FLAGS_FORCE_TABLET_MODE_NAME,
On 2017/03/28 20:32:00, sammiequon wrote:
> On 2017/03/28 19:30:12, Daniel Erat wrote:
> > just to make sure i understand this: changing this will drop the old value
(if
> > anyone has set it) and possibly leave an orphaned pref, right? maybe that's
> not
> > a big deal, but components/flags_ui/feature_entry.h mentions that you
> shouldn't
> > change it.
> 
> I see. Would this work? It wouldn't have "ash-" but then we wouldn't have to
> have two flags showing up with one doing nothing.

yes, i think that what you have now is fine (at least from my reading of this
struct's comments, it sounded like this identifier is just used internally, so i
think it's okay to keep the old value without the "ash-" prefix).

[sammiequon, 2017-03-28 20:44:08]

On 2017/03/28 20:35:33, Daniel Erat wrote:
> lgtm
> 
>
https://codereview.chromium.org/2765373002/diff/20001/chrome/browser/about_fl...
> File chrome/browser/about_flags.cc (right):
> 
>
https://codereview.chromium.org/2765373002/diff/20001/chrome/browser/about_fl...
> chrome/browser/about_flags.cc:2399: {"ash-force-tablet-mode",
> IDS_FLAGS_FORCE_TABLET_MODE_NAME,
> On 2017/03/28 20:32:00, sammiequon wrote:
> > On 2017/03/28 19:30:12, Daniel Erat wrote:
> > > just to make sure i understand this: changing this will drop the old value
> (if
> > > anyone has set it) and possibly leave an orphaned pref, right? maybe
that's
> > not
> > > a big deal, but components/flags_ui/feature_entry.h mentions that you
> > shouldn't
> > > change it.
> > 
> > I see. Would this work? It wouldn't have "ash-" but then we wouldn't have to
> > have two flags showing up with one doing nothing.
> 
> yes, i think that what you have now is fine (at least from my reading of this
> struct's comments, it sounded like this identifier is just used internally, so
i
> think it's okay to keep the old value without the "ash-" prefix).

Thanks!

[sammiequon, 2017-03-28 21:46:49]

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

[commit-bot: I haz the power, 2017-03-28 21:47:11]

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, 2017-03-28 22:19:32]

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

[commit-bot: I haz the power, 2017-03-28 22:19:33]

Dry run: Try jobs failed on following builders:
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[sammiequon, 2017-03-28 22:21:59]

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

[commit-bot: I haz the power, 2017-03-28 22:22:37]

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, 2017-03-28 23:48:54]

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

[commit-bot: I haz the power, 2017-03-28 23:48:55]

Dry run: Try jobs failed on following builders:
  ios-simulator on master.tryserver.chromium.mac (JOB_TIMED_OUT, build hasn't
started yet, builder probably lacks capacity)

[sammiequon, 2017-03-29 04:15:00]

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

[commit-bot: I haz the power, 2017-03-29 04:15:39]

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, 2017-03-29 05:42:56]

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

[commit-bot: I haz the power, 2017-03-29 05:42:57]

Dry run: This issue passed the CQ dry run.

[sammiequon, 2017-03-29 15:11:02]

The CQ bit was checked by sammiequon@chromium.org

[sammiequon, 2017-03-29 15:11:03]

The patchset sent to the CQ was uploaded after l-g-t-m from derat@chromium.org
Link to the patchset: https://codereview.chromium.org/2765373002/#ps60001
(title: "Rebased.")

[commit-bot: I haz the power, 2017-03-29 15:11:41]

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, 2017-03-29 15:17:07]

CQ is committing da patch.

Bot data: {"patchset_id": 60001, "attempt_start_ts": 1490800262954580,
"parent_rev": "21879589342d3991449a8f6eef8ef40802a61d5d", "commit_rev":
"df9bf7c32b4361a7b66a241317ca3571f58c28b3"}

[commit-bot: I haz the power, 2017-03-29 15:17:59]

Description was changed from

==========
CrOS: Move force UI mode switch from switches to ash switches.

We will need this switch in ash switches to use with ash. See dependent patchset
for usage.

TEST=unit_tests --gtest_filter="AboutFlagsHistogramTest.*"
BUG=701135
==========

to

==========
CrOS: Move force UI mode switch from switches to ash switches.

We will need this switch in ash switches to use with ash. See dependent patchset
for usage.

TEST=unit_tests --gtest_filter="AboutFlagsHistogramTest.*"
BUG=701135

Review-Url: https://codereview.chromium.org/2765373002
Cr-Commit-Position: refs/heads/master@{#460391}
Committed:
https://chromium.googlesource.com/chromium/src/+/df9bf7c32b4361a7b66a241317ca...
==========

[commit-bot: I haz the power, 2017-03-29 15:18:00]

Committed patchset #4 (id:60001) as
https://chromium.googlesource.com/chromium/src/+/df9bf7c32b4361a7b66a241317ca...