cc: Introduce mechanism to observe CompositorFrames submitted to new surfaceIDs

cc: Introduce mechanism to observe CompositorFrames submitted to new surfaceIDs

In Mus+Ash, SurfaceManager and the display compositor will
live in the GPU. The gpu process needs to signal the
display compositor host for newly allocated
surface IDs so that it can identify the embedders and
update its book-keeping for lifetime management of
surfaces.

TBR=sadrul@chromium.org for content/browser/renderer_host
BUG=647852
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel

Committed: https://crrev.com/d119e3f1f4a9997ab6b29a3598131d2f14cd8538
Cr-Commit-Position: refs/heads/master@{#424651}

[Fady Samuel, 2016-10-07 20:05:37]

Description was changed from

==========
cc:: Introduce mechanism to observe CompositorFrames submitted to new surfaceIDs

In Mus+Ash, SurfaceManager and the display compositor will live in the GPU. The
gpu process needs to signal the display compositor host for newly allocated
surface IDs so that it can propagate the surface ID up to the parent.

The GPU process does not know about embedding hierarchy and so it cannot signal
parents directly. The gpu needs to go to the window server to find the parent
and propagate update up the surface ID. Mus+Ash specific code can track all
new surfaces via this newly renamed (was SurfaceDamageObserver) observer for
SurfaceManager.

BUG=647852
==========

to

==========
cc:: Introduce mechanism to observe CompositorFrames submitted to new surfaceIDs

In Mus+Ash, SurfaceManager and the display compositor will live in the GPU. The
gpu process needs to signal the display compositor host for newly allocated
surface IDs so that it can propagate the surface ID up to the parent.

The GPU process does not know about embedding hierarchy and so it cannot signal
parents directly. The gpu needs to go to the window server to find the parent
and propagate update up the surface ID. Mus+Ash specific code can track all
new surfaces via this newly renamed (was SurfaceDamageObserver) observer for
SurfaceManager.

BUG=647852
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel
==========

[Fady Samuel, 2016-10-07 20:05:54]

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

[commit-bot: I haz the power, 2016-10-07 20:06:12]

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-10-07 20:06:22]

fsamuel@chromium.org changed reviewers:
+ danakj@chromium.org, sadrul@chromium.org

[Fady Samuel, 2016-10-07 20:06:23]

+danakj@ for cc
+sadrul@ for content/browser/renderer_host.

[Fady Samuel, 2016-10-07 20:07:51]

Description was changed from

==========
cc:: Introduce mechanism to observe CompositorFrames submitted to new surfaceIDs

In Mus+Ash, SurfaceManager and the display compositor will live in the GPU. The
gpu process needs to signal the display compositor host for newly allocated
surface IDs so that it can propagate the surface ID up to the parent.

The GPU process does not know about embedding hierarchy and so it cannot signal
parents directly. The gpu needs to go to the window server to find the parent
and propagate update up the surface ID. Mus+Ash specific code can track all
new surfaces via this newly renamed (was SurfaceDamageObserver) observer for
SurfaceManager.

BUG=647852
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel
==========

to

==========
cc: Introduce mechanism to observe CompositorFrames submitted to new surfaceIDs

In Mus+Ash, SurfaceManager and the display compositor will live in the GPU. The
gpu process needs to signal the display compositor host for newly allocated
surface IDs so that it can propagate the surface ID up to the parent.

The GPU process does not know about embedding hierarchy and so it cannot signal
parents directly. The gpu needs to go to the window server to find the parent
and propagate update up the surface ID. Mus+Ash specific code can track all
new surfaces via this newly renamed (was SurfaceDamageObserver) observer for
SurfaceManager.

BUG=647852
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel
==========

[commit-bot: I haz the power, 2016-10-07 21:19:14]

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

[commit-bot: I haz the power, 2016-10-07 21:19:15]

Dry run: This issue passed the CQ dry run.

[Fady Samuel, 2016-10-11 20:10:33]

Description was changed from

