Plumbing input event latency reporting through Mus GPU.

Plumbing input event latency reporting through Mus GPU.

Adds support for input event latency reporting for Mus.
There's two subtle differences between the Mus flow and the non-Mus flow:
- DISPLAY_COMPOSITOR_RECEIVED_FRAME_COMPONENT is logged if there are ANY latency
  components present.
- OnGpuSwapBuffersCompleted() is called if there are ANY latency components
  present. 
In non-Mus flow these are done only when there are browser-specific latency
components present. In practice this shouldn't have any effect on UMA latency
reporting, because currently OnGpuSwapBuffersCompleted() exits early if it doesn't
see RWH-specific latency components. However it should prove useful for getting
latency data for non-blink use cases in the future.

Other changes:
- Adds logging of DISPLAY_COMPOSITOR_RECEIVED_FRAME_COMPONENT in
  SurfaceFactory::SubmitCompositorFrame().
- Plumbs through LatencyInfo propagation for the in-process GPU
  command buffer case (i.e. for Mus GPU).
- Adds LatencyTracker to DisplayOutputSurface, and calls
  LatencyTracker::OnGpuSwapBuffersCompleted from
  DisplayOutputSurface::OnGpuSwapBuffersCompleted, which ensures UMA latency
  data is reported in Mus.

TBR=bbudge@chromium.org,danakj@chromium.org
BUG=686865
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel

Review-Url: https://codereview.chromium.org/2806163004
Cr-Commit-Position: refs/heads/master@{#464664}
Committed: https://chromium.googlesource.com/chromium/src/+/426ea5dd2cf7061a7860a34d502f2c191a343a8e

[mfomitchev, 2017-04-10 15:53:24]

Description was changed from

==========
Plumbing input event latency reporting through Mus GPU.

BUG=686865
==========

to

==========
Plumbing input event latency reporting through Mus GPU.

BUG=686865
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
==========

[mfomitchev, 2017-04-10 15:53:25]

Description was changed from

==========
Plumbing input event latency reporting through Mus GPU.

BUG=686865
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
==========

to

==========
Plumbing input event latency reporting through Mus GPU.

BUG=686865
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
==========

[mfomitchev, 2017-04-10 15:53:56]

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

[commit-bot: I haz the power, 2017-04-10 15:54:21]

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-04-10 17:45:28]

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

[commit-bot: I haz the power, 2017-04-10 17:45:29]

Dry run: This issue passed the CQ dry run.

[mfomitchev, 2017-04-10 19:26:24]

Description was changed from

==========
Plumbing input event latency reporting through Mus GPU.

BUG=686865
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
==========

to

==========
Plumbing input event latency reporting through Mus GPU.

Adds support for input event latency reporting for Mus.
There's two subtle differences between the Mus flow and the non-Mus flow:
- DISPLAY_COMPOSITOR_RECEIVED_FRAME_COMPONENT is logged if there are ANY latency
  components present.
- OnGpuSwapBuffersCompleted() is called if there are ANY latency components
  present. 
In non-Mus flow these are done only when there are browser-specific latency
components present. In practice this shouldn't have any effect on UMA latency
reporting, because currently OnGpuSwapBuffersCompleted() exits early if it
doesn't
see RWH-specific latency components. However it should prove useful for getting
latency data for non-blink use cases in the future.

Other changes:
- Adds logging of DISPLAY_COMPOSITOR_RECEIVED_FRAME_COMPONENT in
  SurfaceFactory::SubmitCompositorFrame().
- Plumbs through LatencyInfo propagation for the in-process GPU
  command buffer case (i.e. for Mus GPU).
- Adds LatencyTracker to DisplayOutputSurface, and calls
  LatencyTracker::OnGpuSwapBuffersCompleted from
  DisplayOutputSurface::OnGpuSwapBuffersCompleted, which ensures UMA latency
  data is reported in Mus.

BUG=686865

CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
==========

[mfomitchev, 2017-04-10 19:40:30]

mfomitchev@chromium.org changed reviewers:
+ tdresser@chromium.org

[mfomitchev, 2017-04-10 19:40:30]

tdresser: Can you please do the initial review from the overal latency tracking
perspective?

[tdresser, 2017-04-10 20:36:14]

LGTM, with one tiny nit.

