exo: Implement wayland gamepad support

components/exo/gamepads.h

+// Copyright 2016 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.
+
+#ifndef COMPONENTS_EXO_GAMEPADS_H_
+#define COMPONENTS_EXO_GAMEPADS_H_
+
+#include <memory>
+
+#include "base/macros.h"
+#include "base/sequenced_task_runner.h"
+#include "base/threading/thread.h"
+#include "base/threading/thread_task_runner_handle.h"
+#include "third_party/WebKit/public/platform/WebGamepads.h"
+
+namespace device {
+class GamepadDataFetcher;
+}
+
+namespace exo {
+class GamepadsDelegate;
+
+// This class represents multiple gamepads, it implements a background thread
+// for polling gamepad devices and notifies the GamepadsDelegate of any
+// changes.
+class Gamepads {

[reveman, 2016/06/27 17:30:35]

I've made a similar comment on the interface change already. Why not singular
Gamepad to be consistent with Pointer, Keyboard, Touch?

[denniskempin, 2016/06/29 17:12:26]

Done.

+ public:
+  // This class will post tasks to invoke the delegate on the thread runner
+  // which is associated with the thread that is creating this instance.
+  Gamepads(GamepadsDelegate* delegate);
+
+  // Allows test cases to specify a GamepadDataFetcher to provide mock gamepad
+  // data.
+  Gamepads(GamepadsDelegate* delegate,
+           std::unique_ptr<device::GamepadDataFetcher> fetcher);

[reveman, 2016/06/27 17:30:35]

Add SetGamepadDataFetcherForTesting function would e preferred to make it more
clear that this is only for testing purposes.

[denniskempin, 2016/06/29 17:12:25]

Not sure we can do that since this class will initialize a platform-based
fetcher in the constructor, which we don't want it to do for tests. I am
following the same layout as content/browser/GamepadService here. 

We could make this constructor private and add a static method along the lines
of CreateWithGamepadDataFetcherForTesting 

Or another alternative.. don't initialize in the constructor but provide a
separate Initialize() method that allow us to use
SetGamepadDataFetcherForTesting before initialization. But generally I'd like to
follow the practice of having the constructor initialize. 

Let me know what you prefer. 

+  virtual ~Gamepads();
+
+ private:
+  // Schedules a poll on the polling thread. Can be called from any thread.
+  void ScheduleOnPoll();
+
+  // Polls devices for new data and posts delegate updates.
+  // Can only be called on the polling thread.
+  void OnPoll();
+
+  // Initializes the underlying gamepad data fetcher.
+  // Can only be called on the polling thread.
+  void InitializePollingThread();
+
+  // The delegate instance that all events are dispatched to.
+  GamepadsDelegate* delegate_;

[reveman, 2016/06/27 17:30:35]

nit: GamepadsDelegate* const delegate_;

[denniskempin, 2016/06/29 17:12:25]

Done.

+
+  // Implements the logic to fetch gamepad information from connected devices.
+  std::unique_ptr<device::GamepadDataFetcher> fetcher_;
+
+  // The thread on which the polling is executed.
+  std::unique_ptr<base::Thread> polling_thread_;

[reveman, 2016/06/27 17:30:35]

Do we need to add a thread for this? Can we use an existing thread instead?
Maybe just pass a gamepad_data_fetch_task_runner to exo::Display ctor and have
the code here be agnostic to what thread that is..

[denniskempin, 2016/06/29 17:12:26]

I've been following the code of GamepadService here as well. I think it's a good
idea to use a separate thread for two reasons:
a) consistency of polling. We don't want the poll to get delayed too much
because something else is running on the task runner. 
b) eventually I'd like to move towards blocking IO (or selecting fd's) instead
of polling. The gamepads api can't do that since not all platforms allow
blocking IO for gamepads, but since we run on linux only, we are free to do so. 

+
+  // The current state of gamepads.
+  blink::WebGamepads state_;
+
+  // Reference to the task runner of the thread that created this instance.
+  scoped_refptr<base::SingleThreadTaskRunner> origin_task_runner_;
+
+  base::ThreadChecker thread_checker_;
+  DISALLOW_COPY_AND_ASSIGN(Gamepads);

[reveman, 2016/06/27 17:30:35]

nit: blank line before this

[denniskempin, 2016/06/29 17:12:26]

Done.

+};
+
+}  // namespace exo
+
+#endif  // COMPONENTS_EXO_GAMEPADS_H_