Decouple PassThroughImageTransportSurface from GpuCommandBufferStub

Decouple PassThroughImageTransportSurface from GpuCommandBufferStub

We would like to reuse PassThroughImageTransportSurface in
InprocessCommandBuffer to track when a swap completes on various
platforms. This signal is used by ozone, in particular, to advance the
BufferQueue when the command buffer is out-of-process. GLRenderer is
moving to the gpu process and so, in the short term, we'd like to use
an InprocessCommandBuffer and InprocessContextProvider without losing
functionality.

BUG=661278
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

Committed: https://crrev.com/2b039432b32361aa1b772aa5fa878d43ac22eff6
Cr-Commit-Position: refs/heads/master@{#431100}

[Fady Samuel, 2016-11-09 20:03:15]

Description was changed from

==========
WIP: Introduce PassthroughImageTransportSurfaceDelegate

BUG=
==========

to

==========
WIP: Introduce PassthroughImageTransportSurfaceDelegate

BUG=
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
==========

[Fady Samuel, 2016-11-09 20:03:23]

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

[commit-bot: I haz the power, 2016-11-09 20:03:49]

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, 2016-11-09 20:24:14]

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

[commit-bot: I haz the power, 2016-11-09 20:24:15]

Dry run: Try jobs failed on following builders:
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)

[Fady Samuel, 2016-11-09 20:57:25]

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

[commit-bot: I haz the power, 2016-11-09 20:57:58]

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, 2016-11-09 21:01:28]

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

[commit-bot: I haz the power, 2016-11-09 21:01:29]

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

[Fady Samuel, 2016-11-09 21:10:15]

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

[commit-bot: I haz the power, 2016-11-09 21:10:52]

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, 2016-11-09 21:27:36]

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

[commit-bot: I haz the power, 2016-11-09 21:27:36]

Dry run: Try jobs failed on following builders:
  mac_optional_gpu_tests_rel on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_optional_gpu_...)

[Fady Samuel, 2016-11-09 21:40:31]

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

[commit-bot: I haz the power, 2016-11-09 21:40:55]

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

[Fady Samuel, 2016-11-09 22:11:54]

Description was changed from

==========
WIP: Introduce PassthroughImageTransportSurfaceDelegate

BUG=
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

==========
Decouple PassThroughImageTransportSurface from GpuCommandBufferStub

We would like to reuse PassThroughImageTransportSurface in 
InprocessCommandBuffer to track when a swap completes on various 
platforms. This signal is used by ozone, in particular, to advance the
BufferQueue when the command buffer is out-of-process. GLRenderer is
moving to the gpu process and so, in the short term, we'd like to use
an InprocessCommandBuffer and InprocessContextProvider without losing
functionality.
BUG=
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
==========

[Fady Samuel, 2016-11-09 22:12:03]

Description was changed from

==========
Decouple PassThroughImageTransportSurface from GpuCommandBufferStub

We would like to reuse PassThroughImageTransportSurface in 
InprocessCommandBuffer to track when a swap completes on various 
platforms. This signal is used by ozone, in particular, to advance the
BufferQueue when the command buffer is out-of-process. GLRenderer is
moving to the gpu process and so, in the short term, we'd like to use
an InprocessCommandBuffer and InprocessContextProvider without losing
functionality.
BUG=
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

==========
Decouple PassThroughImageTransportSurface from GpuCommandBufferStub

We would like to reuse PassThroughImageTransportSurface in 
InprocessCommandBuffer to track when a swap completes on various 
platforms. This signal is used by ozone, in particular, to advance the
BufferQueue when the command buffer is out-of-process. GLRenderer is
moving to the gpu process and so, in the short term, we'd like to use
an InprocessCommandBuffer and InprocessContextProvider without losing
functionality.

BUG=
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
==========

[Fady Samuel, 2016-11-09 22:12:23]

Description was changed from

==========
Decouple PassThroughImageTransportSurface from GpuCommandBufferStub

We would like to reuse PassThroughImageTransportSurface in 
InprocessCommandBuffer to track when a swap completes on various 
platforms. This signal is used by ozone, in particular, to advance the
BufferQueue when the command buffer is out-of-process. GLRenderer is
moving to the gpu process and so, in the short term, we'd like to use
an InprocessCommandBuffer and InprocessContextProvider without losing
functionality.

BUG=
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

==========
Decouple PassThroughImageTransportSurface from GpuCommandBufferStub

