Support offscreen contexts which own their backing surface

This is an updated version of issue 2586803003 which was reverted.
I have removed the problematic "low priority" GL context setting and
made the change conditional on a new ""WebVR experimental rendering
optimizations" feature flag.
    
When enabled, it uses the own_offscreen_surface flag with WebGL contexts
when WebVR is enabled, to allow the surface to be swapped out with a
compatible one if used for VR presentation later. This must be done
before any WebVR API calls have been made, so it also needs to work
for non-WebVR WebGL canvas drawing.
    
The change includes support for canvas attributes such as "antialias".

The feature flag is intended to control features which are considered
experimental or risky, so that they can be manually enabled for initial
testing. It is also in preparation for a finch experiment for future
rollout.

BUG=655733
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel

Review-Url: https://codereview.chromium.org/2698573002
Cr-Commit-Position: refs/heads/master@{#451484}
Committed: https://chromium.googlesource.com/chromium/src/+/89ec62330efc014949d78367b94de89c4a7b5405

[klausw, 2017-02-14 23:11:22]

Description was changed from

==========
Add "WebVR experimental rendering optimizations" flag

This is intended to support features such as direct-draw-to-surface
which are considered experimental or risky, so that they can be
manually enabled for initial testing. It is also in preparation
for a finch experiment for future rollout.

BUG=655733
==========

to

==========
Add "WebVR experimental rendering optimizations" flag

This is intended to support features such as direct-draw-to-surface
which are considered experimental or risky, so that they can be
manually enabled for initial testing. It is also in preparation
for a finch experiment for future rollout.

BUG=655733
==========

[klausw, 2017-02-14 23:11:22]

klausw@chromium.org changed reviewers:
+ amp@chromium.org

[amp, 2017-02-14 23:23:26]

https://codereview.chromium.org/2698573002/diff/1/chrome/browser/about_flags.cc
File chrome/browser/about_flags.cc (right):

https://codereview.chromium.org/2698573002/diff/1/chrome/browser/about_flags....
chrome/browser/about_flags.cc:1614: {"enable-webvr-experimental-rendering",
Let's only show this when webvr is enabled (which should soon be renamed to
enable_vr_platfrom instead of being webvr specific).

[klausw, 2017-02-14 23:31:29]

klausw@chromium.org changed reviewers:
+ jochen@chromium.org, rkaplow@chromium.org

[klausw, 2017-02-14 23:31:29]

jochen@: Please review the content/feature list parts and the histograms change.

rkaplow@: Finch ambassador.

Launch bug is http://crbug.com/670502 "OWP Launch: WebVR"

https://codereview.chromium.org/2698573002/diff/1/chrome/browser/about_flags.cc
File chrome/browser/about_flags.cc (right):

https://codereview.chromium.org/2698573002/diff/1/chrome/browser/about_flags....
chrome/browser/about_flags.cc:1614: {"enable-webvr-experimental-rendering",
On 2017/02/14 23:23:26, amp wrote:
> Let's only show this when webvr is enabled (which should soon be renamed to
> enable_vr_platfrom instead of being webvr specific).

Done.

[klausw, 2017-02-15 00:17:00]

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

[commit-bot: I haz the power, 2017-02-15 00:18:06]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[jochen (gone - plz use gerrit), 2017-02-15 10:00:38]

I can't approve general histograms changes, just use counter additions.

You add four different mechanisms to control a feature in this CL, but nothing
that actually depends on those flags.

Please bundle the additions of the individual mechanisms together with the code
changes (not lgtm)

[amp, 2017-02-15 17:43:35]

On 2017/02/15 10:00:38, jochen wrote:
> I can't approve general histograms changes, just use counter additions.
> 
> You add four different mechanisms to control a feature in this CL, but nothing
> that actually depends on those flags.
> 
> Please bundle the additions of the individual mechanisms together with the
code
> changes (not lgtm)

What are the four mechanisms?  I see the feature flag and the content switch
(which we shouldn't need), and there are the about flag references, but that's
just a way to enable the flag rather than a different mechanism, isn't it?


Re the lack of inclusion of code that this feature flag actually gates: I had
recommended that we add the feature flag first as the underlying code is rather
complex and the flag would just be noise in that review, but apologies if it is
preferred to include everything together.

[amp, 2017-02-15 17:44:03]

https://codereview.chromium.org/2698573002/diff/40001/chrome/browser/about_fl...
File chrome/browser/about_flags.cc (right):

https://codereview.chromium.org/2698573002/diff/40001/chrome/browser/about_fl...
chrome/browser/about_flags.cc:1617:
IDS_FLAGS_WEBVR_EXPERIMENTAL_RENDERING_DESCRIPTION, kOsAll,
Should we limit this to Android only for now?

https://codereview.chromium.org/2698573002/diff/40001/chrome/browser/about_fl...
chrome/browser/about_flags.cc:1618:
SINGLE_VALUE_TYPE(switches::kEnableWebVRExperimentalRendering)},
This should be referring to the feature (like vr shell does below), not to the
switch (which aren't needed).

https://codereview.chromium.org/2698573002/diff/40001/content/public/common/c...
File content/public/common/content_features.cc (right):

https://codereview.chromium.org/2698573002/diff/40001/content/public/common/c...
content/public/common/content_features.cc:258: "WebVrExperimentalRendering",
base::FEATURE_DISABLED_BY_DEFAULT};
WebVR has a capital 'R' in all the other places we use it.  Not sure if it makes
a huge difference, but I'm likely to misspell this on the command line if I try
to use it.

https://codereview.chromium.org/2698573002/diff/40001/content/public/common/c...
File content/public/common/content_switches.h (right):

https://codereview.chromium.org/2698573002/diff/40001/content/public/common/c...
content/public/common/content_switches.h:166: CONTENT_EXPORT extern const char
kEnableWebVRExperimentalRendering[];
You shouldn't need to add a separate switch.  Adding a feature automatically
provides a command line way of enabling it, via something like:
--enable-features=WebVRExperimentalRendering

https://codereview.chromium.org/2698573002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/platform/RuntimeEnabledFeatures.json5 (right):

https://codereview.chromium.org/2698573002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/RuntimeEnabledFeatures.json5:914: name:
"WebVRExperimentalRendering",
You can add a 'depends_on' section here to limit this being turned on only when
WebVR is turned on (see the 'depends_on' docs at the top of the file).

[mthiesse, 2017-02-15 17:48:07]

I agree with jochen though that we should definitely get the code landed with
the runtime flag. Is there any reason to land this on its own?

[klausw, 2017-02-15 19:40:54]

Description was changed from

==========
Add "WebVR experimental rendering optimizations" flag

This is intended to support features such as direct-draw-to-surface
which are considered experimental or risky, so that they can be
manually enabled for initial testing. It is also in preparation
for a finch experiment for future rollout.

BUG=655733
==========

to

==========
Add "WebVR experimental rendering optimizations" flag

This is intended to support features such as direct-draw-to-surface
which are considered experimental or risky, so that they can be
manually enabled for initial testing. It is also in preparation
for a finch experiment for future rollout.

BUG=655733
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
==========

[klausw, 2017-02-15 19:47:20]

klausw@chromium.org changed reviewers:
+ aelias@chromium.org, bajones@chromium.org, dcheng@chromium.org,
jbauman@chromium.org

[klausw, 2017-02-15 19:47:22]

Everyone added: This is an updated version of crrev.com/2586803003 which you had
previously reviewed and which was reverted. I've removed the "low priority" GL
context part which was causing render glitches, and made the change conditional
on a new feature flag (off by default) to enable further testing and a
controlled rollout later.

aelias@chromium.org: Please review changes in 
content/renderer/renderer_blink_platform_impl.cc

dcheng@chromium.org: Please review changes in
gpu/ipc/common/gpu_command_buffer_traits_multi.h

https://codereview.chromium.org/2698573002/diff/40001/chrome/browser/about_fl...
File chrome/browser/about_flags.cc (right):

https://codereview.chromium.org/2698573002/diff/40001/chrome/browser/about_fl...
chrome/browser/about_flags.cc:1617:
IDS_FLAGS_WEBVR_EXPERIMENTAL_RENDERING_DESCRIPTION, kOsAll,
On 2017/02/15 17:44:03, amp wrote:
> Should we limit this to Android only for now?

Done. I assume that kOsAndroid here means to just keep it auto-disabled and
hidden on other platforms? An ifdef wouldn't work since the feature check is in
non-Android-specific code.

https://codereview.chromium.org/2698573002/diff/40001/chrome/browser/about_fl...
chrome/browser/about_flags.cc:1618:
SINGLE_VALUE_TYPE(switches::kEnableWebVRExperimentalRendering)},
On 2017/02/15 17:44:03, amp wrote:
> This should be referring to the feature (like vr shell does below), not to the
> switch (which aren't needed).

Done.

https://codereview.chromium.org/2698573002/diff/40001/content/public/common/c...
File content/public/common/content_features.cc (right):

https://codereview.chromium.org/2698573002/diff/40001/content/public/common/c...
content/public/common/content_features.cc:258: "WebVrExperimentalRendering",
base::FEATURE_DISABLED_BY_DEFAULT};
On 2017/02/15 17:44:03, amp wrote:
> WebVR has a capital 'R' in all the other places we use it.  Not sure if it
makes
> a huge difference, but I'm likely to misspell this on the command line if I
try
> to use it.

