gpu: Add CHROMIUM_image support to in-process command buffer.

gpu: Add CHROMIUM_image support to in-process command buffer.

This makes it possible to use the in-process command buffer
with CHROMIUM_image extension. GpuMemoryBufferManager and
ImageFactory interfaces need to be provided when creating
an in-process command buffer. Existing implementations
of these interfaces in content/ should work just fine.

cc::TestGpuMemoryBufferManager and cc::TestImageFactory
are used for cc pixel tests. They provide a minimal
implementation of these interfaces using shared memory.

Note: shared memory is used for in-process testing as it
maps better to the GpuMemoryBuffer framework that has
been designed for multi-process usage. Non-shared memory
might be a bit more efficient but it would require more
complexity and not match real usage as well.

BUG=423533
Committed: https://crrev.com/2232bce10844b88b9816027c2b546ccee64f46d4
Cr-Commit-Position: refs/heads/master@{#301763}

[reveman, 2014-10-17 20:25:13]

reveman@chromium.org changed reviewers:
+ sievers@chromium.org

[reveman, 2014-10-17 20:25:14]

and now some real changes

[no sievers, 2014-10-17 21:16:50]

https://codereview.chromium.org/665463003/diff/40001/cc/test/test_gpu_memory_...
File cc/test/test_gpu_memory_buffer_manager.cc (right):

https://codereview.chromium.org/665463003/diff/40001/cc/test/test_gpu_memory_...
cc/test/test_gpu_memory_buffer_manager.cc:64: const gfx::Size size_;
nit: could make size_ and format_ const since bad things would happen if they
change

https://codereview.chromium.org/665463003/diff/40001/cc/test/test_image_facto...
File cc/test/test_image_factory.h (right):

https://codereview.chromium.org/665463003/diff/40001/cc/test/test_image_facto...
cc/test/test_image_factory.h:24: };
nit: private: DISALLOW_COPY_AND_ASSIGN()

https://codereview.chromium.org/665463003/diff/40001/gpu/command_buffer/servi...
File gpu/command_buffer/service/in_process_command_buffer.cc (right):

https://codereview.chromium.org/665463003/diff/40001/gpu/command_buffer/servi...
gpu/command_buffer/service/in_process_command_buffer.cc:281: ImageFactory*
image_factory) {
DCHECK(!gpu_memory_buffer_manager || image_factory);

https://codereview.chromium.org/665463003/diff/40001/gpu/command_buffer/servi...
gpu/command_buffer/service/in_process_command_buffer.cc:439:
params.capabilities->image = params.capabilities->image && image_factory_;
isn't params.capabilities uninitialized?

https://codereview.chromium.org/665463003/diff/40001/webkit/common/gpu/webgra...
File webkit/common/gpu/webgraphicscontext3d_in_process_command_buffer_impl.h
(right):

https://codereview.chromium.org/665463003/diff/40001/webkit/common/gpu/webgra...
webkit/common/gpu/webgraphicscontext3d_in_process_command_buffer_impl.h:85:
::gpu::ImageFactory* image_factory);
Do we need these changes?
It's weird in the constructor since either |context| or both
gpu_memory_buffer_manager and image_factory must be NULL.

[reveman, 2014-10-17 22:36:18]

PTAL

https://codereview.chromium.org/665463003/diff/40001/cc/test/test_gpu_memory_...
File cc/test/test_gpu_memory_buffer_manager.cc (right):

https://codereview.chromium.org/665463003/diff/40001/cc/test/test_gpu_memory_...
cc/test/test_gpu_memory_buffer_manager.cc:64: const gfx::Size size_;
On 2014/10/17 21:16:50, sievers wrote:
> nit: could make size_ and format_ const since bad things would happen if they
> change

Done.

https://codereview.chromium.org/665463003/diff/40001/cc/test/test_image_facto...
File cc/test/test_image_factory.h (right):

https://codereview.chromium.org/665463003/diff/40001/cc/test/test_image_facto...
cc/test/test_image_factory.h:24: };
On 2014/10/17 21:16:50, sievers wrote:
> nit: private: DISALLOW_COPY_AND_ASSIGN()

Done. Also in TestGpuMemoryBufferManager.

https://codereview.chromium.org/665463003/diff/40001/gpu/command_buffer/servi...
File gpu/command_buffer/service/in_process_command_buffer.cc (right):

https://codereview.chromium.org/665463003/diff/40001/gpu/command_buffer/servi...
gpu/command_buffer/service/in_process_command_buffer.cc:281: ImageFactory*
image_factory) {
On 2014/10/17 21:16:50, sievers wrote:
> DCHECK(!gpu_memory_buffer_manager || image_factory);

What if an image factory instance is provided by no buffer manager? I think
DCHECKs to prevent this are just hard to read and we might as well allow the
caller to provide just one if it likes to do that. The extension will of course
only be available if both are provided.

https://codereview.chromium.org/665463003/diff/40001/gpu/command_buffer/servi...
gpu/command_buffer/service/in_process_command_buffer.cc:439:
params.capabilities->image = params.capabilities->image && image_factory_;
On 2014/10/17 21:16:50, sievers wrote:
> isn't params.capabilities uninitialized?

we set it above with a call to decoder_->GetCapabilities().

https://codereview.chromium.org/665463003/diff/40001/webkit/common/gpu/webgra...
File webkit/common/gpu/webgraphicscontext3d_in_process_command_buffer_impl.h
(right):

https://codereview.chromium.org/665463003/diff/40001/webkit/common/gpu/webgra...
webkit/common/gpu/webgraphicscontext3d_in_process_command_buffer_impl.h:85:
::gpu::ImageFactory* image_factory);
On 2014/10/17 21:16:50, sievers wrote:
> Do we need these changes?
> It's weird in the constructor since either |context| or both
> gpu_memory_buffer_manager and image_factory must be NULL.

Right, this must have been confusing :) Removed these changes from latest patch.

[no sievers, 2014-10-17 23:37:59]

lgtm

[reveman, 2014-10-28 23:01:22]

reveman@chromium.org changed reviewers:
+ jamesr@chromium.org

[reveman, 2014-10-28 23:01:22]

+jamesr for webkit/

[reveman, 2014-10-28 23:49:57]

reveman@chromium.org changed reviewers:
+ boliu@chromium.org

[reveman, 2014-10-28 23:49:58]

+boliu for android_webview/

[boliu, 2014-10-28 23:53:32]

On 2014/10/28 23:49:58, reveman wrote:
> +boliu for android_webview/

lgtm

[jamesr, 2014-10-29 00:24:43]

webkit/ lgtm

[reveman, 2014-10-29 00:39:09]

The CQ bit was checked by reveman@chromium.org

[commit-bot: I haz the power, 2014-10-29 00:41:12]

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

[commit-bot: I haz the power, 2014-10-29 01:28:27]

Committed patchset #7 (id:120001)

[commit-bot: I haz the power, 2014-10-29 01:29:01]

Patchset 7 (id:??) landed as
https://crrev.com/2232bce10844b88b9816027c2b546ccee64f46d4
Cr-Commit-Position: refs/heads/master@{#301763}