Modified: trunk/Source/WebCore/ChangeLog (170548 => 170549)
--- trunk/Source/WebCore/ChangeLog 2014-06-27 20:31:31 UTC (rev 170548)
+++ trunk/Source/WebCore/ChangeLog 2014-06-27 20:35:55 UTC (rev 170549)
@@ -1,3 +1,31 @@
+2014-06-26 Brady Eidson <[email protected]>
+
+ HIDGamepadProvider should only be active when someone is interested in Gamepads.
+ https://bugs.webkit.org/show_bug.cgi?id=134374
+
+ Reviewed by Darin Adler.
+
+ No new tests (Not yet a tested config)
+
+ * Modules/gamepad/GamepadManager.cpp:
+ (WebCore::GamepadManager::registerNavigator): Add some logging.
+ (WebCore::GamepadManager::unregisterNavigator): Ditto.
+
+ * platform/mac/HIDGamepadProvider.cpp:
+ (WebCore::HIDGamepadProvider::HIDGamepadProvider):
+ (WebCore::HIDGamepadProvider::connectionDelayTimerFired): Stop suppressing connection callbacks
+ (WebCore::HIDGamepadProvider::openAndScheduleManager):
+ (WebCore::HIDGamepadProvider::closeAndUnscheduleManager): Stop listening for gamepad events, and clear
+ all current gamepads.
+ (WebCore::HIDGamepadProvider::startMonitoringGamepads): If the first client, call openAndScheduleManager
+ (WebCore::HIDGamepadProvider::stopMonitoringGamepads): If the last client, call closeAndUnscheduleManager
+ (WebCore::HIDGamepadProvider::deviceAdded): If this callback came while we were suppressing connection
+ callbacks, it is a startup event for already-connected gamepads. Stop suppressing callbacks in a later
+ spin of the runloop.
+ (WebCore::HIDGamepadProvider::deviceRemoved):
+ * platform/mac/HIDGamepadProvider.h:
+ (WebCore::HIDGamepadProvider::setShouldDispatchCallbacks): Deleted.
+
2014-06-27 Alex Christensen <[email protected]>
Prevent unnecessary register saving in css jit.
Modified: trunk/Source/WebCore/Modules/gamepad/GamepadManager.cpp (170548 => 170549)
--- trunk/Source/WebCore/Modules/gamepad/GamepadManager.cpp 2014-06-27 20:31:31 UTC (rev 170548)
+++ trunk/Source/WebCore/Modules/gamepad/GamepadManager.cpp 2014-06-27 20:35:55 UTC (rev 170549)
@@ -29,6 +29,7 @@
#include "Gamepad.h"
#include "GamepadProvider.h"
+#include "Logging.h"
#include "NavigatorGamepad.h"
#include "PlatformGamepad.h"
@@ -63,26 +64,34 @@
void GamepadManager::registerNavigator(NavigatorGamepad* navigator)
{
+ LOG(Gamepad, "GamepadManager registering NavigatorGamepad %p", navigator);
+
ASSERT(!m_navigators.contains(navigator));
m_navigators.add(navigator);
// FIXME: Monitoring gamepads will also be reliant on whether or not there are any
// connected/disconnected event listeners.
// Those event listeners will also need to register with the GamepadManager.
- if (m_navigators.size() == 1)
+ if (m_navigators.size() == 1) {
+ LOG(Gamepad, "GamepadManager registered first navigator, is starting gamepad monitoring");
GamepadProvider::shared().startMonitoringGamepads(this);
+ }
}
void GamepadManager::unregisterNavigator(NavigatorGamepad* navigator)
{
+ LOG(Gamepad, "GamepadManager unregistering NavigatorGamepad %p", navigator);
+
ASSERT(m_navigators.contains(navigator));
m_navigators.remove(navigator);
// FIXME: Monitoring gamepads will also be reliant on whether or not there are any
// connected/disconnected event listeners.
// Those event listeners will also need to register with the GamepadManager.
- if (m_navigators.isEmpty())
+ if (m_navigators.isEmpty()) {
+ LOG(Gamepad, "GamepadManager unregistered last navigator, is stopping gamepad monitoring");
GamepadProvider::shared().stopMonitoringGamepads(this);
+ }
}
} // namespace WebCore
Modified: trunk/Source/WebCore/platform/mac/HIDGamepadProvider.cpp (170548 => 170549)
--- trunk/Source/WebCore/platform/mac/HIDGamepadProvider.cpp 2014-06-27 20:31:31 UTC (rev 170548)
+++ trunk/Source/WebCore/platform/mac/HIDGamepadProvider.cpp 2014-06-27 20:35:55 UTC (rev 170549)
@@ -29,11 +29,13 @@
#if ENABLE(GAMEPAD)
#include "GamepadProviderClient.h"
+#include "Logging.h"
#include "PlatformGamepad.h"
-#include <wtf/MainThread.h>
namespace WebCore {
+static const double ConnectionDelayInterval = 0.5;
+
static RetainPtr<CFDictionaryRef> deviceMatchingDictionary(uint32_t usagePage, uint32_t usage)
{
ASSERT(usagePage);
@@ -78,6 +80,7 @@
HIDGamepadProvider::HIDGamepadProvider()
: m_shouldDispatchCallbacks(false)
+ , m_connectionDelayTimer(this, &HIDGamepadProvider::connectionDelayTimerFired)
{
m_manager = adoptCF(IOHIDManagerCreate(kCFAllocatorDefault, kIOHIDOptionsTypeNone));
@@ -91,17 +94,7 @@
IOHIDManagerSetDeviceMatchingMultiple(m_manager.get(), matchingArray.get());
IOHIDManagerRegisterDeviceMatchingCallback(m_manager.get(), deviceAddedCallback, this);
IOHIDManagerRegisterDeviceRemovalCallback(m_manager.get(), deviceRemovedCallback, this);
- IOHIDManagerScheduleWithRunLoop(m_manager.get(), CFRunLoopGetCurrent(), kCFRunLoopDefaultMode);
- IOHIDManagerOpen(m_manager.get(), kIOHIDOptionsTypeNone);
-
IOHIDManagerRegisterInputValueCallback(m_manager.get(), deviceValuesChangedCallback, this);
-
- // We'll immediately get a series of connection and value callbacks for already-connected gamepads
- // but we don't want to notify WebCore of those events.
- // This callOnMainThread call re-enables those callbacks after the runloop empties out.
- callOnMainThread([]() {
- HIDGamepadProvider::shared().setShouldDispatchCallbacks(true);
- });
}
unsigned HIDGamepadProvider::indexForNewlyConnectedDevice()
@@ -113,19 +106,67 @@
return index;
}
+void HIDGamepadProvider::connectionDelayTimerFired(Timer<HIDGamepadProvider>&)
+{
+ m_shouldDispatchCallbacks = true;
+}
+
+void HIDGamepadProvider::openAndScheduleManager()
+{
+ LOG(Gamepad, "HIDGamepadProvider opening/scheduling HID manager");
+
+ ASSERT(m_gamepadVector.isEmpty());
+ ASSERT(m_gamepadMap.isEmpty());
+
+ m_shouldDispatchCallbacks = false;
+
+ IOHIDManagerScheduleWithRunLoop(m_manager.get(), CFRunLoopGetCurrent(), kCFRunLoopDefaultMode);
+ IOHIDManagerOpen(m_manager.get(), kIOHIDOptionsTypeNone);
+
+ // Any connections we are notified of within the ConnectionDelayInterval of listening likely represent
+ // devices that were already connected, so we suppress notifying clients of these.
+ m_connectionDelayTimer.startOneShot(ConnectionDelayInterval);
+}
+
+void HIDGamepadProvider::closeAndUnscheduleManager()
+{
+ LOG(Gamepad, "HIDGamepadProvider closing/unscheduling HID manager");
+
+ IOHIDManagerUnscheduleFromRunLoop(m_manager.get(), CFRunLoopGetCurrent(), kCFRunLoopDefaultMode);
+ IOHIDManagerClose(m_manager.get(), kIOHIDOptionsTypeNone);
+
+ m_gamepadVector.clear();
+ m_gamepadMap.clear();
+
+ m_connectionDelayTimer.stop();
+}
+
void HIDGamepadProvider::startMonitoringGamepads(GamepadProviderClient* client)
{
+ bool shouldOpenAndScheduleManager = m_clients.isEmpty();
+
+ ASSERT(!m_clients.contains(client));
m_clients.add(client);
+
+ if (shouldOpenAndScheduleManager)
+ openAndScheduleManager();
}
void HIDGamepadProvider::stopMonitoringGamepads(GamepadProviderClient* client)
{
- m_clients.remove(client);
+ ASSERT(m_clients.contains(client));
+
+ bool shouldCloseAndUnscheduleManager = m_clients.remove(client) && m_clients.isEmpty();
+
+ if (shouldCloseAndUnscheduleManager)
+ closeAndUnscheduleManager();
}
void HIDGamepadProvider::deviceAdded(IOHIDDeviceRef device)
{
ASSERT(!m_gamepadMap.get(device));
+ LOG(Gamepad, "HIDGamepadProvider device %p added", device);
+
std::unique_ptr<HIDGamepad> gamepad = std::make_unique<HIDGamepad>(device);
unsigned index = indexForNewlyConnectedDevice();
@@ -135,8 +176,18 @@
m_gamepadVector[index] = gamepad.get();
m_gamepadMap.set(device, std::move(gamepad));
- if (!m_shouldDispatchCallbacks)
+ if (!m_shouldDispatchCallbacks) {
+ // This added device is the result of us starting to monitor gamepads.
+ // We'll get notified of all connected devices during this current spin of the runloop
+ // and we don't want to tell the client about any of them.
+ // The m_connectionDelayTimer fires in a subsequent spin of the runloop after which
+ // any connection events are actual new devices.
+ m_connectionDelayTimer.startOneShot(0);
+
+ LOG(Gamepad, "Device %p was added while suppressing callbacks, so this should be an 'already connected' event", device);
+
return;
+ }
for (auto& client : m_clients)
client->platformGamepadConnected(index);
@@ -144,11 +195,14 @@
void HIDGamepadProvider::deviceRemoved(IOHIDDeviceRef device)
{
+ LOG(Gamepad, "HIDGamepadProvider device %p removed", device);
+
std::pair<std::unique_ptr<HIDGamepad>, unsigned> removedGamepad = removeGamepadForDevice(device);
ASSERT(removedGamepad.first);
- if (!m_shouldDispatchCallbacks)
- return;
+ // Any time we get a device removed callback we know it's a real event and not an 'already connected' event.
+ // We should always stop supressing callbacks when we receive such an event.
+ m_shouldDispatchCallbacks = true;
for (auto& client : m_clients)
client->platformGamepadDisconnected(removedGamepad.second);
Modified: trunk/Source/WebCore/platform/mac/HIDGamepadProvider.h (170548 => 170549)
--- trunk/Source/WebCore/platform/mac/HIDGamepadProvider.h 2014-06-27 20:31:31 UTC (rev 170548)
+++ trunk/Source/WebCore/platform/mac/HIDGamepadProvider.h 2014-06-27 20:35:55 UTC (rev 170549)
@@ -30,6 +30,7 @@
#include "GamepadProvider.h"
#include "HIDGamepad.h"
+#include "Timer.h"
#include <IOKit/hid/IOHIDManager.h>
#include <wtf/Deque.h>
#include <wtf/HashMap.h>
@@ -55,13 +56,16 @@
void deviceRemoved(IOHIDDeviceRef);
void valuesChanged(IOHIDValueRef);
- void setShouldDispatchCallbacks(bool shouldDispatchCallbacks) { m_shouldDispatchCallbacks = shouldDispatchCallbacks; }
-
private:
HIDGamepadProvider();
std::pair<std::unique_ptr<HIDGamepad>, unsigned> removeGamepadForDevice(IOHIDDeviceRef);
+ void openAndScheduleManager();
+ void closeAndUnscheduleManager();
+
+ void connectionDelayTimerFired(Timer<HIDGamepadProvider>&);
+
unsigned indexForNewlyConnectedDevice();
Vector<PlatformGamepad*> m_gamepadVector;
@@ -71,6 +75,8 @@
HashSet<GamepadProviderClient*> m_clients;
bool m_shouldDispatchCallbacks;
+
+ Timer<HIDGamepadProvider> m_connectionDelayTimer;
};
} // namespace WebCore
