Fixed the Touchscreen.TouchEventsEnabled histogram to record the correct values on X11 and Ozone ba…

chrome/browser/metrics/chrome_browser_main_extra_parts_metrics.cc

Index: chrome/browser/metrics/chrome_browser_main_extra_parts_metrics.cc
diff --git a/chrome/browser/metrics/chrome_browser_main_extra_parts_metrics.cc b/chrome/browser/metrics/chrome_browser_main_extra_parts_metrics.cc
index 3680ad957d6ce1a913d5c6a374e9ab86f5a7d0dd..3454083018c2cfc1d6c1551bb08bc6dd121320a0 100644
--- a/chrome/browser/metrics/chrome_browser_main_extra_parts_metrics.cc
+++ b/chrome/browser/metrics/chrome_browser_main_extra_parts_metrics.cc
@@ -23,6 +23,7 @@
 #include "content/public/browser/browser_thread.h"
 #include "ui/base/touch/touch_device.h"
 #include "ui/base/ui_base_switches.h"
+#include "ui/events/devices/device_data_manager.h"
 #include "ui/events/event_switches.h"
 #include "ui/gfx/screen.h"
 
@@ -236,7 +237,6 @@ void RecordTouchEventState() {
     NOTREACHED();
     return;
   }
-
   UMA_HISTOGRAM_ENUMERATION("Touchscreen.TouchEventsEnabled", state,
                             UMA_TOUCH_EVENTS_STATE_COUNT);
 }
@@ -250,6 +250,9 @@ ChromeBrowserMainExtraPartsMetrics::ChromeBrowserMainExtraPartsMetrics()
 ChromeBrowserMainExtraPartsMetrics::~ChromeBrowserMainExtraPartsMetrics() {
   if (is_screen_observer_)
     gfx::Screen::GetNativeScreen()->RemoveObserver(this);
+#if defined(USE_OZONE) || defined(USE_X11)
+  ui::DeviceDataManager::GetInstance()->RemoveObserver(this);
+#endif  // defined(USE_OZONE) || defined(USE_X11)
 }
 
 void ChromeBrowserMainExtraPartsMetrics::PreProfileInit() {
@@ -269,7 +272,15 @@ void ChromeBrowserMainExtraPartsMetrics::PostBrowserStart() {
                             GetLinuxWindowManager(),
                             UMA_LINUX_WINDOW_MANAGER_COUNT);
 #endif
+
+#if defined(USE_OZONE) || defined(USE_X11)

[Rick Byers, 2015/06/16 14:45:47]

Rather than use an #if, couldn't we encapsulate this into DeviceDataManager? 
Eg. a 'areDeviceListsComplete' method?  Wouldn't that also prevent a race on
OZONE/X11 where OnDeviceListsComplete occurred before we happened to get our
observer installed?

[bruthig, 2015/06/16 15:52:45]

Agreed I'm not happy about the 'potential' race condition either, but IIUC, and
spang@ can confirm, the race condition does not exist in the current
implementation.  It would only be introduced if the point at which attaching the
observer changed or if the spot where the device scan is started is changed.

Having said that, from what I can tell the DeviceDataManager is not used (or
created) on the Windows or Android platforms (see IsTouchDevicePresent()
implementations here
https://code.google.com/p/chromium/codesearch#chromium/src/ui/base/touch/touc...
and here
https://code.google.com/p/chromium/codesearch#chromium/src/ui/base/touch/touc...).
 So adding a DeviceDataManager::AreDeviceListsComplete() method is not
sufficient.

IMO the check to determine if the return value for IsTouchDevicePresent() is
valid should live in the same spot as IsTouchDevicePresent() not in
DeviceDataManager.  But this is not trivial to implement since
IsTouchDevicePresent() is a static/global function and for the X11 and Ozone
platforms it requires the asynchronous observer.

If we want to address this concern I think the right thing to do would be to
turn touch_devices.h into a proper class and each platform can have have their
own implementation.

Thoughts?

[spang, 2015/06/16 15:55:40]

A race free way to do it is

  if (ui::DeviceDataManager::GetInstance()->device_lists_complete())
    OnDeviceListsComplete();
  else
    ui::DeviceDataManager::GetInstance()->AddObserver(this);

+  // The touch event state for X11 and Ozone based event sub-systems are based
+  // on device scans that happen asynchronously so we attach an observer to
+  // wait until these scans complete.
+  ui::DeviceDataManager::GetInstance()->AddObserver(this);
+#else
   RecordTouchEventState();
+#endif  // defined(USE_OZONE) || defined(USE_X11)
 
 #if defined(OS_MACOSX) && !defined(OS_IOS)
   RecordMacMetrics();
@@ -311,6 +322,11 @@ void ChromeBrowserMainExtraPartsMetrics::ProfilerFinishedCollectingMetrics() {
   first_web_contents_profiler_.reset();
 }
 
+void ChromeBrowserMainExtraPartsMetrics::OnDeviceListsComplete() {
+  ui::DeviceDataManager::GetInstance()->RemoveObserver(this);
+  RecordTouchEventState();
+}
+
 void ChromeBrowserMainExtraPartsMetrics::EmitDisplaysChangedMetric() {
   int display_count = gfx::Screen::GetNativeScreen()->GetNumDisplays();
   if (display_count != display_count_) {