X11: Better timestamp handling for _NET_ACTIVE_WINDOW

Main changes introduced:
1. Get the X server timestamp in the newly created browser process and
send it over IPC to the old process to update wm_user_time_ms

2. Add logic in set_wm_user_time_ms to avoid setting the time to 0 (or
backwards)

3. If wm_user_time_ms == 0 and we try to activate a window, grab the
server time first

BUG=593516,608521,379615

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

chrome/browser/chrome_browser_main_extra_parts_x11.cc

@@ -5,15 +5,19 @@
 #include "chrome/browser/chrome_browser_main_extra_parts_x11.h"
 
 #include "base/bind.h"
+#include "base/command_line.h"
 #include "base/debug/debugger.h"
 #include "base/location.h"
 #include "base/sequenced_task_runner.h"
+#include "base/strings/string_number_conversions.h"
 #include "base/threading/sequenced_task_runner_handle.h"
 #include "chrome/browser/lifetime/application_lifetime.h"
 #include "chrome/common/chrome_result_codes.h"
+#include "chrome/common/chrome_switches.h"
 #include "content/public/browser/browser_thread.h"
 #include "ui/base/x/x11_util.h"
 #include "ui/base/x/x11_util_internal.h"
+#include "ui/events/platform/x11/x11_event_source.h"
 
 using content::BrowserThread;
 
@@ -93,6 +97,17 @@ void ChromeBrowserMainExtraPartsX11::PostMainMessageLoopStart() {
   // main message loop has started. This will allow us to exit cleanly
   // if X exits before us.
   ui::SetX11ErrorHandlers(BrowserX11ErrorHandler, BrowserX11IOErrorHandler);
+
+#if !defined(OS_CHROMEOS)
+  // Get a timestamp from the X server.  This makes our requests to the server
+  // less likely to be thrown away by the window manager.  Put the timestamp in
+  // a command line flag so we can forward it to an existing browser process if
+  // necessary.
+  base::CommandLine::ForCurrentProcess()->AppendSwitchASCII(
+      switches::kWmUserTimeMs,
+      base::Uint64ToString(
+          ui::X11EventSource::GetInstance()->UpdateLastSeenServerTime()));
+#endif
 }
 
 void ChromeBrowserMainExtraPartsX11::PostMainMessageLoopRun() {

chrome/browser/ui/startup/startup_browser_creator.cc

@@ -103,6 +103,10 @@
 #include "components/search_engines/desktop_search_utils.h"
 #endif
 
+#if defined(OS_LINUX) && !defined(OS_CHROMEOS)
+#include "ui/views/widget/desktop_aura/x11_desktop_handler.h"
+#endif
+
 #if defined(ENABLE_PRINT_PREVIEW)
 #include "chrome/browser/printing/cloud_print/cloud_print_proxy_service.h"
 #include "chrome/browser/printing/cloud_print/cloud_print_proxy_service_factory.h"
@@ -715,6 +719,21 @@ bool StartupBrowserCreator::ProcessCmdLineImpl(
   }
 #endif  // defined(OS_WIN)
 
+#if defined(OS_LINUX) && !defined(OS_CHROMEOS)
+  // Our request to Activate may be discarded on some linux window
+  // managers unless given a recent timestamp, so update the timestamp if
+  // we were given one.
+  if (command_line.HasSwitch(switches::kWmUserTimeMs)) {
+    uint64_t time;
+    std::string switch_value =
+        command_line.GetSwitchValueASCII(switches::kWmUserTimeMs);
+    if (base::StringToUint64(switch_value, &time)) {
+      views::X11DesktopHandler::get()->set_wm_user_time_ms(
+          static_cast<Time>(time));
+    }
+  }
+#endif
+
   chrome::startup::IsProcessStartup is_process_startup = process_startup ?
       chrome::startup::IS_PROCESS_STARTUP :
       chrome::startup::IS_NOT_PROCESS_STARTUP;
@@ -751,7 +770,6 @@ bool StartupBrowserCreator::ProcessCmdLineImpl(
       last_used_profile = last_opened_profiles[0];
     }
 #endif
-
     // Launch the last used profile with the full command line, and the other
     // opened profiles without the URLs to launch.
     base::CommandLine command_line_without_urls(command_line.GetProgram());
@@ -775,7 +793,6 @@ bool StartupBrowserCreator::ProcessCmdLineImpl(
           startup_pref.type == SessionStartupPref::DEFAULT &&
           !HasPendingUncleanExit(*it))
         continue;
-
       if (!LaunchBrowser((*it == last_used_profile) ? command_line
                                                     : command_line_without_urls,
                          *it, cur_dir, is_process_startup, is_first_run))

chrome/common/chrome_switches.cc

@@ -1139,6 +1139,9 @@ const char kHelpShort[]                     = "h";
 
 // Specifies which password store to use (detect, default, gnome, kwallet).
 const char kPasswordStore[]                 = "password-store";
+
+// Updates X11Desktophandler::wm_user_time_ms with the latest X server time.
+const char kWmUserTimeMs[]              = "wm-user-time-ms";
 #endif
 
 #if defined(OS_MACOSX)

chrome/common/chrome_switches.h

@@ -332,6 +332,7 @@ extern const char kOpenAsh[];
 extern const char kHelp[];
 extern const char kHelpShort[];
 extern const char kPasswordStore[];
+extern const char kWmUserTimeMs[];
 #endif
 
 #if defined(OS_MACOSX)

tools/metrics/histograms/histograms.xml

@@ -12341,6 +12341,12 @@ http://cs/file:chrome/histograms.xml - but prefer this file for new entries.
   </summary>
 </histogram>
 
+<histogram name="Event.Latency.X11EventSource.UpdateServerTime"
+    units="microseconds">
+  <owner>thomasanderson@chromium.org</owner>
+  <summary>Time to request a timestamp from the X server.</summary>
+</histogram>
+
 <histogram name="Event.PassiveListeners" enum="EventResultType">
   <owner>dtapuska@chromium.org</owner>
   <summary>

ui/events/platform/x11/x11_event_source.cc

@@ -4,10 +4,12 @@
 
 #include "ui/events/platform/x11/x11_event_source.h"
 
+#include <X11/Xatom.h>
 #include <X11/XKBlib.h>
 #include <X11/Xlib.h>
 
 #include "base/logging.h"
+#include "base/metrics/histogram_macros.h"
 #include "ui/events/devices/x11/device_data_manager_x11.h"
 #include "ui/events/devices/x11/touch_factory_x11.h"
 #include "ui/events/event_utils.h"
@@ -78,6 +80,13 @@ void UpdateDeviceList() {
   DeviceDataManagerX11::GetInstance()->UpdateDeviceList(display);
 }
 
+Bool IsPropertyNotifyForTimestamp(Display* display,
+                                  XEvent* event,
+                                  XPointer arg) {
+  return event->type == PropertyNotify &&
+         event->xproperty.window == *reinterpret_cast<Window*>(arg);
+}
+
 }  // namespace
 
 X11EventSource* X11EventSource::instance_ = nullptr;