We would like to reuse PassThroughImageTransportSurface in 
InprocessCommandBuffer to track when a swap completes on various 
platforms. This signal is used by ozone, in particular, to advance the
BufferQueue when the command buffer is out-of-process. GLRenderer is
moving to the gpu process and so, in the short term, we'd like to use
an InprocessCommandBuffer and InprocessContextProvider without losing
functionality.

BUG=661278
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
==========

[Fady Samuel, 2016-11-09 22:12:31]

fsamuel@chromium.org changed reviewers:
+ piman@chromium.org

[commit-bot: I haz the power, 2016-11-09 22:12:37]

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

[commit-bot: I haz the power, 2016-11-09 22:12:38]

Dry run: Try jobs failed on following builders:
  mac_optional_gpu_tests_rel on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_optional_gpu_...)

[Fady Samuel, 2016-11-09 22:15:51]

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

[commit-bot: I haz the power, 2016-11-09 22:16:35]

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

[piman, 2016-11-09 22:49:37]

lgtm + nits

https://codereview.chromium.org/2492583002/diff/80001/gpu/ipc/service/image_t...
File gpu/ipc/service/image_transport_surface_delegate.h (right):

https://codereview.chromium.org/2492583002/diff/80001/gpu/ipc/service/image_t...
gpu/ipc/service/image_transport_surface_delegate.h:26:
SwapBuffersCompleteParams(SwapBuffersCompleteParams&& other);
nit: rule-of-3, add a move-assign operator?

https://codereview.chromium.org/2492583002/diff/80001/gpu/ipc/service/image_t...
File gpu/ipc/service/image_transport_surface_overlay_mac.mm (right):

https://codereview.chromium.org/2492583002/diff/80001/gpu/ipc/service/image_t...
gpu/ipc/service/image_transport_surface_overlay_mac.mm:29: #include
"gpu/ipc/common/gpu_messages.h"
nit: is this still needed?

https://codereview.chromium.org/2492583002/diff/80001/gpu/ipc/service/pass_th...
File gpu/ipc/service/pass_through_image_transport_surface.cc (right):

https://codereview.chromium.org/2492583002/diff/80001/gpu/ipc/service/pass_th...
gpu/ipc/service/pass_through_image_transport_surface.cc:11: #include
"gpu/ipc/common/gpu_messages.h"
nit: ditto, still needed?

https://codereview.chromium.org/2492583002/diff/80001/gpu/ipc/service/pass_th...
gpu/ipc/service/pass_through_image_transport_surface.cc:29: if (!delegate_)
nit: I think you could DCHECK here, which means we always return true. And same
on the mac one. IOW, I think we might be able to remove a failure path.

https://codereview.chromium.org/2492583002/diff/80001/gpu/ipc/service/pass_th...
gpu/ipc/service/pass_through_image_transport_surface.cc:152:
base::Unretained(this)));
nit: Any reason for the extra function, as opposed to simply
base::Bind(&ImageTransportSurfaceDelegate::UpdateVSyncParameters, delegate_) ?

[commit-bot: I haz the power, 2016-11-09 23:26:13]

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

[commit-bot: I haz the power, 2016-11-09 23:26:14]

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

[Fady Samuel, 2016-11-09 23:28:11]

CQ'ing. Thanks Antoine!

https://codereview.chromium.org/2492583002/diff/80001/gpu/ipc/service/image_t...
File gpu/ipc/service/image_transport_surface_delegate.h (right):

https://codereview.chromium.org/2492583002/diff/80001/gpu/ipc/service/image_t...
gpu/ipc/service/image_transport_surface_delegate.h:26:
SwapBuffersCompleteParams(SwapBuffersCompleteParams&& other);
On 2016/11/09 22:49:36, piman wrote:
> nit: rule-of-3, add a move-assign operator?

Done.

https://codereview.chromium.org/2492583002/diff/80001/gpu/ipc/service/image_t...
File gpu/ipc/service/image_transport_surface_overlay_mac.mm (right):

https://codereview.chromium.org/2492583002/diff/80001/gpu/ipc/service/image_t...
gpu/ipc/service/image_transport_surface_overlay_mac.mm:29: #include
"gpu/ipc/common/gpu_messages.h"
On 2016/11/09 22:49:36, piman wrote:
> nit: is this still needed?

nope, removing.

