Only emit a fallback SurfaceDrawQuad if differs from primary

Only emit a fallback SurfaceDrawQuad if differs from primary

If both the primary and fallback SurfaceInfos in a SurfaceLayerImpl 
point to the same surface ID, then emitting a fallback SurfaceDrawQuad 
unnecessarily bloats a CompositorFrame with a fallback that will always 
be skipped.

This CL also adds two unit tests: one that verifies that we emit two
SurfaceDrawQuads if the primary and fallback differ and one that 
verifies we only emit one SurfaceDrawQuad if they are the same.

BUG=672962
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel

Review-Url: https://codereview.chromium.org/2696303002
Cr-Commit-Position: refs/heads/master@{#450912}
Committed: https://chromium.googlesource.com/chromium/src/+/7f65d49d1f27e4ea128329b61f4b7c809fcca31f

[Fady Samuel, 2017-02-15 23:57:01]

Description was changed from

==========
Only emit a fallback SurfaceDrawQuad if differs from primary

If both the primary and fallback SurfaceInfos in a SurfaceLayerImpl point
to the same surface ID, then emitting a fallback SurfaceDrawQuad unnecesarily
bloats a CompositorFrame with a fallback that will always be skipped.

This CL also adds two unit tests: one that verifies that we emit two
SurfaceDrawQuads if the primary and fallback differ and one that verifies
we only emit one SurfaceDrawQuad if they are the same.

BUG=672962
==========

to

==========
Only emit a fallback SurfaceDrawQuad if differs from primary

If both the primary and fallback SurfaceInfos in a SurfaceLayerImpl point
to the same surface ID, then emitting a fallback SurfaceDrawQuad unnecesarily
bloats a CompositorFrame with a fallback that will always be skipped.

This CL also adds two unit tests: one that verifies that we emit two
SurfaceDrawQuads if the primary and fallback differ and one that verifies
we only emit one SurfaceDrawQuad if they are the same.

BUG=672962
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel
==========

[Fady Samuel, 2017-02-15 23:59:56]

Description was changed from

==========
Only emit a fallback SurfaceDrawQuad if differs from primary

If both the primary and fallback SurfaceInfos in a SurfaceLayerImpl point
to the same surface ID, then emitting a fallback SurfaceDrawQuad unnecesarily
bloats a CompositorFrame with a fallback that will always be skipped.

This CL also adds two unit tests: one that verifies that we emit two
SurfaceDrawQuads if the primary and fallback differ and one that verifies
we only emit one SurfaceDrawQuad if they are the same.

BUG=672962
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel
==========

to

==========
Only emit a fallback SurfaceDrawQuad if differs from primary

If both the primary and fallback SurfaceInfos in a SurfaceLayerImpl 
point to the same surface ID, then emitting a fallback SurfaceDrawQuad 
unnecessarily bloats a CompositorFrame with a fallback that will always 
be skipped.

This CL also adds two unit tests: one that verifies that we emit two
SurfaceDrawQuads if the primary and fallback differ and one that 
verifies we only emit one SurfaceDrawQuad if they are the same.

BUG=672962
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel
==========

[Fady Samuel, 2017-02-16 00:00:10]

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

[Fady Samuel, 2017-02-16 00:00:11]

PTAL Vlad!

[Fady Samuel, 2017-02-16 00:24:14]

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

[commit-bot: I haz the power, 2017-02-16 00:25:10]

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

[vmpstr, 2017-02-16 00:52:12]

lgtm

https://codereview.chromium.org/2696303002/diff/1/cc/layers/surface_layer_imp...
File cc/layers/surface_layer_impl.cc (right):

https://codereview.chromium.org/2696303002/diff/1/cc/layers/surface_layer_imp...
cc/layers/surface_layer_impl.cc:71: if (primary && fallback_surface_info_.id()
!= primary_surface_info_.id()) {
I'm guessing this comparison takes into account that fallback_surface_info_.id()
may not be valid

[Fady Samuel, 2017-02-16 01:16:10]

Thanks Vlad! CQ'ing!

https://codereview.chromium.org/2696303002/diff/1/cc/layers/surface_layer_imp...
File cc/layers/surface_layer_impl.cc (right):

https://codereview.chromium.org/2696303002/diff/1/cc/layers/surface_layer_imp...
cc/layers/surface_layer_impl.cc:71: if (primary && fallback_surface_info_.id()
!= primary_surface_info_.id()) {
On 2017/02/16 00:52:12, vmpstr wrote:
> I'm guessing this comparison takes into account that
fallback_surface_info_.id()
> may not be valid

If the fallback SurfaceId is invalid  then CreateSurfaceDrawQuad will exit
early.

[Fady Samuel, 2017-02-16 01:16:15]

The CQ bit was unchecked by fsamuel@chromium.org

[Fady Samuel, 2017-02-16 02:42:54]

The CQ bit was checked by fsamuel@chromium.org

[commit-bot: I haz the power, 2017-02-16 02:44:04]

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-16 05:14:14]

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

[commit-bot: I haz the power, 2017-02-16 05:14:16]

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

[Fady Samuel, 2017-02-16 08:14:21]

The CQ bit was checked by fsamuel@chromium.org

[commit-bot: I haz the power, 2017-02-16 08:15:20]

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-16 09:16:15]

CQ is committing da patch.

Bot data: {"patchset_id": 1, "attempt_start_ts": 1487232861418690, "parent_rev":
"0e6baaf2652e33f99bb396851b2036fff99b7834", "commit_rev":
"7f65d49d1f27e4ea128329b61f4b7c809fcca31f"}

[commit-bot: I haz the power, 2017-02-16 09:17:02]

Description was changed from

==========
Only emit a fallback SurfaceDrawQuad if differs from primary

If both the primary and fallback SurfaceInfos in a SurfaceLayerImpl 
point to the same surface ID, then emitting a fallback SurfaceDrawQuad 
unnecessarily bloats a CompositorFrame with a fallback that will always 
be skipped.

This CL also adds two unit tests: one that verifies that we emit two
SurfaceDrawQuads if the primary and fallback differ and one that 
verifies we only emit one SurfaceDrawQuad if they are the same.

BUG=672962
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel
==========

to

==========
Only emit a fallback SurfaceDrawQuad if differs from primary

If both the primary and fallback SurfaceInfos in a SurfaceLayerImpl 
point to the same surface ID, then emitting a fallback SurfaceDrawQuad 
unnecessarily bloats a CompositorFrame with a fallback that will always 
be skipped.

This CL also adds two unit tests: one that verifies that we emit two
SurfaceDrawQuads if the primary and fallback differ and one that 
verifies we only emit one SurfaceDrawQuad if they are the same.

BUG=672962
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel

Review-Url: https://codereview.chromium.org/2696303002
Cr-Commit-Position: refs/heads/master@{#450912}
Committed:
https://chromium.googlesource.com/chromium/src/+/7f65d49d1f27e4ea128329b61f4b...
==========

[commit-bot: I haz the power, 2017-02-16 09:17:04]

Committed patchset #1 (id:1) as
https://chromium.googlesource.com/chromium/src/+/7f65d49d1f27e4ea128329b61f4b...