[FastTextAutosizer] Enable FastTextAutosizer on tot/dev by default

[FastTextAutosizer] Enable FastTextAutosizer on tot/dev by default

This patch enables the FastTextAutosizer on tot/dev by default in order
to get some early feedback before flipping this live everywhere.

I've added a tri-state default/enabled/disabled flag in about://flags
where the disabled state takes precedence. By default, chrome
now has the fast text autosizer enabled but content does not.

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

[skobes, 2014-03-19 19:41:29]

lgtm

https://codereview.chromium.org/203473003/diff/1/chrome/browser/android/chrom...
File chrome/browser/android/chrome_startup_flags.cc (right):

https://codereview.chromium.org/203473003/diff/1/chrome/browser/android/chrom...
chrome/browser/android/chrome_startup_flags.cc:59: // Enable the Fast Text
Autosizer on local builds, canary and dev-channel.
Does this mean local builds will use FTA even without
--enable-fast-text-autosizing?  If so, how do we test a local build with the old
autosizer?

[pdr., 2014-03-19 19:48:05]

(+cc johnme and timvolodine in case they have comments as well)

On 2014/03/19 19:41:29, skobes wrote:
> lgtm
> 
>
https://codereview.chromium.org/203473003/diff/1/chrome/browser/android/chrom...
> File chrome/browser/android/chrome_startup_flags.cc (right):
> 
>
https://codereview.chromium.org/203473003/diff/1/chrome/browser/android/chrom...
> chrome/browser/android/chrome_startup_flags.cc:59: // Enable the Fast Text
> Autosizer on local builds, canary and dev-channel.
> Does this mean local builds will use FTA even without
> --enable-fast-text-autosizing?  If so, how do we test a local build with the
old
> autosizer?

We would have to manually comment out this line :/ My thinking was that this
would be a 1 week trial run. Does that sound reasonable or should we add a flag
for disabling the fast text autosizer?

[skobes, 2014-03-19 20:26:29]

On 2014/03/19 19:48:05, pdr wrote:
> We would have to manually comment out this line :/

That will get annoying quickly. :)  Can we turn it on for dev and canary, but
not local builds?

[timvolodine, 2014-03-20 11:48:14]

On 2014/03/19 20:26:29, skobes wrote:
> On 2014/03/19 19:48:05, pdr wrote:
> > We would have to manually comment out this line :/
> 
> That will get annoying quickly. :)  Can we turn it on for dev and canary, but
> not local builds?

For testing it would be nice to have a way to switch between the two. When I was
looking at some pages recently I actually flipped the switch quite often, but
that may be me trying to obsessively compare the algorithms ;)
How about having a MULTI_VALUE_TYPE in about:flags?

[pdr., 2014-03-20 20:25:01]

On 2014/03/20 11:48:14, timvolodine wrote:
> On 2014/03/19 20:26:29, skobes wrote:
> > On 2014/03/19 19:48:05, pdr wrote:
> > > We would have to manually comment out this line :/
> > 
> > That will get annoying quickly. :)  Can we turn it on for dev and canary,
but
> > not local builds?
> 
> For testing it would be nice to have a way to switch between the two. When I
was
> looking at some pages recently I actually flipped the switch quite often, but
> that may be me trying to obsessively compare the algorithms ;)
> How about having a MULTI_VALUE_TYPE in about:flags?

Done :) This will be better long-term as well.

We now have a tri-state flag in about://flags with default (enabled on clank
dev, disabled elsewhere), enabled, and disabled.

PTAL

[skobes, 2014-03-20 20:48:18]

lgtm

https://codereview.chromium.org/203473003/diff/20001/content/child/runtime_fe...
File content/child/runtime_features.cc (right):

https://codereview.chromium.org/203473003/diff/20001/content/child/runtime_fe...
content/child/runtime_features.cc:161:
WebRuntimeFeatures::enableFastTextAutosizing(false);
The default is false in RuntimeEnabledFeatures, right?  So you could just do

if (command_line.HasSwitch(switches::kEnableFastTextAutosizing) &&
    !command_line.HasSwitch(switches::kDisableFastTextAutosizing))
  WebRuntimeFeatures::enableFastTextAutosizing(true);

