Make Mac swap code like other platforms

Make Mac swap code like other platforms

Swap buffer completion is sent from the GPU process to the browser
process via the IPC GpuCommandBufferMsg_SwapBuffersCompleted on all
platforms except for Mac, which uses the IPC
GpuHostMsg_AcceleratedSurfaceBuffersSwapped.

Mac needs (for now) to send a handful of additional parameters from the
gpu process at every swap. These parameters are used to display in the
browser process the CALayer tree that is created in the gpu process.
Add these parameters to GpuHostMsg_AcceleratedSurfaceBuffersSwapped
under a platform ifdef. These parameters will be able to be removed when
layer tree construction is moved to be in the browser process.

Pass these parameters along to GpuBrowserCompositorOutputSurface::
SwapBuffers, where they are consumed in Mac-specific code. This wart,
the gpu::GpuProcessHostedCALayerTreeParamsMac structure being passed
along, will be removed when the CALayer tree is constructed in the
browser process.

Swaps are acknowledged by the browser process to the gpu process in the
AcceleratedSurfaceMsg_BufferPresented IPC. The purpose of this IPC was
to send vsync data to the gpu process to coordinate swaps, but this
data is no longer used by the gpu process, is entirely dead code, and
can be removed.

Remove other supporting code that is no longer needed. Not all supporting
code is removed in this patch.

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

