exo: Cleanup and make buffer release code more robust.

exo: Cleanup and make buffer release code more robust.

Since the introduction of CompositorFrameSinkHolder it's possible
for release callbacks to be lost. This is because we rely on the
use count to drop to 0 but we're only guaranteed to get a callback
for the last frame as we only keep one CompositorFrameSinkHolder
reference.

This removes the use count in favor of a simple cancelable callback.
The use count was never necessary as attaching the buffer to
multiple surfaces is a client behavior that results in undefined
release callback behavior. Running the callback when the last
attachment is released is not worse then sending it when all
attachments have been released.

BUG=659601
TEST=exo_unittests --gtest_filter=BufferTest.*

Review-Url: https://codereview.chromium.org/2666233002
Cr-Commit-Position: refs/heads/master@{#447593}
Committed: https://chromium.googlesource.com/chromium/src/+/2cd69789c0cc1bf7b35f6c5673ba1db6f2273cf0

[reveman, 2017-02-01 01:10:20]

reveman@chromium.org changed reviewers:
+ dcastagna@chromium.org

[Alex Z., 2017-02-01 14:34:51]

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

[Alex Z., 2017-02-01 14:34:52]

lgtm with one nit

https://codereview.chromium.org/2666233002/diff/1/components/exo/buffer.h
File components/exo/buffer.h (right):

https://codereview.chromium.org/2666233002/diff/1/components/exo/buffer.h#new...
components/exo/buffer.h:94: // Notifies the client that buffer as been released
if no longer attached
nit: s/as/has

[reveman, 2017-02-01 17:07:24]

https://codereview.chromium.org/2666233002/diff/1/components/exo/buffer.h
File components/exo/buffer.h (right):

https://codereview.chromium.org/2666233002/diff/1/components/exo/buffer.h#new...
components/exo/buffer.h:94: // Notifies the client that buffer as been released
if no longer attached
On 2017/02/01 at 14:34:51, StarAZ1 wrote:
> nit: s/as/has

Done.

[reveman, 2017-02-01 17:07:28]

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

[commit-bot: I haz the power, 2017-02-01 17:07:44]

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, 2017-02-01 17:47:08]

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

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

Dry run: This issue passed the CQ dry run.

[reveman, 2017-02-01 19:26:02]

The CQ bit was checked by reveman@chromium.org

[reveman, 2017-02-01 19:26:02]

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

[commit-bot: I haz the power, 2017-02-01 19:26:34]

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-01 19:26:35]

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

[commit-bot: I haz the power, 2017-02-01 19:26:36]

No L-G-T-M from a valid reviewer yet. 
CQ run can only be started by full committers or once the patch has
received an L-G-T-M from a full committer.
Even if an L-G-T-M may have been provided, it was from a non-committer,
_not_ a full super star committer.
See http://www.chromium.org/getting-involved/become-a-committer
Note that this has nothing to do with OWNERS files.

[Daniele Castagna, 2017-02-01 19:50:47]

lgtm

[reveman, 2017-02-01 20:14:36]

The CQ bit was checked by reveman@chromium.org

[commit-bot: I haz the power, 2017-02-01 20:15: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-01 20:19:48]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1485980076502480,
"parent_rev": "03f7a2cb6811d08b7fc8dd2de21c1fefa34415b1", "commit_rev":
"2cd69789c0cc1bf7b35f6c5673ba1db6f2273cf0"}

[commit-bot: I haz the power, 2017-02-01 20:20:25]

Description was changed from

==========
exo: Cleanup and make buffer release code more robust.

Since the introduction of CompositorFrameSinkHolder it's possible
for release callbacks to be lost. This is because we rely on the
use count to drop to 0 but we're only guaranteed to get a callback
for the last frame as we only keep one CompositorFrameSinkHolder
reference.

This removes the use count in favor of a simple cancelable callback.
The use count was never necessary as attaching the buffer to
multiple surfaces is a client behavior that results in undefined
release callback behavior. Running the callback when the last
attachment is released is not worse then sending it when all
attachments have been released.

BUG=659601
TEST=exo_unittests --gtest_filter=BufferTest.*
==========

to

==========
exo: Cleanup and make buffer release code more robust.

Since the introduction of CompositorFrameSinkHolder it's possible
for release callbacks to be lost. This is because we rely on the
use count to drop to 0 but we're only guaranteed to get a callback
for the last frame as we only keep one CompositorFrameSinkHolder
reference.

This removes the use count in favor of a simple cancelable callback.
The use count was never necessary as attaching the buffer to
multiple surfaces is a client behavior that results in undefined
release callback behavior. Running the callback when the last
attachment is released is not worse then sending it when all
attachments have been released.

BUG=659601
TEST=exo_unittests --gtest_filter=BufferTest.*

Review-Url: https://codereview.chromium.org/2666233002
Cr-Commit-Position: refs/heads/master@{#447593}
Committed:
https://chromium.googlesource.com/chromium/src/+/2cd69789c0cc1bf7b35f6c5673ba...
==========

[commit-bot: I haz the power, 2017-02-01 20:20:26]

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