==========
cc: Introduce mechanism to observe CompositorFrames submitted to new surfaceIDs

In Mus+Ash, SurfaceManager and the display compositor will live in the GPU. The
gpu process needs to signal the display compositor host for newly allocated
surface IDs so that it can propagate the surface ID up to the parent.

The GPU process does not know about embedding hierarchy and so it cannot signal
parents directly. The gpu needs to go to the window server to find the parent
and propagate update up the surface ID. Mus+Ash specific code can track all
new surfaces via this newly renamed (was SurfaceDamageObserver) observer for
SurfaceManager.

BUG=647852
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel
==========

to

==========
cc: Introduce mechanism to observe CompositorFrames submitted to new surfaceIDs

In Mus+Ash, SurfaceManager and the display compositor will live in the GPU. The
gpu process needs to signal the display compositor host for newly allocated
surface IDs so that it can identify the embedders and 
update its book-keeping for lifetime management of 
surfaces.

BUG=647852
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel
==========

[Fady Samuel, 2016-10-11 20:11:58]

Description was changed from

==========
cc: Introduce mechanism to observe CompositorFrames submitted to new surfaceIDs

In Mus+Ash, SurfaceManager and the display compositor will live in the GPU. The
gpu process needs to signal the display compositor host for newly allocated
surface IDs so that it can identify the embedders and 
update its book-keeping for lifetime management of 
surfaces.

BUG=647852
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel
==========

to

==========
cc: Introduce mechanism to observe CompositorFrames submitted to new surfaceIDs

In Mus+Ash, SurfaceManager and the display compositor will 
live in the GPU. The gpu process needs to signal the 
display compositor host for newly allocated
surface IDs so that it can identify the embedders and 
update its book-keeping for lifetime management of 
surfaces.

BUG=647852
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel
==========

[Fady Samuel, 2016-10-11 21:48:45]

fsamuel@chromium.org changed reviewers:
+ enne@chromium.org
- danakj@chromium.org

[Fady Samuel, 2016-10-11 21:48:46]

+enne, -danakj@ since Dana is OOO.

enne: this is necessary for surface ID bookkeeping in Mus and orthogonal to
resize. Thanks!

[vmpstr, 2016-10-11 21:59:03]

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

[vmpstr, 2016-10-11 21:59:04]

(drive-by)

https://codereview.chromium.org/2403573002/diff/1/cc/surfaces/surface_factory.cc
File cc/surfaces/surface_factory.cc (right):

https://codereview.chromium.org/2403573002/diff/1/cc/surfaces/surface_factory...
cc/surfaces/surface_factory.cc:79: const CompositorFrame& previous_frame =
it->second->GetEligibleFrame();
Can you add a comment explaining this logic, please?