Committed: https://crrev.com/433204258d41867ad95c2af17c1248dfc64032ac
Cr-Commit-Position: refs/heads/master@{#392419}

[ccameron, 2016-05-08 05:31:50]

Description was changed from

==========
Make Mac less special

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

to

==========
Make Mac less special

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

[ccameron, 2016-05-08 19:17:21]

Description was changed from

==========
Make Mac less special

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

to

==========
Make Mac swap code like other platforms

Swap buffer completion is sent from the GPU process to the browser
process via the IPC GpuCommandBufferMsg_SwapBuffersCompleted on all
platforms except for Mac, which uses the IPC
GpuHostMsg_AcceleratedSurfaceBuffersSwapped.

Mac needs (for now) to send a handful of additional parameters from the
gpu process at every swap. These parameters are used to display in the
browser process the CALayer tree that is created in the gpu process.
Add these parameters to GpuHostMsg_AcceleratedSurfaceBuffersSwapped
under a platform ifdef. These parameters will be able to be removed when
layer tree construction is moved to be in the browser process.

Pass these parameters along to GpuBrowserCompositorOutputSurface::
SwapBuffers, where they are consumed in Mac-specific code. This wart,
the gpu::GpuProcessHostedCALayerTreeParamsMac structure being passed
along, will be removed when the CALayer tree is constructed in the
browser process.

Swaps are acknowledged by the browser process to the gpu process in the
AcceleratedSurfaceMsg_BufferPresented IPC. The purpose of this IPC was
to send vsync data to the gpu process to coordinate swaps, but this
data is no longer used by the gpu process, is entirely dead code, and
can be removed. 

Remove other supporting code that is no longer needed. Not all supporting
code is removed in this patch.
==========

[ccameron, 2016-05-08 19:17:21]

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

[ccameron, 2016-05-08 19:22:17]

This has been overdue for a while. Mac's special IPCs no longer made sense when
we switched to surfaceless output in M46.

https://codereview.chromium.org/1962493002/diff/60001/content/browser/composi...
File content/browser/compositor/gpu_browser_compositor_output_surface.cc
(right):

https://codereview.chromium.org/1962493002/diff/60001/content/browser/composi...
content/browser/compositor/gpu_browser_compositor_output_surface.cc:129: #if
defined(OS_MACOSX)
I plumbed the gpu::GpuProcessHostedCALayerTreeParamsMac all the way here instead
of just making the ui::AcceleratedWidgetMacGotFrame calls in
CommandBufferProxyImpl::OnSwapBuffersCompleted because this location seems more
natural (in particular, the concept of SurfaceIsSuspendForRecycle, etc, is now
local instead of having to be looked up, so we can delete all of that
complicated code soon).

I do acknowledge that that plumbing is sort of ugly (it can all be killed soon,
though).

[piman, 2016-05-09 16:23:59]

lgtm

https://codereview.chromium.org/1962493002/diff/60001/content/browser/composi...
File content/browser/compositor/gpu_browser_compositor_output_surface.cc
(right):

https://codereview.chromium.org/1962493002/diff/60001/content/browser/composi...
content/browser/compositor/gpu_browser_compositor_output_surface.cc:129: #if
defined(OS_MACOSX)
On 2016/05/08 19:22:17, ccameron wrote:
> I plumbed the gpu::GpuProcessHostedCALayerTreeParamsMac all the way here
instead
> of just making the ui::AcceleratedWidgetMacGotFrame calls in
> CommandBufferProxyImpl::OnSwapBuffersCompleted because this location seems
more
> natural (in particular, the concept of SurfaceIsSuspendForRecycle, etc, is now
> local instead of having to be looked up, so we can delete all of that
> complicated code soon).

This is the right place, I think. CommandBufferProxyImpl doesn't/shouldn't know
about the GpuSurfaceTracker and so on.

> I do acknowledge that that plumbing is sort of ugly (it can all be killed
soon,
> though).

Well, this is an improvement over the previous code, so we can live with it, but
I'm looking forward to further improvements :)

https://codereview.chromium.org/1962493002/diff/60001/content/browser/composi...
content/browser/compositor/gpu_browser_compositor_output_surface.cc:172:
SurfaceShouldNotShowFramesAfterSuspendForRecycle() const {
Am I right to assume that this (and maybe SetSurfaceSuspendedForRecycle  and
associated state) can be removed as well (as a follow up maybe)?

https://codereview.chromium.org/1962493002/diff/60001/content/browser/rendere...
File content/browser/renderer_host/compositor_impl_android.h (right):

https://codereview.chromium.org/1962493002/diff/60001/content/browser/rendere...
content/browser/renderer_host/compositor_impl_android.h:43: }
nit: doesn't look like it's used here.

https://codereview.chromium.org/1962493002/diff/60001/gpu/ipc/client/gpu_proc...
File gpu/ipc/client/gpu_process_hosted_ca_layer_tree_params.h (right):

https://codereview.chromium.org/1962493002/diff/60001/gpu/ipc/client/gpu_proc...
gpu/ipc/client/gpu_process_hosted_ca_layer_tree_params.h:22:
base::ScopedCFTypeRef<IOSurfaceRef> io_surface;
FYI, just a thought, don't worry about it since it sounds all this is somewhat
temporary, but you could define a
IPC::ParamTraits<base::ScopedCFTypeRef<IOSurfaceRef>> which does the mac port
creation/lookup, and then it would be trivial to pass the
GpuProcessHostedCALayerTreeParamsMac directly in the
GpuCommandBufferMsg_SwapBuffersCompleted_Params.

[ccameron, 2016-05-09 18:38:48]

Description was changed from

==========
Make Mac swap code like other platforms

Swap buffer completion is sent from the GPU process to the browser
process via the IPC GpuCommandBufferMsg_SwapBuffersCompleted on all
platforms except for Mac, which uses the IPC
GpuHostMsg_AcceleratedSurfaceBuffersSwapped.

Mac needs (for now) to send a handful of additional parameters from the
gpu process at every swap. These parameters are used to display in the
browser process the CALayer tree that is created in the gpu process.
Add these parameters to GpuHostMsg_AcceleratedSurfaceBuffersSwapped
under a platform ifdef. These parameters will be able to be removed when
layer tree construction is moved to be in the browser process.

Pass these parameters along to GpuBrowserCompositorOutputSurface::
SwapBuffers, where they are consumed in Mac-specific code. This wart,
the gpu::GpuProcessHostedCALayerTreeParamsMac structure being passed
along, will be removed when the CALayer tree is constructed in the
browser process.

Swaps are acknowledged by the browser process to the gpu process in the
AcceleratedSurfaceMsg_BufferPresented IPC. The purpose of this IPC was
to send vsync data to the gpu process to coordinate swaps, but this
data is no longer used by the gpu process, is entirely dead code, and
can be removed. 

Remove other supporting code that is no longer needed. Not all supporting
code is removed in this patch.
==========

to

==========
Make Mac swap code like other platforms

Swap buffer completion is sent from the GPU process to the browser
process via the IPC GpuCommandBufferMsg_SwapBuffersCompleted on all
platforms except for Mac, which uses the IPC
GpuHostMsg_AcceleratedSurfaceBuffersSwapped.

Mac needs (for now) to send a handful of additional parameters from the
gpu process at every swap. These parameters are used to display in the
browser process the CALayer tree that is created in the gpu process.
Add these parameters to GpuHostMsg_AcceleratedSurfaceBuffersSwapped
under a platform ifdef. These parameters will be able to be removed when
layer tree construction is moved to be in the browser process.

Pass these parameters along to GpuBrowserCompositorOutputSurface::
SwapBuffers, where they are consumed in Mac-specific code. This wart,
the gpu::GpuProcessHostedCALayerTreeParamsMac structure being passed
along, will be removed when the CALayer tree is constructed in the
browser process.

Swaps are acknowledged by the browser process to the gpu process in the
AcceleratedSurfaceMsg_BufferPresented IPC. The purpose of this IPC was
to send vsync data to the gpu process to coordinate swaps, but this
data is no longer used by the gpu process, is entirely dead code, and
can be removed. 

Remove other supporting code that is no longer needed. Not all supporting
code is removed in this patch.
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel;tryserver.chromium.win:win_optional_gpu_tests_rel
==========

[ccameron, 2016-05-09 18:40:36]

ccameron@chromium.org changed reviewers:
+ fsamuel@chromium.org, wfh@chromium.org

[ccameron, 2016-05-09 18:40:37]

Thanks

Adding fsamuel@ for components/mus/gpu stamp.

Adding wfh for _messages security. This patch removes 2 IPCs and rolls the
arguments from one of them into another.

https://codereview.chromium.org/1962493002/diff/60001/content/browser/composi...
File content/browser/compositor/gpu_browser_compositor_output_surface.cc
(right):

https://codereview.chromium.org/1962493002/diff/60001/content/browser/composi...
content/browser/compositor/gpu_browser_compositor_output_surface.cc:172:
SurfaceShouldNotShowFramesAfterSuspendForRecycle() const {
On 2016/05/09 16:23:58, piman wrote:
> Am I right to assume that this (and maybe SetSurfaceSuspendedForRecycle  and
> associated state) can be removed as well (as a follow up maybe)?

Once we're doing the CALayer tree construction in the browser process, we'll be
able to get rid of this (it exists to work around synchronization issues across
the processes).

https://codereview.chromium.org/1962493002/diff/60001/content/browser/rendere...
File content/browser/renderer_host/compositor_impl_android.h (right):

https://codereview.chromium.org/1962493002/diff/60001/content/browser/rendere...
content/browser/renderer_host/compositor_impl_android.h:43: }
On 2016/05/09 16:23:59, piman wrote:
> nit: doesn't look like it's used here.

You're right -- it can go down into the .cc file.

https://codereview.chromium.org/1962493002/diff/60001/gpu/ipc/client/gpu_proc...
File gpu/ipc/client/gpu_process_hosted_ca_layer_tree_params.h (right):

https://codereview.chromium.org/1962493002/diff/60001/gpu/ipc/client/gpu_proc...
gpu/ipc/client/gpu_process_hosted_ca_layer_tree_params.h:22:
base::ScopedCFTypeRef<IOSurfaceRef> io_surface;
On 2016/05/09 16:23:59, piman wrote:
> FYI, just a thought, don't worry about it since it sounds all this is somewhat
> temporary, but you could define a
> IPC::ParamTraits<base::ScopedCFTypeRef<IOSurfaceRef>> which does the mac port
> creation/lookup, and then it would be trivial to pass the
> GpuProcessHostedCALayerTreeParamsMac directly in the
> GpuCommandBufferMsg_SwapBuffersCompleted_Params.

That's an interesting idea -- I'll keep that in mind.

[Will Harris, 2016-05-09 18:42:02]

bug id, for context?

[Fady Samuel, 2016-05-09 18:43:05]

mus lgtm

[ccameron, 2016-05-09 19:39:18]

Description was changed from

==========
Make Mac swap code like other platforms

Swap buffer completion is sent from the GPU process to the browser
process via the IPC GpuCommandBufferMsg_SwapBuffersCompleted on all
platforms except for Mac, which uses the IPC
GpuHostMsg_AcceleratedSurfaceBuffersSwapped.

Mac needs (for now) to send a handful of additional parameters from the
gpu process at every swap. These parameters are used to display in the
browser process the CALayer tree that is created in the gpu process.
Add these parameters to GpuHostMsg_AcceleratedSurfaceBuffersSwapped
under a platform ifdef. These parameters will be able to be removed when
layer tree construction is moved to be in the browser process.

Pass these parameters along to GpuBrowserCompositorOutputSurface::
SwapBuffers, where they are consumed in Mac-specific code. This wart,
the gpu::GpuProcessHostedCALayerTreeParamsMac structure being passed
along, will be removed when the CALayer tree is constructed in the
browser process.

Swaps are acknowledged by the browser process to the gpu process in the
AcceleratedSurfaceMsg_BufferPresented IPC. The purpose of this IPC was
to send vsync data to the gpu process to coordinate swaps, but this
data is no longer used by the gpu process, is entirely dead code, and
can be removed. 

Remove other supporting code that is no longer needed. Not all supporting
code is removed in this patch.
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel;tryserver.chromium.win:win_optional_gpu_tests_rel
==========

to

==========
Make Mac swap code like other platforms

Swap buffer completion is sent from the GPU process to the browser
process via the IPC GpuCommandBufferMsg_SwapBuffersCompleted on all
platforms except for Mac, which uses the IPC
GpuHostMsg_AcceleratedSurfaceBuffersSwapped.

Mac needs (for now) to send a handful of additional parameters from the
gpu process at every swap. These parameters are used to display in the
browser process the CALayer tree that is created in the gpu process.
Add these parameters to GpuHostMsg_AcceleratedSurfaceBuffersSwapped
under a platform ifdef. These parameters will be able to be removed when
layer tree construction is moved to be in the browser process.

Pass these parameters along to GpuBrowserCompositorOutputSurface::
SwapBuffers, where they are consumed in Mac-specific code. This wart,
the gpu::GpuProcessHostedCALayerTreeParamsMac structure being passed
along, will be removed when the CALayer tree is constructed in the
browser process.

Swaps are acknowledged by the browser process to the gpu process in the
AcceleratedSurfaceMsg_BufferPresented IPC. The purpose of this IPC was
to send vsync data to the gpu process to coordinate swaps, but this
data is no longer used by the gpu process, is entirely dead code, and
can be removed. 

Remove other supporting code that is no longer needed. Not all supporting
code is removed in this patch.

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

[ccameron, 2016-05-09 19:41:08]

On 2016/05/09 18:42:02, Will Harris wrote:
> bug id, for context?

I put in crbug.com/604052, which is where this came up. This is just general
cleanup, though.

[Will Harris, 2016-05-09 19:46:31]

*_messages.h lgtm

[ccameron, 2016-05-09 19:49:15]

The CQ bit was checked by ccameron@chromium.org

[ccameron, 2016-05-09 19:49:15]

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/1962493002/#ps80001
(title: "Incorporate review feedback")

[commit-bot: I haz the power, 2016-05-09 19:49:43]

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

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

Description was changed from

==========
Make Mac swap code like other platforms

Swap buffer completion is sent from the GPU process to the browser
process via the IPC GpuCommandBufferMsg_SwapBuffersCompleted on all
platforms except for Mac, which uses the IPC
GpuHostMsg_AcceleratedSurfaceBuffersSwapped.

Mac needs (for now) to send a handful of additional parameters from the
gpu process at every swap. These parameters are used to display in the
browser process the CALayer tree that is created in the gpu process.
Add these parameters to GpuHostMsg_AcceleratedSurfaceBuffersSwapped
under a platform ifdef. These parameters will be able to be removed when
layer tree construction is moved to be in the browser process.

Pass these parameters along to GpuBrowserCompositorOutputSurface::
SwapBuffers, where they are consumed in Mac-specific code. This wart,
the gpu::GpuProcessHostedCALayerTreeParamsMac structure being passed
along, will be removed when the CALayer tree is constructed in the
browser process.

Swaps are acknowledged by the browser process to the gpu process in the
AcceleratedSurfaceMsg_BufferPresented IPC. The purpose of this IPC was
to send vsync data to the gpu process to coordinate swaps, but this
data is no longer used by the gpu process, is entirely dead code, and
can be removed. 

Remove other supporting code that is no longer needed. Not all supporting
code is removed in this patch.

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

to

==========
Make Mac swap code like other platforms

Swap buffer completion is sent from the GPU process to the browser
process via the IPC GpuCommandBufferMsg_SwapBuffersCompleted on all
platforms except for Mac, which uses the IPC
GpuHostMsg_AcceleratedSurfaceBuffersSwapped.

Mac needs (for now) to send a handful of additional parameters from the
gpu process at every swap. These parameters are used to display in the
browser process the CALayer tree that is created in the gpu process.
Add these parameters to GpuHostMsg_AcceleratedSurfaceBuffersSwapped
under a platform ifdef. These parameters will be able to be removed when
layer tree construction is moved to be in the browser process.

Pass these parameters along to GpuBrowserCompositorOutputSurface::
SwapBuffers, where they are consumed in Mac-specific code. This wart,
the gpu::GpuProcessHostedCALayerTreeParamsMac structure being passed
along, will be removed when the CALayer tree is constructed in the
browser process.

Swaps are acknowledged by the browser process to the gpu process in the
AcceleratedSurfaceMsg_BufferPresented IPC. The purpose of this IPC was
to send vsync data to the gpu process to coordinate swaps, but this
data is no longer used by the gpu process, is entirely dead code, and
can be removed. 

Remove other supporting code that is no longer needed. Not all supporting
code is removed in this patch.

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

[commit-bot: I haz the power, 2016-05-09 20:58:12]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2016-05-09 20:59:49]