Done.

https://codereview.chromium.org/2698573002/diff/40001/content/public/common/c...
File content/public/common/content_switches.h (right):

https://codereview.chromium.org/2698573002/diff/40001/content/public/common/c...
content/public/common/content_switches.h:166: CONTENT_EXPORT extern const char
kEnableWebVRExperimentalRendering[];
On 2017/02/15 17:44:03, amp wrote:
> You shouldn't need to add a separate switch.  Adding a feature automatically
> provides a command line way of enabling it, via something like:
> --enable-features=WebVRExperimentalRendering

Done, removed. I was following the WebVR example, didn't realize that I got the
two separate mechanisms mixed up.

https://codereview.chromium.org/2698573002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/platform/RuntimeEnabledFeatures.json5 (right):

https://codereview.chromium.org/2698573002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/RuntimeEnabledFeatures.json5:914: name:
"WebVRExperimentalRendering",
On 2017/02/15 17:44:03, amp wrote:
> You can add a 'depends_on' section here to limit this being turned on only
when
> WebVR is turned on (see the 'depends_on' docs at the top of the file).

Declaring it as a dependency means that there's no way to turn on the
optimizations in "origin trial only" mode. I've added a code comment to explain.

[amp, 2017-02-15 20:04:22]

https://codereview.chromium.org/2698573002/diff/40001/chrome/browser/about_fl...
File chrome/browser/about_flags.cc (right):