[pdr., 2014-03-20 21:07:36]

On 2014/03/20 20:48:18, skobes wrote:
> lgtm
> 
>
https://codereview.chromium.org/203473003/diff/20001/content/child/runtime_fe...
> File content/child/runtime_features.cc (right):
> 
>
https://codereview.chromium.org/203473003/diff/20001/content/child/runtime_fe...
> content/child/runtime_features.cc:161:
> WebRuntimeFeatures::enableFastTextAutosizing(false);
> The default is false in RuntimeEnabledFeatures, right?  So you could just do
> 
> if (command_line.HasSwitch(switches::kEnableFastTextAutosizing) &&
>     !command_line.HasSwitch(switches::kDisableFastTextAutosizing))
>   WebRuntimeFeatures::enableFastTextAutosizing(true);

Done. I also removed an extra space in content_switches.cc so everything lines
up properly. I think we're ready to roll.

Adding @jochen and @piman for chrome/ and content/ OWNERS

[piman, 2014-03-20 21:16:04]

lgtm

https://codereview.chromium.org/203473003/diff/40001/content/browser/renderer...
File content/browser/renderer_host/render_process_host_impl.cc (right):

https://codereview.chromium.org/203473003/diff/40001/content/browser/renderer...
content/browser/renderer_host/render_process_host_impl.cc:1044:
switches::kDisableFastTextAutosizing,
Can you add this (and the enable one too, I think) to
chrome/browser/chromeos/login/chrome_restart_request.cc so that it applies in
guest mode too?

[pdr., 2014-03-20 21:23:45]

On 2014/03/20 21:16:04, piman wrote:
> lgtm
> 
>
https://codereview.chromium.org/203473003/diff/40001/content/browser/renderer...
> File content/browser/renderer_host/render_process_host_impl.cc (right):
> 
>
https://codereview.chromium.org/203473003/diff/40001/content/browser/renderer...
> content/browser/renderer_host/render_process_host_impl.cc:1044:
> switches::kDisableFastTextAutosizing,
> Can you add this (and the enable one too, I think) to
> chrome/browser/chromeos/login/chrome_restart_request.cc so that it applies in
> guest mode too?

I'm not very familiar with ChromeOS but this feature only applies to Android so
I don't think we should add this to the ChromeOS guest mode. Text autosizing is
really only useful on very small screens.

I may be misunderstanding. Does that sound reasonable to you?

[piman, 2014-03-20 21:42:32]

On Thu, Mar 20, 2014 at 2:23 PM, <pdr@chromium.org> wrote:

> On 2014/03/20 21:16:04, piman wrote:
>
>> lgtm
>>
>
>
> https://codereview.chromium.org/203473003/diff/40001/
> content/browser/renderer_host/render_process_host_impl.cc
>
>> File content/browser/renderer_host/render_process_host_impl.cc (right):
>>
>
>
> https://codereview.chromium.org/203473003/diff/40001/
> content/browser/renderer_host/render_process_host_impl.cc#newcode1044
>
>> content/browser/renderer_host/render_process_host_impl.cc:1044:
>> switches::kDisableFastTextAutosizing,
>> Can you add this (and the enable one too, I think) to
>> chrome/browser/chromeos/login/chrome_restart_request.cc so that it
>> applies in
>> guest mode too?
>>
>
> I'm not very familiar with ChromeOS but this feature only applies to
> Android so
> I don't think we should add this to the ChromeOS guest mode. Text
> autosizing is
> really only useful on very small screens.
>
> I may be misunderstanding. Does that sound reasonable to you?
>

Then put the flag definition and usage in a #ifdef OS_ANDROID.

That said, who knows what the future is made of, if it works on other
platforms, why disable the flags there (may help testing/debugging etc.).
Either way, let's be consistent.


> https://codereview.chromium.org/203473003/
>

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

[pdr., 2014-03-20 21:42:58]

The CQ bit was checked by pdr@chromium.org

[pdr., 2014-03-20 21:43:15]

The CQ bit was unchecked by pdr@chromium.org

[pdr., 2014-03-20 21:43:38]

