Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(1076)

Unified Diff: ui/events/x/events_x_utils.cc

Issue 1982203002: Replace DCHECK() upon unexpected x11 event time with a workaround (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: bool -> enum, renamed metric Created 4 years, 7 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: ui/events/x/events_x_utils.cc
diff --git a/ui/events/x/events_x_utils.cc b/ui/events/x/events_x_utils.cc
index aab272373068313cf5ae95cd5fbb27b34fa12287..b7685f19f40fa6dcd2a3670d2b51bed20acd21a4 100644
--- a/ui/events/x/events_x_utils.cc
+++ b/ui/events/x/events_x_utils.cc
@@ -17,12 +17,14 @@
#include "base/logging.h"
#include "base/macros.h"
#include "base/memory/singleton.h"
+#include "base/metrics/histogram_macros.h"
#include "build/build_config.h"
#include "ui/display/display.h"
#include "ui/display/screen.h"
#include "ui/events/devices/x11/device_data_manager_x11.h"
#include "ui/events/devices/x11/device_list_cache_x11.h"
#include "ui/events/devices/x11/touch_factory_x11.h"
+#include "ui/events/event_utils.h"
#include "ui/events/keycodes/keyboard_code_conversion_x.h"
#include "ui/gfx/geometry/point.h"
#include "ui/gfx/geometry/rect.h"
@@ -306,12 +308,20 @@ bool GetGestureTimes(const XEvent& xev, double* start_time, double* end_time) {
return true;
}
+namespace {
int64_t g_last_seen_timestamp_ms = 0;
-// accumulated rollover time.
int64_t g_rollover_ms = 0;
+bool g_bogus_x11_timestamps = false;
base::LazyInstance<std::unique_ptr<base::TickClock>>::Leaky g_tick_clock =
LAZY_INSTANCE_INITIALIZER;
+enum UMAEventTimestampValidity {
+ UMA_EVENT_TIMESTAMPS_HAVE_INVALID_TIMEBASE,
+ UMA_EVENT_TIMESTAMPS_HAVE_VALID_TIMEBASE
+};
+
+} // namespace
+
// Takes Xlib Time and returns a time delta that is immune to timer rollover.
// This function is not thread safe as we do not use a lock.
base::TimeDelta TimeDeltaFromXEventTime(Time timestamp) {
@@ -320,6 +330,9 @@ base::TimeDelta TimeDeltaFromXEventTime(Time timestamp) {
if (!timestamp)
return base::TimeDelta();
+ if (g_bogus_x11_timestamps)
+ return ui::EventTimeForNow();
+
// If this is the first event that we get, assume the time stamp roll-over
// might have happened before the process was started.
// Register a rollover if the distance between last timestamp and current one
@@ -343,11 +356,21 @@ base::TimeDelta TimeDeltaFromXEventTime(Time timestamp) {
g_rollover_ms = now_ms & ~static_cast<int64_t>(UINT32_MAX);
uint32_t delta = static_cast<uint32_t>(now_ms - timestamp);
- // If using a mock clock, all bets are off -- in some tests, actual X11 events
- // come through with real timestamps.
- DCHECK(delta < 60 * 1000 || g_tick_clock.Get() != nullptr)
- << "Unexpected X11 event time, now: " << now_ticks
- << " event at: " << timestamp;
+ if (!g_tick_clock.Get()) {
+ if (delta > 60 * 1000) {
+ // x11 timestamps don't seem to be using the same time base as TimeTicks,
+ // so ignore them altogether and always use current time instead.
+ delta = 0;
+ g_bogus_x11_timestamps = true;
+ LOG(WARNING)
+ << "Unexpected x11 timestamps, will use browser time instead.";
+ }
+ UMA_HISTOGRAM_BOOLEAN("Event.TimestampHasValidTimebase",
+ g_bogus_x11_timestamps
+ ? UMA_EVENT_TIMESTAMPS_HAVE_INVALID_TIMEBASE
+ : UMA_EVENT_TIMESTAMPS_HAVE_VALID_TIMEBASE);
Ilya Sherman 2016/05/23 18:36:33 nit: No need to use an enum in the C++ code -- you
+ }
+
return base::TimeDelta::FromMilliseconds(now_ms - delta);
}
@@ -813,6 +836,7 @@ void ResetTimestampRolloverCountersForTesting(
std::unique_ptr<base::TickClock> tick_clock) {
g_last_seen_timestamp_ms = 0;
g_rollover_ms = 0;
+ g_bogus_x11_timestamps = false;
g_tick_clock.Get() = std::move(tick_clock);
}
« tools/metrics/histograms/histograms.xml ('K') | « tools/metrics/histograms/histograms.xml ('k') | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698