Enable by default threaded compositing on windows and FCM on mac

Enable by default:
1. threaded compositing on windows (except XP)
2. force compositing mode on mac and linux with 30% probability (for channels < beta)

This patch also gives priority to the command line switches so that both threaded
and force compositing mode can be turned off via the about:flags page or command line.

In the near future, we will use the Finch framework to tweak the experiment percentages.

It also disables ExtensionApiTest.CaptureVisibleTabJPeg as it triggers flakiness on it. chrome.tabs.captureVisibleTab is inherently racy as there is no way to guarantee that the tab has been painted at least once when that call is made. Note how the other variants of CaptureVisibleTab tests have already been disabled for similar reasons.

BUG=143809, 128387, 146861
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=158707

[Vangelis Kokkevis, 2012-09-13 01:32:58]

thakis, can you please take a look?

[Nico, 2012-09-13 01:37:21]

lgtm, but someone from the the field trial folks should take a look too.

https://codereview.chromium.org/10914247/diff/1/chrome/browser/chrome_gpu_uti...
File chrome/browser/chrome_gpu_util.cc (right):

https://codereview.chromium.org/10914247/diff/1/chrome/browser/chrome_gpu_uti...
chrome/browser/chrome_gpu_util.cc:101: // threaded-compositing on with 1/3
probability each.
This is gonna be hell for bug triage. What are the plans to mitigate this?

[vangelis, 2012-09-13 01:47:22]

On 2012/09/13 01:37:21, Nico wrote:
> lgtm, but someone from the the field trial folks should take a look too.
> 
>
https://codereview.chromium.org/10914247/diff/1/chrome/browser/chrome_gpu_uti...
> File chrome/browser/chrome_gpu_util.cc (right):
> 
>
https://codereview.chromium.org/10914247/diff/1/chrome/browser/chrome_gpu_uti...
> chrome/browser/chrome_gpu_util.cc:101: // threaded-compositing on with 1/3
> probability each.
> This is gonna be hell for bug triage. What are the plans to mitigate this?

This was an incomplete upload as it failed to include the removal of the old
field trial code.

As for bug triage, we have already been running these experiments in one form of
another.  We have a couple of ways of dealing with issues:

1. On issues posted to the issue tracker, we ask folks for their about:gpu page.
 From that we can tell which mode they are on (regular, force compositing of
fcm+thread)
2. In the crash database we can identify the mode they're in by using a
combination of the field trial ids and the command line flags.

It is a bit of a pain but works ok. Comparing behaviors in various modes is the
whole purpose of running field trials.

[Nico, 2012-09-13 01:49:37]

lgtm

https://codereview.chromium.org/10914247/diff/4001/chrome/browser/chrome_gpu_...
File chrome/browser/chrome_gpu_util.cc (right):

https://codereview.chromium.org/10914247/diff/4001/chrome/browser/chrome_gpu_...
chrome/browser/chrome_gpu_util.cc:92: threaded_compositing_probability = 3;
nit: Set this to kDivisor instead

https://codereview.chromium.org/10914247/diff/4001/chrome/browser/chrome_gpu_...
chrome/browser/chrome_gpu_util.cc:95: force_compositing_mode_probability = 3;
same

[Alexei Svitkine (slow), 2012-09-13 13:37:45]

I see there's also a lot of gpu-related test failures on Mac and Windows where
you're flipping the modes. I suspect you'll need to fix these before enabling
these.

https://codereview.chromium.org/10914247/diff/4001/chrome/browser/chrome_gpu_...
File chrome/browser/chrome_gpu_util.cc (right):

https://codereview.chromium.org/10914247/diff/4001/chrome/browser/chrome_gpu_...
chrome/browser/chrome_gpu_util.cc:77: "disable", 2017, 12, 31, NULL));
The goal of the expiry date is to set an expectation of when you're likely to
have gotten all you needed out of the field trial and are no longer interested
in it. That way, the system can automatically disable it at that point and also
gives a hint that the code can be deleted.

I doubt you expect to still be using this trial in 2016, so please set this to
something more reasonable.

https://codereview.chromium.org/10914247/diff/4001/chrome/browser/chrome_gpu_...
chrome/browser/chrome_gpu_util.cc:103: threaded_compositing_mode_probability =
1;
The variable name you use elsewhere is |threaded_compositing_probability|
(without the "mode"), so this won't compile.

