Adding SatisfySequence and RequireSequence to SurfaceManager

Adding SatisfySequence and RequireSequence to SurfaceManager

Now that satisfies_sequences in CompositorFrameMetadata is gone, there 
is absolutely no reason for SurfaceManager::DidSatisfySequences to take
a vector of sequences as opposed to one.
This CL replaces DidSatisfySequence with SatisfySequence and also creates
SurfaceManager::RequireSequence which helps remove duplicate code
throughout the code base.

TBR=piman@chromium.org
BUG=675054
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel

Review-Url: https://codereview.chromium.org/2614423003
Cr-Commit-Position: refs/heads/master@{#442461}
Committed: https://chromium.googlesource.com/chromium/src/+/1221800150ab0969c63166809616392db5a02729

[Saman Sami, 2017-01-09 16:40:49]

Description was changed from

==========
Adding SatisfySequence and RequireSequence to SurfaceManager

BUG=675054
==========

to

==========
Adding SatisfySequence and RequireSequence to SurfaceManager

BUG=675054
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
==========

[Saman Sami, 2017-01-09 16:40:49]

Description was changed from

==========
Adding SatisfySequence and RequireSequence to SurfaceManager

BUG=675054
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
==========

to

==========
Adding SatisfySequence and RequireSequence to SurfaceManager

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

[Saman Sami, 2017-01-09 16:41:05]

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

[commit-bot: I haz the power, 2017-01-09 16:41:27]

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

[Saman Sami, 2017-01-09 16:45:35]

Description was changed from

==========
Adding SatisfySequence and RequireSequence to SurfaceManager

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

to

==========
Adding SatisfySequence and RequireSequence to SurfaceManager

Now that satisfies_sequences in CompositorFrameMetadata is gone, there 
is absolutely no reason for SurfaceManager::DidSatisfySequences to take a vector
of sequences as opposed to one.
This CL replaces DidSatisfySequence with SatisfySequence and also creates
SurfaceManager::RequireSequence which helps remove duplicate code
throughout the code base.

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

[Saman Sami, 2017-01-09 16:45:35]

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

[Saman Sami, 2017-01-09 16:45:47]

Description was changed from

==========
Adding SatisfySequence and RequireSequence to SurfaceManager

Now that satisfies_sequences in CompositorFrameMetadata is gone, there 
is absolutely no reason for SurfaceManager::DidSatisfySequences to take a vector
of sequences as opposed to one.
This CL replaces DidSatisfySequence with SatisfySequence and also creates
SurfaceManager::RequireSequence which helps remove duplicate code
throughout the code base.

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

to

==========
Adding SatisfySequence and RequireSequence to SurfaceManager

Now that satisfies_sequences in CompositorFrameMetadata is gone, there 
is absolutely no reason for SurfaceManager::DidSatisfySequences to take
a vector of sequences as opposed to one.
This CL replaces DidSatisfySequence with SatisfySequence and also creates
SurfaceManager::RequireSequence which helps remove duplicate code
throughout the code base.

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

[Saman Sami, 2017-01-09 16:47:17]

samans@chromium.org changed reviewers:
+ danakj@chromium.org

[Fady Samuel, 2017-01-09 17:48:36]

lgtm

[commit-bot: I haz the power, 2017-01-09 18:01:07]

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

[commit-bot: I haz the power, 2017-01-09 18:01:08]

Dry run: This issue passed the CQ dry run.

[Saman Sami, 2017-01-09 18:16:36]

samans@chromium.org changed reviewers:
+ jbauman@chromium.org

[Saman Sami, 2017-01-09 18:26:21]

danakj and jbauman: Please review all files.

[Saman Sami, 2017-01-09 18:52:54]

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

[Saman Sami, 2017-01-09 18:53:41]

Description was changed from

==========
Adding SatisfySequence and RequireSequence to SurfaceManager

Now that satisfies_sequences in CompositorFrameMetadata is gone, there 
is absolutely no reason for SurfaceManager::DidSatisfySequences to take
a vector of sequences as opposed to one.
This CL replaces DidSatisfySequence with SatisfySequence and also creates
SurfaceManager::RequireSequence which helps remove duplicate code
throughout the code base.

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

to

==========
Adding SatisfySequence and RequireSequence to SurfaceManager

Now that satisfies_sequences in CompositorFrameMetadata is gone, there 
is absolutely no reason for SurfaceManager::DidSatisfySequences to take
a vector of sequences as opposed to one.
This CL replaces DidSatisfySequence with SatisfySequence and also creates
SurfaceManager::RequireSequence which helps remove duplicate code
throughout the code base.

TBR=piman@chromium.org
BUG=675054
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel
==========

[danakj, 2017-01-09 19:07:43]

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

https://codereview.chromium.org/2614423003/diff/1/cc/surfaces/surface_manager...
cc/surfaces/surface_manager.cc:84: LOG(ERROR) << "Attempting to require callback
on nonexistent surface";
We don't usually put LOGs in code that we ship, I'm not sure how this got there.
This should be DLOG probably.

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

https://codereview.chromium.org/2614423003/diff/1/cc/surfaces/surface_manager...
cc/surfaces/surface_manager.h:66: void RequireSequence(const SurfaceId&
surface_id,
Can you document these methods?

[Saman Sami, 2017-01-09 19:18:53]

PTAL

[danakj, 2017-01-09 19:27:10]

LGTM

https://codereview.chromium.org/2614423003/diff/20001/cc/surfaces/surface_man...
File cc/surfaces/surface_manager.cc (right):

https://codereview.chromium.org/2614423003/diff/20001/cc/surfaces/surface_man...
cc/surfaces/surface_manager.cc:84: DLOG(ERROR) << "Attempting to require
callback on nonexistent surface";
Maybe mention in a comment that this can occur from bad actors over ipc.
Otherwise it looks like this should be a dcheck.

[jbauman, 2017-01-09 23:36:23]

lgtm

[Saman Sami, 2017-01-10 00:05:25]

The CQ bit was checked by samans@chromium.org

[Saman Sami, 2017-01-10 00:05:26]

The patchset sent to the CQ was uploaded after l-g-t-m from fsamuel@chromium.org
Link to the patchset: https://codereview.chromium.org/2614423003/#ps20001
(title: "up")

[commit-bot: I haz the power, 2017-01-10 00:06:33]

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-01-10 03:10:46]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1484006725545830,
"parent_rev": "948cc62e307ddf74b9d846d2fa51c4b2bfe92f1f", "commit_rev":
"1221800150ab0969c63166809616392db5a02729"}

