Move GPU blacklist calculation to GPU proc

Move GPU raster decision making to GPU proc

This change moves decision making about whether to use GPU raster
from the browser process to the GPU process. This allows the
decision to be made based on more detailed GPU information, which
is not available to the browser. The new decision making process
is as follows:

The GPU process loads the GPU blacklist at startup, using more
complete GPUInfo than is available to the browser. This data is
combined with flags (forwarded from the browser) to
generate a GpuFeatureInfo struct which contains enabled/
blacklisted/disabled values for each GPU feature (currently
only populated for GPU raster).

The GPU process notifies the browser of its decision
by passing the GpuFeatureInfo via GpuHostMessage_Initialized.
The browser uses this info solely to populate about:gpu.

The GPU process notifies the renderer process of its decision
via context capabilities. The renderer uses this info to make
decisions on how to rasterize.

In the Render process, we now recieve GPU raster information
at a later point (when we create our CompositorFrameSink), so
we can't store this info in LayerTreeSettings. This requires
a bit of refactoring to have tests use context Capabilities
rather than LayerTreeSettings to provide this information.

BUG=648493
R=zmo@chromium.org
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/2654993004
Cr-Commit-Position: refs/heads/master@{#449729}
Committed: https://chromium.googlesource.com/chromium/src/+/41a1579e255266ab1f48bfd3c963aebde8579f44

[ericrk, 2017-01-25 19:43:49]

Description was changed from

==========
Add GPU blacklist calculation to GPU proc and pass data back to browser

This is part 1 of a multi-part patch. Broke it up to make things easier
to review, but will only commit when all parts are complete.

R=zmo@chromium.org
==========

to

==========
Add GPU blacklist calculation to GPU proc and pass data back to browser

This is part 1 of a multi-part patch. Broke it up to make things easier
to review, but will only commit when all parts are complete.

R=zmo@chromium.org
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
==========

[ericrk, 2017-01-25 19:53:21]

Hi Mo,

This is part 1 of a few dependent CLs (which I will eventually commit together,
as they aren't truly seperable). Wanted to make sure I was going in the right
direction now.

Note that the "enabled features" part of the GPUFeatureStatus struct could
probably be removed if we are OK folding command-line and finch feature
disablement into "blacklisted features". The only downside here (from what I can
see) is that we lose this differentiation in about:gpu.

[Zhenyao Mo, 2017-01-25 22:37:09]

Looks like the right direction to go.

https://codereview.chromium.org/2654993004/diff/20001/gpu/config/gpu_feature_...
File gpu/config/gpu_feature_status.h (right):

https://codereview.chromium.org/2654993004/diff/20001/gpu/config/gpu_feature_...
gpu/config/gpu_feature_status.h:14: std::set<int> enabled_features;
I suggest to defined an enum of {Blacklisted, Enabled, } etc, and the struct
contains a list of enums, one for each feature.  This is cleaner.

In the future we want to track a feature is enabled in software fallback, etc,
so we can easily just expand the enum.

https://codereview.chromium.org/2654993004/diff/20001/gpu/config/gpu_features.h
File gpu/config/gpu_features.h (right):

https://codereview.chromium.org/2654993004/diff/20001/gpu/config/gpu_features...
gpu/config/gpu_features.h:18: GPU_EXPORT extern const base::Feature
kDefaultEnableGpuRasterization;
You don't need this.  You should just use the GPU_FEATURE_RASTERIZATION.

[ericrk, 2017-01-28 01:09:07]

Description was changed from

==========
Add GPU blacklist calculation to GPU proc and pass data back to browser

This is part 1 of a multi-part patch. Broke it up to make things easier
to review, but will only commit when all parts are complete.

R=zmo@chromium.org
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

==========
Move GPU raster decision making to GPU proc

This change moves decision making about whether to use GPU raster
from the browser process to the GPU process. This allows the
decision to be made based on more detailed GPU information, which
is not available to the browser. The new decision making process
is as follows:

The GPU process loads the GPU blacklist at startup, using more
complete GPUInfo than is available to the browser. This data is
combined with flags (forwarded from the browser) to
generate a GpuFeatureInfo struct which contains enabled/
blacklisted/disabled values for each GPU feature (currently
only populated for GPU raster).

The GPU process notifies the browser of its decision via the
by passing the GpuFeatureInfo via GpuHostMessage_Initialized.
The browser uses this info solely to populate about:gpu.

The GPU process notifies the renderer process of its decision
via context capabilities. The renderer uses this info to make
decisions on how to rasterize.

In the Render process, we now recieve GPU raster information
at a later point (when we create our CompositorFrameSink), so
we can't store this info in LayerTreeSettings. This requires
a bit of refactoring to have tests use context Capabilities
rather than LayerTreeSettings to provide this information.

BUG=648493
R=zmo@chromium.org
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
==========

[ericrk, 2017-01-30 15:36:07]

Here's the updated CL, let me know how this looks.

https://codereview.chromium.org/2654993004/diff/20001/gpu/config/gpu_feature_...
File gpu/config/gpu_feature_status.h (right):

https://codereview.chromium.org/2654993004/diff/20001/gpu/config/gpu_feature_...
gpu/config/gpu_feature_status.h:14: std::set<int> enabled_features;
On 2017/01/25 22:37:09, Zhenyao Mo wrote:
> I suggest to defined an enum of {Blacklisted, Enabled, } etc, and the struct
> contains a list of enums, one for each feature.  This is cleaner.
> 
> In the future we want to track a feature is enabled in software fallback, etc,
> so we can easily just expand the enum.

sgtm

https://codereview.chromium.org/2654993004/diff/20001/gpu/config/gpu_features.h
File gpu/config/gpu_features.h (right):

https://codereview.chromium.org/2654993004/diff/20001/gpu/config/gpu_features...
gpu/config/gpu_features.h:18: GPU_EXPORT extern const base::Feature
kDefaultEnableGpuRasterization;
On 2017/01/25 22:37:09, Zhenyao Mo wrote:
> You don't need this.  You should just use the GPU_FEATURE_RASTERIZATION.

These are Finch features, not GPU ones - the naming is a bit confusing (too many
things called "feature"), but it lines up with the existing naming in content
"content_features.h", and these Finch objects are called base::Feature.

[ericrk, 2017-01-30 15:41:24]

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

https://codereview.chromium.org/2654993004/diff/40001/content/browser/gpu/gpu...
content/browser/gpu/gpu_data_manager_impl_private.cc:307: return
gpu_feature_info_
We could also put an IsFeatureEnabled on GpuFeatureInfo ... but seemed weird for
a mostly data-only struct.... WDYT?

[ericrk, 2017-02-03 19:38:17]

On 2017/01/30 15:41:24, ericrk wrote:
>
https://codereview.chromium.org/2654993004/diff/40001/content/browser/gpu/gpu...
> File content/browser/gpu/gpu_data_manager_impl_private.cc (right):
> 
>
https://codereview.chromium.org/2654993004/diff/40001/content/browser/gpu/gpu...
> content/browser/gpu/gpu_data_manager_impl_private.cc:307: return
> gpu_feature_info_
> We could also put an IsFeatureEnabled on GpuFeatureInfo ... but seemed weird
for
> a mostly data-only struct.... WDYT?

Ping - zmo@, can you take a look when you have a chance? Thanks!

[ericrk, 2017-02-03 23:33:46]

Description was changed from

==========
Move GPU raster decision making to GPU proc

This change moves decision making about whether to use GPU raster
from the browser process to the GPU process. This allows the
decision to be made based on more detailed GPU information, which
is not available to the browser. The new decision making process
is as follows:

The GPU process loads the GPU blacklist at startup, using more
complete GPUInfo than is available to the browser. This data is
combined with flags (forwarded from the browser) to
generate a GpuFeatureInfo struct which contains enabled/
blacklisted/disabled values for each GPU feature (currently
only populated for GPU raster).

The GPU process notifies the browser of its decision via the
by passing the GpuFeatureInfo via GpuHostMessage_Initialized.
The browser uses this info solely to populate about:gpu.

The GPU process notifies the renderer process of its decision
via context capabilities. The renderer uses this info to make
decisions on how to rasterize.

In the Render process, we now recieve GPU raster information
at a later point (when we create our CompositorFrameSink), so
we can't store this info in LayerTreeSettings. This requires
a bit of refactoring to have tests use context Capabilities
rather than LayerTreeSettings to provide this information.

BUG=648493
R=zmo@chromium.org
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

==========
Move GPU raster decision making to GPU proc

This change moves decision making about whether to use GPU raster
from the browser process to the GPU process. This allows the
decision to be made based on more detailed GPU information, which
is not available to the browser. The new decision making process
is as follows:

The GPU process loads the GPU blacklist at startup, using more
complete GPUInfo than is available to the browser. This data is
combined with flags (forwarded from the browser) to
generate a GpuFeatureInfo struct which contains enabled/
blacklisted/disabled values for each GPU feature (currently
only populated for GPU raster).

The GPU process notifies the browser of its decision
by passing the GpuFeatureInfo via GpuHostMessage_Initialized.
The browser uses this info solely to populate about:gpu.

The GPU process notifies the renderer process of its decision
via context capabilities. The renderer uses this info to make
decisions on how to rasterize.

In the Render process, we now recieve GPU raster information
at a later point (when we create our CompositorFrameSink), so
we can't store this info in LayerTreeSettings. This requires
a bit of refactoring to have tests use context Capabilities
rather than LayerTreeSettings to provide this information.

BUG=648493
R=zmo@chromium.org
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
==========

[Zhenyao Mo, 2017-02-03 23:57:32]

Sorry about the delay.  LGTM with one question. (I didn't look at cc though).

The question leads to this situation: if I open about:gpu before GPU process
launches and sends back GpuFeatureInfo, we should say info not ready yet and
there should be a refresh somewhere in the future when the info is available,
not saying features are disabled like the current design.

That's not critical though, so feel free to address it in a follow up.

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

https://codereview.chromium.org/2654993004/diff/40001/content/browser/gpu/gpu...
content/browser/gpu/gpu_data_manager_impl_private.cc:305: bool
GpuDataManagerImplPrivate::IsFeatureEnabled(int feature) const {
What is the design for the situation if this is called before the GPU process
side sends back the GpuFeatureInfo?
Should we add an actual enum "undefined" for GpuFeatureStatus? And by default a
feature status should be "undefined" rather than "disabled"

https://codereview.chromium.org/2654993004/diff/40001/content/browser/gpu/gpu...
content/browser/gpu/gpu_data_manager_impl_private.cc:307: return
gpu_feature_info_
On 2017/01/30 15:41:24, ericrk wrote:
> We could also put an IsFeatureEnabled on GpuFeatureInfo ... but seemed weird
for
> a mostly data-only struct.... WDYT?

Sounds good.

https://codereview.chromium.org/2654993004/diff/40001/gpu/config/gpu_features.h
File gpu/config/gpu_features.h (right):

https://codereview.chromium.org/2654993004/diff/40001/gpu/config/gpu_features...
gpu/config/gpu_features.h:5: // This file defines all the public
base::FeatureList features for the gpu
What about be more specific to avoid confusion?  Maybe just name the file
gpu_finch_features.h

https://codereview.chromium.org/2654993004/diff/40001/gpu/config/gpu_util.cc
File gpu/config/gpu_util.cc (right):

https://codereview.chromium.org/2654993004/diff/40001/gpu/config/gpu_util.cc#...
gpu/config/gpu_util.cc:177: const base::CommandLine& command_line) {
Can we DCHECK it's either in-process gpu mode or it's actually in GPU process? 
If it's too much efforts, then forget about it.

[ericrk, 2017-02-06 19:16:05]

Patchset #4 (id:60001) has been deleted

[ericrk, 2017-02-06 19:36:13]

Patchset #4 (id:80001) has been deleted

[ericrk, 2017-02-06 19:45:29]

ericrk@chromium.org changed reviewers:
+ enne@chromium.org

[ericrk, 2017-02-06 19:47:57]

ericrk@chromium.org changed reviewers:
+ jochen@chromium.org

[ericrk, 2017-02-06 19:48:36]

ericrk@chromium.org changed reviewers:
+ rjkroege@chromium.org

[ericrk, 2017-02-06 19:50:00]

ericrk@chromium.org changed reviewers:
+ dcheng@chromium.org

[ericrk, 2017-02-06 19:51:37]

ericrk@chromium.org changed reviewers:
- dcheng@chromium.org, enne@chromium.org, jochen@chromium.org,
rjkroege@chromium.org

[ericrk, 2017-02-06 19:51:38]

zmo@, created a bug to track the about:gpu info. Addressed other comments.
Thanks!

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

https://codereview.chromium.org/2654993004/diff/40001/content/browser/gpu/gpu...
content/browser/gpu/gpu_data_manager_impl_private.cc:305: bool
GpuDataManagerImplPrivate::IsFeatureEnabled(int feature) const {
On 2017/02/03 23:57:32, Zhenyao Mo wrote:
> What is the design for the situation if this is called before the GPU process
> side sends back the GpuFeatureInfo?
> Should we add an actual enum "undefined" for GpuFeatureStatus? And by default
a
> feature status should be "undefined" rather than "disabled"

makes sense - added disabled.

https://codereview.chromium.org/2654993004/diff/40001/gpu/config/gpu_features.h
File gpu/config/gpu_features.h (right):

https://codereview.chromium.org/2654993004/diff/40001/gpu/config/gpu_features...
gpu/config/gpu_features.h:5: // This file defines all the public
base::FeatureList features for the gpu
On 2017/02/03 23:57:32, Zhenyao Mo wrote:
> What about be more specific to avoid confusion?  Maybe just name the file
> gpu_finch_features.h

sgtm.

https://codereview.chromium.org/2654993004/diff/40001/gpu/config/gpu_util.cc
File gpu/config/gpu_util.cc (right):

https://codereview.chromium.org/2654993004/diff/40001/gpu/config/gpu_util.cc#...
gpu/config/gpu_util.cc:177: const base::CommandLine& command_line) {
On 2017/02/03 23:57:32, Zhenyao Mo wrote:
> Can we DCHECK it's either in-process gpu mode or it's actually in GPU process?

> If it's too much efforts, then forget about it.

I did a bit of searching and I couldn't find a good way to DCHECK what process
we are in. I'll add a comment to the .h indicating that this should only be used
with the GPU proc or Browser proc in in-proc GPU mode.

[ericrk, 2017-02-06 20:01:51]

ericrk@chromium.org changed reviewers:
+ dcheng@chromium.org, enne@chromium.org, jochen@chromium.org,
rjkroege@chromium.org

[ericrk, 2017-02-06 20:01:51]

+enne@ for cc/ - We now get the GPU raster status from context caps rather than
layer tree settings. This required a couple small code changes, and a bunch of
unit test tweaks (no functional changes).

+jochen@ for content/* (other than what zmo@ already reviewed) - in general,
we've moved a few switches and a finch flag from content > gpu. Additionally, we
create a new GpuFeatureInfo struct which is stored on GpuChildThread (and passed
back to the browser).

+rjkroege@ for services/ui/gpu - We need to store the GpuFeatureInfo struct on
GpuService - it's later used to create the GpuChannelManager.

+dcheng@ for IPC changes - This change adds a new struct to
GpuHostMsg_Initialized - GpuFeatureInfo. This struct contains a list of GPU
features, such as rasterization, WebGL, etc.. and an enum value indicating
whether each one is (enabled, disabled, blacklisted). We used to calculate this
directly in the Browser proc, but there wasn't enough info there to make good
decisions. Additionally, we pass one of these values, gpu_rasterization, from
the Gpu proc to the renderer proc via ContextCapabilities. We used to pass this
from the browser to the renderer via flags.

[ericrk, 2017-02-06 20:02:05]

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

[commit-bot: I haz the power, 2017-02-06 20:03:18]

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-06 20:08:25]

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

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

Dry run: Try jobs failed on following builders:
  cast_shell_linux on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...)
  linux_chromium_compile_dbg_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
  mac_optional_gpu_tests_rel on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_optional_gpu_...)

[enne (OOO), 2017-02-06 20:14:53]

cc lgtm

[ericrk, 2017-02-06 21:18:19]

Patchset #6 (id:140001) has been deleted

[ericrk, 2017-02-06 21:45:46]

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

[commit-bot: I haz the power, 2017-02-06 21:46:34]

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-06 21:51:10]

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

[commit-bot: I haz the power, 2017-02-06 21:51:11]

Dry run: 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...)
  ios-simulator-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator-xco...)
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
  mac_optional_gpu_tests_rel on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_optional_gpu_...)

[ericrk, 2017-02-06 21:55:04]

+jbauman FYI

[ericrk, 2017-02-06 23:09:26]

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

[commit-bot: I haz the power, 2017-02-06 23:10:32]

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-06 23:23:08]

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

[commit-bot: I haz the power, 2017-02-06 23:23:09]

Dry run: Try jobs failed on following builders:
  cast_shell_android on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/cast_shell_a...)
  linux_optional_gpu_tests_rel on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_optional_...)
  mac_optional_gpu_tests_rel on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_optional_gpu_...)

[rjkroege, 2017-02-07 00:42:41]

services/ui lgtm

https://codereview.chromium.org/2654993004/diff/180001/gpu/config/gpu_util.h
File gpu/config/gpu_util.h (right):

https://codereview.chromium.org/2654993004/diff/180001/gpu/config/gpu_util.h#...
gpu/config/gpu_util.h:43: GPU_EXPORT GpuFeatureInfo
can you say if it's safe to call this function after entering the sandbox in a
comment?

[dcheng, 2017-02-07 00:58:22]

https://codereview.chromium.org/2654993004/diff/180001/gpu/config/gpu_feature...
File gpu/config/gpu_feature_info.h (right):

https://codereview.chromium.org/2654993004/diff/180001/gpu/config/gpu_feature...
gpu/config/gpu_feature_info.h:20: kGpuFeatureStatusMax
kGpuFeatureStatusMax = kGpuFeatureStatusUndefined

How many features does this thing have? Does it make sense to just skip the
vector and just make this an inline array to avoid the indirection through the
heap allocation?

[jochen (gone - plz use gerrit), 2017-02-07 15:09:50]

content/ lgtm

[ericrk, 2017-02-08 21:19:55]

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

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

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

[ericrk, 2017-02-08 21:29:46]

Thanks for the feedback. Updated.

https://codereview.chromium.org/2654993004/diff/180001/gpu/config/gpu_feature...
File gpu/config/gpu_feature_info.h (right):

https://codereview.chromium.org/2654993004/diff/180001/gpu/config/gpu_feature...
gpu/config/gpu_feature_info.h:20: kGpuFeatureStatusMax
On 2017/02/07 00:58:22, dcheng wrote:
> kGpuFeatureStatusMax = kGpuFeatureStatusUndefined
> 
> How many features does this thing have? Does it make sense to just skip the
> vector and just make this an inline array to avoid the indirection through the
> heap allocation?

Makes sense - switched this to a fixed-size array.

https://codereview.chromium.org/2654993004/diff/180001/gpu/config/gpu_util.h
File gpu/config/gpu_util.h (right):

https://codereview.chromium.org/2654993004/diff/180001/gpu/config/gpu_util.h#...
gpu/config/gpu_util.h:43: GPU_EXPORT GpuFeatureInfo
On 2017/02/07 00:42:41, rjkroege wrote:
> can you say if it's safe to call this function after entering the sandbox in a
> comment?

Yup - you can call this at any point - added a comment.

[commit-bot: I haz the power, 2017-02-08 21:51:39]

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

[commit-bot: I haz the power, 2017-02-08 21:51:41]

Dry run: Try jobs failed on following builders:
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[dcheng, 2017-02-08 22:33:11]

ipc lgtm, but I think you'll need to fix the fuzzer to understand how to handle
arrays.

https://codereview.chromium.org/2654993004/diff/200001/gpu/config/gpu_feature...
File gpu/config/gpu_feature_info.h (right):

https://codereview.chromium.org/2654993004/diff/200001/gpu/config/gpu_feature...
gpu/config/gpu_feature_info.h:8: #include <array>
Nit: Not used?

[ericrk, 2017-02-10 02:36:06]

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

[commit-bot: I haz the power, 2017-02-10 02:36:34]

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

[ericrk, 2017-02-10 02:38:56]

Patchset #9 (id:220001) has been deleted

[commit-bot: I haz the power, 2017-02-10 02:39:32]

Dry run: Try jobs failed on following builders:
  cast_shell_android on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/cast_shell_a...)
  chromeos_daisy_chromium_compile_only_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_daisy_...)
  linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)
  mac_optional_gpu_tests_rel on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_optional_gpu_...)

[ericrk, 2017-02-10 02:56:37]

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

[commit-bot: I haz the power, 2017-02-10 02:57: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-10 03:10:13]

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

[commit-bot: I haz the power, 2017-02-10 03:10:14]

Dry run: 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_...)
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_optional_gpu_tests_rel on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_optional_...)

[ericrk, 2017-02-10 03:23:06]

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

[commit-bot: I haz the power, 2017-02-10 03:23:29]

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-10 04:32:49]

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

[commit-bot: I haz the power, 2017-02-10 04:32:50]

Dry run: Try jobs failed on following builders:
  win_clang on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_clang/builds/...)

[ericrk, 2017-02-10 18:41:20]

On 2017/02/08 22:33:11, dcheng wrote:
> ipc lgtm, but I think you'll need to fix the fuzzer to understand how to
handle
> arrays.
> 
>
https://codereview.chromium.org/2654993004/diff/200001/gpu/config/gpu_feature...
> File gpu/config/gpu_feature_info.h (right):
> 
>
https://codereview.chromium.org/2654993004/diff/200001/gpu/config/gpu_feature...
> gpu/config/gpu_feature_info.h:8: #include <array>
> Nit: Not used?

Thanks. If you're referring to the build error in fuzzer, that was actually
unrelated to the arrays change.

[ericrk, 2017-02-10 19:27:46]

https://codereview.chromium.org/2654993004/diff/200001/gpu/config/gpu_feature...
File gpu/config/gpu_feature_info.h (right):

https://codereview.chromium.org/2654993004/diff/200001/gpu/config/gpu_feature...
gpu/config/gpu_feature_info.h:8: #include <array>
On 2017/02/08 22:33:11, dcheng wrote:
> Nit: Not used?

removed.

[ericrk, 2017-02-10 19:28:02]

The CQ bit was checked by ericrk@chromium.org

[ericrk, 2017-02-10 19:28:02]

The patchset sent to the CQ was uploaded after l-g-t-m from zmo@chromium.org,
enne@chromium.org, rjkroege@chromium.org, jochen@chromium.org,
dcheng@chromium.org
Link to the patchset: https://codereview.chromium.org/2654993004/#ps250001
(title: "fix win clang build")

[commit-bot: I haz the power, 2017-02-10 19:28:48]

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-10 20:56:32]

CQ is committing da patch.

Bot data: {"patchset_id": 250001, "attempt_start_ts": 1486754882137950,
"parent_rev": "40029084fa291f3a0ef6cf22a90ff86e281f9cf6", "commit_rev":
"41a1579e255266ab1f48bfd3c963aebde8579f44"}

[commit-bot: I haz the power, 2017-02-10 20:57:09]

Description was changed from

==========
Move GPU raster decision making to GPU proc

This change moves decision making about whether to use GPU raster
from the browser process to the GPU process. This allows the
decision to be made based on more detailed GPU information, which
is not available to the browser. The new decision making process
is as follows:

The GPU process loads the GPU blacklist at startup, using more
complete GPUInfo than is available to the browser. This data is
combined with flags (forwarded from the browser) to
generate a GpuFeatureInfo struct which contains enabled/
blacklisted/disabled values for each GPU feature (currently
only populated for GPU raster).

The GPU process notifies the browser of its decision
by passing the GpuFeatureInfo via GpuHostMessage_Initialized.
The browser uses this info solely to populate about:gpu.

The GPU process notifies the renderer process of its decision
via context capabilities. The renderer uses this info to make
decisions on how to rasterize.

In the Render process, we now recieve GPU raster information
at a later point (when we create our CompositorFrameSink), so
we can't store this info in LayerTreeSettings. This requires
a bit of refactoring to have tests use context Capabilities
rather than LayerTreeSettings to provide this information.

BUG=648493
R=zmo@chromium.org
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

==========
Move GPU raster decision making to GPU proc

This change moves decision making about whether to use GPU raster
from the browser process to the GPU process. This allows the
decision to be made based on more detailed GPU information, which
is not available to the browser. The new decision making process
is as follows:

The GPU process loads the GPU blacklist at startup, using more
complete GPUInfo than is available to the browser. This data is
combined with flags (forwarded from the browser) to
generate a GpuFeatureInfo struct which contains enabled/
blacklisted/disabled values for each GPU feature (currently
only populated for GPU raster).

The GPU process notifies the browser of its decision
by passing the GpuFeatureInfo via GpuHostMessage_Initialized.
The browser uses this info solely to populate about:gpu.

The GPU process notifies the renderer process of its decision
via context capabilities. The renderer uses this info to make
decisions on how to rasterize.

In the Render process, we now recieve GPU raster information
at a later point (when we create our CompositorFrameSink), so
we can't store this info in LayerTreeSettings. This requires
a bit of refactoring to have tests use context Capabilities
rather than LayerTreeSettings to provide this information.

BUG=648493
R=zmo@chromium.org
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/2654993004
Cr-Commit-Position: refs/heads/master@{#449729}
Committed:
https://chromium.googlesource.com/chromium/src/+/41a1579e255266ab1f48bfd3c963...
==========

[commit-bot: I haz the power, 2017-02-10 20:57:11]

Committed patchset #10 (id:250001) as
https://chromium.googlesource.com/chromium/src/+/41a1579e255266ab1f48bfd3c963...