https://codereview.chromium.org/2403573002/diff/1/cc/surfaces/surface_factory...
cc/surfaces/surface_factory.cc:84:
frame.delegated_frame_data->render_pass_list.size() > 0) {
nit: !frame.delegated_frame_data->render_pass_list.empty()

https://codereview.chromium.org/2403573002/diff/1/cc/surfaces/surface_factory...
File cc/surfaces/surface_factory_unittest.cc (right):

https://codereview.chromium.org/2403573002/diff/1/cc/surfaces/surface_factory...
cc/surfaces/surface_factory_unittest.cc:160: SurfaceId last_seen_surface_id_;
last_created_surface_id_ maybe? Or is the only way to "see" a surface is to
create it?

https://codereview.chromium.org/2403573002/diff/1/cc/surfaces/surface_manager...
File cc/surfaces/surface_manager_observer.h (right):

https://codereview.chromium.org/2403573002/diff/1/cc/surfaces/surface_manager...
cc/surfaces/surface_manager_observer.h:1: // Copyright 2014 The Chromium
Authors. All rights reserved.
nit: 2016 (although I know this is just renaming files...)

[Fady Samuel, 2016-10-11 23:11:45]

Thanks for the review, Vlad! PTAL!

https://codereview.chromium.org/2403573002/diff/1/cc/surfaces/surface_factory.cc
File cc/surfaces/surface_factory.cc (right):

https://codereview.chromium.org/2403573002/diff/1/cc/surfaces/surface_factory...
cc/surfaces/surface_factory.cc:79: const CompositorFrame& previous_frame =
it->second->GetEligibleFrame();
On 2016/10/11 21:59:04, vmpstr wrote:
> Can you add a comment explaining this logic, please?

Done.

https://codereview.chromium.org/2403573002/diff/1/cc/surfaces/surface_factory...
cc/surfaces/surface_factory.cc:84:
frame.delegated_frame_data->render_pass_list.size() > 0) {
On 2016/10/11 21:59:04, vmpstr wrote:
> nit: !frame.delegated_frame_data->render_pass_list.empty()

Done.

https://codereview.chromium.org/2403573002/diff/1/cc/surfaces/surface_factory...
File cc/surfaces/surface_factory_unittest.cc (right):

https://codereview.chromium.org/2403573002/diff/1/cc/surfaces/surface_factory...
cc/surfaces/surface_factory_unittest.cc:160: SurfaceId last_seen_surface_id_;
On 2016/10/11 21:59:04, vmpstr wrote:
> last_created_surface_id_ maybe? Or is the only way to "see" a surface is to
> create it?

renamed to last_created_surface_id.

https://codereview.chromium.org/2403573002/diff/1/cc/surfaces/surface_manager...
File cc/surfaces/surface_manager_observer.h (right):

https://codereview.chromium.org/2403573002/diff/1/cc/surfaces/surface_manager...
cc/surfaces/surface_manager_observer.h:1: // Copyright 2014 The Chromium
Authors. All rights reserved.
On 2016/10/11 21:59:04, vmpstr wrote:
> nit: 2016 (although I know this is just renaming files...)

Done.

[Fady Samuel, 2016-10-11 23:11:55]

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

[commit-bot: I haz the power, 2016-10-11 23:12:39]

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

[vmpstr, 2016-10-11 23:31:29]

(drive-by) lgtm

[Fady Samuel, 2016-10-11 23:35:16]

Thanks Vlad! I'm TBR'ing sadrul

[Fady Samuel, 2016-10-11 23:36:24]

Description was changed from

==========
cc: Introduce mechanism to observe CompositorFrames submitted to new surfaceIDs

In Mus+Ash, SurfaceManager and the display compositor will 
live in the GPU. The gpu process needs to signal the 
display compositor host for newly allocated
surface IDs so that it can identify the embedders and 
update its book-keeping for lifetime management of 
surfaces.

BUG=647852
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel
==========

to

==========
cc: Introduce mechanism to observe CompositorFrames submitted to new surfaceIDs

In Mus+Ash, SurfaceManager and the display compositor will 
live in the GPU. The gpu process needs to signal the 
display compositor host for newly allocated
surface IDs so that it can identify the embedders and 
update its book-keeping for lifetime management of 
surfaces.

TBR=sadrul@chromium.org for content/browser/renderer_host
BUG=647852
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel
==========

[sadrul, 2016-10-11 23:39:04]

stamp lgtm

[enne (OOO), 2016-10-12 00:07:20]

https://codereview.chromium.org/2403573002/diff/1/cc/surfaces/surface_manager...
File cc/surfaces/surface_manager_observer.h (right):

https://codereview.chromium.org/2403573002/diff/1/cc/surfaces/surface_manager...
cc/surfaces/surface_manager_observer.h:16: class SurfaceManagerObserver {
bikeshed: what about just SurfaceObserver? Is that too ambiguous?

[commit-bot: I haz the power, 2016-10-12 00:30:40]

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

[commit-bot: I haz the power, 2016-10-12 00:30:41]

Dry run: This issue passed the CQ dry run.

[Fady Samuel, 2016-10-12 00:34:28]

Addressed nit! Thanks Enne! CQ'ing.

https://codereview.chromium.org/2403573002/diff/1/cc/surfaces/surface_manager...
File cc/surfaces/surface_manager_observer.h (right):

https://codereview.chromium.org/2403573002/diff/1/cc/surfaces/surface_manager...
cc/surfaces/surface_manager_observer.h:16: class SurfaceManagerObserver {
On 2016/10/12 00:07:20, enne wrote:
> bikeshed: what about just SurfaceObserver? Is that too ambiguous?

Done.

[Fady Samuel, 2016-10-12 00:34:32]

The CQ bit was checked by fsamuel@chromium.org

[Fady Samuel, 2016-10-12 00:34:33]

The patchset sent to the CQ was uploaded after l-g-t-m from sadrul@chromium.org,
vmpstr@chromium.org
Link to the patchset: https://codereview.chromium.org/2403573002/#ps40001
(title: "SurfaceManagerObserver => SurfaceObserver")

[commit-bot: I haz the power, 2016-10-12 00:35:06]

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-10-12 00:49:20]

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

[commit-bot: I haz the power, 2016-10-12 00:49:22]

Try jobs failed on following builders:
  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...)

[Fady Samuel, 2016-10-12 01:18:42]

The CQ bit was checked by fsamuel@chromium.org

[Fady Samuel, 2016-10-12 01:18:43]

The patchset sent to the CQ was uploaded after l-g-t-m from sadrul@chromium.org,
vmpstr@chromium.org
Link to the patchset: https://codereview.chromium.org/2403573002/#ps60001
(title: "Updated changes missed")

[commit-bot: I haz the power, 2016-10-12 01:19:15]

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-10-12 02:23:49]

