921565 - [Gamepad] Watch for device change notifications on Windows

Status: VERIFIED FIXED

(Core :: Hardware Abstraction Layer (HAL), defect)

Description

[(not currently active) Ted Mielczarek]

I was hoping bug 680326 would get us the notifications we needed to notice when controllers were added or removed, but that became hard because the hidden window went away. We can just roll our own in HAL though, jimm pointed out that the Windows battery code already does this:
http://mxr.mozilla.org/mozilla-central/source/hal/windows/WindowsBattery.cpp#65

We basically need to copy that code, which creates a single window with a custom window class, but have a window proc that looks for WM_DEVICECHANGE, similar to attachment 579755 [details] [diff] [review]. Then that code could trigger a rescan for new devices.

Comment 1

[(not currently active) Ted Mielczarek]

Attachment: look for WM_DEVICECHANGE in Windows gamepad backend and rescan for devices

This patch copies what the WindowsBattery code is doing by creating its own window and listening for WM_DEVICECHANGE on it and using that to force a device rescan.

Comment 2

[Jim Mathies [:jimm]]

Comment on attachment 821873 [details] [diff] [review]
look for WM_DEVICECHANGE in Windows gamepad backend and rescan for devices

Review of attachment 821873 [details] [diff] [review]:
-----------------------------------------------------------------

::: hal/windows/WindowsGamepad.cpp
@@ +633,5 @@
> +  if (msg == WM_DEVICECHANGE &&
> +      (wParam == DBT_DEVICEARRIVAL ||
> +       wParam == DBT_DEVICEREMOVECOMPLETE ||
> +       wParam == DBT_DEVNODES_CHANGED)) {
> +    if (gService)

nit - brackets

@@ +679,5 @@
>    gService->Shutdown();
>    delete gService;
>    gService = nullptr;
> +
> +  if (sHWnd) {

nit - despite checking for a valid gService in the window procedure, still probably better to have this above the destruction of the service.

Comment 3

[(not currently active) Ted Mielczarek]

https://hg.mozilla.org/integration/mozilla-inbound/rev/1f56a7160c76

Comment 4

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/1f56a7160c76

Comment 5

[(not currently active) Ted Mielczarek]

This is QA-able, FWIW. I guess this bug doesn't really have much in the way of description, but the steps to test this (it's Windows-only) would be:
1. Start Firefox
2. Load a gamepad demo, like http://luser.github.io/gamepadtest/
3. Connect a gamepad to your system
4. Press a button on the gamepad

The gamepad should show up in the demo page.

Comment 6

[u279076]

Thanks Ted. Alexandra, can you please make sure this gets tested in Firefox 28?

Comment 7

[Ada [:adalucinet]]

Mozilla/5.0 (Windows NT 5.1; rv:28.0) Gecko/20100101 Firefox/28.0
Mozilla/5.0 (Windows NT 6.0; rv:28.0) Gecko/20100101 Firefox/28.0
Mozilla/5.0 (Windows NT 6.1; rv:28.0) Gecko/20100101 Firefox/28.0
Mozilla/5.0 (Windows NT 6.2; WOW64; rv:28.0) Gecko/20100101 Firefox/28.0
Mozilla/5.0 (Windows NT 6.3; WOW64; rv:28.0) Gecko/20100101 Firefox/28.0

Verified as fixed with latest Aurora (Build ID: 20140119004001) with STR from comment 5.
Tested with Xbox 360 controller and Logitech Gamepad F310.