New issue
Advanced search Search tips

Issue 771165 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Nov 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 1
Type: Bug-Regression



Sign in to add a comment

chrome://tracing reports MouseMove LatencyInfo doesn't end

Project Member Reported by dtapu...@chromium.org, Oct 3 2017

Issue description

1) Load www.google.com
2) Open chrome://tracing
3) Record Input Latency Trace
4) Switch tab back to google.com
5) Move mouse around
6) Stop trace
7) Observe the Input Latency for MouseMove events don't end correctly and there is a long tree of them pending. Some are indicating "Slice has no matching END. End time has been adjusted."


Per revision bisect points to:

You are probably looking for a change made after 497298 (known good), but no later than 497299 (first known bad).
CHANGELOG URL:
The script might not always return single CL as suspect as some perf builds might get missing due to failure.
  https://chromium.googlesource.com/chromium/src/+log/322031ea3956b9e32face8cc012db9347c78ac4b..95d3ef0e9c58dd9509745f5feaa5f209e11378ac

Which I think https://chromium.googlesource.com/catapult.git/+/e37aa9d6016b3a6fec594409dd651029d7201099

is the culprit.
 
I wonder if this isn't an issue with the importer but a revelation that we aren't terminating some latency info objects.

I'm looking into this as well from my end.
Cc: -tdres...@chromium.org benjhayden@chromium.org
Components: -Speed>Tracing Blink>Input
Owner: tdres...@chromium.org
Actually caused by Tim's change.

https://chromium.googlesource.com/chromium/src/+/7a5ff008cb7963c34c5207fa60145a1d7d9ea99a

I believe we shouldn't clobber the trace id in 
https://cs.chromium.org/chromium/src/ui/latency/latency_info.cc?type=cs&q=AddNewLatencyFrom&sq=package:chromium&l=202

A quick one off here shows that commenting out that line fixes the issue. Although I don't know the impact of making such a change.
Status: Started (was: Assigned)
Project Member

Comment 4 by bugdroid1@chromium.org, Oct 17 2017

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

commit 5f1830ac7534281927550d142a61d282eb2ae85f
Author: Tim Dresser <tdresser@chromium.org>
Date: Tue Oct 17 16:51:59 2017

Fix latency info termination.

Previously, we incorrectly modified the trace ID of LatencyInfo
objects which were being coalesced. This caused their async events
to never terminate.

Now we don't modify the trace ID during coalescing.

Bug:  771165 
Change-Id: I1e73b97579333b9be67886b4a93e53e8315d30cb
Reviewed-on: https://chromium-review.googlesource.com/700406
Reviewed-by: Dave Tapuska <dtapuska@chromium.org>
Commit-Queue: Timothy Dresser <tdresser@chromium.org>
Cr-Commit-Position: refs/heads/master@{#509417}
[modify] https://crrev.com/5f1830ac7534281927550d142a61d282eb2ae85f/content/browser/renderer_host/input/mouse_latency_browsertest.cc
[modify] https://crrev.com/5f1830ac7534281927550d142a61d282eb2ae85f/ui/latency/latency_info.cc

Project Member

Comment 5 by bugdroid1@chromium.org, Oct 26 2017

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

commit 56960bf35257549f2181a4ea5acd869de26f6ef1
Author: Dave Tapuska <dtapuska@chromium.org>
Date: Thu Oct 26 21:40:20 2017

LatencyInfo wasn't propogating all fields over IPC.

The coalesced field needs to be replicated on the param traits. It is
done for mojo and wasn't done for Chrome IPC.

BUG= 771165 

Change-Id: I0e22c798968188b075847376b1d5b0752cc15749
Reviewed-on: https://chromium-review.googlesource.com/738441
Reviewed-by: Timothy Dresser <tdresser@chromium.org>
Reviewed-by: Ken Buchanan <kenrb@chromium.org>
Commit-Queue: Dave Tapuska <dtapuska@chromium.org>
Cr-Commit-Position: refs/heads/master@{#511968}
[modify] https://crrev.com/56960bf35257549f2181a4ea5acd869de26f6ef1/ui/latency/ipc/latency_info_param_traits.cc

Project Member

Comment 6 by bugdroid1@chromium.org, Oct 26 2017

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

commit 0981af299f30526170a20702b1afd663196ea94a
Author: Dave Tapuska <dtapuska@chromium.org>
Date: Thu Oct 26 23:52:46 2017

Ensure that coalesced events are properly terminated.

Coalesced events that scheduled rendering were not terminated properly
because they weren't passed to the LatencyInfo Swap promise monitor.
Ensure that if a coalesced event was received we add a no swap component
on the latency info.

BUG= 771165 

Change-Id: I3a4ef851211c1ab87f1a181076884553607f08e0
Reviewed-on: https://chromium-review.googlesource.com/738554
Commit-Queue: Dave Tapuska <dtapuska@chromium.org>
Reviewed-by: Timothy Dresser <tdresser@chromium.org>
Cr-Commit-Position: refs/heads/master@{#512027}
[modify] https://crrev.com/0981af299f30526170a20702b1afd663196ea94a/content/browser/renderer_host/input/mouse_latency_browsertest.cc
[modify] https://crrev.com/0981af299f30526170a20702b1afd663196ea94a/content/browser/renderer_host/input/render_widget_host_latency_tracker.cc

Status: Fixed (was: Started)

Sign in to add a comment