Initialize XInput2 on demand

base/message_pump_x.cc

Index: base/message_pump_x.cc
diff --git a/base/message_pump_x.cc b/base/message_pump_x.cc
index e7deff0f602316625093be29971493743f3acf22..2e05cb398a05493258bb9d8218738444b67bb29f 100644
--- a/base/message_pump_x.cc
+++ b/base/message_pump_x.cc
@@ -37,8 +37,9 @@ GSourceFuncs XSourceFuncs = {
   NULL
 };
 
-// The opcode used for checking events.
-int xiopcode = -1;
+// The status of XInput2 initialization.
+bool xinput2_initialized = false;

[Mark Mentovai, 2012/05/17 14:17:34]

This can just be a static bool inside InitializeXInput2IfNeeded…

+bool xinput2_supported = false;
 
 // The message-pump opens a connection to the display and owns it.
 Display* g_xdisplay = NULL;
@@ -47,16 +48,20 @@ Display* g_xdisplay = NULL;
 // is specified.
 base::MessagePumpDispatcher* g_default_dispatcher = NULL;
 
-void InitializeXInput2(void) {
+void InitializeXInput2IfNeeded(void) {
+  if (xinput2_initialized)

[Mark Mentovai, 2012/05/17 14:17:34]

…but this pattern is a little bit of a pet peeve. You’re effectively using one
static (xinput2_initialized) as a guard variable around another static
(xinput2_supported), but statics can already have their own guard variables that
the compiler spits out automatically, so why not just leverage that?

Here’s how I would usually write this pattern:

bool InitializeXInput2Internal() {
  Display* display = base::MessagePumpX::GetDefaultXDisplay();
  // blah blah blah, returning true if Xinput2 is supported, and false if not.
}

bool InitializeXInput2() {
  static bool xinput2_supported = InitializeXInput2Internal();
  return xinput2_supported;
}

Now you only have one static, it tracks whether Xinput2 is supported, it lives
right in InitializeXInput2, and the compiler gives you the guard variable for it
so that its initializer (InitializeXInput2Internal) is only called once.

[Rick Byers, 2012/05/17 15:13:15]

I agree this is better, thanks!  Done.

+    return;
+  xinput2_initialized = true;
+
   Display* display = base::MessagePumpX::GetDefaultXDisplay();
   if (!display)
     return;
 
   int event, err;
 
+  int xiopcode;
   if (!XQueryExtension(display, "XInputExtension", &xiopcode, &event, &err)) {
     DVLOG(1) << "X Input extension not available.";
-    xiopcode = -1;
     return;
   }
 
@@ -68,17 +73,17 @@ void InitializeXInput2(void) {
 #endif
   if (XIQueryVersion(display, &major, &minor) == BadRequest) {
     DVLOG(1) << "XInput2 not supported in the server.";
-    xiopcode = -1;
     return;
   }
 #if defined(USE_XI2_MT)
   if (major < 2 || (major == 2 && minor < USE_XI2_MT)) {
     DVLOG(1) << "XI version on server is " << major << "." << minor << ". "
             << "But 2." << USE_XI2_MT << " is required.";
-    xiopcode = -1;
     return;
   }
 #endif
+
+  xinput2_supported = true;
 }
 
 }  // namespace
@@ -87,7 +92,7 @@ namespace base {
 
 MessagePumpX::MessagePumpX() : MessagePumpGlib(),
     x_source_(NULL) {
-  InitializeXInput2();
+  InitializeXInput2IfNeeded();
   InitXSource();
 }
 
@@ -107,7 +112,8 @@ Display* MessagePumpX::GetDefaultXDisplay() {
 
 // static
 bool MessagePumpX::HasXInput2() {
-  return xiopcode != -1;
+  InitializeXInput2IfNeeded();
+  return xinput2_supported;
 }
 
 // static