Initialize XInput2 on demand

base/message_pump_x.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 "base/message_pump_x.h"
 
 #include <X11/extensions/XInput2.h>
 
 #include "base/basictypes.h"
 #include "base/message_loop.h"
@@ skipping 19 matching lines @@
   return pump->DispatchXEvents();
 }
 
 GSourceFuncs XSourceFuncs = {
   XSourcePrepare,
   XSourceCheck,
   XSourceDispatch,
   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;
 
 // The default dispatcher to process native events when no dispatcher
 // 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;
   }
 
 #if defined(USE_XI2_MT)
   // USE_XI2_MT also defines the required XI2 minor minimum version.
   int major = 2, minor = USE_XI2_MT;
 #else
   int major = 2, minor = 0;
 #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
 
 namespace base {
 
 MessagePumpX::MessagePumpX() : MessagePumpGlib(),
     x_source_(NULL) {
-  InitializeXInput2();
+  InitializeXInput2IfNeeded();
   InitXSource();
 }
 
 MessagePumpX::~MessagePumpX() {
   g_source_destroy(x_source_);
   g_source_unref(x_source_);
   XCloseDisplay(g_xdisplay);
   g_xdisplay = NULL;
 }
 
 // static
 Display* MessagePumpX::GetDefaultXDisplay() {
   if (!g_xdisplay)
     g_xdisplay = XOpenDisplay(NULL);
   return g_xdisplay;
 }
 
 // static
 bool MessagePumpX::HasXInput2() {
-  return xiopcode != -1;
+  InitializeXInput2IfNeeded();
+  return xinput2_supported;
 }
 
 // static
 void MessagePumpX::SetDefaultDispatcher(MessagePumpDispatcher* dispatcher) {
   DCHECK(!g_default_dispatcher || !dispatcher);
   g_default_dispatcher = dispatcher;
 }
 
 void MessagePumpX::InitXSource() {
   // CHECKs are to help track down crbug.com/113106.
@@ skipping 64 matching lines @@
       return true;
   }
   return false;
 }
 
 void MessagePumpX::DidProcessXEvent(XEvent* xevent) {
   FOR_EACH_OBSERVER(MessagePumpObserver, observers(), DidProcessEvent(xevent));
 }
 
 }  // namespace base