Bug 1657404 - Implement a strongly-typed, service-dependent gamepad handle r=handyman,aklotz

Currently, the gamepad code uses a uint32_t as a handle and does some trickery
with it by trying to create a unique ID and adding an offset to it for VR code.

This can (and has) led to errors where the developer forgets to perform the
arithmetic and sends the wrong number to the wrong manager.

This change created a strongly-typed handle that remembers which service it
belongs to. This should eliminate such accidents.

Differential Revision: https://phabricator.services.mozilla.com/D96273

Author: Chris Martin

dom/base/nsGlobalWindowInner.cpp

@@ -116,6 +116,7 @@
 #include "mozilla/dom/EventTarget.h"
 #include "mozilla/dom/Fetch.h"
 #include "mozilla/dom/Gamepad.h"
+#include "mozilla/dom/GamepadHandle.h"
 #include "mozilla/dom/GamepadManager.h"
 #include "mozilla/dom/HashChangeEvent.h"
 #include "mozilla/dom/HashChangeEventBinding.h"
@@ -339,6 +340,7 @@ using namespace mozilla::dom;
 using namespace mozilla::dom::ipc;
 using mozilla::TimeDuration;
 using mozilla::TimeStamp;
+using mozilla::dom::GamepadHandle;
 using mozilla::dom::cache::CacheStorage;
 
 #define FORWARD_TO_OUTER(method, args, err_rval)                     \
@@ -6535,7 +6537,7 @@ void nsGlobalWindowInner::AddSizeOfIncludingThis(
   }
 }
 