https://codereview.chromium.org/2806163004/diff/1/services/ui/surfaces/displa...
File services/ui/surfaces/display_output_surface.cc (right):

https://codereview.chromium.org/2806163004/diff/1/services/ui/surfaces/displa...
services/ui/surfaces/display_output_surface.cc:93:
context_provider_->ContextSupport()->Swap(/*frame.latency_info*/);
?

[mfomitchev, 2017-04-10 21:31:00]

mfomitchev@chromium.org changed reviewers:
+ fsamuel@chromium.org, piman@chromium.org

[mfomitchev, 2017-04-10 21:31:01]

fsamuel@chromium.org: Please review changes in 
services/ui/surfaces/

piman@chromium.org: Please review changes in 
cc/, gpu/, ppapi/

https://codereview.chromium.org/2806163004/diff/1/services/ui/surfaces/displa...
File services/ui/surfaces/display_output_surface.cc (right):

https://codereview.chromium.org/2806163004/diff/1/services/ui/surfaces/displa...
services/ui/surfaces/display_output_surface.cc:93:
context_provider_->ContextSupport()->Swap(/*frame.latency_info*/);
On 2017/04/10 20:36:14, tdresser wrote:
> ?

Oops. Removed, thanks!

[Fady Samuel, 2017-04-10 21:35:16]

How does this plumbing differ for non-Mus Chrome? Can we unify? Thanks!

[mfomitchev, 2017-04-10 21:41:39]

On 2017/04/10 21:35:16, Fady Samuel wrote:
> How does this plumbing differ for non-Mus Chrome? Can we unify? Thanks!

Both Classic Chrome and Mus use LatencyTracker::OnGpuSwapBuffersCompleted().
LatencyTracker was factored is getting factored out here:
https://codereview.chromium.org/2814483002/
I don't see opportunities to unify beyond this, since the flows are different.
Classic chrome goes 
renderer->browser display compositor->out of process gpu command buffer->chrome,
while Mus goes 
renderer->Mus display compositor->in-process gpu command buffer.

[piman, 2017-04-10 23:50:34]

https://codereview.chromium.org/2806163004/diff/20001/gpu/ipc/in_process_comm...
File gpu/ipc/in_process_command_buffer.cc (right):

