Bug 214010 – Get rid of concept of "initial connected gamepads"

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[Brady Eidson]

Get rid of concept of "initial connected gamepads"

It was meant to manage the concept of when gamepads become visible to the page or not. But it's only used in the HID provider, not GameController provider, and it's not quite right anyways.

Everybody can get away with gamepad events instead having a "make gamepads visible" bit along with them.

[Brady Eidson]

Created attachment 403630 [details]
Patch

[Tim Horton]

Comment on attachment 403630 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=403630&action=review

> Source/WebCore/platform/gamepad/mac/HIDGamepadProvider.cpp:241
> +        m_inputNotificationTimer.startOneShot(0_s);

Is this a potentially-forever-looping 0-delay timer‽ Very suspicious.

[Brady Eidson]

Created attachment 403632 [details]
Patch

[Brady Eidson]

Created attachment 403633 [details]
Patch

[Brady Eidson]

(In reply to Tim Horton from comment #2)
> Comment on attachment 403630 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=403630&action=review
> 
> > Source/WebCore/platform/gamepad/mac/HIDGamepadProvider.cpp:241
> > +        m_inputNotificationTimer.startOneShot(0_s);
> 
> Is this a potentially-forever-looping 0-delay timer‽ Very suspicious.

There's "Reasons" why not a problem in practice, but making it explicit is also perfectly great :) (done in new patch)

[Tim Horton]

Comment on attachment 403633 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=403633&action=review

> Source/WebCore/platform/gamepad/mac/HIDGamepadProvider.cpp:241
> +        if (!m_inputNotificationTimer.isActive())

I’m confused how this changes things. Aren’t we still in the callback for the timer that we’re starting here? (Thus if connected stays false we’ll reschedule it forever?)

[Brady Eidson]

Created attachment 403649 [details]
For landing

[Brady Eidson]

(In reply to Tim Horton from comment #6)
> Comment on attachment 403633 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=403633&action=review
> 
> > Source/WebCore/platform/gamepad/mac/HIDGamepadProvider.cpp:241
> > +        if (!m_inputNotificationTimer.isActive())
> 
> I’m confused how this changes things. Aren’t we still in the callback for
> the timer that we’re starting here? (Thus if connected stays false we’ll
> reschedule it forever?)

The key is the timer fires, and sets the flag to true (and it never resets to false ever)

[EWS]

Committed r264004: <https://trac.webkit.org/changeset/264004>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 403649 [details].

[Radar WebKit Bug Importer]

<rdar://problem/65157303>