(Apologies, I misclicked the button)

[pdr., 2014-03-20 21:48:13]

On 2014/03/20 21:42:32, piman wrote:
> On Thu, Mar 20, 2014 at 2:23 PM, <mailto:pdr@chromium.org> wrote:
> 
> > On 2014/03/20 21:16:04, piman wrote:
> >
> >> lgtm
> >>
> >
> >
> > https://codereview.chromium.org/203473003/diff/40001/
> > content/browser/renderer_host/render_process_host_impl.cc
> >
> >> File content/browser/renderer_host/render_process_host_impl.cc (right):
> >>
> >
> >
> > https://codereview.chromium.org/203473003/diff/40001/
> > content/browser/renderer_host/render_process_host_impl.cc#newcode1044
> >
> >> content/browser/renderer_host/render_process_host_impl.cc:1044:
> >> switches::kDisableFastTextAutosizing,
> >> Can you add this (and the enable one too, I think) to
> >> chrome/browser/chromeos/login/chrome_restart_request.cc so that it
> >> applies in
> >> guest mode too?
> >>
> >
> > I'm not very familiar with ChromeOS but this feature only applies to
> > Android so
> > I don't think we should add this to the ChromeOS guest mode. Text
> > autosizing is
> > really only useful on very small screens.
> >
> > I may be misunderstanding. Does that sound reasonable to you?
> >
> 
> Then put the flag definition and usage in a #ifdef OS_ANDROID.
> 
> That said, who knows what the future is made of, if it works on other
> platforms, why disable the flags there (may help testing/debugging etc.).
> Either way, let's be consistent.

Sounds reasonable to me. I've gone ahead and added it there. Thanks for the
review!

[pdr., 2014-03-20 21:48:16]

The CQ bit was checked by pdr@chromium.org

[commit-bot: I haz the power, 2014-03-20 21:52:14]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/pdr@chromium.org/203473003/60001

[commit-bot: I haz the power, 2014-03-20 22:26:22]

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

[commit-bot: I haz the power, 2014-03-20 22:26:22]

Retried try job too often on chromium_presubmit for step(s) presubmit
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=chromium_p...

[pdr., 2014-03-20 22:28:38]

On 2014/03/20 22:26:22, I haz the power (commit-bot) wrote:
> Retried try job too often on chromium_presubmit for step(s) presubmit
>
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=chromium_p...

+cc bulach, tedchoc for chrome/browser/android OWNER

[bulach, 2014-03-21 09:07:23]

lgtm, thanks!

[Ted C, 2014-03-21 10:23:47]

lgtm (w/ nit)

https://codereview.chromium.org/203473003/diff/60001/content/child/runtime_fe...
File content/child/runtime_features.cc (right):

https://codereview.chromium.org/203473003/diff/60001/content/child/runtime_fe...
content/child/runtime_features.cc:161: &&
!command_line.HasSwitch(switches::kDisableFastTextAutosizing))
+2 indent

[pdr., 2014-03-21 16:29:51]

On 2014/03/21 10:23:47, Ted C wrote:
> lgtm (w/ nit)
> 
>
https://codereview.chromium.org/203473003/diff/60001/content/child/runtime_fe...
> File content/child/runtime_features.cc (right):
> 
>
https://codereview.chromium.org/203473003/diff/60001/content/child/runtime_fe...
> content/child/runtime_features.cc:161: &&
> !command_line.HasSwitch(switches::kDisableFastTextAutosizing))
> +2 indent

Blink engineer trying to code in chrome...isn't fooling anyone :P

Fixed

[pdr., 2014-03-21 16:29:58]

The CQ bit was checked by pdr@chromium.org

[commit-bot: I haz the power, 2014-03-21 16:30:47]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/pdr@chromium.org/203473003/80001

[commit-bot: I haz the power, 2014-03-21 16:32:49]

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

[commit-bot: I haz the power, 2014-03-21 16:32:49]

Try jobs failed on following builders:
  tryserver.chromium on linux_chromium_clang_dbg

[pdr., 2014-03-21 16:36:08]

The CQ bit was checked by pdr@chromium.org

