Revert "[Reland 1] Use accurate X11 event timestamp computation."""

This reverts commit 2c72cd9089bc3278bf3d2852c971325a9c07f097.

Reason for revert: bugs 915277 and 899881

Original change's description:
> [Reland 1] Use accurate X11 event timestamp computation.""
> 
> The original CL didn't allow negative time skew from the time server. While this
> is likely correct, this caused unit tests to fail as they relied on negative time
> skew. This CL allows negative time skew, and also removes another source of
> non-determinism from the tests.
> 
> Original change's description:
> > Use accurate X11 event timestamp computation.
> >
> > X events have a timestamp which is only well defined relative to the X11 Server
> > time. The previous computation for timestamp for X11 events was making the
> > assumption that Server time and Chrome time were the same. This assumption is
> > not always true -- this is likely the root cause of "bad" timestamps observed in
> > https://bugs.chromium.org/p/chromium/issues/detail?id=650338#c1
> >
> > This CL changes event timestamp computation to make a roundtrip to the X11
> > Server to get an accurate base::TimeTicks. This logic was lifted out of the
> > responsiveness calculator, which was already doing this computation. The latter
> > will subsequently be changed to use the computation in this CL.
> >
> > Change-Id: I963019cd8bfb8ce14e06b3743a159c9c85f2cb82
> > Bug: 859155
> > Reviewed-on: https://chromium-review.googlesource.com/1249383
> > Reviewed-by: Thomas Anderson <thomasanderson@chromium.org>
> > Reviewed-by: Avi Drissman <avi@chromium.org>
> > Commit-Queue: Erik Chen <erikchen@chromium.org>
> > Cr-Commit-Position: refs/heads/master@{#594844}
> 
> Change-Id: I6595f6aa163a18c9724c50546ac8c84bc5c84a03
> Bug: 859155
> Reviewed-on: https://chromium-review.googlesource.com/1252685
> Reviewed-by: Thomas Anderson <thomasanderson@chromium.org>
> Reviewed-by: Avi Drissman <avi@chromium.org>
> Commit-Queue: Erik Chen <erikchen@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#595454}

TBR=avi@chromium.org,erikchen@chromium.org,thomasanderson@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: 859155,899881,915277
Change-Id: Ib85a5f5d7bf9469076a334d77ef92771199d6a2d
Reviewed-on: https://chromium-review.googlesource.com/c/1382868
Reviewed-by: Thomas Anderson <thomasanderson@chromium.org>
Commit-Queue: Thomas Anderson <thomasanderson@chromium.org>
Cr-Commit-Position: refs/heads/master@{#617593}
diff --git a/ui/events/event_unittest.cc b/ui/events/event_unittest.cc
index 27fd65f..fee9fc1 100644
--- a/ui/events/event_unittest.cc
+++ b/ui/events/event_unittest.cc
@@ -25,7 +25,6 @@
 
 #if defined(USE_X11)
 #include "ui/events/test/events_test_utils_x11.h"
-#include "ui/events/x/events_x_utils.h"  // nogncheck
 #include "ui/gfx/x/x11.h"        // nogncheck
 #include "ui/gfx/x/x11_types.h"  // nogncheck
 #endif
@@ -431,15 +430,6 @@
 #if defined(USE_X11)
 namespace {
 
-class MockTimestampServer : public ui::TimestampServer {
- public:
-  Time GetCurrentServerTime() override { return base_time_; }
-  void SetBaseTime(Time time) { base_time_ = time; }
-
- private:
-  Time base_time_ = 0;
-};
-
 void SetKeyEventTimestamp(XEvent* event, int64_t time) {
   event->xkey.time = time & UINT32_MAX;
 }
@@ -450,29 +440,7 @@
 
 }  // namespace
 
-class X11EventTest : public testing::Test {
- public:
-  X11EventTest() {}
-  ~X11EventTest() override {}
-
-  void SetUp() override {
-    SetTimestampServer(&server_);
-    SetUseFixedTimeForXEventTesting(true);
-  }
-
-  void TearDown() override {
-    SetTimestampServer(nullptr);
-    SetUseFixedTimeForXEventTesting(false);
-  }
-
- protected:
-  MockTimestampServer server_;
-
- private:
-  DISALLOW_COPY_AND_ASSIGN(X11EventTest);
-};
-
-TEST_F(X11EventTest, AutoRepeat) {
+TEST(EventTest, AutoRepeat) {
   const uint16_t kNativeCodeA =
       ui::KeycodeConverter::DomCodeToNativeKeycode(DomCode::US_A);
   const uint16_t kNativeCodeB =
@@ -500,7 +468,6 @@
 
   int64_t ticks_base =
       (base::TimeTicks::Now() - base::TimeTicks()).InMilliseconds() - 5000;
-  server_.SetBaseTime(static_cast<Time>(ticks_base));
   SetKeyEventTimestamp(native_event_a_pressed, ticks_base);
   SetKeyEventTimestamp(native_event_a_pressed_1500, ticks_base + 1500);
   SetKeyEventTimestamp(native_event_a_pressed_3000, ticks_base + 3000);
diff --git a/ui/events/platform/x11/x11_event_source.cc b/ui/events/platform/x11/x11_event_source.cc
index a68188d..7e9f6df 100644
--- a/ui/events/platform/x11/x11_event_source.cc
+++ b/ui/events/platform/x11/x11_event_source.cc
@@ -100,7 +100,6 @@
       distribution_(0, 999) {
   DCHECK(!instance_);
   instance_ = this;
-  SetTimestampServer(this);
 
   DCHECK(delegate_);
   DCHECK(display_);
@@ -111,7 +110,6 @@
 X11EventSource::~X11EventSource() {
   DCHECK_EQ(this, instance_);
   instance_ = nullptr;
-  SetTimestampServer(nullptr);
   if (dummy_initialized_)
     XDestroyWindow(display_, dummy_window_);
 }
diff --git a/ui/events/platform/x11/x11_event_source.h b/ui/events/platform/x11/x11_event_source.h
index c2ee795..e818c4a 100644
--- a/ui/events/platform/x11/x11_event_source.h
+++ b/ui/events/platform/x11/x11_event_source.h
@@ -14,7 +14,6 @@
 #include "base/optional.h"
 #include "build/build_config.h"
 #include "ui/events/events_export.h"
-#include "ui/events/x/events_x_utils.h"
 #include "ui/gfx/x/x11_types.h"
 
 using Time = unsigned long;
@@ -47,7 +46,7 @@
 
 // Receives X11 events and sends them to X11EventSourceDelegate. Handles
 // receiving, pre-process and post-processing XEvents.
-class EVENTS_EXPORT X11EventSource : TimestampServer {
+class EVENTS_EXPORT X11EventSource {
  public:
   X11EventSource(X11EventSourceDelegate* delegate, XDisplay* display);
   ~X11EventSource();
@@ -83,7 +82,7 @@
 
   // Explicitly asks the X11 server for the current timestamp, and updates
   // |last_seen_server_time_| with this value.
-  Time GetCurrentServerTime() override;
+  Time GetCurrentServerTime();
 
  protected:
   // Extracts cookie data from |xevent| if it's of GenericType, and dispatches
diff --git a/ui/events/x/events_x.cc b/ui/events/x/events_x.cc
index 6253031..b8df7b0 100644
--- a/ui/events/x/events_x.cc
+++ b/ui/events/x/events_x.cc
@@ -81,7 +81,9 @@
 }
 
 base::TimeTicks EventTimeFromNative(const PlatformEvent& native_event) {
-  return EventTimeFromXEvent(*native_event);
+  base::TimeTicks timestamp = EventTimeFromXEvent(*native_event);
+  ValidateEventTimeClock(&timestamp);
+  return timestamp;
 }
 
 gfx::PointF EventLocationFromNative(const PlatformEvent& native_event) {
diff --git a/ui/events/x/events_x_unittest.cc b/ui/events/x/events_x_unittest.cc
index 3419309..897c5c5 100644
--- a/ui/events/x/events_x_unittest.cc
+++ b/ui/events/x/events_x_unittest.cc
@@ -76,15 +76,6 @@
   return rotation_angle;
 }
 
-class MockTimestampServer : public ui::TimestampServer {
- public:
-  Time GetCurrentServerTime() override { return base_time_; }
-  void SetBaseTime(Time time) { base_time_ = time; }
-
- private:
-  Time base_time_ = 0;
-};
-
 }  // namespace
 
 class EventsXTest : public testing::Test {
@@ -93,15 +84,13 @@
   ~EventsXTest() override {}
 
   void SetUp() override {
-    SetTimestampServer(&server_);
     DeviceDataManagerX11::CreateInstance();
     ui::TouchFactory::GetInstance()->ResetForTest();
+    ResetTimestampRolloverCountersForTesting();
   }
-
-  void TearDown() override { SetTimestampServer(nullptr); }
+  void TearDown() override { ResetTimestampRolloverCountersForTesting(); }
 
  private:
-  MockTimestampServer server_;
   DISALLOW_COPY_AND_ASSIGN(EventsXTest);
 };
 
@@ -553,4 +542,52 @@
   EXPECT_EQ(ui::ET_UNKNOWN, ui::EventTypeFromNative(xev));
 }
 
+namespace {
+
+// Returns a fake TimeTicks based on the given millisecond offset.
+base::TimeTicks TimeTicksFromMillis(int64_t millis) {
+  return base::TimeTicks() + base::TimeDelta::FromMilliseconds(millis);
+}
+
+}  // namespace
+
+TEST_F(EventsXTest, TimestampRolloverAndAdjustWhenDecreasing) {
+  XEvent event;
+  InitButtonEvent(&event, true, gfx::Point(5, 10), 1, 0);
+
+  test::ScopedEventTestTickClock clock;
+  clock.SetNowTicks(TimeTicksFromMillis(0x100000001));
+  ResetTimestampRolloverCountersForTesting();
+
+  event.xbutton.time = 0xFFFFFFFF;
+  EXPECT_EQ(TimeTicksFromMillis(0xFFFFFFFF), ui::EventTimeFromNative(&event));
+
+  clock.SetNowTicks(TimeTicksFromMillis(0x100000007));
+  ResetTimestampRolloverCountersForTesting();
+
+  event.xbutton.time = 3;
+  EXPECT_EQ(TimeTicksFromMillis(0x100000000 + 3),
+            ui::EventTimeFromNative(&event));
+}
+
+TEST_F(EventsXTest, NoTimestampRolloverWhenMonotonicIncreasing) {
+  XEvent event;
+  InitButtonEvent(&event, true, gfx::Point(5, 10), 1, 0);
+
+  test::ScopedEventTestTickClock clock;
+  clock.SetNowTicks(TimeTicksFromMillis(10));
+  ResetTimestampRolloverCountersForTesting();
+
+  event.xbutton.time = 6;
+  EXPECT_EQ(TimeTicksFromMillis(6), ui::EventTimeFromNative(&event));
+  event.xbutton.time = 7;
+  EXPECT_EQ(TimeTicksFromMillis(7), ui::EventTimeFromNative(&event));
+
+  clock.SetNowTicks(TimeTicksFromMillis(0x100000005));
+  ResetTimestampRolloverCountersForTesting();
+
+  event.xbutton.time = 0xFFFFFFFF;
+  EXPECT_EQ(TimeTicksFromMillis(0xFFFFFFFF), ui::EventTimeFromNative(&event));
+}
+
 }  // namespace ui
diff --git a/ui/events/x/events_x_utils.cc b/ui/events/x/events_x_utils.cc
index 8dec888..86fbd19 100644
--- a/ui/events/x/events_x_utils.cc
+++ b/ui/events/x/events_x_utils.cc
@@ -27,25 +27,6 @@
 
 namespace {
 
-ui::TimestampServer* g_timestamp_server = nullptr;
-bool g_use_fixed_time_for_testing = false;
-
-// Clamps a TimeDelta to be within [-30 seconds, 30 seconds].
-base::TimeDelta ClampDeltaFromExternalSource(const base::TimeDelta& delta) {
-  // Ignore pathologically long deltas. External source is probably having
-  // issues.
-  constexpr base::TimeDelta pathologically_long_duration =
-      base::TimeDelta::FromSeconds(30);
-  if (delta > pathologically_long_duration)
-    return base::TimeDelta();
-
-  // Ignore negative deltas. External source is probably having issues.
-  if (delta < -pathologically_long_duration)
-    return base::TimeDelta();
-
-  return delta;
-}
-
 // Scroll amount for each wheelscroll event. 53 is also the value used for GTK+.
 const int kWheelScrollAmount = 53;
 
@@ -327,36 +308,40 @@
   return true;
 }
 
+int64_t g_last_seen_timestamp_ms = 0;
+int64_t g_rollover_ms = 0;
+
+// 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::TimeTicks TimeTicksFromXEventTime(Time timestamp) {
-  // There's no way to convert from an X time to a base::TimeTicks without
-  // knowing the current X server time.
-  if (!g_timestamp_server)
-    return base::TimeTicks();
+  int64_t timestamp64 = timestamp;
 
-  // X11 uses a uint32_t on the wire protocol. Xlib casts this to an unsigned
-  // long by prepending with 0s. We cast back to a uint32_t so that subtraction
-  // works properly when the timestamp overflows back to 0.
-  uint32_t event_server_time_ms = static_cast<uint32_t>(timestamp);
-  uint32_t current_server_time_ms =
-      static_cast<uint32_t>(g_timestamp_server->GetCurrentServerTime());
+  if (!timestamp)
+    return ui::EventTimeForNow();
 
-  // On X11, event times are in X11 Server time. To convert to base::TimeTicks,
-  // we perform a round-trip to the X11 Server, subtract the two times to get a
-  // TimeDelta, and then subtract that from base::TimeTicks::Now(). Since we're
-  // working with units of time from an external source, we clamp the TimeDelta
-  // to reasonable values.
-  int64_t delta_ms = static_cast<int64_t>(current_server_time_ms) -
-                     static_cast<int64_t>(event_server_time_ms);
-  base::TimeDelta delta = base::TimeDelta::FromMilliseconds(delta_ms);
-  base::TimeDelta sanitized = ClampDeltaFromExternalSource(delta);
+  // 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
+  // is larger than half the width. This avoids false rollovers even in a case
+  // where X server delivers reasonably close events out-of-order.
+  bool had_recent_rollover =
+      !g_last_seen_timestamp_ms ||
+      g_last_seen_timestamp_ms - timestamp64 > (UINT32_MAX >> 1);
 
-  base::TimeTicks now;
-  if (g_use_fixed_time_for_testing)
-    now = base::TimeTicks() + base::TimeDelta::FromDays(1);
-  else
-    now = base::TimeTicks::Now();
+  g_last_seen_timestamp_ms = timestamp64;
+  if (!had_recent_rollover)
+    return base::TimeTicks() +
+        base::TimeDelta::FromMilliseconds(g_rollover_ms + timestamp);
 
-  return now - sanitized;
+  DCHECK(timestamp64 <= UINT32_MAX)
+      << "X11 Time does not roll over 32 bit, the below logic is likely wrong";
+
+  base::TimeTicks now_ticks = ui::EventTimeForNow();
+  int64_t now_ms = (now_ticks - base::TimeTicks()).InMilliseconds();
+
+  g_rollover_ms = now_ms & ~static_cast<int64_t>(UINT32_MAX);
+  uint32_t delta = static_cast<uint32_t>(now_ms - timestamp);
+  return base::TimeTicks() + base::TimeDelta::FromMilliseconds(now_ms - delta);
 }
 
 }  // namespace
@@ -846,16 +831,9 @@
   return XModifierStateWatcher::GetInstance()->state() & Mod1Mask;
 }
 
