Cleaning up compositing field trial code and enable FCM in beta/stable

Cleaned up compositing mode field trial code by:

1. Move the code that checks whether FCM and/or ThreadedCompositing is enabled to a separate file (compositor_utils) that can be accessed from all the spots that need the info.
2. Cache results results for checks so that we don't have to check them every frame (string compares were actually showing up in profile results)
3. Some cosmetic changes to variable names to keep them consistent.
4. Re-arranged to code that decides what trials run in which channels.

This CL also enables FCM on beta/stable by default but keeps the previous 1/3 experiment running on the other channels.

BUG=


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=150359

[vangelis, 2012-08-03 19:10:42]

Can you please have a look?

[jbates, 2012-08-03 20:47:58]

LGTM!

[commit-bot: I haz the power, 2012-08-03 20:54:37]

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

[commit-bot: I haz the power, 2012-08-03 20:54:41]

Presubmit check for 10824168-1001 failed and returned exit status 1.

Running presubmit commit checks ...

** Presubmit Messages **
If this change has an associated bug, add BUG=[bug number].

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

Presubmit checks took 1.0s to calculate.

[vangelis, 2012-08-03 20:59:16]

Dain, mind doing an OWNERS review?

[darin (slow to review), 2012-08-03 21:16:06]

https://chromiumcodereview.appspot.com/10824168/diff/1001/content/browser/ren...
File content/browser/renderer_host/render_widget_host_impl.cc (right):

https://chromiumcodereview.appspot.com/10824168/diff/1001/content/browser/ren...
content/browser/renderer_host/render_widget_host_impl.cc:184:
is_threaded_compositing_enabled_ =
nit: place the above code into a static helper function named
ShouldEnableThreadedCompositing().

https://chromiumcodereview.appspot.com/10824168/diff/1001/content/common/gpu/...
File content/common/gpu/client/webgraphicscontext3d_command_buffer_impl.cc
(right):

https://chromiumcodereview.appspot.com/10824168/diff/1001/content/common/gpu/...
content/common/gpu/client/webgraphicscontext3d_command_buffer_impl.cc:157: //
Get ViewMsg_SwapBuffers_ACK from browser for single-threaded path.
this is exactly the same logic as in render_view_impl.cc, right?  sounds like
ShouldEnableThreadedCompositing() should be pulled out into a helper function
that lives in content/common/.  threaded_compositor_field_trial.{h,cc}?

[vangelis, 2012-08-04 23:33:35]

Moved the compositing mode detection code to a separate file so that it can be
shared by all the spots that need to check it. 

I would have liked to move the initialization of the field trial into the same
file but unfortunately for now we need access to the current channel which isn't
available in content.  I think this is fine.  The experiments are somewhat
temporary anyway but the compositor_util will be a bit more long lived.

I took the opportunity to sneak in enabling FCM on stable/beta with this CL. 
This is mostly to make sure the change can land before the branch point.

[vangelis, 2012-08-06 17:03:52]

Ready for another review round.  Darin, can you please take a look? I'm hoping I
can check it in before the branch deadline. Thanks!

[darin (slow to review), 2012-08-06 20:39:38]

https://chromiumcodereview.appspot.com/10824168/diff/6004/chrome/browser/gpu_...
File chrome/browser/gpu_util.cc (right):

https://chromiumcodereview.appspot.com/10824168/diff/6004/chrome/browser/gpu_...
chrome/browser/gpu_util.cc:211: base::FieldTrial::Probability
forceCompositingModeProbability = 0;
nit: google_style_variables

https://chromiumcodereview.appspot.com/10824168/diff/6004/content/public/comm...
File content/public/common/compositor_util.h (right):

https://chromiumcodereview.appspot.com/10824168/diff/6004/content/public/comm...
content/public/common/compositor_util.h:10: namespace compositor_util {
code under src/content/ should be in the content:: namespace.

nit:  maybe the compositor_util:: namespace is unnecessary?  i know we have
historically used namespaces like this for *_util files, but i'm not sure that
is so helpful.  the style guide doesn't actually encourage this pattern.

[vangelis, 2012-08-06 23:39:59]

On 2012/08/06 20:39:38, darin wrote:
>
https://chromiumcodereview.appspot.com/10824168/diff/6004/chrome/browser/gpu_...
> File chrome/browser/gpu_util.cc (right):
> 
>
https://chromiumcodereview.appspot.com/10824168/diff/6004/chrome/browser/gpu_...
> chrome/browser/gpu_util.cc:211: base::FieldTrial::Probability
> forceCompositingModeProbability = 0;
> nit: google_style_variables
> 
> 

Done.

https://chromiumcodereview.appspot.com/10824168/diff/6004/content/public/comm...
> File content/public/common/compositor_util.h (right):
> 
>
https://chromiumcodereview.appspot.com/10824168/diff/6004/content/public/comm...
> content/public/common/compositor_util.h:10: namespace compositor_util {
> code under src/content/ should be in the content:: namespace.
> 
> nit:  maybe the compositor_util:: namespace is unnecessary?  i know we have
> historically used namespaces like this for *_util files, but i'm not sure that
> is so helpful.  the style guide doesn't actually encourage this pattern.

Done.

[vangelis, 2012-08-06 23:41:20]

Thanks. PTAL.  If it looks ok, would you mind sending it through the commit
queue as well?

[darin (slow to review), 2012-08-07 05:25:13]

LGTM, thanks for the extra rounds.

[commit-bot: I haz the power, 2012-08-07 05:25:47]

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

[darin (slow to review), 2012-08-07 05:29:30]

@jam pointed out that compositor_util.cc should live in src/content/common/
instead since it is implementation of public API.

[commit-bot: I haz the power, 2012-08-07 15:23:16]

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

[vangelis, 2012-08-07 15:23:19]

On 2012/08/07 05:29:30, darin wrote:
> @jam pointed out that compositor_util.cc should live in src/content/common/
> instead since it is implementation of public API.

ok, thanks.  File moved.

[commit-bot: I haz the power, 2012-08-07 16:38:57]

Change committed as 150359