[commit-bot: I haz the power, 2014-03-21 16:37:52]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/pdr@chromium.org/203473003/80001

[commit-bot: I haz the power, 2014-03-21 16:39:24]

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

[commit-bot: I haz the power, 2014-03-21 16:39:25]

Try jobs failed on following builders:
  tryserver.chromium on linux_chromium_clang_dbg

[pdr., 2014-03-21 18:38:26]

The CQ bit was checked by pdr@chromium.org

[commit-bot: I haz the power, 2014-03-21 18:39:04]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/pdr@chromium.org/203473003/80001

[commit-bot: I haz the power, 2014-03-21 18:42:53]

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

[commit-bot: I haz the power, 2014-03-21 18:42:53]

Try jobs failed on following builders:
  tryserver.chromium on linux_chromium_clang_dbg

[pdr., 2014-03-21 18:45:00]

The CQ bit was checked by pdr@chromium.org

[commit-bot: I haz the power, 2014-03-21 18:46:02]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/pdr@chromium.org/203473003/80001

[commit-bot: I haz the power, 2014-03-21 20:27:36]

Change committed as 258661

[Nico, 2014-04-22 22:14:51]

Just happened to see this while looking for supporting data for my rant on
https://chromiumcodereview.appspot.com/229533002/ . 

We try not to do things depending on the channel you're on, with the reasoning
being that earlier channels are supposed to test what we ship on the more mature
channels. Why don't you just turn this on by default, and if it's not ready turn
it off again? The bug says that you're worried about this since you don't want
to lose test coverage for the old way, but that won't regress in a few days, and
that's how we roll out almost all flag flips.

(If you're super worried I suppose you could launch this behind a finch flag and
then turn that on globally for a short while, but then you get the aura issue of
the perfbots not using your change.)

[chromium-reviews, 2014-04-22 22:33:07]

We wanted to turn it on for a subset of users to get feedback before the
full launch.  Is there a preferred way of doing that?  It seems like a
common requirement.

Philip investigated doing a Finch trial but concluded it was not
appropriate for this, I don't remember what the reason was.


On Tue, Apr 22, 2014 at 3:14 PM, <thakis@chromium.org> wrote:

> Just happened to see this while looking for supporting data for my rant on
> https://chromiumcodereview.appspot.com/229533002/ .
>
> We try not to do things depending on the channel you're on, with the
> reasoning
> being that earlier channels are supposed to test what we ship on the more
> mature
> channels. Why don't you just turn this on by default, and if it's not
> ready turn
> it off again? The bug says that you're worried about this since you don't
> want
> to lose test coverage for the old way, but that won't regress in a few
> days, and
> that's how we roll out almost all flag flips.
>
> (If you're super worried I suppose you could launch this behind a finch
> flag and
> then turn that on globally for a short while, but then you get the aura
> issue of
> the perfbots not using your change.)
>
> https://codereview.chromium.org/203473003/
>

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

[Nico, 2014-04-22 22:33:53]

On Tue, Apr 22, 2014 at 3:32 PM, Steve Kobes <skobes@google.com> wrote:

> We wanted to turn it on for a subset of users to get feedback before the
> full launch.  Is there a preferred way of doing that?  It seems like a
> common requirement.
>

The preferred way is to turn it on on dev / beta.


>
> Philip investigated doing a Finch trial but concluded it was not
> appropriate for this, I don't remember what the reason was.
>
>
> On Tue, Apr 22, 2014 at 3:14 PM, <thakis@chromium.org> wrote:
>
>> Just happened to see this while looking for supporting data for my rant on
>> https://chromiumcodereview.appspot.com/229533002/ .
>>
>> We try not to do things depending on the channel you're on, with the
>> reasoning
>> being that earlier channels are supposed to test what we ship on the more
>> mature
>> channels. Why don't you just turn this on by default, and if it's not
>> ready turn
>> it off again? The bug says that you're worried about this since you don't
>> want
>> to lose test coverage for the old way, but that won't regress in a few
>> days, and
>> that's how we roll out almost all flag flips.
>>
>> (If you're super worried I suppose you could launch this behind a finch
>> flag and
>> then turn that on globally for a short while, but then you get the aura
>> issue of
>> the perfbots not using your change.)
>>
>> https://codereview.chromium.org/203473003/
>>
>
>

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