-void SetTimestampServer(TimestampServer* server) {
-  // This method must be setting or unsetting a timestamp server. It should
-  // never replace an existing timestamp server, nor change from
-  // nullptr->nullptr.
-  CHECK(!!g_timestamp_server ^ !!server);
-  g_timestamp_server = server;
-}
-
-void SetUseFixedTimeForXEventTesting(bool use_fixed_time) {
-  g_use_fixed_time_for_testing = use_fixed_time;
+void ResetTimestampRolloverCountersForTesting() {
+  g_last_seen_timestamp_ms = 0;
+  g_rollover_ms = 0;
 }
 
 }  // namespace ui
diff --git a/ui/events/x/events_x_utils.h b/ui/events/x/events_x_utils.h
index ad11a08..64814e8 100644
--- a/ui/events/x/events_x_utils.h
+++ b/ui/events/x/events_x_utils.h
@@ -16,8 +16,6 @@
 #include "ui/gfx/geometry/point.h"
 #include "ui/gfx/x/x11_types.h"
 
-using Time = unsigned long;
-
 namespace ui {
 
 // Gets the EventType from a XEvent.
@@ -93,18 +91,6 @@
 
 EVENTS_X_EXPORT void ResetTimestampRolloverCountersForTesting();
 
-// Conversion from X Time to base::TimeTicks requires checking the current X
-// Server Time. This functionality is provided by X11EventSource, but due to odd
-// layering that cannot be referenced directly.
-class TimestampServer {
- public:
-  virtual Time GetCurrentServerTime() = 0;
-};
-EVENTS_X_EXPORT void SetTimestampServer(TimestampServer* server);
-
-// Allows tests to force a fixed time..
-EVENTS_X_EXPORT void SetUseFixedTimeForXEventTesting(bool use_fixed_time);
-
 }  // namespace ui
 
 #endif  // UI_EVENTS_X_EVENTS_X_UTILS_H_