Bug 214010

Summary: Get rid of concept of "initial connected gamepads"
Product: WebKit Reporter: Brady Eidson <beidson>
Component: WebCore Misc.Assignee: Brady Eidson <beidson>
Status: RESOLVED FIXED    
Severity: Normal CC: thorton, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
thorton: review+
Patch
none
Patch
thorton: review+
For landing none

Description Brady Eidson 2020-07-06 15:46:21 PDT 
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.
Comment 1 Brady Eidson 2020-07-06 16:19:33 PDT 
Created attachment 403630 [details]
Patch
Comment 2 Tim Horton 2020-07-06 16:24:13 PDT 
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.
Comment 3 Brady Eidson 2020-07-06 16:24:47 PDT 
Created attachment 403632 [details]
Patch
Comment 4 Brady Eidson 2020-07-06 16:27:52 PDT 
Created attachment 403633 [details]
Patch
Comment 5 Brady Eidson 2020-07-06 16:28:32 PDT 
(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)
Comment 6 Tim Horton 2020-07-06 17:46:39 PDT 
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?)
Comment 7 Brady Eidson 2020-07-06 17:58:51 PDT 
Created attachment 403649 [details]
For landing
Comment 8 Brady Eidson 2020-07-06 17:59:32 PDT 
(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)
Comment 9 EWS 2020-07-06 18:34:53 PDT 
Committed r264004: <https://trac.webkit.org/changeset/264004>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 403649 [details].
Comment 10 Radar WebKit Bug Importer 2020-07-06 18:35:16 PDT 
<rdar://problem/65157303>