exo: Implement presentation interface.

exo: Implement presentation interface.

This implements the wayland presentation interface for
presentation timing feedback. The motion event client
has been updated to use this interface for accurate
display latency measurements.

This also cleans up the Surfce/CompositorFrameSinkHolder
communication slighly and fixes a problem in the motion
event client that prevented it from working without
--use-drm flag on ozone devices.

BUG=661010
TEST=wayland_motion_events

Review-Url: https://codereview.chromium.org/2610513006
Cr-Commit-Position: refs/heads/master@{#442421}
Committed: https://chromium.googlesource.com/chromium/src/+/211cf808aabe5487f7db53d7bcc896ff02008dc2

[reveman, 2017-01-03 13:15:48]

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

[reveman, 2017-01-04 13:30:57]

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

[commit-bot: I haz the power, 2017-01-04 13:31:08]

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-04 14:23:40]

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

[commit-bot: I haz the power, 2017-01-04 14:23:41]

Dry run: This issue passed the CQ dry run.

[Daniele Castagna, 2017-01-09 17:01:33]

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

https://codereview.chromium.org/2610513006/diff/1/components/exo/surface.h#ne...
components/exo/surface.h:363: std::list<PresentationCallback>
pending_presentation_callbacks_;
Given the small size of these containers, why do you prefer lists over vectors
here?
Why not forward_list?

https://codereview.chromium.org/2610513006/diff/1/components/exo/wayland/clie...
File components/exo/wayland/clients/motion_events.cc (right):

https://codereview.chromium.org/2610513006/diff/1/components/exo/wayland/clie...
components/exo/wayland/clients/motion_events.cc:649: std::unique_ptr<Frame>
frame(new Frame);
nit: I'd go with auto frame = make_unique

https://codereview.chromium.org/2610513006/diff/1/components/exo/wayland/serv...
File components/exo/wayland/server.cc (right):

https://codereview.chromium.org/2610513006/diff/1/components/exo/wayland/serv...
components/exo/wayland/server.cc:2661: if (microseconds >=
base::Time::kMicrosecondsPerSecond) {
Isn't this just:
seconds = presentation_time / base::Time::kMicrosecondsPerSecond
microseconds = presentation_time % base::Time::kMicrosecondsPerSecond

https://codereview.chromium.org/2610513006/diff/1/components/exo/wayland/serv...
components/exo/wayland/server.cc:2690:
base::CancelableCallback<void(base::TimeTicks, base::TimeDelta)>>
nit: auto cancelable_callback = make_unique... to avoid repeating the type
twice.

[reveman, 2017-01-09 17:55:15]

ptal

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

https://codereview.chromium.org/2610513006/diff/1/components/exo/surface.h#ne...
components/exo/surface.h:363: std::list<PresentationCallback>
pending_presentation_callbacks_;
On 2017/01/09 at 17:01:33, Daniele Castagna wrote:
> Given the small size of these containers, why do you prefer lists over vectors
here?
> Why not forward_list?

For now, I'd just want to keep the code consistent. We can change both these and
the frame callbacks in a separate CL if that makes sense. Imo, std::list makes
for easy to read code and there's no data showing that we need to make it more
complicated.

https://codereview.chromium.org/2610513006/diff/1/components/exo/wayland/clie...
File components/exo/wayland/clients/motion_events.cc (right):

https://codereview.chromium.org/2610513006/diff/1/components/exo/wayland/clie...
components/exo/wayland/clients/motion_events.cc:649: std::unique_ptr<Frame>
frame(new Frame);
On 2017/01/09 at 17:01:33, Daniele Castagna wrote:
> nit: I'd go with auto frame = make_unique

Done.

https://codereview.chromium.org/2610513006/diff/1/components/exo/wayland/serv...
File components/exo/wayland/server.cc (right):

https://codereview.chromium.org/2610513006/diff/1/components/exo/wayland/serv...
components/exo/wayland/server.cc:2661: if (microseconds >=
base::Time::kMicrosecondsPerSecond) {
On 2017/01/09 at 17:01:33, Daniele Castagna wrote:
> Isn't this just:
> seconds = presentation_time / base::Time::kMicrosecondsPerSecond
> microseconds = presentation_time % base::Time::kMicrosecondsPerSecond

Good call. Removed the branch.

https://codereview.chromium.org/2610513006/diff/1/components/exo/wayland/serv...
components/exo/wayland/server.cc:2690:
base::CancelableCallback<void(base::TimeTicks, base::TimeDelta)>>
On 2017/01/09 at 17:01:33, Daniele Castagna wrote:
> nit: auto cancelable_callback = make_unique... to avoid repeating the type
twice.

Done.

[Daniele Castagna, 2017-01-09 18:42:14]

LGTM

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

https://codereview.chromium.org/2610513006/diff/1/components/exo/surface.h#ne...
components/exo/surface.h:363: std::list<PresentationCallback>
pending_presentation_callbacks_;
On 2017/01/09 at 17:55:15, reveman wrote:
> On 2017/01/09 at 17:01:33, Daniele Castagna wrote:
> > Given the small size of these containers, why do you prefer lists over
vectors here?
> > Why not forward_list?
> 
> For now, I'd just want to keep the code consistent. We can change both these
and the frame callbacks in a separate CL if that makes sense. Imo, std::list
makes for easy to read code and there's no data showing that we need to make it
more complicated.

Ack.

[reveman, 2017-01-09 22:34:17]

The CQ bit was checked by reveman@chromium.org

[commit-bot: I haz the power, 2017-01-09 22:35:02]

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 00:31:06]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1484001257183420,
"parent_rev": "e2ce5fc799c1488d4c857505baae8b3a20f9cd76", "commit_rev":
"211cf808aabe5487f7db53d7bcc896ff02008dc2"}

[commit-bot: I haz the power, 2017-01-10 00:31:38]

Description was changed from

==========
exo: Implement presentation interface.

This implements the wayland presentation interface for
presentation timing feedback. The motion event client
has been updated to use this interface for accurate
display latency measurements.

This also cleans up the Surfce/CompositorFrameSinkHolder
communication slighly and fixes a problem in the motion
event client that prevented it from working without
--use-drm flag on ozone devices.

BUG=661010
TEST=wayland_motion_events
==========

to

==========
exo: Implement presentation interface.

This implements the wayland presentation interface for
presentation timing feedback. The motion event client
has been updated to use this interface for accurate
display latency measurements.

This also cleans up the Surfce/CompositorFrameSinkHolder
communication slighly and fixes a problem in the motion
event client that prevented it from working without
--use-drm flag on ozone devices.

BUG=661010
TEST=wayland_motion_events

Review-Url: https://codereview.chromium.org/2610513006
Cr-Commit-Position: refs/heads/master@{#442421}
Committed:
https://chromium.googlesource.com/chromium/src/+/211cf808aabe5487f7db53d7bcc8...
==========

[commit-bot: I haz the power, 2017-01-10 00:31:38]

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