Gamepads are a system-level thing that are them broadcast to listening windows, so dispatching them to the system group should be fine.

Comment on attachment 8883696 [details] [diff] [review] label GamepadUpdateRunnable Review of attachment 8883696 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/gamepad/ipc/GamepadEventChannelChild.cpp @@ +39,5 @@ > const GamepadChangeEvent& aGamepadEvent) > { > DebugOnly<nsresult> rv = > + SystemGroup::Dispatch("GamepadUpdate", TaskCategory::Other, > + do_AddRef(new GamepadUpdateRunnable(aGamepadEvent))); I think that GamepadUpdateRunnable touches the DOM, so SystemGroup won't work here. One thing that's odd, though, is that I see no reason for this dispatch. I think we could just do the Update call right here. We should be running on the main thread (which could be asserted). Might want to ask qdot first though. This doesn't really fix much since we'll still have to label PGamepadEventChannel::Msg_GamepadUpdate and PVRManager::Msg_GamepadUpdate. But at least it kills one useless runnable. The main work that needs to happen to actually label Gamepad is to: 1. Dispatch a separate runnable in each iteration of this loop, with the runnable directed at the |listener| window: http://searchfox.org/mozilla-central/rev/e8f4f51cd543f203e9cb861cecb7545ac43c836c/dom/gamepad/GamepadManager.cpp#540-541 This shouldn't be too expensive since we're only doing it for foreground windows that listen for gamepad events, so probably just one. We'll be adding an extra roundtrip through the event loop, but you can also remove the one above. 2. Convert PGamepadEventChannel::Msg_GamepadUpdate and PVRManager::Msg_GamepadUpdate to SystemGroup. This could be done with the GetSpecificMessageEventTarget override on the top-level actor.

Thanks for the review! (In reply to Bill McCloskey (:billm) from comment #2) > ::: dom/gamepad/ipc/GamepadEventChannelChild.cpp > @@ +39,5 @@ > > const GamepadChangeEvent& aGamepadEvent) > > { > > DebugOnly<nsresult> rv = > > + SystemGroup::Dispatch("GamepadUpdate", TaskCategory::Other, > > + do_AddRef(new GamepadUpdateRunnable(aGamepadEvent))); > > I think that GamepadUpdateRunnable touches the DOM, so SystemGroup won't > work here. One thing that's odd, though, is that I see no reason for this > dispatch. I think we could just do the Update call right here. We should be > running on the main thread (which could be asserted). Might want to ask qdot > first though. qDot? > This doesn't really fix much since we'll still have to label > PGamepadEventChannel::Msg_GamepadUpdate and PVRManager::Msg_GamepadUpdate. > But at least it kills one useless runnable. > > The main work that needs to happen to actually label Gamepad is to: > 1. Dispatch a separate runnable in each iteration of this loop, with the > runnable directed at the |listener| window: > http://searchfox.org/mozilla-central/rev/ > e8f4f51cd543f203e9cb861cecb7545ac43c836c/dom/gamepad/GamepadManager.cpp#540- > 541 > This shouldn't be too expensive since we're only doing it for foreground > windows that listen for gamepad events, so probably just one. We'll be > adding an extra roundtrip through the event loop, but you can also remove > the one above. > > 2. Convert PGamepadEventChannel::Msg_GamepadUpdate and > PVRManager::Msg_GamepadUpdate to SystemGroup. This could be done with the > GetSpecificMessageEventTarget override on the top-level actor. Does this plan sound at all sensible?

(In reply to Nathan Froyd [:froydnj] from comment #3) > > I think that GamepadUpdateRunnable touches the DOM, so SystemGroup won't > > work here. One thing that's odd, though, is that I see no reason for this > > dispatch. I think we could just do the Update call right here. We should be > > running on the main thread (which could be asserted). Might want to ask qdot > > first though. > > qDot? Huh. Yeah. No idea why that runnable is there in GamepadEventChannelChild, as we're receiving that message on the main thread anyways. I think that can be removed. > Does this plan sound at all sensible? I think that plan should work, though do we need to label any time we're firing events to listeners? If so, we may also need to add runnables in GamepadManager::NewConnectionEvent, as we send gamepad connect/disconnect events to listeners from there, and those should be scheduled with the gamepad updates.