Bug 214010 - Get rid of concept of "initial connected gamepads"
Summary: Get rid of concept of "initial connected gamepads"
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Brady Eidson
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2020-07-06 15:46 PDT by Brady Eidson
Modified: 2020-07-06 18:35 PDT (History)
2 users (show)

See Also:


Attachments
Patch (32.19 KB, patch)
2020-07-06 16:19 PDT, Brady Eidson
thorton: review+
Details | Formatted Diff | Diff
Patch (32.19 KB, patch)
2020-07-06 16:24 PDT, Brady Eidson
no flags Details | Formatted Diff | Diff
Patch (32.24 KB, patch)
2020-07-06 16:27 PDT, Brady Eidson
thorton: review+
Details | Formatted Diff | Diff
For landing (32.23 KB, patch)
2020-07-06 17:58 PDT, Brady Eidson
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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>