[commit-bot: I haz the power, 2017-01-10 03:11:15]

Description was changed from

==========
Adding SatisfySequence and RequireSequence to SurfaceManager

Now that satisfies_sequences in CompositorFrameMetadata is gone, there 
is absolutely no reason for SurfaceManager::DidSatisfySequences to take
a vector of sequences as opposed to one.
This CL replaces DidSatisfySequence with SatisfySequence and also creates
SurfaceManager::RequireSequence which helps remove duplicate code
throughout the code base.

TBR=piman@chromium.org
BUG=675054
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel
==========

to

==========
Adding SatisfySequence and RequireSequence to SurfaceManager

Now that satisfies_sequences in CompositorFrameMetadata is gone, there 
is absolutely no reason for SurfaceManager::DidSatisfySequences to take
a vector of sequences as opposed to one.
This CL replaces DidSatisfySequence with SatisfySequence and also creates
SurfaceManager::RequireSequence which helps remove duplicate code
throughout the code base.

TBR=piman@chromium.org
BUG=675054
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel

Review-Url: https://codereview.chromium.org/2614423003
Cr-Commit-Position: refs/heads/master@{#442461}
Committed:
https://chromium.googlesource.com/chromium/src/+/1221800150ab0969c63166809616...
==========

[commit-bot: I haz the power, 2017-01-10 03:11:17]

Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/chromium/src/+/1221800150ab0969c63166809616...