https://codereview.chromium.org/2806163004/diff/20001/gpu/ipc/in_process_comm...
gpu/ipc/in_process_command_buffer.cc:561: if
(ui::LatencyInfo::Verify(latency_info_,
latency_info_ is written on another thread (without lock), so it's unsafe to
read (or modify) here.

I think what should happen instead is that InProcessCommandBuffer::Flush should
extract the latency_info_ and pass it to the queued task (i.e. bound to this
function).

[mfomitchev, 2017-04-13 03:00:00]

https://codereview.chromium.org/2806163004/diff/20001/gpu/ipc/in_process_comm...
File gpu/ipc/in_process_command_buffer.cc (right):

https://codereview.chromium.org/2806163004/diff/20001/gpu/ipc/in_process_comm...
gpu/ipc/in_process_command_buffer.cc:561: if
(ui::LatencyInfo::Verify(latency_info_,
On 2017/04/10 23:50:34, piman - OOO back 4-27 wrote:
> latency_info_ is written on another thread (without lock), so it's unsafe to
> read (or modify) here.
> 
> I think what should happen instead is that InProcessCommandBuffer::Flush
should
> extract the latency_info_ and pass it to the queued task (i.e. bound to this
> function).

Done.

[mfomitchev, 2017-04-13 03:00:14]

mfomitchev@chromium.org changed reviewers:
- piman@chromium.org

[mfomitchev, 2017-04-13 03:03:57]

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

[mfomitchev, 2017-04-13 03:03:57]

-piman@ since he is on vacation now.

jbauman@chromium.org: Please review changes in 
gpu/ and cc/

fsamuel: Please review changes in 
services/ui/surfaces

[Fady Samuel, 2017-04-13 03:32:18]

https://codereview.chromium.org/2806163004/diff/60001/services/ui/surfaces/di...
File services/ui/surfaces/display_output_surface.cc (right):

https://codereview.chromium.org/2806163004/diff/60001/services/ui/surfaces/di...
services/ui/surfaces/display_output_surface.cc:27: latency_tracker_(new
LatencyTracker),
nit: base::MakeUnique<LatencyTracker>()  or better yet, why is this allocated on
the heap? Why not keep it inline?

[mfomitchev, 2017-04-13 17:15:22]

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

[mfomitchev, 2017-04-13 17:15:29]

https://codereview.chromium.org/2806163004/diff/60001/services/ui/surfaces/di...
File services/ui/surfaces/display_output_surface.cc (right):

https://codereview.chromium.org/2806163004/diff/60001/services/ui/surfaces/di...
services/ui/surfaces/display_output_surface.cc:27: latency_tracker_(new
LatencyTracker),
On 2017/04/13 03:32:17, Fady Samuel wrote:
> nit: base::MakeUnique<LatencyTracker>()  or better yet, why is this allocated
on
> the heap? Why not keep it inline?

Done.

[commit-bot: I haz the power, 2017-04-13 17:16:20]

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-04-13 18:00:35]

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

[commit-bot: I haz the power, 2017-04-13 18:00:37]

Dry run: Try jobs failed on following builders:
  linux_trusty_blink_rel on master.tryserver.blink (JOB_FAILED,
http://build.chromium.org/p/tryserver.blink/builders/linux_trusty_blink_rel/b...)

[jbauman, 2017-04-13 22:27:04]

lgtm

[mfomitchev, 2017-04-14 02:10:41]

Description was changed from

==========
Plumbing input event latency reporting through Mus GPU.

Adds support for input event latency reporting for Mus.
There's two subtle differences between the Mus flow and the non-Mus flow:
- DISPLAY_COMPOSITOR_RECEIVED_FRAME_COMPONENT is logged if there are ANY latency
  components present.
- OnGpuSwapBuffersCompleted() is called if there are ANY latency components
  present. 
In non-Mus flow these are done only when there are browser-specific latency
components present. In practice this shouldn't have any effect on UMA latency
reporting, because currently OnGpuSwapBuffersCompleted() exits early if it
doesn't
see RWH-specific latency components. However it should prove useful for getting
latency data for non-blink use cases in the future.

Other changes:
- Adds logging of DISPLAY_COMPOSITOR_RECEIVED_FRAME_COMPONENT in
  SurfaceFactory::SubmitCompositorFrame().
- Plumbs through LatencyInfo propagation for the in-process GPU
  command buffer case (i.e. for Mus GPU).
- Adds LatencyTracker to DisplayOutputSurface, and calls
  LatencyTracker::OnGpuSwapBuffersCompleted from
  DisplayOutputSurface::OnGpuSwapBuffersCompleted, which ensures UMA latency
  data is reported in Mus.

BUG=686865

CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
==========

to

==========
Plumbing input event latency reporting through Mus GPU.

Adds support for input event latency reporting for Mus.
There's two subtle differences between the Mus flow and the non-Mus flow:
- DISPLAY_COMPOSITOR_RECEIVED_FRAME_COMPONENT is logged if there are ANY latency
  components present.
- OnGpuSwapBuffersCompleted() is called if there are ANY latency components
  present. 
In non-Mus flow these are done only when there are browser-specific latency
components present. In practice this shouldn't have any effect on UMA latency
reporting, because currently OnGpuSwapBuffersCompleted() exits early if it
doesn't
see RWH-specific latency components. However it should prove useful for getting
latency data for non-blink use cases in the future.

Other changes:
- Adds logging of DISPLAY_COMPOSITOR_RECEIVED_FRAME_COMPONENT in
  SurfaceFactory::SubmitCompositorFrame().
- Plumbs through LatencyInfo propagation for the in-process GPU
  command buffer case (i.e. for Mus GPU).
- Adds LatencyTracker to DisplayOutputSurface, and calls
  LatencyTracker::OnGpuSwapBuffersCompleted from
  DisplayOutputSurface::OnGpuSwapBuffersCompleted, which ensures UMA latency
  data is reported in Mus.

TBR=bbudge@chromium.org,danakj@chromium.org
BUG=686865

CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
==========

[mfomitchev, 2017-04-14 02:11:29]

TBR bbudge@chromium.org and danakj@chromium.org for mechanical changes in ppapi/
and cc/test/ respectively

[Fady Samuel, 2017-04-14 02:15:47]

lgtm

[mfomitchev, 2017-04-14 02:19:11]

The CQ bit was checked by mfomitchev@chromium.org

[mfomitchev, 2017-04-14 02:19:12]

The patchset sent to the CQ was uploaded after l-g-t-m from
tdresser@chromium.org
Link to the patchset: https://codereview.chromium.org/2806163004/#ps80001
(title: "Inline LatencyTracker in DIsplayOutputService.")

[commit-bot: I haz the power, 2017-04-14 02:19:25]

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-04-14 03:03:43]

CQ is committing da patch.

Bot data: {"patchset_id": 80001, "attempt_start_ts": 1492136351856430,
"parent_rev": "629219eb67f0a5d7ce30f0729870b584d307c897", "commit_rev":
"a54c0417a83da731338d1bc173b1210afbbe951c"}

[commit-bot: I haz the power, 2017-04-14 03:09:53]

CQ is committing da patch.

Bot data: {"patchset_id": 80001, "attempt_start_ts": 1492136351856430,
"parent_rev": "9df825af4caa97dd4a0dc3d130833db68bb20b72", "commit_rev":
"426ea5dd2cf7061a7860a34d502f2c191a343a8e"}

[commit-bot: I haz the power, 2017-04-14 03:11:13]

Description was changed from

==========
Plumbing input event latency reporting through Mus GPU.

Adds support for input event latency reporting for Mus.
There's two subtle differences between the Mus flow and the non-Mus flow:
- DISPLAY_COMPOSITOR_RECEIVED_FRAME_COMPONENT is logged if there are ANY latency
  components present.
- OnGpuSwapBuffersCompleted() is called if there are ANY latency components
  present. 
In non-Mus flow these are done only when there are browser-specific latency
components present. In practice this shouldn't have any effect on UMA latency
reporting, because currently OnGpuSwapBuffersCompleted() exits early if it
doesn't
see RWH-specific latency components. However it should prove useful for getting
latency data for non-blink use cases in the future.

Other changes:
- Adds logging of DISPLAY_COMPOSITOR_RECEIVED_FRAME_COMPONENT in
  SurfaceFactory::SubmitCompositorFrame().
- Plumbs through LatencyInfo propagation for the in-process GPU
  command buffer case (i.e. for Mus GPU).
- Adds LatencyTracker to DisplayOutputSurface, and calls
  LatencyTracker::OnGpuSwapBuffersCompleted from
  DisplayOutputSurface::OnGpuSwapBuffersCompleted, which ensures UMA latency
  data is reported in Mus.

TBR=bbudge@chromium.org,danakj@chromium.org
BUG=686865

CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
==========

to

==========
Plumbing input event latency reporting through Mus GPU.

Adds support for input event latency reporting for Mus.
There's two subtle differences between the Mus flow and the non-Mus flow:
- DISPLAY_COMPOSITOR_RECEIVED_FRAME_COMPONENT is logged if there are ANY latency
  components present.
- OnGpuSwapBuffersCompleted() is called if there are ANY latency components
  present. 
In non-Mus flow these are done only when there are browser-specific latency
components present. In practice this shouldn't have any effect on UMA latency
reporting, because currently OnGpuSwapBuffersCompleted() exits early if it
doesn't
see RWH-specific latency components. However it should prove useful for getting
latency data for non-blink use cases in the future.

Other changes:
- Adds logging of DISPLAY_COMPOSITOR_RECEIVED_FRAME_COMPONENT in
  SurfaceFactory::SubmitCompositorFrame().
- Plumbs through LatencyInfo propagation for the in-process GPU
  command buffer case (i.e. for Mus GPU).
- Adds LatencyTracker to DisplayOutputSurface, and calls
  LatencyTracker::OnGpuSwapBuffersCompleted from
  DisplayOutputSurface::OnGpuSwapBuffersCompleted, which ensures UMA latency
  data is reported in Mus.

TBR=bbudge@chromium.org,danakj@chromium.org
BUG=686865

CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel

Review-Url: https://codereview.chromium.org/2806163004
Cr-Commit-Position: refs/heads/master@{#464664}
Committed:
https://chromium.googlesource.com/chromium/src/+/426ea5dd2cf7061a7860a34d502f...
==========

[commit-bot: I haz the power, 2017-04-14 03:11:14]

Committed patchset #5 (id:80001) as
https://chromium.googlesource.com/chromium/src/+/426ea5dd2cf7061a7860a34d502f...