Add a --use-angle flag for selecting the ANGLE renderer.

Add a --use-angle flag for selecting the ANGLE renderer.

 * Add support for selecting the new OpenGL renderer.
 * Remove the --use-warp flag and merge it into the --use-angle flag.

BUG=485541
Committed: https://crrev.com/b23448010805bf493f7a7104a73b378968b228da
Cr-Commit-Position: refs/heads/master@{#329436}

[Geoff Lang, 2015-05-07 18:38:27]

geofflang@chromium.org changed reviewers:
+ jmadill@chromium.org, kbr@chromium.org

[Geoff Lang, 2015-05-07 18:38:41]

@jmadill, kbr: PTAL

[Jamie Madill, 2015-05-07 18:53:13]

Also try running 'git cl format' to clear up formatting.

https://codereview.chromium.org/1123343004/diff/40001/ui/gl/gl_surface_egl.cc
File ui/gl/gl_surface_egl.cc (right):

https://codereview.chromium.org/1123343004/diff/40001/ui/gl/gl_surface_egl.cc...
ui/gl/gl_surface_egl.cc:195: case ANGLE_OPENGLES:
maybe we wouldn't want to immediately expose ES if it won't be supported for a
while (possibly quite a while?)

https://codereview.chromium.org/1123343004/diff/40001/ui/gl/gl_surface_egl.cc...
ui/gl/gl_surface_egl.cc:248: 
could probably simplify this float bit by separating out the default case

https://codereview.chromium.org/1123343004/diff/40001/ui/gl/gl_switches.cc
File ui/gl/gl_switches.cc (left):

https://codereview.chromium.org/1123343004/diff/40001/ui/gl/gl_switches.cc#ol...
ui/gl/gl_switches.cc:59: const char kUseWarp[] = "use-warp";
you might want to check with someone about disabling this flag

[Ken Russell (switch to Gerrit), 2015-05-07 20:58:10]

This LGTM overall modulo Jamie's comments. I think removing the --use-warp flag
is fine; it doesn't seem to be used by any of our bots. +zmo for other comments.

https://codereview.chromium.org/1123343004/diff/40001/ui/gl/gl_surface_egl.cc
File ui/gl/gl_surface_egl.cc (right):

https://codereview.chromium.org/1123343004/diff/40001/ui/gl/gl_surface_egl.cc...
ui/gl/gl_surface_egl.cc:289: init_displays->push_back(DEFAULT);
There's a fair bit of logic here and it would be ideal to unit test it. Could
you see whether it's feasible to add a test to the gl_unittests target in
src/ui/gl/gl_tests.gyp (which would be the first one)?

https://codereview.chromium.org/1123343004/diff/40001/ui/gl/gl_surface_egl.cc...
ui/gl/gl_surface_egl.cc:489: ExtensionsContain(client_extensions,
"EGL_ANGLE_platform_angle_opengl");
The use of EGL_ for this extension but not the others seems strange.

[Zhenyao Mo, 2015-05-07 22:08:35]

zmo@chromium.org changed reviewers:
+ zmo@chromium.org

[Zhenyao Mo, 2015-05-07 22:08:35]

cc cpu for removing kUseWarp.

https://codereview.chromium.org/1123343004/diff/40001/content/browser/gpu/gpu...
File content/browser/gpu/gpu_data_manager_impl_private.cc (right):

https://codereview.chromium.org/1123343004/diff/40001/content/browser/gpu/gpu...
content/browser/gpu/gpu_data_manager_impl_private.cc:418: return use_warp_ ||
angle_impl_flag == gfx::kANGLEImplementationWARPName;
The only place use_warp_ is set to true is ForceWarpModeForTesting(), which is
not referenced.  So you should just remove both.

[Geoff Lang, 2015-05-11 13:31:50]

Updated, added tests, PTAL

https://codereview.chromium.org/1123343004/diff/40001/content/browser/gpu/gpu...
File content/browser/gpu/gpu_data_manager_impl_private.cc (right):

https://codereview.chromium.org/1123343004/diff/40001/content/browser/gpu/gpu...
content/browser/gpu/gpu_data_manager_impl_private.cc:418: return use_warp_ ||
angle_impl_flag == gfx::kANGLEImplementationWARPName;
On 2015/05/07 22:08:35, Zhenyao Mo wrote:
> The only place use_warp_ is set to true is ForceWarpModeForTesting(), which is
> not referenced.  So you should just remove both.

Looks like use_warp_ is referenced in
GpuDataManagerImplPrivate::EnableWarpIfNecessary too but doesn't show up on the
code search because it is only compiled on Windows.

https://codereview.chromium.org/1123343004/diff/40001/ui/gl/gl_surface_egl.cc
File ui/gl/gl_surface_egl.cc (right):

https://codereview.chromium.org/1123343004/diff/40001/ui/gl/gl_surface_egl.cc...
ui/gl/gl_surface_egl.cc:195: case ANGLE_OPENGLES:
On 2015/05/07 18:53:13, Jamie Madill wrote:
> maybe we wouldn't want to immediately expose ES if it won't be supported for a
> while (possibly quite a while?)

I'm not sure, the OpenGL renderer will be fairly unstable in chrome for a while
too. Since it will never be selected by default, I don't think it'll cause any
issues.

https://codereview.chromium.org/1123343004/diff/40001/ui/gl/gl_surface_egl.cc...
ui/gl/gl_surface_egl.cc:248: 
On 2015/05/07 18:53:13, Jamie Madill wrote:
> could probably simplify this float bit by separating out the default case

Done, reorganized a bit.

https://codereview.chromium.org/1123343004/diff/40001/ui/gl/gl_surface_egl.cc...
ui/gl/gl_surface_egl.cc:289: init_displays->push_back(DEFAULT);
On 2015/05/07 20:58:09, Ken Russell wrote:
> There's a fair bit of logic here and it would be ideal to unit test it. Could
> you see whether it's feasible to add a test to the gl_unittests target in
> src/ui/gl/gl_tests.gyp (which would be the first one)?

Done, added a test.

https://codereview.chromium.org/1123343004/diff/40001/ui/gl/gl_surface_egl.cc...
ui/gl/gl_surface_egl.cc:489: ExtensionsContain(client_extensions,
"EGL_ANGLE_platform_angle_opengl");
On 2015/05/07 20:58:09, Ken Russell wrote:
> The use of EGL_ for this extension but not the others seems strange.

Fixed, we should always be using EGL_

[Ken Russell (switch to Gerrit), 2015-05-11 21:42:03]

Very nice. Still LGTM.

[Zhenyao Mo, 2015-05-11 23:55:35]

LGTM

[Geoff Lang, 2015-05-12 15:30:37]

The CQ bit was checked by geofflang@chromium.org

[commit-bot: I haz the power, 2015-05-12 15:30:50]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1123343004/60001

[commit-bot: I haz the power, 2015-05-12 15:40:32]

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

[commit-bot: I haz the power, 2015-05-12 15:40:33]

Try jobs failed on following builders:
  ios_rel_device_ninja on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_rel_device_ni...)

[Geoff Lang, 2015-05-12 16:56:15]

The CQ bit was checked by geofflang@chromium.org

[Geoff Lang, 2015-05-12 16:56:16]

The patchset sent to the CQ was uploaded after l-g-t-m from kbr@chromium.org,
zmo@chromium.org
Link to the patchset: https://codereview.chromium.org/1123343004/#ps80001
(title: "Move dependency on gl.gyp into conditional.")

[commit-bot: I haz the power, 2015-05-12 16:56:29]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1123343004/80001

[commit-bot: I haz the power, 2015-05-12 17:54:33]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2015-05-12 17:55:34]

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