[Vangelis Kokkevis, 2012-09-17 20:03:07]

On 2012/09/13 13:37:45, Alexei Svitkine wrote:
> I see there's also a lot of gpu-related test failures on Mac and Windows where
> you're flipping the modes. I suspect you'll need to fix these before enabling
> these.
> 

I believe with this latest patch the test failures should be fixed.  I am though
waiting for webkit to roll past http://trac.webkit.org/changeset/128785

as some of the tests have a dependency to this.

>
https://codereview.chromium.org/10914247/diff/4001/chrome/browser/chrome_gpu_...
> File chrome/browser/chrome_gpu_util.cc (right):
> 
>
https://codereview.chromium.org/10914247/diff/4001/chrome/browser/chrome_gpu_...
> chrome/browser/chrome_gpu_util.cc:77: "disable", 2017, 12, 31, NULL));
> The goal of the expiry date is to set an expectation of when you're likely to
> have gotten all you needed out of the field trial and are no longer interested
> in it. That way, the system can automatically disable it at that point and
also
> gives a hint that the code can be deleted.
> 

> I doubt you expect to still be using this trial in 2016, so please set this to
> something more reasonable.
> 

We are using this field trial as an emergency switch in case something blows up
when threaded compositing reaches the stable channel.  Once we are certain that
it will stick (maybe by M24 branch), I'll be removing all this code. I don't
expect to reach the termination date but I didn't want to be surprised either by
it switching on me. I can switch it to say a year from now if you feel that's
better although it's equally arbitrary.  


>
https://codereview.chromium.org/10914247/diff/4001/chrome/browser/chrome_gpu_...
> chrome/browser/chrome_gpu_util.cc:103: threaded_compositing_mode_probability =
> 1;
> The variable name you use elsewhere is |threaded_compositing_probability|
> (without the "mode"), so this won't compile.

Yep, thanks!  Caught by the linux trybot as well and I fixed it.

[Alexei Svitkine (slow), 2012-09-17 20:06:55]

>
> https://codereview.chromium.**org/10914247/diff/4001/chrome/**
>
browser/chrome_gpu_util.cc#**newcode77<https://codereview.chromium.org/10914247/diff/4001/chrome/browser/chrome_gpu_util.cc#newcode77>
>
>> chrome/browser/chrome_gpu_**util.cc:77: "disable", 2017, 12, 31, NULL));
>> The goal of the expiry date is to set an expectation of when you're
>> likely to
>> have gotten all you needed out of the field trial and are no longer
>> interested
>> in it. That way, the system can automatically disable it at that point and
>>
> also
>
>> gives a hint that the code can be deleted.
>>
>
>
>  I doubt you expect to still be using this trial in 2016, so please set
>> this to
>> something more reasonable.
>>
>
>
> We are using this field trial as an emergency switch in case something
> blows up
> when threaded compositing reaches the stable channel.  Once we are certain
> that
> it will stick (maybe by M24 branch), I'll be removing all this code. I
> don't
> expect to reach the termination date but I didn't want to be surprised
> either by
> it switching on me. I can switch it to say a year from now if you feel
> that's
> better although it's equally arbitrary.


Please do. Thanks!

-Alexei

[Vangelis Kokkevis, 2012-09-17 20:16:51]

On 2012/09/17 20:06:55, Alexei Svitkine wrote:
> >
> > https://codereview.chromium.**org/10914247/diff/4001/chrome/**
> >
>
browser/chrome_gpu_util.cc#**newcode77<https://codereview.chromium.org/10914247/diff/4001/chrome/browser/chrome_gpu_util.cc#newcode77>
> >
> >> chrome/browser/chrome_gpu_**util.cc:77: "disable", 2017, 12, 31, NULL));
> >> The goal of the expiry date is to set an expectation of when you're
> >> likely to
> >> have gotten all you needed out of the field trial and are no longer
> >> interested
> >> in it. That way, the system can automatically disable it at that point and
> >>
> > also
> >
> >> gives a hint that the code can be deleted.
> >>
> >
> >
> >  I doubt you expect to still be using this trial in 2016, so please set
> >> this to
> >> something more reasonable.
> >>
> >
> >
> > We are using this field trial as an emergency switch in case something
> > blows up
> > when threaded compositing reaches the stable channel.  Once we are certain
> > that
> > it will stick (maybe by M24 branch), I'll be removing all this code. I
> > don't
> > expect to reach the termination date but I didn't want to be surprised
> > either by
> > it switching on me. I can switch it to say a year from now if you feel
> > that's
> > better although it's equally arbitrary.
> 
> 
> Please do. Thanks!
> 
> -Alexei

