Modified: trunk/Source/WebCore/ChangeLog (264388 => 264389)
--- trunk/Source/WebCore/ChangeLog 2020-07-15 05:54:46 UTC (rev 264388)
+++ trunk/Source/WebCore/ChangeLog 2020-07-15 08:00:13 UTC (rev 264389)
@@ -1,3 +1,59 @@
+2020-07-15 Brady Eidson <[email protected]>
+
+ Resolve race between IOHIDManager and GameController framework.
+ <rdar://problem/65554490> and https://bugs.webkit.org/show_bug.cgi?id=214245
+
+ Reviewed by Tim Horton.
+
+ No automated testing available.
+
+ IOHIDDevices and IOHIDServices are two separate things.
+
+ An IOHIDDevice and IOHIDService often have a 1-to-1 correlation, but the IOHIDDevice might be
+ published to the application before the IOHIDService is.
+
+ WebKit's HID gamepad manager uses IOHIDDevice.
+ GameController.framework uses IOHIDServices.
+
+ When we added the ability for WebKit to switch between HID and GCF, the following happens:
+
+ - Sometimes the IOHIDServiceClient will get published first. Then when the IOHIDDevice is published,
+ and WebKit was deciding which gamepad manager to use, the device's services are available,
+ and we get the right answer.
+
+ - Sometimes, the IOHIDDevice is published first. Then when WebKit is deciding which gamepad
+ manager to use, it can't check the IOHIDServiceClient against GameController framework.
+ So we have the HID manager handle the device... but then GCF comes along a split second later
+ and ALSO handles it.
+
+ The "device before service" scenario results in the same gamepad showing up twice.
+
+ To resolve this, we do the following:
+
+ 1 - If an IOHIDDevice attaches and its services aren't available yet, we delay managing it.
+ 2 - When we delay managing a device, we start listening for IOHIDServiceClient additions.
+ 3 - Each time a GamePad service is published, we once again try to determine if GCF will handle
+ the device.
+ 4 - As long as the answer is "Maybe" - instead of "Yes" or "No" - we refuse to manage the device.
+ 5 - After a brief delay (currently 1 second), we will give up waiting for published services
+ and manage the device with the HID gamepad provider.
+
+ In my testing, when the service publishes after the devices, it's always within 50ms,
+ (and usually just a spin or two of the runloop) so the 1s delay seems sufficient.
+
+ NOTE: The above all holds true with the "MultiGamepadProvider" on Catalina.
+ On Big Sur, there's brand new GameController framework API that makes this much easier. 👍
+
+ * platform/gamepad/mac/HIDGamepadProvider.h:
+ * platform/gamepad/mac/HIDGamepadProvider.mm:
+ (WebCore::deviceAddedCallback):
+ (WebCore::gameControllerFrameworkWillHandleHIDDevice):
+ (WebCore::HIDGamepadProvider::waitForManagementDecisionForDevice):
+ (WebCore::HIDGamepadProvider::removeDeviceWaitingForManagementDecision):
+ (WebCore::HIDGamepadProvider::newGamePadServicePublished):
+ (WebCore::HIDGamepadProvider::deviceAdded):
+ (WebCore::HIDGamepadProvider::deviceRemoved):
+
2020-07-14 Zalan Bujtas <[email protected]>
Caret leaves trails behind when the editable content is subpixel positioned
Modified: trunk/Source/WebCore/PAL/pal/spi/mac/IOKitSPIMac.h (264388 => 264389)
--- trunk/Source/WebCore/PAL/pal/spi/mac/IOKitSPIMac.h 2020-07-15 05:54:46 UTC (rev 264388)
+++ trunk/Source/WebCore/PAL/pal/spi/mac/IOKitSPIMac.h 2020-07-15 08:00:13 UTC (rev 264389)
@@ -35,9 +35,12 @@
WTF_EXTERN_C_BEGIN
typedef struct CF_BRIDGED_TYPE(id) __IOHIDServiceClient * IOHIDServiceClientRef;
typedef struct CF_BRIDGED_TYPE(id) __IOHIDEventSystemClient * IOHIDEventSystemClientRef;
+typedef void (^IOHIDServiceClientBlock)(void *, void *, IOHIDServiceClientRef);
+
IOHIDEventSystemClientRef IOHIDEventSystemClientCreate(CFAllocatorRef);
void IOHIDEventSystemClientSetMatching(IOHIDEventSystemClientRef, CFDictionaryRef);
IOHIDServiceClientRef IOHIDEventSystemClientCopyServiceForRegistryID(IOHIDEventSystemClientRef, uint64_t registryID);
+void IOHIDEventSystemClientRegisterDeviceMatchingBlock(IOHIDEventSystemClientRef, IOHIDServiceClientBlock, void *, void *);
WTF_EXTERN_C_END
#endif // USE(APPLE_INTERNAL_SDK)
Modified: trunk/Source/WebCore/platform/gamepad/mac/HIDGamepadProvider.h (264388 => 264389)
--- trunk/Source/WebCore/platform/gamepad/mac/HIDGamepadProvider.h 2020-07-15 05:54:46 UTC (rev 264388)
+++ trunk/Source/WebCore/platform/gamepad/mac/HIDGamepadProvider.h 2020-07-15 08:00:13 UTC (rev 264389)
@@ -31,9 +31,11 @@
#include "HIDGamepad.h"
#include "Timer.h"
#include <IOKit/hid/IOHIDManager.h>
+#include <pal/spi/mac/IOKitSPIMac.h>
#include <wtf/Deque.h>
#include <wtf/HashMap.h>
#include <wtf/RetainPtr.h>
+#include <wtf/WorkQueue.h>
namespace WebCore {
@@ -51,8 +53,12 @@
WEBCORE_EXPORT void stopMonitoringInput();
WEBCORE_EXPORT void startMonitoringInput();
-
- void deviceAdded(IOHIDDeviceRef);
+
+ enum class AllowManagementDecisionDelay : bool {
+ No,
+ Yes
+ };
+ void deviceAdded(IOHIDDeviceRef, AllowManagementDecisionDelay);
void deviceRemoved(IOHIDDeviceRef);
void valuesChanged(IOHIDValueRef);
@@ -73,7 +79,6 @@
Vector<PlatformGamepad*> m_gamepadVector;
HashMap<IOHIDDeviceRef, std::unique_ptr<HIDGamepad>> m_gamepadMap;
- HashSet<IOHIDDeviceRef> m_gameControllerManagedGamepads;
RetainPtr<IOHIDManagerRef> m_manager;
@@ -82,6 +87,19 @@
Timer m_initialGamepadsConnectedTimer;
Timer m_inputNotificationTimer;
+
+#if HAVE(MULTIGAMEPADPROVIDER_SUPPORT)
+ HashSet<IOHIDDeviceRef> m_gameControllerManagedGamepads;
+#if !HAVE(GCCONTROLLER_HID_DEVICE_CHECK)
+ void waitForManagementDecisionForDevice(IOHIDDeviceRef);
+ bool removeDeviceWaitingForManagementDecision(IOHIDDeviceRef);
+ void gamePadServiceWasPublished();
+
+ HashSet<IOHIDDeviceRef> m_devicesWaitingManagementDecision;
+ RefPtr<WorkQueue> m_delayManagementDecisionQueue;
+ RetainPtr<IOHIDEventSystemClientRef> m_eventSystemClient;
+#endif // !HAVE(GCCONTROLLER_HID_DEVICE_CHECK)
+#endif // HAVE(MULTIGAMEPADPROVIDER_SUPPORT)
};
} // namespace WebCore
Modified: trunk/Source/WebCore/platform/gamepad/mac/HIDGamepadProvider.mm (264388 => 264389)
--- trunk/Source/WebCore/platform/gamepad/mac/HIDGamepadProvider.mm 2020-07-15 05:54:46 UTC (rev 264388)
+++ trunk/Source/WebCore/platform/gamepad/mac/HIDGamepadProvider.mm 2020-07-15 08:00:13 UTC (rev 264389)
@@ -34,6 +34,10 @@
#import <pal/spi/mac/IOKitSPIMac.h>
#import <wtf/NeverDestroyed.h>
+#if HAVE(GCCONTROLLER_HID_DEVICE_CHECK)
+#import <GameController/GCController.h>
+#endif
+
#import "GameControllerSoftLink.h"
namespace WebCore {
@@ -58,7 +62,7 @@
static void deviceAddedCallback(void* context, IOReturn, void*, IOHIDDeviceRef device)
{
HIDGamepadProvider* listener = static_cast<HIDGamepadProvider*>(context);
- listener->deviceAdded(device);
+ listener->deviceAdded(device, HIDGamepadProvider::AllowManagementDecisionDelay::Yes);
}
static void deviceRemovedCallback(void* context, IOReturn, void*, IOHIDDeviceRef device)
@@ -180,61 +184,169 @@
}
#if HAVE(MULTIGAMEPADPROVIDER_SUPPORT)
-static bool gameControllerFrameworkWillHandleHIDDevice(IOHIDDeviceRef device)
+
+enum class GameControllerFrameworkHandlesDevice : uint8_t {
+ No,
+ Yes,
+ Maybe,
+};
+
+static GameControllerFrameworkHandlesDevice gameControllerFrameworkWillHandleHIDDevice(IOHIDDeviceRef device)
{
if (!isGameControllerFrameworkAvailable())
- return false;
+ return GameControllerFrameworkHandlesDevice::No;
+#if HAVE(GCCONTROLLER_HID_DEVICE_CHECK)
+ return [getGCControllerClass() supportsHIDDevice:device] ? GameControllerFrameworkHandlesDevice::Yes : GameControllerFrameworkHandlesDevice::No;
+#endif
+
auto deviceService = IOHIDDeviceGetService(device);
if (!deviceService)
- return false;
+ return GameControllerFrameworkHandlesDevice::Maybe;
// Check the service directly backing this device
uint64_t registryID;
if (IORegistryEntryGetRegistryEntryID(deviceService, ®istryID) != KERN_SUCCESS)
- return false;
+ return GameControllerFrameworkHandlesDevice::Maybe;
auto eventSystemClient = adoptCF(IOHIDEventSystemClientCreate(nullptr));
IOHIDEventSystemClientSetMatching(eventSystemClient.get(), nullptr);
auto serviceClient = adoptCF(IOHIDEventSystemClientCopyServiceForRegistryID(eventSystemClient.get(), registryID));
- if (serviceClient) {
- if (ControllerClassForService(serviceClient.get()))
- return true;
- }
+ if (serviceClient && ControllerClassForService(serviceClient.get()))
+ return GameControllerFrameworkHandlesDevice::Yes;
// Otherwise, check its grandchild service
io_registry_entry_t child;
if (IORegistryEntryGetChildEntry(deviceService, kIOServicePlane, &child) != KERN_SUCCESS)
- return false;
+ return GameControllerFrameworkHandlesDevice::Maybe;
if (!child)
- return false;
+ return GameControllerFrameworkHandlesDevice::Maybe;
if (IORegistryEntryGetChildEntry(child, kIOServicePlane, &child) != KERN_SUCCESS)
- return false;
+ return GameControllerFrameworkHandlesDevice::Maybe;
if (!child)
- return false;
+ return GameControllerFrameworkHandlesDevice::Maybe;
if (IORegistryEntryGetRegistryEntryID(child, ®istryID) != KERN_SUCCESS)
- return false;
+ return GameControllerFrameworkHandlesDevice::Maybe;
serviceClient = adoptCF(IOHIDEventSystemClientCopyServiceForRegistryID(eventSystemClient.get(), registryID));
if (!serviceClient)
+ return GameControllerFrameworkHandlesDevice::Maybe;
+
+ return ControllerClassForService(serviceClient.get()) ? GameControllerFrameworkHandlesDevice::Yes : GameControllerFrameworkHandlesDevice::No;
+}
+
+#if !HAVE(GCCONTROLLER_HID_DEVICE_CHECK)
+
+static const Seconds managementDecisionDelay { 1000_ms };
+
+void HIDGamepadProvider::waitForManagementDecisionForDevice(IOHIDDeviceRef device)
+{
+ LOG(Gamepad, "Unable to determine if GameController framework will handle newly attached device %p - Delaying management decision", device);
+ m_devicesWaitingManagementDecision.add(device);
+
+ if (!m_delayManagementDecisionQueue)
+ m_delayManagementDecisionQueue = WorkQueue::create("com.apple.WebKit.GamepadManagementDecision", WorkQueue::Type::Serial, WorkQueue::QOS::Utility);
+
+ auto delayedDevice = RetainPtr<IOHIDDeviceRef>(device);
+ m_delayManagementDecisionQueue->dispatchAfter(managementDecisionDelay, [this, delayedDevice = WTFMove(delayedDevice)] {
+ // If this device was still waiting for its management decision, then the HID provider will handle it.
+ if (removeDeviceWaitingForManagementDecision(delayedDevice.get()))
+ deviceAdded(delayedDevice.get(), AllowManagementDecisionDelay::No);
+ else
+ LOG(Gamepad, "Management decision delay for device %p has expired, but device has already been handled", delayedDevice.get());
+ });
+
+ if (m_devicesWaitingManagementDecision.size() > 1)
+ return;
+
+ m_eventSystemClient = adoptCF(IOHIDEventSystemClientCreateWithType(kCFAllocatorDefault, kIOHIDEventSystemClientTypeMonitor, 0));
+
+ NSMutableArray *matchingAttributes = [NSMutableArray array];
+ [matchingAttributes addObject:@{@(kIOHIDDeviceUsagePageKey) : @(kHIDPage_GenericDesktop), @(kIOHIDDeviceUsageKey) : @(kHIDUsage_GD_GamePad)}];
+ [matchingAttributes addObject:@{@(kIOHIDDeviceUsagePageKey) : @(kHIDPage_GenericDesktop), @(kIOHIDDeviceUsageKey) : @(kHIDUsage_GD_Joystick)}];
+
+ IOHIDEventSystemClientSetMatchingMultiple(m_eventSystemClient.get(), (__bridge CFArrayRef)matchingAttributes);
+ IOHIDEventSystemClientScheduleWithDispatchQueue(m_eventSystemClient.get(), dispatch_get_main_queue());
+ IOHIDEventSystemClientRegisterDeviceMatchingBlock(m_eventSystemClient.get(), ^(void *, void *, IOHIDServiceClientRef) {
+ // We don't actually care about the actual published service.
+ // Any new GamePad service is worth re-checking every pending IOHIDDevice against GameController framework.
+ HIDGamepadProvider::singleton().gamePadServiceWasPublished();
+ }, nullptr, nullptr);
+}
+
+bool HIDGamepadProvider::removeDeviceWaitingForManagementDecision(IOHIDDeviceRef device)
+{
+ if (!m_devicesWaitingManagementDecision.remove(device))
return false;
- return ControllerClassForService(serviceClient.get());
+ if (m_devicesWaitingManagementDecision.isEmpty()) {
+ // That was the last device, so we can stop monitoring for IOHIDServiceClients
+ ASSERT(m_eventSystemClient);
+ IOHIDEventSystemClientUnregisterDeviceMatchingBlock(m_eventSystemClient.get());
+ m_eventSystemClient = nullptr;
+ m_delayManagementDecisionQueue = nullptr;
+ }
+
+ return true;
}
-#endif // HAVE(MULTIGAMEPADPROVIDER_SUPPORT)
+void HIDGamepadProvider::gamePadServiceWasPublished()
+{
+ LOG(Gamepad, "HIDGamepadProvider::gamePadServiceWasPublished()");
+ IOHIDDeviceRef matchedDevice = nullptr;
+ for (auto device : m_devicesWaitingManagementDecision) {
+ auto result = gameControllerFrameworkWillHandleHIDDevice(device);
+ if (result == GameControllerFrameworkHandlesDevice::Yes || result == GameControllerFrameworkHandlesDevice::No) {
+ matchedDevice = device;
+ break;
+ }
+ }
+ if (matchedDevice) {
+ LOG(Gamepad, "Device %p pending management decision can now be managed", matchedDevice);
+ removeDeviceWaitingForManagementDecision(matchedDevice);
+ deviceAdded(matchedDevice, AllowManagementDecisionDelay::No);
+ }
+}
-void HIDGamepadProvider::deviceAdded(IOHIDDeviceRef device)
+#endif // !HAVE(GCCONTROLLER_HID_DEVICE_CHECK)
+#endif // HAVE(MULTIGAMEPADPROVIDER_SUPPORT)
+
+void HIDGamepadProvider::deviceAdded(IOHIDDeviceRef device, AllowManagementDecisionDelay allowManagementDecisionDelay)
{
#if HAVE(MULTIGAMEPADPROVIDER_SUPPORT)
- if (m_ignoresGameControllerFrameworkDevices && gameControllerFrameworkWillHandleHIDDevice(device)) {
- LOG(Gamepad, "GameController framework will handle attached device %p - HIDGamepadProvider ignoring it", device);
- m_gameControllerManagedGamepads.add(device);
- return;
+ if (m_ignoresGameControllerFrameworkDevices) {
+ auto result = gameControllerFrameworkWillHandleHIDDevice(device);
+ switch (result) {
+ case GameControllerFrameworkHandlesDevice::Yes:
+ LOG(Gamepad, "GameController framework will handle newly attached device %p - HIDGamepadProvider ignoring it", device);
+ m_gameControllerManagedGamepads.add(device);
+ return;
+
+ case GameControllerFrameworkHandlesDevice::Maybe:
+#if HAVE(GCCONTROLLER_HID_DEVICE_CHECK)
+ UNUSED_PARAM(allowManagementDecisionDelay);
+ RELEASE_ASSERT_NOT_REACHED();
+#else
+ if (allowManagementDecisionDelay == AllowManagementDecisionDelay::No) {
+ LOG(Gamepad, "Unable to determine if GameController framework will handle attached device %p - HIDGamepadProvider will handle it", device);
+ break;
+ }
+
+ // Give IOHIDServiceClient callbacks a chance to confirm whether or not
+ // this device will be handled by GameController framework
+ waitForManagementDecisionForDevice(device);
+ return;
+#endif // HAVE(GCCONTROLLER_HID_DEVICE_CHECK)
+ case GameControllerFrameworkHandlesDevice::No:
+ LOG(Gamepad, "GameController framework does not handle attached device %p - HIDGamepadProvider will handle it", device);
+ break;
+ }
}
+#else
+ UNUSED_PARAM(allowManagementDecisionDelay);
#endif // HAVE(MULTIGAMEPADPROVIDER_SUPPORT)
ASSERT(!m_gamepadMap.get(device));
@@ -272,12 +384,29 @@
void HIDGamepadProvider::deviceRemoved(IOHIDDeviceRef device)
{
+#if HAVE(MULTIGAMEPADPROVIDER_SUPPORT) && !HAVE(GCCONTROLLER_HID_DEVICE_CHECK)
+ bool wasRemoved = removeDeviceWaitingForManagementDecision(device);
+#endif
+
std::unique_ptr<HIDGamepad> removedGamepad = removeGamepadForDevice(device);
if (!removedGamepad) {
+#if HAVE(MULTIGAMEPADPROVIDER_SUPPORT)
auto taken = m_gameControllerManagedGamepads.take(device);
- ASSERT_UNUSED(taken, taken);
- LOG(Gamepad, "HIDGamepadProvider informed of removal of device %p, which is managed by GameController framework. Ignoring.", device);
+
+ // If this device didn't show up in the gamepad map, then it was either managed by GameController framework
+ // or we were still waiting to decide which provider would manage it.
+#if HAVE(GCCONTROLLER_HID_DEVICE_CHECK)
+ ASSERT(taken);
+#else
+ ASSERT(taken || wasRemoved);
+#endif
+
+ if (taken)
+ LOG(Gamepad, "HIDGamepadProvider informed of removal of device %p, which is managed by GameController framework. Ignoring.", device);
+ else
+ LOG(Gamepad, "HIDGamepadProvider informed of removal of device %p, which was not yet managed. Ignoring.", device);
+#endif
return;
}