Description was changed from

==========
Make Mac swap code like other platforms

Swap buffer completion is sent from the GPU process to the browser
process via the IPC GpuCommandBufferMsg_SwapBuffersCompleted on all
platforms except for Mac, which uses the IPC
GpuHostMsg_AcceleratedSurfaceBuffersSwapped.

Mac needs (for now) to send a handful of additional parameters from the
gpu process at every swap. These parameters are used to display in the
browser process the CALayer tree that is created in the gpu process.
Add these parameters to GpuHostMsg_AcceleratedSurfaceBuffersSwapped
under a platform ifdef. These parameters will be able to be removed when
layer tree construction is moved to be in the browser process.

Pass these parameters along to GpuBrowserCompositorOutputSurface::
SwapBuffers, where they are consumed in Mac-specific code. This wart,
the gpu::GpuProcessHostedCALayerTreeParamsMac structure being passed
along, will be removed when the CALayer tree is constructed in the
browser process.

Swaps are acknowledged by the browser process to the gpu process in the
AcceleratedSurfaceMsg_BufferPresented IPC. The purpose of this IPC was
to send vsync data to the gpu process to coordinate swaps, but this
data is no longer used by the gpu process, is entirely dead code, and
can be removed. 

Remove other supporting code that is no longer needed. Not all supporting
code is removed in this patch.

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