Done!

[Alexei Svitkine (slow), 2012-09-17 20:23:47]

LGTM with nits, assuming tests are green. Thanks!

https://codereview.chromium.org/10914247/diff/7005/chrome/test/gpu/gpu_featur...
File chrome/test/gpu/gpu_feature_browsertest.cc (right):

https://codereview.chromium.org/10914247/diff/7005/chrome/test/gpu/gpu_featur...
chrome/test/gpu/gpu_feature_browsertest.cc:40: #define SWAP_BUFFERS_EVENT       
         "SwapBuffers"
Nit: Declare these per the Chromium style convention as:

const char kSwapBuffersEvent[] = "SwapBuffers";

https://codereview.chromium.org/10914247/diff/7005/chrome/test/gpu/gpu_featur...
chrome/test/gpu/gpu_feature_browsertest.cc:141: EXPECT_GT(event_count,
size_t(0));
Nit: Instead of size_t(0), use 0U.

https://codereview.chromium.org/10914247/diff/7005/chrome/test/gpu/gpu_featur...
chrome/test/gpu/gpu_feature_browsertest.cc:184: virtual void
SetUpInProcessBrowserTestFixture() {
Nit: OVERRIDE.

[Vangelis Kokkevis, 2012-09-17 21:47:54]

Thanks a lot, Alexei.  I will do another try run before checking it in.

https://codereview.chromium.org/10914247/diff/7005/chrome/test/gpu/gpu_featur...
File chrome/test/gpu/gpu_feature_browsertest.cc (right):

https://codereview.chromium.org/10914247/diff/7005/chrome/test/gpu/gpu_featur...
chrome/test/gpu/gpu_feature_browsertest.cc:40: #define SWAP_BUFFERS_EVENT       
         "SwapBuffers"
On 2012/09/17 20:23:47, Alexei Svitkine wrote:
> Nit: Declare these per the Chromium style convention as:
> 
> const char kSwapBuffersEvent[] = "SwapBuffers";

Done.

https://codereview.chromium.org/10914247/diff/7005/chrome/test/gpu/gpu_featur...
chrome/test/gpu/gpu_feature_browsertest.cc:141: EXPECT_GT(event_count,
size_t(0));
On 2012/09/17 20:23:47, Alexei Svitkine wrote:
> Nit: Instead of size_t(0), use 0U.

Done.

https://codereview.chromium.org/10914247/diff/7005/chrome/test/gpu/gpu_featur...
chrome/test/gpu/gpu_feature_browsertest.cc:184: virtual void
SetUpInProcessBrowserTestFixture() {
On 2012/09/17 20:23:47, Alexei Svitkine wrote:
> Nit: OVERRIDE.

Done.

[commit-bot: I haz the power, 2012-09-25 21:44:18]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/vangelis@chromium.org/10914247/42001

[commit-bot: I haz the power, 2012-09-25 21:44:27]

Presubmit check for 10914247-42001 failed and returned exit status 1.


Running presubmit commit checks ...

** Presubmit ERRORS **
Missing LGTM from an OWNER for files in these directories:
    content/common

Presubmit checks took 1.1s to calculate.

[Vangelis Kokkevis, 2012-09-25 21:47:08]

I need an OWNER for the content/common change.  Darin, would you mind doing the
honors? Thanks!

On 2012/09/25 21:44:27, I haz the power (commit-bot) wrote:
> Presubmit check for 10914247-42001 failed and returned exit status 1.
> 
> 
> Running presubmit commit checks ...
> 
> ** Presubmit ERRORS **
> Missing LGTM from an OWNER for files in these directories:
>     content/common
> 
> Presubmit checks took 1.1s to calculate.

[darin (slow to review), 2012-09-25 22:45:21]

LGTM

[commit-bot: I haz the power, 2012-09-25 22:46:57]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/vangelis@chromium.org/10914247/42001

[commit-bot: I haz the power, 2012-09-26 00:10:52]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/vangelis@chromium.org/10914247/42001

[commit-bot: I haz the power, 2012-09-26 01:08:32]

Change committed as 158707

[commit-bot: I haz the power, 2012-09-26 04:29:36]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/vangelis@chromium.org/10914247/48002