[chromium-reviews, 2014-04-22 22:38:32]

I'm confused.  You just said "We try not to do things depending on the
channel you're on..."?

Why is it bad for dev and beta to be different, but ok for beta and stable
to be different?


On Tue, Apr 22, 2014 at 3:33 PM, Nico Weber <thakis@chromium.org> wrote:

> On Tue, Apr 22, 2014 at 3:32 PM, Steve Kobes <skobes@google.com> wrote:
>
>> We wanted to turn it on for a subset of users to get feedback before the
>> full launch.  Is there a preferred way of doing that?  It seems like a
>> common requirement.
>>
>
> The preferred way is to turn it on on dev / beta.
>
>
>>
>> Philip investigated doing a Finch trial but concluded it was not
>> appropriate for this, I don't remember what the reason was.
>>
>>
>> On Tue, Apr 22, 2014 at 3:14 PM, <thakis@chromium.org> wrote:
>>
>>> Just happened to see this while looking for supporting data for my rant
>>> on
>>> https://chromiumcodereview.appspot.com/229533002/ .
>>>
>>> We try not to do things depending on the channel you're on, with the
>>> reasoning
>>> being that earlier channels are supposed to test what we ship on the
>>> more mature
>>> channels. Why don't you just turn this on by default, and if it's not
>>> ready turn
>>> it off again? The bug says that you're worried about this since you
>>> don't want
>>> to lose test coverage for the old way, but that won't regress in a few
>>> days, and
>>> that's how we roll out almost all flag flips.
>>>
>>> (If you're super worried I suppose you could launch this behind a finch
>>> flag and
>>> then turn that on globally for a short while, but then you get the aura
>>> issue of
>>> the perfbots not using your change.)
>>>
>>> https://codereview.chromium.org/203473003/
>>>
>>
>>
>

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

[Nico, 2014-04-22 22:40:08]

On Tue, Apr 22, 2014 at 3:38 PM, Steve Kobes <skobes@google.com> wrote:

> I'm confused.  You just said "We try not to do things depending on the
> channel you're on..."?
>
> Why is it bad for dev and beta to be different, but ok for beta and stable
> to be different?
>

Sorry, that was worded poorly. With "turn it on on dev / beta" I mean "turn
it on, unconditionally, wait for that to reach dev / beta" (and then just
let it reach stable if it's ready).


>
>
> On Tue, Apr 22, 2014 at 3:33 PM, Nico Weber <thakis@chromium.org> wrote:
>
>> On Tue, Apr 22, 2014 at 3:32 PM, Steve Kobes <skobes@google.com> wrote:
>>
>>> We wanted to turn it on for a subset of users to get feedback before the
>>> full launch.  Is there a preferred way of doing that?  It seems like a
>>> common requirement.
>>>
>>
>> The preferred way is to turn it on on dev / beta.
>>
>>
>>>
>>> Philip investigated doing a Finch trial but concluded it was not
>>> appropriate for this, I don't remember what the reason was.
>>>
>>>
>>> On Tue, Apr 22, 2014 at 3:14 PM, <thakis@chromium.org> wrote:
>>>
>>>> Just happened to see this while looking for supporting data for my rant
>>>> on
>>>> https://chromiumcodereview.appspot.com/229533002/ .
>>>>
>>>> We try not to do things depending on the channel you're on, with the
>>>> reasoning
>>>> being that earlier channels are supposed to test what we ship on the
>>>> more mature
>>>> channels. Why don't you just turn this on by default, and if it's not
>>>> ready turn
>>>> it off again? The bug says that you're worried about this since you
>>>> don't want
>>>> to lose test coverage for the old way, but that won't regress in a few
>>>> days, and
>>>> that's how we roll out almost all flag flips.
>>>>
>>>> (If you're super worried I suppose you could launch this behind a finch
>>>> flag and
>>>> then turn that on globally for a short while, but then you get the aura
>>>> issue of
>>>> the perfbots not using your change.)
>>>>
>>>> https://codereview.chromium.org/203473003/
>>>>
>>>
>>>
>>
>

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