-void nsGlobalWindowInner::AddGamepad(uint32_t aIndex, Gamepad* aGamepad) {
+void nsGlobalWindowInner::AddGamepad(GamepadHandle aHandle, Gamepad* aGamepad) {
   // Create the index we will present to content based on which indices are
   // already taken, as required by the spec.
   // https://w3c.github.io/gamepad/gamepad.html#widl-Gamepad-index
@@ -6545,17 +6547,17 @@ void nsGlobalWindowInner::AddGamepad(uint32_t aIndex, Gamepad* aGamepad) {
   }
   mGamepadIndexSet.Put(index);
   aGamepad->SetIndex(index);
-  mGamepads.Put(aIndex, RefPtr{aGamepad});
+  mGamepads.Put(aHandle, RefPtr{aGamepad});
 }
 
-void nsGlobalWindowInner::RemoveGamepad(uint32_t aIndex) {
+void nsGlobalWindowInner::RemoveGamepad(GamepadHandle aHandle) {
   RefPtr<Gamepad> gamepad;
-  if (!mGamepads.Get(aIndex, getter_AddRefs(gamepad))) {
+  if (!mGamepads.Get(aHandle, getter_AddRefs(gamepad))) {
     return;
   }
   // Free up the index we were using so it can be reused
   mGamepadIndexSet.Remove(gamepad->Index());
-  mGamepads.Remove(aIndex);
+  mGamepads.Remove(aHandle);
 }
 
 void nsGlobalWindowInner::GetGamepads(nsTArray<RefPtr<Gamepad>>& aGamepads) {
@@ -6576,10 +6578,11 @@ void nsGlobalWindowInner::GetGamepads(nsTArray<RefPtr<Gamepad>>& aGamepads) {
   }
 }
 
-already_AddRefed<Gamepad> nsGlobalWindowInner::GetGamepad(uint32_t aIndex) {
+already_AddRefed<Gamepad> nsGlobalWindowInner::GetGamepad(
+    GamepadHandle aHandle) {
   RefPtr<Gamepad> gamepad;
 
-  if (mGamepads.Get(aIndex, getter_AddRefs(gamepad))) {
+  if (mGamepads.Get(aHandle, getter_AddRefs(gamepad))) {
     return gamepad.forget();
   }
 

dom/base/nsGlobalWindowInner.h

@@ -35,6 +35,7 @@
 #include "mozilla/dom/BindingDeclarations.h"
 #include "mozilla/dom/ChromeMessageBroadcaster.h"
 #include "mozilla/dom/DebuggerNotificationManager.h"
+#include "mozilla/dom/GamepadHandle.h"
 #include "mozilla/dom/Location.h"
 #include "mozilla/dom/NavigatorBinding.h"
 #include "mozilla/dom/StorageEvent.h"
@@ -491,10 +492,12 @@ class nsGlobalWindowInner final : public mozilla::dom::EventTarget,
                                           const double aDuration);
 
   // Inner windows only.
-  void AddGamepad(uint32_t aIndex, mozilla::dom::Gamepad* aGamepad);
-  void RemoveGamepad(uint32_t aIndex);
+  void AddGamepad(mozilla::dom::GamepadHandle aHandle,
+                  mozilla::dom::Gamepad* aGamepad);
+  void RemoveGamepad(mozilla::dom::GamepadHandle aHandle);
   void GetGamepads(nsTArray<RefPtr<mozilla::dom::Gamepad>>& aGamepads);
-  already_AddRefed<mozilla::dom::Gamepad> GetGamepad(uint32_t aIndex);
+  already_AddRefed<mozilla::dom::Gamepad> GetGamepad(
+      mozilla::dom::GamepadHandle aHandle);
   void SetHasSeenGamepadInput(bool aHasSeen);
   bool HasSeenGamepadInput();
   void SyncGamepadState();
@@ -1323,7 +1326,9 @@ class nsGlobalWindowInner final : public mozilla::dom::EventTarget,
   bool mHintedWasLoading : 1;
 
   nsCheapSet<nsUint32HashKey> mGamepadIndexSet;
-  nsRefPtrHashtable<nsUint32HashKey, mozilla::dom::Gamepad> mGamepads;
+  nsRefPtrHashtable<nsGenericHashKey<mozilla::dom::GamepadHandle>,
+                    mozilla::dom::Gamepad>
+      mGamepads;
 
   RefPtr<nsScreen> mScreen;
 

dom/gamepad/Gamepad.cpp

@@ -36,14 +36,14 @@ void Gamepad::UpdateTimestamp() {
 }
 
 Gamepad::Gamepad(nsISupports* aParent, const nsAString& aID, int32_t aIndex,
-                 uint32_t aHashKey, GamepadMappingType aMapping,
+                 GamepadHandle aHandle, GamepadMappingType aMapping,
                  GamepadHand aHand, uint32_t aDisplayID, uint32_t aNumButtons,
                  uint32_t aNumAxes, uint32_t aNumHaptics,
                  uint32_t aNumLightIndicator, uint32_t aNumTouchEvents)
     : mParent(aParent),
       mID(aID),
       mIndex(aIndex),
-      mHashKey(aHashKey),
+      mHandle(aHandle),
       mDisplayId(aDisplayID),
       mTouchIdHashValue(0),
       mMapping(aMapping),
@@ -59,11 +59,11 @@ Gamepad::Gamepad(nsISupports* aParent, const nsAString& aID, int32_t aIndex,
   mPose = new GamepadPose(aParent);
   for (uint32_t i = 0; i < aNumHaptics; ++i) {
     mHapticActuators.AppendElement(
-        new GamepadHapticActuator(mParent, mHashKey, i));
+        new GamepadHapticActuator(mParent, mHandle, i));
   }
   for (uint32_t i = 0; i < aNumLightIndicator; ++i) {
     mLightIndicators.AppendElement(
-        new GamepadLightIndicator(mParent, mHashKey, i));
+        new GamepadLightIndicator(mParent, mHandle, i));
   }
   for (uint32_t i = 0; i < aNumTouchEvents; ++i) {
     mTouchEvents.AppendElement(new GamepadTouch(mParent));
@@ -177,7 +177,7 @@ void Gamepad::SyncState(Gamepad* aOther) {
 
 already_AddRefed<Gamepad> Gamepad::Clone(nsISupports* aParent) {
   RefPtr<Gamepad> out =
-      new Gamepad(aParent, mID, mIndex, mHashKey, mMapping, mHand, mDisplayId,
+      new Gamepad(aParent, mID, mIndex, mHandle, mMapping, mHand, mDisplayId,
                   mButtons.Length(), mAxes.Length(), mHapticActuators.Length(),
                   mLightIndicators.Length(), mTouchEvents.Length());
   out->SyncState(this);

dom/gamepad/Gamepad.h

@@ -9,6 +9,7 @@
 
 #include "mozilla/dom/GamepadBinding.h"
 #include "mozilla/dom/GamepadButton.h"
+#include "mozilla/dom/GamepadHandle.h"
 #include "mozilla/dom/GamepadPose.h"
 #include "mozilla/dom/GamepadHapticActuator.h"
 #include "mozilla/dom/GamepadLightIndicator.h"
@@ -40,7 +41,7 @@ const int kRightStickYAxis = 3;
 class Gamepad final : public nsISupports, public nsWrapperCache {
  public:
   Gamepad(nsISupports* aParent, const nsAString& aID, int32_t aIndex,
-          uint32_t aHashKey, GamepadMappingType aMapping, GamepadHand aHand,
+          GamepadHandle aHandle, GamepadMappingType aMapping, GamepadHand aHand,
           uint32_t aDisplayID, uint32_t aNumButtons, uint32_t aNumAxes,
           uint32_t aNumHaptics, uint32_t aNumLightIndicator,
           uint32_t aNumTouchEvents);
@@ -84,8 +85,6 @@ class Gamepad final : public nsISupports, public nsWrapperCache {
 
   int32_t Index() const { return mIndex; }
 
-  uint32_t HashKey() const { return mHashKey; }
-
   void GetButtons(nsTArray<RefPtr<GamepadButton>>& aButtons) const {
     aButtons = mButtons.Clone();
   }
@@ -108,6 +107,8 @@ class Gamepad final : public nsISupports, public nsWrapperCache {
     aTouchEvents = mTouchEvents.Clone();
   }
 
+  GamepadHandle GetHandle() const { return mHandle; }
+
  private:
   virtual ~Gamepad() = default;
   void UpdateTimestamp();
@@ -117,7 +118,7 @@ class Gamepad final : public nsISupports, public nsWrapperCache {
   nsString mID;
   int32_t mIndex;
   // the gamepad hash key in GamepadManager
-  uint32_t mHashKey;
+  GamepadHandle mHandle;
   uint32_t mDisplayId;
   uint32_t mTouchIdHashValue;
   // The mapping in use.

dom/gamepad/GamepadHandle.cpp

@@ -0,0 +1,40 @@
+/* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
+/* vim: set ts=8 sts=2 et sw=2 tw=80: */
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+#include "GamepadHandle.h"
+#include "mozilla/Assertions.h"
+#include "mozilla/HashFunctions.h"
+
+namespace mozilla::dom {
+
+GamepadHandle::GamepadHandle(uint32_t aValue, GamepadHandleKind aKind)
+    : mValue(aValue), mKind(aKind) {
+  MOZ_RELEASE_ASSERT(mValue);
+}
+
+GamepadHandleKind GamepadHandle::GetKind() const { return mKind; }
+
+PLDHashNumber GamepadHandle::Hash() const {
+  return HashGeneric(mValue, uint8_t(mKind));
+}
+
+bool operator==(const GamepadHandle& a, const GamepadHandle& b) {
+  return (a.mValue == b.mValue) && (a.mKind == b.mKind);
+}
+
+bool operator!=(const GamepadHandle& a, const GamepadHandle& b) {
+  return !(a == b);
+}
+
+bool operator<(const GamepadHandle& a, const GamepadHandle& b) {
+  if (a.mKind == b.mKind) {
+    return a.mValue < b.mValue;
+  }
+  // Arbitrarily order them by kind
+  return uint8_t(a.mKind) < uint8_t(b.mKind);
+}
+
+}  // namespace mozilla::dom

dom/gamepad/GamepadHandle.h

@@ -0,0 +1,89 @@
+/* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
+/* vim: set ts=8 sts=2 et sw=2 tw=80: */
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+// This file defines a strongly-typed opaque gamepad handle
+//
+// The handle is designed to be copied around and passed over IPC. It keeps
+// track of which "provider" created the handle so it can ensure that
+// providers are never mixed. It also allows each provider to have its own
+// algorithm for generating gamepad IDs, since the VR and Platform services
+// do it differently.
+
+#ifndef mozilla_dom_gamepad_GamepadHandle_h
+#define mozilla_dom_gamepad_GamepadHandle_h
+#include "mozilla/Tuple.h"
+#include "PLDHashTable.h"
+#include <type_traits>
+#include <cinttypes>
+
+namespace IPC {
+
+template <class>
+struct ParamTraits;
+
+}  // namespace IPC
+
+namespace mozilla::gfx {
+
+class VRDisplayClient;
+class VRManager;
+
+}  // namespace mozilla::gfx
+
+namespace mozilla::dom {
+
+class GamepadPlatformService;
+class GamepadServiceTest;
+class XRInputSource;
+
+// The "kind" of a gamepad handle is based on which provider created it
+enum class GamepadHandleKind : uint8_t {
+  GamepadPlatformManager,
+  VR,
+};
+
+class GamepadHandle {
+ public:
+  // Allow handle to be passed around as a simple object
+  GamepadHandle() = default;
+  GamepadHandle(const GamepadHandle&) = default;
+  GamepadHandle& operator=(const GamepadHandle&) = default;
+
+  // Helps code know which manager to send requests to
+  GamepadHandleKind GetKind() const;
+
+  // Define operators so the handle can compared and stored in maps
+  friend bool operator==(const GamepadHandle& a, const GamepadHandle& b);
+  friend bool operator!=(const GamepadHandle& a, const GamepadHandle& b);
+  friend bool operator<(const GamepadHandle& a, const GamepadHandle& b);
+
+  PLDHashNumber Hash() const;
+
+ private:
+  explicit GamepadHandle(uint32_t aValue, GamepadHandleKind aKind);
+  uint32_t GetValue() const { return mValue; }
+
+  uint32_t mValue{0};
+  GamepadHandleKind mKind{GamepadHandleKind::GamepadPlatformManager};
+
+  // These are the classes that are "gamepad managers". They are allowed to
+  // create new handles and inspect their actual value
+  friend class mozilla::dom::GamepadPlatformService;
+  friend class mozilla::dom::GamepadServiceTest;
+  friend class mozilla::dom::XRInputSource;
+  friend class mozilla::gfx::VRDisplayClient;
+  friend class mozilla::gfx::VRManager;
+
+  // Allow IPDL to serialize us
+  friend struct IPC::ParamTraits<mozilla::dom::GamepadHandle>;
+};
+
+static_assert(std::is_trivially_copyable<GamepadHandle>::value,
+              "GamepadHandle must be trivially copyable");
+
+}  // namespace mozilla::dom
+
+#endif  // mozilla_dom_gamepad_GamepadHandle_h

dom/gamepad/GamepadHapticActuator.cpp

@@ -21,10 +21,10 @@ NS_INTERFACE_MAP_END
 NS_IMPL_CYCLE_COLLECTION_WRAPPERCACHE(GamepadHapticActuator, mParent)
 
 GamepadHapticActuator::GamepadHapticActuator(nsISupports* aParent,
-                                             uint32_t aGamepadId,
+                                             GamepadHandle aGamepadHandle,
                                              uint32_t aIndex)
     : mParent(aParent),
-      mGamepadId(aGamepadId),
+      mGamepadHandle(aGamepadHandle),
       mType(GamepadHapticActuatorType::Vibration),
       mIndex(aIndex) {}
 
@@ -55,7 +55,7 @@ already_AddRefed<Promise> GamepadHapticActuator::Pulse(double aValue,
   switch (mType) {
     case GamepadHapticActuatorType::Vibration: {
       RefPtr<Promise> promise = gamepadManager->VibrateHaptic(
-          mGamepadId, mIndex, value, duration, global, aRv);
+          mGamepadHandle, mIndex, value, duration, global, aRv);
       if (!promise) {
         return nullptr;
       }
@@ -72,7 +72,7 @@ already_AddRefed<Promise> GamepadHapticActuator::Pulse(double aValue,
 GamepadHapticActuatorType GamepadHapticActuator::Type() const { return mType; }
 
 void GamepadHapticActuator::Set(const GamepadHapticActuator* aOther) {
-  mGamepadId = aOther->mGamepadId;
+  mGamepadHandle = aOther->mGamepadHandle;
   mType = aOther->mType;
   mIndex = aOther->mIndex;
 }

dom/gamepad/GamepadHapticActuator.h

@@ -11,14 +11,15 @@
 #include "nsWrapperCache.h"
 #include "mozilla/dom/GamepadHapticActuatorBinding.h"
 #include "mozilla/dom/Gamepad.h"
+#include "mozilla/dom/GamepadHandle.h"
 
 namespace mozilla {
 namespace dom {
 class Promise;
 
 class GamepadHapticActuator : public nsISupports, public nsWrapperCache {
  public:
-  GamepadHapticActuator(nsISupports* aParent, uint32_t aGamepadId,
+  GamepadHapticActuator(nsISupports* aParent, GamepadHandle aGamepadHandle,
                         uint32_t aIndex);
 
   NS_DECL_CYCLE_COLLECTING_ISUPPORTS
@@ -41,7 +42,7 @@ class GamepadHapticActuator : public nsISupports, public nsWrapperCache {
 
  protected:
   nsCOMPtr<nsISupports> mParent;
-  uint32_t mGamepadId;
+  GamepadHandle mGamepadHandle;
   GamepadHapticActuatorType mType;
   uint32_t mIndex;
 };