https://codereview.chromium.org/2492583002/diff/80001/gpu/ipc/service/pass_th...
File gpu/ipc/service/pass_through_image_transport_surface.cc (right):

https://codereview.chromium.org/2492583002/diff/80001/gpu/ipc/service/pass_th...
gpu/ipc/service/pass_through_image_transport_surface.cc:11: #include
"gpu/ipc/common/gpu_messages.h"
On 2016/11/09 22:49:36, piman wrote:
> nit: ditto, still needed?

Nope removed.

https://codereview.chromium.org/2492583002/diff/80001/gpu/ipc/service/pass_th...
gpu/ipc/service/pass_through_image_transport_surface.cc:29: if (!delegate_)
On 2016/11/09 22:49:36, piman wrote:
> nit: I think you could DCHECK here, which means we always return true. And
same
> on the mac one. IOW, I think we might be able to remove a failure path.

done. I'll remove the bool in a separate CL.

https://codereview.chromium.org/2492583002/diff/80001/gpu/ipc/service/pass_th...
gpu/ipc/service/pass_through_image_transport_surface.cc:152:
base::Unretained(this)));
On 2016/11/09 22:49:36, piman wrote:
> nit: Any reason for the extra function, as opposed to simply
> base::Bind(&ImageTransportSurfaceDelegate::UpdateVSyncParameters, delegate_) ?

Done.

[Fady Samuel, 2016-11-09 23:35:40]

The CQ bit was checked by fsamuel@chromium.org

[Fady Samuel, 2016-11-09 23:35:40]

The patchset sent to the CQ was uploaded after l-g-t-m from piman@chromium.org
Link to the patchset: https://codereview.chromium.org/2492583002/#ps100001
(title: "Addressed Antoine's comments")

[commit-bot: I haz the power, 2016-11-09 23:36:32]

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, 2016-11-10 00:34:33]

Description was changed from

==========
Decouple PassThroughImageTransportSurface from GpuCommandBufferStub

We would like to reuse PassThroughImageTransportSurface in 
InprocessCommandBuffer to track when a swap completes on various 
platforms. This signal is used by ozone, in particular, to advance the
BufferQueue when the command buffer is out-of-process. GLRenderer is
moving to the gpu process and so, in the short term, we'd like to use
an InprocessCommandBuffer and InprocessContextProvider without losing
functionality.

BUG=661278
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

==========
Decouple PassThroughImageTransportSurface from GpuCommandBufferStub

We would like to reuse PassThroughImageTransportSurface in 
InprocessCommandBuffer to track when a swap completes on various 
platforms. This signal is used by ozone, in particular, to advance the
BufferQueue when the command buffer is out-of-process. GLRenderer is
moving to the gpu process and so, in the short term, we'd like to use
an InprocessCommandBuffer and InprocessContextProvider without losing
functionality.

BUG=661278
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
==========

[commit-bot: I haz the power, 2016-11-10 00:34:36]

Committed patchset #6 (id:100001)

[commit-bot: I haz the power, 2016-11-10 00:38:23]

Description was changed from

==========
Decouple PassThroughImageTransportSurface from GpuCommandBufferStub

We would like to reuse PassThroughImageTransportSurface in 
InprocessCommandBuffer to track when a swap completes on various 
platforms. This signal is used by ozone, in particular, to advance the
BufferQueue when the command buffer is out-of-process. GLRenderer is
moving to the gpu process and so, in the short term, we'd like to use
an InprocessCommandBuffer and InprocessContextProvider without losing
functionality.

BUG=661278
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

==========
Decouple PassThroughImageTransportSurface from GpuCommandBufferStub

We would like to reuse PassThroughImageTransportSurface in
InprocessCommandBuffer to track when a swap completes on various
platforms. This signal is used by ozone, in particular, to advance the
BufferQueue when the command buffer is out-of-process. GLRenderer is
moving to the gpu process and so, in the short term, we'd like to use
an InprocessCommandBuffer and InprocessContextProvider without losing
functionality.

BUG=661278
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

Committed: https://crrev.com/2b039432b32361aa1b772aa5fa878d43ac22eff6
Cr-Commit-Position: refs/heads/master@{#431100}
==========

[commit-bot: I haz the power, 2016-11-10 00:38:26]

Patchset 6 (id:??) landed as
https://crrev.com/2b039432b32361aa1b772aa5fa878d43ac22eff6
Cr-Commit-Position: refs/heads/master@{#431100}