Expand event timestamp validation check to more platforms

ui/events/event_utils.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "ui/events/event_utils.h"
 
 #include <vector>
 
 #include "base/metrics/histogram.h"
 #include "ui/display/display.h"
@@ skipping 44 matching lines @@
       break;
   }
   return event;
 }
 
 int RegisterCustomEventType() {
   return ++g_custom_event_types;
 }
 
 void ValidateEventTimeClock(base::TimeTicks* timestamp) {
-// Restrict this validation to DCHECK builds except when using X11 which is
-// known to provide bogus timestamps that require correction (crbug.com/611950).
-#if defined(USE_X11) || DCHECK_IS_ON()
   if (base::debug::BeingDebugged())
     return;
 
   base::TimeTicks now = EventTimeForNow();
   int64_t delta = (now - *timestamp).InMilliseconds();

[Ilya Sherman, 2016/09/26 19:04:44]

nit: Please write this as

bool has_valid_timebase = delta >= 0 && delta <= 60 * 1000;
UMA_HISTOGRAM_BOOLEAN("Event.TimestampHasValidTimebase.Browser",
has_valid_timebase);
if (!has_valid_timebase) {
  ...
}

[majidvp, 2016/10/11 16:02:07]

Done.

   if (delta < 0 || delta > 60 * 1000) {
-    UMA_HISTOGRAM_BOOLEAN("Event.TimestampHasValidTimebase", false);
+    UMA_HISTOGRAM_BOOLEAN("Event.TimestampHasValidTimebase.Browser", false);

[Ilya Sherman, 2016/09/26 19:04:44]

Where is the .Renderer version of this histogram?

[majidvp, 2016/10/11 16:02:07]

It is more involved so I am working on adding it in a separate patch here:
https://codereview.chromium.org/1936853002

[Ilya Sherman, 2016/10/11 23:54:08]

In general, it's best to modify histograms.xml in precisely the same CL as the
one where the new metric is being added.

[majidvp, 2016/10/12 13:44:21]

I thought it will be odd to have a histogram_suffixes with a 
single suffix. But if that is acceptable I will change.

[Ilya Sherman, 2016/10/13 00:40:17]

It's acceptable, yeah.  It's not a big deal either way in this particular case,
though: Mainly the reason that it helps to change histograms.xml in the same CL
is that it loops in a metrics reviewer, and we look at the C++ code as well as
the XML changes.  In this case I've already glanced at your follow-up CL and it
looks fine, so there's not necessarily a need for the additional metrics review
there.

 #if defined(USE_X11)
+    // Restrict this correction to X11 which is known to provide bogus
+    // timestamps that require correction (crbug.com/611950).
     *timestamp = now;
-#else
-    NOTREACHED() << "Unexpected event timestamp, now:" << now
-                 << " event timestamp:" << *timestamp;
 #endif
+  } else {
+    UMA_HISTOGRAM_BOOLEAN("Event.TimestampHasValidTimebase.Browser", true);
   }
-
-  UMA_HISTOGRAM_BOOLEAN("Event.TimestampHasValidTimebase", true);
-#endif
 }
 
 bool ShouldDefaultToNaturalScroll() {
   return GetInternalDisplayTouchSupport() ==
          display::Display::TOUCH_SUPPORT_AVAILABLE;
 }
 
 display::Display::TouchSupport GetInternalDisplayTouchSupport() {
   display::Screen* screen = display::Screen::GetScreen();
   // No screen in some unit tests.
@@ skipping 30 matching lines @@
     case ET_TOUCH_RELEASED:
       UMA_HISTOGRAM_CUSTOM_COUNTS("Event.Latency.OS.TOUCH_RELEASED",
                                   delta.InMicroseconds(), 1, 1000000, 50);
       return;
     default:
       return;
   }
 }
 
 }  // namespace ui