New issue
Advanced search Search tips

Issue 788446 link

Starred by 1 user

Issue metadata

Status: Started
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug



Sign in to add a comment

Move more of RenderWidgetHostLatencyTracker into LatencyTracker

Project Member Reported by sadrul@chromium.org, Nov 24 2017

Issue description

Move more code out of RenderWidgetHostLatencyTracker into LatencyTracker. Ideally, all of the code should go in LatencyTracker. But RenderWidgetHostLatencyTracker currently has some dependency on input events, which LatencyTracker can't depend on right now. It may make sense to split LatencyTracker out into its own component, so that it can depend on blink events, without introducing circular dependencies.
 
Project Member

Comment 1 by bugdroid1@chromium.org, Nov 24 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/68404c8e32b30b09e4f3bd8551bf722f4bd4e6e3

commit 68404c8e32b30b09e4f3bd8551bf722f4bd4e6e3
Author: Sadrul Habib Chowdhury <sadrul@chromium.org>
Date: Fri Nov 24 17:17:16 2017

latency: Move the UkmSourceId into LatencyTracker.

Instead of having the UkmSourceId in RenderWidgetHostLatencyTracker, move
it into ui::LatencyTracker. Also, inject the id during creation, instead
of later on demand to make the code simpler.

Also, make ui::LatencyTracker responsible for assigning the trace-id for
the LatencyInfo.

BUG=788446

Change-Id: I4601c9d7ab47d90f58fcffeafa1428457e422b0c
Reviewed-on: https://chromium-review.googlesource.com/788457
Commit-Queue: Sadrul Chowdhury <sadrul@chromium.org>
Reviewed-by: Timothy Dresser <tdresser@chromium.org>
Cr-Commit-Position: refs/heads/master@{#519138}
[modify] https://crrev.com/68404c8e32b30b09e4f3bd8551bf722f4bd4e6e3/content/browser/renderer_host/input/render_widget_host_latency_tracker.cc
[modify] https://crrev.com/68404c8e32b30b09e4f3bd8551bf722f4bd4e6e3/content/browser/renderer_host/input/render_widget_host_latency_tracker.h
[modify] https://crrev.com/68404c8e32b30b09e4f3bd8551bf722f4bd4e6e3/content/browser/renderer_host/input/render_widget_host_latency_tracker_unittest.cc
[modify] https://crrev.com/68404c8e32b30b09e4f3bd8551bf722f4bd4e6e3/content/browser/renderer_host/render_widget_host_impl.cc
[modify] https://crrev.com/68404c8e32b30b09e4f3bd8551bf722f4bd4e6e3/ui/latency/latency_tracker.cc
[modify] https://crrev.com/68404c8e32b30b09e4f3bd8551bf722f4bd4e6e3/ui/latency/latency_tracker.h

Project Member

Comment 2 by bugdroid1@chromium.org, Nov 24 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/f1f9c81fb04aa7b2d0e063fd15497765a59a50e3

commit f1f9c81fb04aa7b2d0e063fd15497765a59a50e3
Author: Sadrul Habib Chowdhury <sadrul@chromium.org>
Date: Fri Nov 24 17:30:03 2017

latency: Remove some dead code.

RenderWidgetHostLatencyTracker no longer does anything with the
device-scale-factor. So remove associated code.

Also, remove an unused field in RenderWidgetHostViewAura.

BUG=788446

Change-Id: I66a1521f4dec3aae28b023e41cc28817941eab9c
Reviewed-on: https://chromium-review.googlesource.com/788570
Commit-Queue: Sadrul Chowdhury <sadrul@chromium.org>
Reviewed-by: Timothy Dresser <tdresser@chromium.org>
Cr-Commit-Position: refs/heads/master@{#519139}
[modify] https://crrev.com/f1f9c81fb04aa7b2d0e063fd15497765a59a50e3/content/browser/renderer_host/input/render_widget_host_latency_tracker.cc
[modify] https://crrev.com/f1f9c81fb04aa7b2d0e063fd15497765a59a50e3/content/browser/renderer_host/input/render_widget_host_latency_tracker.h
[modify] https://crrev.com/f1f9c81fb04aa7b2d0e063fd15497765a59a50e3/content/browser/renderer_host/render_widget_host_impl.cc
[modify] https://crrev.com/f1f9c81fb04aa7b2d0e063fd15497765a59a50e3/content/browser/renderer_host/render_widget_host_view_aura.h

Project Member

Comment 3 by bugdroid1@chromium.org, Nov 24 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/aadeba2f78f9a150a2143f3f175f921ab3962378

commit aadeba2f78f9a150a2143f3f175f921ab3962378
Author: Sadrul Habib Chowdhury <sadrul@chromium.org>
Date: Fri Nov 24 18:54:56 2017

latency: Make some assumptions more explicit.

Make the code more explicit about the components that are expected to
exist in LatencyInfo. Specifically:
 . LatencyInfo should not already have a BEGIN_RWH component set when it
   arrives in RenderWidgetHostLatencyTracker::OnInputEvent(), since that
   is the only place that adds this component.
 . The BEGIN_RWH component must be present in OnInputEventAck(), since it
   must have gone through OnInputEvent() first, which always adds the
   component.
 . If gpu-swap happened for a LatencyInfo, then it must have SWAP_BEGIN,
   RENDERING_SCHEDULED (on impl or main), RENDERER_SWAP, and
   DISPLAY_COMPOSITOR_RECEIVED_FRAME components set. The LatencyInfo in
   this callback (OnGpuSwapBuffersCompleted()) may come from untrusted
   source though, so be lenient about these validations.

BUG=788446

Change-Id: Ide32d6b1e3ef4b263b1b0b1ad0f7f68878cda4d5
Reviewed-on: https://chromium-review.googlesource.com/788459
Reviewed-by: Timothy Dresser <tdresser@chromium.org>
Commit-Queue: Sadrul Chowdhury <sadrul@chromium.org>
Cr-Commit-Position: refs/heads/master@{#519149}
[modify] https://crrev.com/aadeba2f78f9a150a2143f3f175f921ab3962378/content/browser/renderer_host/input/render_widget_host_latency_tracker.cc
[modify] https://crrev.com/aadeba2f78f9a150a2143f3f175f921ab3962378/content/browser/renderer_host/input/render_widget_host_latency_tracker_unittest.cc
[modify] https://crrev.com/aadeba2f78f9a150a2143f3f175f921ab3962378/ui/latency/latency_tracker.cc

Sign in to add a comment