@@ -87,6 +96,7 @@ X11EventSource::X11EventSource(X11EventSourceDelegate* delegate,
     : delegate_(delegate),
       display_(display),
       last_seen_server_time_(CurrentTime),
+      dummy_initialized_(false),
       continue_stream_(true) {
   DCHECK(!instance_);
   instance_ = this;
@@ -100,6 +110,8 @@ X11EventSource::X11EventSource(X11EventSourceDelegate* delegate,
 X11EventSource::~X11EventSource() {
   DCHECK_EQ(this, instance_);
   instance_ = nullptr;
+  if (dummy_initialized_)
+    XDestroyWindow(display_, dummy_window_);
 }
 
 // static
@@ -134,6 +146,38 @@ void X11EventSource::BlockUntilWindowMapped(XID window) {
   } while (event.type != MapNotify);
 }
 
+Time X11EventSource::UpdateLastSeenServerTime() {
+  base::TimeTicks start = base::TimeTicks::Now();
+
+  DCHECK(display_);
+
+  if (!dummy_initialized_) {
+    // Create a new Window and Atom that will be used for the property change.
+    dummy_window_ = XCreateSimpleWindow(display_, DefaultRootWindow(display_),
+                                        0, 0, 1, 1, 0, 0, 0);
+    dummy_atom_ = XInternAtom(display_, "CHROMIUM_TIMESTAMP", False);
+    XSelectInput(display_, dummy_window_, PropertyChangeMask);
+    dummy_initialized_ = true;
+  }
+
+  // Make a no-op property change on |dummy_window_|.
+  XChangeProperty(display_, dummy_window_, dummy_atom_, XA_STRING, 8,
+                  PropModeAppend, nullptr, 0);
+
+  // Observe the resulting PropertyNotify event to obtain the timestamp.
+  XEvent event;
+  XIfEvent(display_, &event, IsPropertyNotifyForTimestamp,
+           reinterpret_cast<XPointer>(&dummy_window_));
+
+  last_seen_server_time_ = event.xproperty.time;
+
+  UMA_HISTOGRAM_CUSTOM_COUNTS(
+      "Event.Latency.X11EventSource.UpdateServerTime",
+      (base::TimeTicks::Now() - start).InMicroseconds(), 0,
+      base::TimeDelta::FromMilliseconds(1).InMicroseconds(), 50);
+  return last_seen_server_time_;
+}
+
 ////////////////////////////////////////////////////////////////////////////////
 // X11EventSource, protected
 

ui/events/platform/x11/x11_event_source.h

@@ -16,6 +16,7 @@
 using Time = unsigned long;
 using XEvent = union _XEvent;
 using XID = unsigned long;
+using XWindow = unsigned long;
 
 namespace ui {
 
@@ -62,6 +63,10 @@ class EVENTS_EXPORT X11EventSource {
   XDisplay* display() { return display_; }
   Time last_seen_server_time() const { return last_seen_server_time_; }
 
+  // Explicitly asks the X11 server for the current timestamp, and updates
+  // |last_seen_server_time| with this value.
+  Time UpdateLastSeenServerTime();
+
   void StopCurrentEventStream();
   void OnDispatcherListChanged();
 
@@ -85,6 +90,11 @@ class EVENTS_EXPORT X11EventSource {
   // The last timestamp seen in an XEvent.
   Time last_seen_server_time_;
 
+  // State necessary for UpdateLastSeenServerTime
+  bool dummy_initialized_;
+  XWindow dummy_window_;
+  XAtom dummy_atom_;
+
   // Keeps track of whether this source should continue to dispatch all the
   // available events.
   bool continue_stream_ = true;

ui/views/widget/desktop_aura/x11_desktop_handler.cc

@@ -43,7 +43,7 @@ X11DesktopHandler::X11DesktopHandler()
     : xdisplay_(gfx::GetXDisplay()),
       x_root_window_(DefaultRootWindow(xdisplay_)),
       x_active_window_(None),
-      wm_user_time_ms_(0),
+      wm_user_time_ms_(CurrentTime),
       current_window_(None),
       current_window_active_state_(NOT_ACTIVE),
       atom_cache_(xdisplay_, kAtomsToCache),
@@ -91,6 +91,10 @@ void X11DesktopHandler::ActivateWindow(::Window window) {
 
     // If the window is not already active, send a hint to activate it
     if (x_active_window_ != window) {
+      if (wm_user_time_ms_ == CurrentTime) {
+        set_wm_user_time_ms(
+            ui::X11EventSource::GetInstance()->UpdateLastSeenServerTime());
+      }
       XEvent xclient;
       memset(&xclient, 0, sizeof(xclient));
       xclient.type = ClientMessage;
@@ -120,6 +124,18 @@ void X11DesktopHandler::ActivateWindow(::Window window) {
   }
 }
 
+void X11DesktopHandler::set_wm_user_time_ms(Time time_ms) {
+  if (time_ms != CurrentTime) {
+    int64_t event_time_64 = time_ms;
+    int64_t time_difference = wm_user_time_ms_ - event_time_64;
+    // Ignore timestamps that go backwards. However, X server time is a 32-bit
+    // millisecond counter, so if the time goes backwards by more than half the
+    // range of the 32-bit counter, treat it as a rollover.
+    if (time_difference < 0 || time_difference > (UINT32_MAX >> 1))
+      wm_user_time_ms_ = time_ms;
+  }
+}
+
 void X11DesktopHandler::DeactivateWindow(::Window window) {
   if (!IsActiveWindow(window))
     return;

ui/views/widget/desktop_aura/x11_desktop_handler.h

@@ -15,6 +15,7 @@
 #include "base/macros.h"
 #include "ui/aura/env_observer.h"
 #include "ui/events/platform/platform_event_dispatcher.h"
+#include "ui/events/platform/x11/x11_event_source.h"
 #include "ui/gfx/x/x11_atom_cache.h"
 #include "ui/gfx/x/x11_types.h"
 #include "ui/views/views_export.h"
@@ -36,12 +37,8 @@ class VIEWS_EXPORT X11DesktopHandler : public ui::PlatformEventDispatcher,
 
   // Gets/sets the X11 server time of the most recent mouse click, touch or
   // key press on a Chrome window.
-  int wm_user_time_ms() const {
-    return wm_user_time_ms_;
-  }
-  void set_wm_user_time_ms(unsigned long time_ms) {
-    wm_user_time_ms_ = time_ms;
-  }
+  int wm_user_time_ms() const { return wm_user_time_ms_; }
+  void set_wm_user_time_ms(Time time_ms);
 
   // Sends a request to the window manager to activate |window|.
   // This method should only be called if the window is already mapped.
@@ -95,7 +92,7 @@ class VIEWS_EXPORT X11DesktopHandler : public ui::PlatformEventDispatcher,
 
   // The X11 server time of the most recent mouse click, touch, or key press
   // on a Chrome window.
-  unsigned long wm_user_time_ms_;
+  Time wm_user_time_ms_;
 
   // The active window according to X11 server.
   ::Window current_window_;