New issue
Advanced search Search tips

Issue 611950 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Jul 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux
Pri: 3
Type: Bug



Sign in to add a comment

DCHECK failure in TimeDeltaFromXEventTime()

Project Member Reported by thestig@chromium.org, May 14 2016

Issue description

Running a debug build at r393725 in tightvncserver 1.3.9-6.4ubuntu1.

A few seconds after starting Chromium, it crashes with:

[FATAL:events_x_utils.cc(348)] Check failed: delta < 60 * 1000 || g_tick_clock.Get() != nullptr. Unexpected X11 event time, now: 11268870119366 bogo-microseconds event at: 2912789934

I tried a second time and got similar results.
 

Comment 1 by caseq@chromium.org, May 16 2016

Cc: majidvp@chromium.org dtapu...@chromium.org tdres...@chromium.org rbyers@chromium.org
Components: -UI>Browser UI>Input
Status: Assigned (was: Untriaged)
Ugh. It looks like we're getting bogus timestamps from tightvncserver :( Since this will affect the metrics we report and the time stamps we expose to the web platform, I think we'll just need to discard timestamps if these don't look plausible and replace them with current tick count instead of DCHECK()'ing on these. Does this sound reasonable?
Yeah, that seems pretty reasonable.

One option would be to add a bit to the event indicating if the event timestamp is synthetic, so we could avoid mixing up the real and synthetic timestamps in our metrics.

I suspect this is rare enough that we don't really need to worry about it though.


The timestamp on the X11 field is actually the client's timestamp...


https://www.x.org/archive/X11R7.5/doc/x11proto/proto.pdf

Timestamp
A timestamp is a time value, expressed in milliseconds. It typically is the time since the last
server reset. Timestamp values wrap around (after about 49.7 days). The server, giv en its
current time is represented by timestamp T, always interprets timestamps from clients by
treating half of the timestamp space as being earlier in time than T and half of the timestamp
space as being later in time than T. One timestamp value (named CurrentTime) is
never generated by the server. This value is reserved for use in requests to represent the current
server time.

Comment 4 by caseq@chromium.org, May 16 2016

I think this passage refers to the client-provided timestamps, and in case of input events we're talking of server-produced timestamp -- that is, unless it results from something like XSendEvent() (which I'm about to check now just in case we generate events with bad timestamps).
Project Member

Comment 5 by bugdroid1@chromium.org, Jun 2 2016

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

commit 905cedbf079a83c048c9cfcf526210efcf7c4abd
Author: caseq <caseq@chromium.org>
Date: Thu Jun 02 06:34:05 2016

Replace DCHECK() upon unexpected x11 event time with a workaround

BUG= 611950 

Review-Url: https://codereview.chromium.org/1982203002
Cr-Commit-Position: refs/heads/master@{#397315}

[modify] https://crrev.com/905cedbf079a83c048c9cfcf526210efcf7c4abd/tools/metrics/histograms/histograms.xml
[modify] https://crrev.com/905cedbf079a83c048c9cfcf526210efcf7c4abd/ui/events/x/events_x_utils.cc

Comment 6 by caseq@chromium.org, Jul 22 2016

Status: Fixed (was: Assigned)
Project Member

Comment 7 by bugdroid1@chromium.org, Oct 13 2016

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

commit 758cb6139c130fca8d6153d621203c18980005df
Author: majidvp <majidvp@chromium.org>
Date: Thu Oct 13 15:08:41 2016

Expand event timestamp validation check to more platforms

Collect UMA metric on validity of event timestamp clock on platforms
where we currently check the timebase (i.e., X11, Mac)*.
We still only correct invalid timestamps for X11 but will monitor this histogram
to see if there is a need for similar corrections on other platforms.

Also the histogram is now suffixed to differentiate between BROWSER and
RENDERER processes in anticipation of supporting a similar check
on renderer process.

* The check will cover more platforms in https://codereview.chromium.org/2408063005/

BUG= 611950 

Review-Url: https://codereview.chromium.org/2295923003
Cr-Commit-Position: refs/heads/master@{#425032}

[modify] https://crrev.com/758cb6139c130fca8d6153d621203c18980005df/tools/metrics/histograms/histograms.xml
[modify] https://crrev.com/758cb6139c130fca8d6153d621203c18980005df/ui/events/event_utils.cc

Sign in to add a comment