[chromium-reviews, 2014-04-22 22:49:15]

Ah, I see now.  I think we wanted to launch to a subset without being tied
to a particular milestone release.  Like, if we turned it on
unconditionally but decided it wasn't ready for beta then we would have to
turn it off (unconditionally) before it hits beta, even if we would rather
leave it on in dev.

But your point about testing what we ship makes sense.  At this point I
think we have enough data on the known issues that I'd be ok with turning
it off.  Philip WDYT?


On Tue, Apr 22, 2014 at 3:40 PM, Nico Weber <thakis@chromium.org> wrote:

> On Tue, Apr 22, 2014 at 3:38 PM, Steve Kobes <skobes@google.com> wrote:
>
>> I'm confused.  You just said "We try not to do things depending on the
>> channel you're on..."?
>>
>> Why is it bad for dev and beta to be different, but ok for beta and
>> stable to be different?
>>
>
> Sorry, that was worded poorly. With "turn it on on dev / beta" I mean
> "turn it on, unconditionally, wait for that to reach dev / beta" (and then
> just let it reach stable if it's ready).
>
>
>>
>>
>> On Tue, Apr 22, 2014 at 3:33 PM, Nico Weber <thakis@chromium.org> wrote:
>>
>>> On Tue, Apr 22, 2014 at 3:32 PM, Steve Kobes <skobes@google.com> wrote:
>>>
>>>> We wanted to turn it on for a subset of users to get feedback before
>>>> the full launch.  Is there a preferred way of doing that?  It seems like a
>>>> common requirement.
>>>>
>>>
>>> The preferred way is to turn it on on dev / beta.
>>>
>>>
>>>>
>>>> Philip investigated doing a Finch trial but concluded it was not
>>>> appropriate for this, I don't remember what the reason was.
>>>>
>>>>
>>>> On Tue, Apr 22, 2014 at 3:14 PM, <thakis@chromium.org> wrote:
>>>>
>>>>> Just happened to see this while looking for supporting data for my
>>>>> rant on
>>>>> https://chromiumcodereview.appspot.com/229533002/ .
>>>>>
>>>>> We try not to do things depending on the channel you're on, with the
>>>>> reasoning
>>>>> being that earlier channels are supposed to test what we ship on the
>>>>> more mature
>>>>> channels. Why don't you just turn this on by default, and if it's not
>>>>> ready turn
>>>>> it off again? The bug says that you're worried about this since you
>>>>> don't want
>>>>> to lose test coverage for the old way, but that won't regress in a few
>>>>> days, and
>>>>> that's how we roll out almost all flag flips.
>>>>>
>>>>> (If you're super worried I suppose you could launch this behind a
>>>>> finch flag and
>>>>> then turn that on globally for a short while, but then you get the
>>>>> aura issue of
>>>>> the perfbots not using your change.)
>>>>>
>>>>> https://codereview.chromium.org/203473003/
>>>>>
>>>>
>>>>
>>>
>>
>

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

[pdr., 2014-04-22 22:59:52]

I'm happy to say this is going away soon because we're launching.

This feature is a core change and we have almost no tests that hit the code
(only ~100). We only had real data from one user (me) plus the real world impact
script that tests static views of the top 1,000 pages. We discussed this with
Grace and this seemed to be a special case where the additional data was worth
the channel skew. In retrospect, I think you're right that we should have just
flipped this on in the first place.

[Nico, 2014-04-22 23:00:33]

On Tue, Apr 22, 2014 at 3:48 PM, Steve Kobes <skobes@google.com> wrote:

> Ah, I see now.  I think we wanted to launch to a subset without being tied
> to a particular milestone release.  Like, if we turned it on
> unconditionally but decided it wasn't ready for beta then we would have to
> turn it off (unconditionally) before it hits beta, even if we would rather
> leave it on in dev.
>

I think the usual feature progression is:
1.) Feature is behind a flag. There's enough stuff broken with it on that
you don't need data to turn it on. You fix all known critical bugs.
2.) Flag on by default. Two possible futures:
2a: You learn about some new bugs, but nothing you can't fix. You fix bugs
as they come in, feature stays on, launches.
2b: You learn about major new bugs you weren't aware of. In this case, goto
1 as having it on won't give you useful new data.

