exo: Make sure vsync observer is removed when Surface is destroyed.

exo: Make sure vsync observer is removed when Surface is destroyed.

If the window is still in the tree when Surface is destroyed
then we need to remove the vsync observer in the dtor.

BUG=679787
Review-Url: https://codereview.chromium.org/2627713004
Cr-Commit-Position: refs/heads/master@{#442996}
Committed: https://chromium.googlesource.com/chromium/src/+/b622b0bbb1c98da5d7ce4b9e7749698959b93fa1

[reveman, 2017-01-11 19:33:45]

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

[reveman, 2017-01-11 19:33:50]

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

[commit-bot: I haz the power, 2017-01-11 19:34:47]

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-01-11 20:05:17]

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

[commit-bot: I haz the power, 2017-01-11 20:05:18]

Dry run: This issue passed the CQ dry run.

[Daniele Castagna, 2017-01-11 20:30:41]

lgtm

[reveman, 2017-01-11 20:30:53]

The CQ bit was checked by reveman@chromium.org

[commit-bot: I haz the power, 2017-01-11 20:32:11]

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

[Fady Samuel, 2017-01-11 20:33:04]

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

[Fady Samuel, 2017-01-11 20:33:04]

https://codereview.chromium.org/2627713004/diff/1/components/exo/surface.cc
File components/exo/surface.cc (right):

https://codereview.chromium.org/2627713004/diff/1/components/exo/surface.cc#n...
components/exo/surface.cc:209:
window_->layer()->GetCompositor()->vsync_manager()->RemoveObserver(this);
drive-by question: What does exo use the vsync manager for? It should be using
BeginFrames instead?

[reveman, 2017-01-11 21:52:08]

https://codereview.chromium.org/2627713004/diff/1/components/exo/surface.cc
File components/exo/surface.cc (right):

https://codereview.chromium.org/2627713004/diff/1/components/exo/surface.cc#n...
components/exo/surface.cc:209:
window_->layer()->GetCompositor()->vsync_manager()->RemoveObserver(this);
On 2017/01/11 at 20:33:04, Fady Samuel wrote:
> drive-by question: What does exo use the vsync manager for? It should be using
BeginFrames instead?

We use it to implement the presentation time protocol
(https://cs.chromium.org/chromium/src/third_party/wayland-protocols/src/stable...).
It provides clients with presentation timing feedback. The motion event wayland
perftest client is using this to accurately measure end-to-end latency and arc++
will use this in the future to drive vsync events. Arc++ currently uses a custom
protocol for this that also rely on vsync manager but the plan is to replace
that with this upstream presentation-time protocol.

[commit-bot: I haz the power, 2017-01-11 21:52:51]

CQ is committing da patch.

Bot data: {"patchset_id": 1, "attempt_start_ts": 1484166653490210, "parent_rev":
"b322c318a830e539f8ca1695d7f5e8e9fde6f5a7", "commit_rev":
"b622b0bbb1c98da5d7ce4b9e7749698959b93fa1"}

[commit-bot: I haz the power, 2017-01-11 21:53:23]

Description was changed from

==========
exo: Make sure vsync observer is removed when Surface is destroyed.

If the window is still in the tree when Surface is destroyed
then we need to remove the vsync observer in the dtor.

BUG=679787
==========

to

==========
exo: Make sure vsync observer is removed when Surface is destroyed.

If the window is still in the tree when Surface is destroyed
then we need to remove the vsync observer in the dtor.

BUG=679787

Review-Url: https://codereview.chromium.org/2627713004
Cr-Commit-Position: refs/heads/master@{#442996}
Committed:
https://chromium.googlesource.com/chromium/src/+/b622b0bbb1c98da5d7ce4b9e7749...
==========

[commit-bot: I haz the power, 2017-01-11 21:53:24]

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