Tab switcher theme colors no longer behind flag

Tab switcher theme colors no longer behind flag

Tab switcher theme colors are now on by default.

BUG=624944
Committed: https://crrev.com/cbac917bbb4eaaf08438358505f00f66999cd355
Cr-Commit-Position: refs/heads/master@{#415078}

[mdjones, 2016-07-28 01:08:43]

mdjones@chromium.org changed reviewers:
+ dfalcantara@chromium.org

[mdjones, 2016-07-28 01:08:44]

ptal

[gone, 2016-07-28 16:20:18]

You'll need the metrics folks to approve the histogram change.

https://chromiumcodereview.appspot.com/2163393003/diff/40001/tools/metrics/hi...
File tools/metrics/histograms/histograms.xml (left):

https://chromiumcodereview.appspot.com/2163393003/diff/40001/tools/metrics/hi...
tools/metrics/histograms/histograms.xml:82358: -  <int value="423855924"
label="enable-tab-switcher-theme-colors"/>
Feels like littering, but you never remove a flag from here, AIUI.

[gone, 2016-07-28 16:20:38]

lgtm for the rest, though.

[mdjones, 2016-07-28 16:49:38]

mdjones@chromium.org changed reviewers:
+ rkaplow@chromium.org

[mdjones, 2016-07-28 16:49:38]

+rkaplow for histograms.xml

[mdjones, 2016-07-28 16:50:42]

https://codereview.chromium.org/2163393003/diff/40001/tools/metrics/histogram...
File tools/metrics/histograms/histograms.xml (left):

https://codereview.chromium.org/2163393003/diff/40001/tools/metrics/histogram...
tools/metrics/histograms/histograms.xml:82358: -  <int value="423855924"
label="enable-tab-switcher-theme-colors"/>
On 2016/07/28 16:20:18, dfalcantara wrote:
> Feels like littering, but you never remove a flag from here, AIUI.

I know that's the rule for histograms, I didn't think that was true for flags as
well; we'll see what rkaplow@ says.

[rkaplow, 2016-07-28 19:47:23]

https://codereview.chromium.org/2163393003/diff/40001/tools/metrics/histogram...
File tools/metrics/histograms/histograms.xml (left):

https://codereview.chromium.org/2163393003/diff/40001/tools/metrics/histogram...
tools/metrics/histograms/histograms.xml:82358: -  <int value="423855924"
label="enable-tab-switcher-theme-colors"/>
On 2016/07/28 16:50:42, mdjones wrote:
> On 2016/07/28 16:20:18, dfalcantara wrote:
> > Feels like littering, but you never remove a flag from here, AIUI.
> 
> I know that's the rule for histograms, I didn't think that was true for flags
as
> well; we'll see what rkaplow@ says.

Dan's right, it is preferable to keep the enum, as otherwise it will show up as
423855924 which can get annoying. We suggest you rename it to |name| (obsolete)
though.

[mdjones, 2016-07-28 20:31:34]

https://codereview.chromium.org/2163393003/diff/40001/tools/metrics/histogram...
File tools/metrics/histograms/histograms.xml (left):

https://codereview.chromium.org/2163393003/diff/40001/tools/metrics/histogram...
tools/metrics/histograms/histograms.xml:82358: -  <int value="423855924"
label="enable-tab-switcher-theme-colors"/>
On 2016/07/28 19:47:23, rkaplow wrote:
> On 2016/07/28 16:50:42, mdjones wrote:
> > On 2016/07/28 16:20:18, dfalcantara wrote:
> > > Feels like littering, but you never remove a flag from here, AIUI.
> > 
> > I know that's the rule for histograms, I didn't think that was true for
flags
> as
> > well; we'll see what rkaplow@ says.
> 
> Dan's right, it is preferable to keep the enum, as otherwise it will show up
as
> 423855924 which can get annoying. We suggest you rename it to |name|
(obsolete)
> though.

Done.

[rkaplow, 2016-07-29 20:20:56]

lgtm

thanks!

[mdjones, 2016-07-29 20:47:02]

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

[commit-bot: I haz the power, 2016-07-29 20:47:25]

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-07-29 20:50:21]

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

[commit-bot: I haz the power, 2016-07-29 20:50:22]

Dry run: Try jobs failed on following builders:
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)
  ios-device-gn on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device-gn/bui...)
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)
  ios-simulator-gn on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator-gn/...)
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[mdjones, 2016-07-29 22:05:33]

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

[commit-bot: I haz the power, 2016-07-29 22:06:06]

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-07-29 22:51:47]

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

[commit-bot: I haz the power, 2016-07-29 22:51:49]

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_...)

[mdjones, 2016-08-11 18:01:21]

The CQ bit was checked by mdjones@chromium.org

[mdjones, 2016-08-11 18:01:22]

The patchset sent to the CQ was uploaded after l-g-t-m from
dfalcantara@chromium.org, rkaplow@chromium.org
Link to the patchset: https://codereview.chromium.org/2163393003/#ps80001
(title: "rebase")

[commit-bot: I haz the power, 2016-08-11 18:02:11]

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-08-11 18:41:05]

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

[commit-bot: I haz the power, 2016-08-11 18:41:06]

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

[mdjones, 2016-08-11 18:50:32]

On 2016/07/29 20:20:56, rkaplow wrote:
> lgtm
> 
> thanks!

Changing the flag name appears to mean I need to change the ID. Doesn't the ID
need to remain the same?

[mdjones, 2016-08-29 15:42:41]

mdjones@chromium.org changed reviewers:
+ jwd@chromium.org

[mdjones, 2016-08-29 15:42:42]

+jwd since rkaplow is ooo. How should the obsolete flag be renamed? Adding
"obsolete" makes the ID invalid and I believe we want the ID to stay the same.

[jwd, 2016-08-29 20:04:06]

On 2016/08/29 15:42:42, mdjones wrote:
> +jwd since rkaplow is ooo. How should the obsolete flag be renamed? Adding
> "obsolete" makes the ID invalid and I believe we want the ID to stay the same.

Hm, so it looks like these values are just left there as is when flags are
removed.

[mdjones, 2016-08-29 21:28:59]

On 2016/08/29 20:04:06, jwd wrote:
> On 2016/08/29 15:42:42, mdjones wrote:
> > +jwd since rkaplow is ooo. How should the obsolete flag be renamed? Adding
> > "obsolete" makes the ID invalid and I believe we want the ID to stay the
same.
> 
> Hm, so it looks like these values are just left there as is when flags are
> removed.

Ok, thanks for the info.

[mdjones, 2016-08-29 21:29:17]

The CQ bit was checked by mdjones@chromium.org

[mdjones, 2016-08-29 21:29:17]

The patchset sent to the CQ was uploaded after l-g-t-m from
rkaplow@chromium.org, dfalcantara@chromium.org
Link to the patchset: https://codereview.chromium.org/2163393003/#ps100001
(title: "Littering flags")

[commit-bot: I haz the power, 2016-08-29 21:43:21]

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-08-30 04:12:16]

Committed patchset #6 (id:100001)

[commit-bot: I haz the power, 2016-08-30 04:14:00]

Description was changed from

==========
Tab switcher theme colors no longer behind flag

Tab switcher theme colors are now on by default.

BUG=624944
==========

to

==========
Tab switcher theme colors no longer behind flag

Tab switcher theme colors are now on by default.

BUG=624944

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

[commit-bot: I haz the power, 2016-08-30 04:14:02]

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