Description was changed from

==========
cc: Introduce mechanism to observe CompositorFrames submitted to new surfaceIDs

In Mus+Ash, SurfaceManager and the display compositor will 
live in the GPU. The gpu process needs to signal the 
display compositor host for newly allocated
surface IDs so that it can identify the embedders and 
update its book-keeping for lifetime management of 
surfaces.

TBR=sadrul@chromium.org for content/browser/renderer_host
BUG=647852
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel
==========

to

==========
cc: Introduce mechanism to observe CompositorFrames submitted to new surfaceIDs

In Mus+Ash, SurfaceManager and the display compositor will 
live in the GPU. The gpu process needs to signal the 
display compositor host for newly allocated
surface IDs so that it can identify the embedders and 
update its book-keeping for lifetime management of 
surfaces.

TBR=sadrul@chromium.org for content/browser/renderer_host
BUG=647852
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel
==========

[commit-bot: I haz the power, 2016-10-12 02:23:50]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-10-12 02:25:51]

Description was changed from

==========
cc: Introduce mechanism to observe CompositorFrames submitted to new surfaceIDs

In Mus+Ash, SurfaceManager and the display compositor will 
live in the GPU. The gpu process needs to signal the 
display compositor host for newly allocated
surface IDs so that it can identify the embedders and 
update its book-keeping for lifetime management of 
surfaces.

TBR=sadrul@chromium.org for content/browser/renderer_host
BUG=647852
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel
==========

to

==========
cc: Introduce mechanism to observe CompositorFrames submitted to new surfaceIDs

In Mus+Ash, SurfaceManager and the display compositor will
live in the GPU. The gpu process needs to signal the
display compositor host for newly allocated
surface IDs so that it can identify the embedders and
update its book-keeping for lifetime management of
surfaces.

TBR=sadrul@chromium.org for content/browser/renderer_host
BUG=647852
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel

Committed: https://crrev.com/d119e3f1f4a9997ab6b29a3598131d2f14cd8538
Cr-Commit-Position: refs/heads/master@{#424651}
==========

[commit-bot: I haz the power, 2016-10-12 02:25:52]

Patchset 4 (id:??) landed as
https://crrev.com/d119e3f1f4a9997ab6b29a3598131d2f14cd8538
Cr-Commit-Position: refs/heads/master@{#424651}