With this progression, it's possible that beta arrives at some inopportune
moment during 2a. In that case, you can turn your feature off on beta, but
if you time 2 right that should be relatively rare.


>
> But your point about testing what we ship makes sense.  At this point I
> think we have enough data on the known issues that I'd be ok with turning
> it off.  Philip WDYT?
>
>
> On Tue, Apr 22, 2014 at 3:40 PM, Nico Weber <thakis@chromium.org> wrote:
>
>> On Tue, Apr 22, 2014 at 3:38 PM, Steve Kobes <skobes@google.com> wrote:
>>
>>> I'm confused.  You just said "We try not to do things depending on the
>>> channel you're on..."?
>>>
>>> Why is it bad for dev and beta to be different, but ok for beta and
>>> stable to be different?
>>>
>>
>> Sorry, that was worded poorly. With "turn it on on dev / beta" I mean
>> "turn it on, unconditionally, wait for that to reach dev / beta" (and then
>> just let it reach stable if it's ready).
>>
>>
>>>
>>>
>>> On Tue, Apr 22, 2014 at 3:33 PM, Nico Weber <thakis@chromium.org> wrote:
>>>
>>>> On Tue, Apr 22, 2014 at 3:32 PM, Steve Kobes <skobes@google.com> wrote:
>>>>
>>>>> We wanted to turn it on for a subset of users to get feedback before
>>>>> the full launch.  Is there a preferred way of doing that?  It seems like
a
>>>>> common requirement.
>>>>>
>>>>
>>>> The preferred way is to turn it on on dev / beta.
>>>>
>>>>
>>>>>
>>>>> Philip investigated doing a Finch trial but concluded it was not
>>>>> appropriate for this, I don't remember what the reason was.
>>>>>
>>>>>
>>>>> On Tue, Apr 22, 2014 at 3:14 PM, <thakis@chromium.org> wrote:
>>>>>
>>>>>> Just happened to see this while looking for supporting data for my
>>>>>> rant on
>>>>>> https://chromiumcodereview.appspot.com/229533002/ .
>>>>>>
>>>>>> We try not to do things depending on the channel you're on, with the
>>>>>> reasoning
>>>>>> being that earlier channels are supposed to test what we ship on the
>>>>>> more mature
>>>>>> channels. Why don't you just turn this on by default, and if it's not
>>>>>> ready turn
>>>>>> it off again? The bug says that you're worried about this since you
>>>>>> don't want
>>>>>> to lose test coverage for the old way, but that won't regress in a
>>>>>> few days, and
>>>>>> that's how we roll out almost all flag flips.
>>>>>>
>>>>>> (If you're super worried I suppose you could launch this behind a
>>>>>> finch flag and
>>>>>> then turn that on globally for a short while, but then you get the
>>>>>> aura issue of
>>>>>> the perfbots not using your change.)
>>>>>>
>>>>>> https://codereview.chromium.org/203473003/
>>>>>>
>>>>>
>>>>>
>>>>
>>>
>>
>

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

[pdr., 2014-04-22 23:22:08]

On 2014/04/22 23:00:33, Nico wrote:

Steve and I chatted about this. crbug.com/365911 is the new bug and I'll be
reverting the dev-channel special code tomorrow (so it's on for m36).

[chromium-reviews, 2014-04-22 23:28:03]

On Tue, Apr 22, 2014 at 4:00 PM, Nico Weber <thakis@chromium.org> wrote:

> I think the usual feature progression is:
> 1.) Feature is behind a flag. There's enough stuff broken with it on that
> you don't need data to turn it on. You fix all known critical bugs.
> 2.) Flag on by default. Two possible futures:
> 2a: You learn about some new bugs, but nothing you can't fix. You fix bugs
> as they come in, feature stays on, launches.
> 2b: You learn about major new bugs you weren't aware of. In this case,
> goto 1 as having it on won't give you useful new data.
>

We should put this on a wiki somewhere. :)

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