Allow gaming_seat to use ozone gamepad as back-end

components/exo/gaming_seat.h

Index: components/exo/gaming_seat.h
diff --git a/components/exo/gaming_seat.h b/components/exo/gaming_seat.h
index 40e44ae33352eeb4cf86c473177ce37d6e52c548..b523e5874557e12c4d5d7e0a6d2221a7d9bd02cb 100644
--- a/components/exo/gaming_seat.h
+++ b/components/exo/gaming_seat.h
@@ -13,10 +13,23 @@
 #include "base/synchronization/lock.h"
 #include "base/threading/thread.h"
 #include "base/threading/thread_task_runner_handle.h"
+#include "components/exo/gaming_seat.h"

[reveman, 2017/06/05 22:48:04]

avoid including yourself

[jkwang, 2017/06/06 20:03:07]

Sorry. Copy/paste mistake.
Done.

 #include "components/exo/wm_helper.h"
 #include "device/gamepad/gamepad_data_fetcher.h"
 #include "ui/aura/client/focus_change_observer.h"
 
+#if defined(USE_OZONE_GAMEPAD)
+#include "ui/events/ozone/gamepad/gamepad_observer.h"
+
+namespace ui {
+class GamepadProviderOzone;
+}
+#endif
+
+namespace base {
+class Thread;
+}
+
 namespace exo {
 class GamingSeatDelegate;
 class GamepadDelegate;
@@ -24,29 +37,57 @@ class GamepadDelegate;
 using CreateGamepadDataFetcherCallback =
     base::Callback<std::unique_ptr<device::GamepadDataFetcher>()>;
 
-// This class represents one gaming seat, it uses a background thread
-// for polling gamepad devices and notifies the corresponding GampadDelegate of
-// any changes.
-class GamingSeat : public WMHelper::FocusObserver {
+// TODO(jkwang) always use ozone_gamepad when ozone is default for all chrome os
+// build.
+class GamingSeat : public WMHelper::FocusObserver
+#if defined(USE_OZONE_GAMEPAD)
+                   ,
+                   public ui::GamepadObserver
+#endif
+{
  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.
+  // This function will return pointer to polling thread if it will be needed by
+  // gaming_seat. Otherwise, it will return nullptr. The pointer should be later
+  // used to construct GamingSeat.
+  static base::Thread* CreatePollingThreadIfNeeded();

[reveman, 2017/06/05 22:48:03]

please remove this and always create this thread as before. thread has minimal
cost when not used.

[jkwang, 2017/06/06 20:03:07]

Done.

+
+  // This class will monitor gamepad connection change and manage gamepad
+  // returned by gaming_seat_delegate.
   GamingSeat(GamingSeatDelegate* gaming_seat_delegate,
-             base::SingleThreadTaskRunner* polling_task_runner);
+             base::Thread* polling_thread);

[reveman, 2017/06/05 22:48:03]

"base::SingleThreadTaskRunner* polling_task_runner" as before

[jkwang, 2017/06/06 20:03:07]

Done.

 
+#if !defined(USE_OZONE_GAMEPAD)

[reveman, 2017/06/05 22:48:03]

Let's just remove this testing only ctor a related tests as ozone gamepad is the
future and the code that we care about testing.

[jkwang, 2017/06/06 20:03:07]

Make sense. If we are not going to change code of the old gaming_seat impl.
Done.

   // Allows test cases to specify a CreateGamepadDataFetcherCallback that
   // overrides the default GamepadPlatformDataFetcher.
   GamingSeat(GamingSeatDelegate* gaming_seat_delegate,
              base::SingleThreadTaskRunner* polling_task_runner,
              CreateGamepadDataFetcherCallback create_fetcher_callback);
+#endif
 
   ~GamingSeat() override;
 
   // Overridden WMHelper::FocusObserver:

[reveman, 2017/06/05 22:48:03]

while here, please change to: // Overridden from WMHelper::FocusObserver:

[jkwang, 2017/06/06 20:03:06]

Done.

   void OnWindowFocused(aura::Window* gained_focus,
                        aura::Window* lost_focus) override;
-
  private:
+  // The delegate that handles gamepad_added.
+  GamingSeatDelegate* const delegate_;
+
+#if defined(USE_OZONE_GAMEPAD)
+  // Overriden ui::GamepadObserver. These functions are overrided as private

[reveman, 2017/06/05 22:48:03]

s/Overriden/Overridden/ and make it "// Overridden from ui::GamepadObserver:"
without a follow up comment.

Also, please don't make this private as you can always cast this to
ui::GamepadObserver and then it's not private.

[jkwang, 2017/06/06 20:03:07]

Done.

+  // because they should only be called from a base class(GamepadObserver).
+  void OnGamepadDevicesUpdated() override;
+  void OnGamepadEvent(const ui::GamepadEvent& event) override;
+
+  // Enable or disable this gaming seat.
+  void Enable(bool enable);
+
+  // This map maps device id to gamepad delegates.

[reveman, 2017/06/05 22:48:03]

nit: describe what is stored in the map instead of what it's used for. E.g.
"Contains the delegate for each gamepad device."

[jkwang, 2017/06/06 20:03:06]

Done.

+  std::map<int, GamepadDelegate*> gamepads_;

[reveman, 2017/06/05 22:48:04]

base::flat_map

[jkwang, 2017/06/06 20:03:06]

Done.

+
+  ui::GamepadProviderOzone* gamepad_provider_;

[reveman, 2017/06/05 22:48:04]

nit: short comment above this for consistency

[jkwang, 2017/06/06 20:03:07]

Done.

+

[reveman, 2017/06/05 22:48:04]

nit: remove this blank line

[jkwang, 2017/06/06 20:03:07]

Done.

+#else
   class ThreadSafeGamepadChangeFetcher;
 
   // Processes updates of gamepad data and passes changes on to delegate.
@@ -56,9 +97,6 @@ class GamingSeat : public WMHelper::FocusObserver {
   // polling thread.
   scoped_refptr<ThreadSafeGamepadChangeFetcher> gamepad_change_fetcher_;
 
-  // The delegate that handles gamepad_added.
-  GamingSeatDelegate* const delegate_;
-
   // The delegate instances that all other events are dispatched to.
   GamepadDelegate* gamepad_delegates_[device::Gamepads::kItemsLengthCap];
 
@@ -66,13 +104,12 @@ class GamingSeat : public WMHelper::FocusObserver {
   device::Gamepads pad_state_;
 
   // ThreadChecker for the origin thread.
-  base::ThreadChecker thread_checker_;
+  THREAD_CHECKER(thread_checker_);
 
   base::WeakPtrFactory<GamingSeat> weak_ptr_factory_;
-
+#endif
   DISALLOW_COPY_AND_ASSIGN(GamingSeat);

[reveman, 2017/06/05 22:48:03]

nit: blank line before this

[jkwang, 2017/06/06 20:03:06]

Done.

 };
 
 }  // namespace exo
-
 #endif  // COMPONENTS_EXO_GAMING_SEAT_H_