https://codereview.chromium.org/2698573002/diff/40001/chrome/browser/about_fl...
chrome/browser/about_flags.cc:1617:
IDS_FLAGS_WEBVR_EXPERIMENTAL_RENDERING_DESCRIPTION, kOsAll,
On 2017/02/15 19:47:22, klausw wrote:
> On 2017/02/15 17:44:03, amp wrote:
> > Should we limit this to Android only for now?
> 
> Done. I assume that kOsAndroid here means to just keep it auto-disabled and
> hidden on other platforms? An ifdef wouldn't work since the feature check is
in
> non-Android-specific code.

I'm not actually sure what the os param does.  Maybe it grays it out on other
platforms (but we are hiding it behind a build flag so it won't show up anyway).

Either looks good to keep the os param in sync with VrShell below it.

https://codereview.chromium.org/2698573002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/webgl/WebGLRenderingContextBase.cpp
(right):

https://codereview.chromium.org/2698573002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/webgl/WebGLRenderingContextBase.cpp:632: // if
called with a scriptState, not for a canvas.
Comment seems out of sync with code which does check the canvas for an origin
trial.

[klausw, 2017-02-15 20:04:56]

On 2017/02/15 10:00:38, jochen wrote:
> I can't approve general histograms changes, just use counter additions.
> 
> You add four different mechanisms to control a feature in this CL, but nothing
> that actually depends on those flags.
> 
> Please bundle the additions of the individual mechanisms together with the
code
> changes (not lgtm)

This should be moot now, I've removed the histograms.xml change since the code
no longer adds a flag.

[klausw, 2017-02-15 20:06:52]

On 2017/02/15 20:04:56, klausw wrote:
> On 2017/02/15 10:00:38, jochen wrote:
> > I can't approve general histograms changes, just use counter additions.
> > 
> > You add four different mechanisms to control a feature in this CL, but
nothing
> > that actually depends on those flags.
> > 
> > Please bundle the additions of the individual mechanisms together with the
> code
> > changes (not lgtm)
> 
> This should be moot now, I've removed the histograms.xml change since the code
> no longer adds a flag.

Sorry, hit "send" too early. I've bundled in the mechanism that depends on the
change, and I think the control mechanisms should now be limited to those
necessary for the feature flag to work, without a separate command line flag.
Please let me know if I have remaining redundancies.

[klausw, 2017-02-15 20:07:02]

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

[commit-bot: I haz the power, 2017-02-15 20:08:08]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[amp, 2017-02-15 20:13:56]

On 2017/02/15 20:06:52, klausw wrote:
> On 2017/02/15 20:04:56, klausw wrote:
> > On 2017/02/15 10:00:38, jochen wrote:
> > > I can't approve general histograms changes, just use counter additions.
> > > 
> > > You add four different mechanisms to control a feature in this CL, but
> nothing
> > > that actually depends on those flags.
> > > 
> > > Please bundle the additions of the individual mechanisms together with the
> > code
> > > changes (not lgtm)
> > 
> > This should be moot now, I've removed the histograms.xml change since the
code
> > no longer adds a flag.
> 
> Sorry, hit "send" too early. I've bundled in the mechanism that depends on the
> change, and I think the control mechanisms should now be limited to those
> necessary for the feature flag to work, without a separate command line flag.
> Please let me know if I have remaining redundancies.

I think the histogram change is from adding anything to about://flags, even if
it isn't defined in a command line switch.

I'm not sure, though so check if the unit test for about flags histograms
passes. If it does you should be fine.

[klausw, 2017-02-15 20:17:41]

Description was changed from

==========
Add "WebVR experimental rendering optimizations" flag

This is intended to support features such as direct-draw-to-surface
which are considered experimental or risky, so that they can be
manually enabled for initial testing. It is also in preparation
for a finch experiment for future rollout.

BUG=655733
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
==========

to

==========
This is an updated version of issue 2586803003 which was reverted.
I have removed the problematic "low priority" GL context setting and
made the change conditional on a new ""WebVR experimental rendering
optimizations" feature flag.
    
When enabled, it uses the own_offscreen_surface flag with WebGL contexts
when WebVR is enabled, to allow the surface to be swapped out with a
compatible one if used for VR presentation later. This must be done
before any WebVR API calls have been made, so it also needs to work
for non-WebVR WebGL canvas drawing.
    
The change includes support for canvas attributes such as "antialias".

The feature flag is intended to control features which are considered
experimental or risky, so that they can be manually enabled for initial
testing. It is also in preparation for a finch experiment for future
rollout.

BUG=655733
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
==========

[klausw, 2017-02-15 20:21:16]

amp@ wrote:
> I think the histogram change is from adding anything to about://flags, even if
> it isn't defined in a command line switch.
> 
> I'm not sure, though so check if the unit test for about flags histograms
> passes. If it does you should be fine

The test does pass without any addition to histograms.xml, seems like it's only
enforced for actual command line switches.

https://codereview.chromium.org/2698573002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/webgl/WebGLRenderingContextBase.cpp
(right):

https://codereview.chromium.org/2698573002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/webgl/WebGLRenderingContextBase.cpp:632: // if
called with a scriptState, not for a canvas.
On 2017/02/15 20:04:22, amp wrote:
> Comment seems out of sync with code which does check the canvas for an origin
> trial.

Done.

[mthiesse, 2017-02-15 21:36:42]

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

[mthiesse, 2017-02-15 21:36:43]

https://codereview.chromium.org/2698573002/diff/100001/content/renderer/rende...
File content/renderer/renderer_blink_platform_impl.cc (right):

https://codereview.chromium.org/2698573002/diff/100001/content/renderer/rende...
content/renderer/renderer_blink_platform_impl.cc:1029: attributes.depth_size =
0;
Are you missing attributes.alpha_size = -1? Or was leaving that out intentional?

[klausw, 2017-02-15 22:16:33]

https://codereview.chromium.org/2698573002/diff/100001/content/renderer/rende...
File content/renderer/renderer_blink_platform_impl.cc (right):

https://codereview.chromium.org/2698573002/diff/100001/content/renderer/rende...
content/renderer/renderer_blink_platform_impl.cc:1029: attributes.depth_size =
0;
On 2017/02/15 21:36:43, mthiesse wrote:
> Are you missing attributes.alpha_size = -1? Or was leaving that out
intentional?

Re-added.

I had removed it in http://crrev.com/2586803003#ps220001 at dcheng@'s
suggestion since it's the default, but on second thought I do think it's
clearer to keep it in and match the pre-patch code path.

[aelias_OOO_until_Jul13, 2017-02-16 03:25:51]

https://codereview.chromium.org/2698573002/diff/120001/content/renderer/rende...
File content/renderer/renderer_blink_platform_impl.cc (right):

https://codereview.chromium.org/2698573002/diff/120001/content/renderer/rende...
content/renderer/renderer_blink_platform_impl.cc:1021: if
(web_attributes.supportOwnOffscreenSurface) {
It doesn't look like this outer if statement is needed because the ternary
operator on each individual attribute covers it, so please remove it.

https://codereview.chromium.org/2698573002/diff/120001/content/renderer/rende...
content/renderer/renderer_blink_platform_impl.cc:1041: 
nit: unnecessary newline

https://codereview.chromium.org/2698573002/diff/120001/third_party/WebKit/pub...
File third_party/WebKit/public/platform/Platform.h (right):

https://codereview.chromium.org/2698573002/diff/120001/third_party/WebKit/pub...
third_party/WebKit/public/platform/Platform.h:465: bool supportAlpha = true;
These "true"s mixed in look arbitrary, can this be all false?  And set them
explicitly where you care if you're relying on the defaults.

[klausw, 2017-02-16 20:06:58]

dcheng@: I think this no longer needs your review, I've removed the IPC changes.

I've refactored it a bit:

- Move the supportOwnOffscreenSurface conditional into
toPlatformContextAttributes so that it only asks for alpha/depth etc if it
intends to use it.

- Add supportOwnOffscreenSurface check to maybeRestoreContext.

- remove the GPU-level "own_offscreen_surface" attribute. Instead, ask for a
customized surface iff the requested format is incompatible with the default
surface.

What do you think?

https://codereview.chromium.org/2698573002/diff/120001/content/renderer/rende...
File content/renderer/renderer_blink_platform_impl.cc (right):

https://codereview.chromium.org/2698573002/diff/120001/content/renderer/rende...
content/renderer/renderer_blink_platform_impl.cc:1021: if
(web_attributes.supportOwnOffscreenSurface) {
On 2017/02/16 03:25:51, aelias wrote:
> It doesn't look like this outer if statement is needed because the ternary
> operator on each individual attribute covers it, so please remove it.

Done, and also updated the comment above to match the new behavior.

https://codereview.chromium.org/2698573002/diff/120001/content/renderer/rende...
content/renderer/renderer_blink_platform_impl.cc:1041: 
On 2017/02/16 03:25:51, aelias wrote:
> nit: unnecessary newline

Done.

https://codereview.chromium.org/2698573002/diff/120001/third_party/WebKit/pub...
File third_party/WebKit/public/platform/Platform.h (right):

https://codereview.chromium.org/2698573002/diff/120001/third_party/WebKit/pub...
third_party/WebKit/public/platform/Platform.h:465: bool supportAlpha = true;
On 2017/02/16 03:25:51, aelias wrote:
> These "true"s mixed in look arbitrary, can this be all false?  And set them
> explicitly where you care if you're relying on the defaults.

Done.

[klausw, 2017-02-16 20:16:30]

On 2017/02/16 20:06:58, klausw wrote:
> - remove the GPU-level "own_offscreen_surface" attribute. Instead, ask for a
> customized surface iff the requested format is incompatible with the default
> surface.

I've now also removed the Platform ContextAttribute-level
supportOwnOffscreenSurface
attribute, it's now implied by setting any of the supportAlpha etc. attributes.
Updated the comments to match.

[klausw, 2017-02-16 20:31:09]

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

[commit-bot: I haz the power, 2017-02-16 20:32:58]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[aelias_OOO_until_Jul13, 2017-02-16 21:55:46]

content/renderer lgtm

[bajones, 2017-02-16 22:05:41]

modules/webgl LGTM

https://codereview.chromium.org/2698573002/diff/160001/gpu/command_buffer/com...
File gpu/command_buffer/common/gles2_cmd_utils.h (left):

https://codereview.chromium.org/2698573002/diff/160001/gpu/command_buffer/com...
gpu/command_buffer/common/gles2_cmd_utils.h:342: 
Unneeded newline removal.

[klausw, 2017-02-16 22:17:10]

https://codereview.chromium.org/2698573002/diff/160001/gpu/command_buffer/com...
File gpu/command_buffer/common/gles2_cmd_utils.h (left):

https://codereview.chromium.org/2698573002/diff/160001/gpu/command_buffer/com...
gpu/command_buffer/common/gles2_cmd_utils.h:342: 
On 2017/02/16 22:05:41, bajones wrote:
> Unneeded newline removal.

Reverted.

[klausw, 2017-02-16 22:35:43]

klausw@chromium.org changed reviewers:
- dcheng@chromium.org

[klausw, 2017-02-16 22:37:38]

PTAL:

jochen@: please review the feature changes in content/*features* and
third_party/WebKit/Source/platform/RuntimeEnabledFeatures.json5

jbauman@: please review the gpu/ and ui/gl/ changes

[jbauman, 2017-02-17 00:38:19]

lgtm

[klausw, 2017-02-17 00:42:03]

klausw@chromium.org changed reviewers:
+ mpearson@chromium.org

[klausw, 2017-02-17 00:43:33]

mpearson@: please review the histograms.xml additions for the feature flag.

(I mistakenly thought earlier that this was not needed due to running the
desktop unit_tests. The Android version does insist on them.)

[klausw, 2017-02-17 02:06:31]

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

[commit-bot: I haz the power, 2017-02-17 02:08:13]

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, 2017-02-17 04:05:47]

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

[commit-bot: I haz the power, 2017-02-17 04:05:48]

Dry run: Try jobs failed on following builders:
  android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...)

[Mark P, 2017-02-17 06:23:30]

histograms.xml lgtm

[klausw, 2017-02-17 07:37:03]

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

[commit-bot: I haz the power, 2017-02-17 07:37:36]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[jochen (gone - plz use gerrit), 2017-02-17 08:34:33]

lgtm with comment

https://codereview.chromium.org/2698573002/diff/200001/third_party/WebKit/Sou...
File third_party/WebKit/Source/platform/RuntimeEnabledFeatures.json5 (right):

https://codereview.chromium.org/2698573002/diff/200001/third_party/WebKit/Sou...
third_party/WebKit/Source/platform/RuntimeEnabledFeatures.json5:917: status:
"test",
I think this should be experimental not just test

[commit-bot: I haz the power, 2017-02-17 09:05:28]

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

[commit-bot: I haz the power, 2017-02-17 09:05:30]

Dry run: This issue passed the CQ dry run.

[klausw, 2017-02-17 15:40:31]

The CQ bit was checked by klausw@chromium.org

[klausw, 2017-02-17 15:40:32]

The patchset sent to the CQ was uploaded after l-g-t-m from aelias@chromium.org,
bajones@chromium.org, jbauman@chromium.org, jochen@chromium.org,
mpearson@chromium.org
Link to the patchset: https://codereview.chromium.org/2698573002/#ps220001
(title: "Change flag status to "experimental" at jochen@'s request")

[commit-bot: I haz the power, 2017-02-17 15:40:58]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[klausw, 2017-02-17 15:41:19]

Thank you!

https://codereview.chromium.org/2698573002/diff/200001/third_party/WebKit/Sou...
File third_party/WebKit/Source/platform/RuntimeEnabledFeatures.json5 (right):

https://codereview.chromium.org/2698573002/diff/200001/third_party/WebKit/Sou...
third_party/WebKit/Source/platform/RuntimeEnabledFeatures.json5:917: status:
"test",
On 2017/02/17 08:34:33, jochen wrote:
> I think this should be experimental not just test

Done.

[rkaplow, 2017-02-17 16:42:08]

lgtm

[commit-bot: I haz the power, 2017-02-17 18:06:10]

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

[commit-bot: I haz the power, 2017-02-17 18:06:13]

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

[klausw, 2017-02-17 18:08:18]

The CQ bit was checked by klausw@chromium.org

[commit-bot: I haz the power, 2017-02-17 18:08:48]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[klausw, 2017-02-17 19:53:59]

The CQ bit was unchecked by klausw@chromium.org

[klausw, 2017-02-17 19:54:06]

The CQ bit was checked by klausw@chromium.org

[commit-bot: I haz the power, 2017-02-17 19:54:58]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[klausw, 2017-02-17 20:41:01]

The CQ bit was unchecked by klausw@chromium.org

[klausw, 2017-02-17 20:41:11]

The CQ bit was checked by klausw@chromium.org

[amp, 2017-02-17 20:41:37]

lgtm

[commit-bot: I haz the power, 2017-02-17 20:41:50]

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, 2017-02-17 23:02:52]

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

[commit-bot: I haz the power, 2017-02-17 23:02:56]

Failed to apply patch for content/public/common/content_features.h:
While running git apply --index -p1;
  error: patch failed: content/public/common/content_features.h:67
  error: content/public/common/content_features.h: patch does not apply

Patch:       content/public/common/content_features.h
Index: content/public/common/content_features.h
diff --git a/content/public/common/content_features.h
b/content/public/common/content_features.h
index
359a95152ef08c00415f7b88dc65c689dd054337..a81a3fd08ddce5b97fcc4f4b67403e6b313392a8
100644
--- a/content/public/common/content_features.h
+++ b/content/public/common/content_features.h
@@ -67,6 +67,7 @@ CONTENT_EXPORT extern const base::Feature kWebRtcEcdsaDefault;
 CONTENT_EXPORT extern const base::Feature kWebRtcHWH264Encoding;
 CONTENT_EXPORT extern const base::Feature kWebRtcUseGpuMemoryBufferVideoFrames;
 CONTENT_EXPORT extern const base::Feature kWebUsb;
+CONTENT_EXPORT extern const base::Feature kWebVRExperimentalRendering;
 CONTENT_EXPORT
 extern const base::Feature kSendBeaconThrowForBlobWithNonSimpleType;

[klausw, 2017-02-17 23:10:36]

The CQ bit was checked by klausw@chromium.org

[klausw, 2017-02-17 23:10:36]

The patchset sent to the CQ was uploaded after l-g-t-m from aelias@chromium.org,
jochen@chromium.org, rkaplow@chromium.org, amp@chromium.org,
mpearson@chromium.org, jbauman@chromium.org, bajones@chromium.org
Link to the patchset: https://codereview.chromium.org/2698573002/#ps260001
(title: "Rebase to ToT")

[commit-bot: I haz the power, 2017-02-17 23:11:44]

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, 2017-02-18 00:26:53]

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

[commit-bot: I haz the power, 2017-02-18 00:26:55]

Try jobs failed on following builders:
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)

[klausw, 2017-02-18 01:24:10]

The CQ bit was checked by klausw@chromium.org

[klausw, 2017-02-18 01:24:10]

The patchset sent to the CQ was uploaded after l-g-t-m from aelias@chromium.org,
jochen@chromium.org, rkaplow@chromium.org, amp@chromium.org,
mpearson@chromium.org, jbauman@chromium.org, bajones@chromium.org
Link to the patchset: https://codereview.chromium.org/2698573002/#ps280001
(title: "Rebase again. I'm pretty sure it wasn't me breaking iOS.")

[commit-bot: I haz the power, 2017-02-18 01:25:14]

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, 2017-02-18 03:27:40]

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

[commit-bot: I haz the power, 2017-02-18 03:27:42]

Try jobs failed on following builders:
  cast_shell_linux on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build
URL)
  chromeos_daisy_chromium_compile_only_ng on master.tryserver.chromium.linux
(JOB_TIMED_OUT, no build URL)
  chromium_presubmit on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build
URL)
  linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT,
no build URL)
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux
(JOB_TIMED_OUT, no build URL)
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT, no
build URL)
  linux_chromium_tsan_rel_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT,
no build URL)
  linux_optional_gpu_tests_rel on master.tryserver.chromium.linux
(JOB_TIMED_OUT, no build URL)

[klausw, 2017-02-18 03:36:37]

The CQ bit was checked by klausw@chromium.org

[klausw, 2017-02-18 03:36:38]

The patchset sent to the CQ was uploaded after l-g-t-m from aelias@chromium.org,
jochen@chromium.org, rkaplow@chromium.org, amp@chromium.org,
mpearson@chromium.org, jbauman@chromium.org, bajones@chromium.org
Link to the patchset: https://codereview.chromium.org/2698573002/#ps290001
(title: "Rebase and retry again")

[commit-bot: I haz the power, 2017-02-18 03:37:13]

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, 2017-02-18 05:38:24]

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

[commit-bot: I haz the power, 2017-02-18 05:38:26]

Try jobs failed on following builders:
  cast_shell_linux on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build
URL)
  linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
(JOB_TIMED_OUT, no build URL)
  linux_chromium_compile_dbg_ng on master.tryserver.chromium.linux
(JOB_TIMED_OUT, no build URL)

[klausw, 2017-02-18 06:18:39]

The CQ bit was checked by klausw@chromium.org

[commit-bot: I haz the power, 2017-02-18 06:19:04]

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, 2017-02-18 07:54:03]

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

[commit-bot: I haz the power, 2017-02-18 07:54:05]

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

[klausw, 2017-02-18 17:47:27]

The CQ bit was checked by klausw@chromium.org

[commit-bot: I haz the power, 2017-02-18 17:47:37]

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, 2017-02-18 18:48:02]

CQ is committing da patch.

Bot data: {"patchset_id": 290001, "attempt_start_ts": 1487440047393760,
"parent_rev": "4ed8b41d5e7e3b6cbf292f3a8d3533b767fd0417", "commit_rev":
"89ec62330efc014949d78367b94de89c4a7b5405"}

[commit-bot: I haz the power, 2017-02-18 18:48:41]

Description was changed from

==========
This is an updated version of issue 2586803003 which was reverted.
I have removed the problematic "low priority" GL context setting and
made the change conditional on a new ""WebVR experimental rendering
optimizations" feature flag.
    
When enabled, it uses the own_offscreen_surface flag with WebGL contexts
when WebVR is enabled, to allow the surface to be swapped out with a
compatible one if used for VR presentation later. This must be done
before any WebVR API calls have been made, so it also needs to work
for non-WebVR WebGL canvas drawing.
    
The change includes support for canvas attributes such as "antialias".

The feature flag is intended to control features which are considered
experimental or risky, so that they can be manually enabled for initial
testing. It is also in preparation for a finch experiment for future
rollout.

BUG=655733
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
==========

to

==========
This is an updated version of issue 2586803003 which was reverted.
I have removed the problematic "low priority" GL context setting and
made the change conditional on a new ""WebVR experimental rendering
optimizations" feature flag.
    
When enabled, it uses the own_offscreen_surface flag with WebGL contexts
when WebVR is enabled, to allow the surface to be swapped out with a
compatible one if used for VR presentation later. This must be done
before any WebVR API calls have been made, so it also needs to work
for non-WebVR WebGL canvas drawing.
    
The change includes support for canvas attributes such as "antialias".

The feature flag is intended to control features which are considered
experimental or risky, so that they can be manually enabled for initial
testing. It is also in preparation for a finch experiment for future
rollout.

BUG=655733
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel

Review-Url: https://codereview.chromium.org/2698573002
Cr-Commit-Position: refs/heads/master@{#451484}
Committed:
https://chromium.googlesource.com/chromium/src/+/89ec62330efc014949d78367b94d...
==========

[commit-bot: I haz the power, 2017-02-18 18:48:43]

Committed patchset #16 (id:290001) as
https://chromium.googlesource.com/chromium/src/+/89ec62330efc014949d78367b94d...