to

==========
Make Mac swap code like other platforms

Swap buffer completion is sent from the GPU process to the browser
process via the IPC GpuCommandBufferMsg_SwapBuffersCompleted on all
platforms except for Mac, which uses the IPC
GpuHostMsg_AcceleratedSurfaceBuffersSwapped.

Mac needs (for now) to send a handful of additional parameters from the
gpu process at every swap. These parameters are used to display in the
browser process the CALayer tree that is created in the gpu process.
Add these parameters to GpuHostMsg_AcceleratedSurfaceBuffersSwapped
under a platform ifdef. These parameters will be able to be removed when
layer tree construction is moved to be in the browser process.

Pass these parameters along to GpuBrowserCompositorOutputSurface::
SwapBuffers, where they are consumed in Mac-specific code. This wart,
the gpu::GpuProcessHostedCALayerTreeParamsMac structure being passed
along, will be removed when the CALayer tree is constructed in the
browser process.

Swaps are acknowledged by the browser process to the gpu process in the
AcceleratedSurfaceMsg_BufferPresented IPC. The purpose of this IPC was
to send vsync data to the gpu process to coordinate swaps, but this
data is no longer used by the gpu process, is entirely dead code, and
can be removed.

Remove other supporting code that is no longer needed. Not all supporting
code is removed in this patch.

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

Committed: https://crrev.com/433204258d41867ad95c2af17c1248dfc64032ac
Cr-Commit-Position: refs/heads/master@{#392419}
==========

[commit-bot: I haz the power, 2016-05-09 20:59:50]

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