Bug 160760 - [WK2] Don't monitor gamepads at all unless an interested WebPageProxy is in the active window
Summary: [WK2] Don't monitor gamepads at all unless an interested WebPageProxy is in t...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit2 (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Brady Eidson
URL:
Keywords:
Depends on:
Blocks: 134076 134675
  Show dependency treegraph
 
Reported: 2016-08-10 17:00 PDT by Brady Eidson
Modified: 2016-08-24 17:33 PDT (History)
2 users (show)

See Also:


Attachments
Patch v1 (10.89 KB, patch)
2016-08-10 17:05 PDT, Brady Eidson
thorton: review+
thorton: commit-queue-
Details | Formatted Diff | Diff
PFL (11.21 KB, patch)
2016-08-11 18:01 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 2016-08-10 17:00:50 PDT
[WK2] Don't monitor gamepads at all unless an interested WebPageProxy is in the active window
Comment 1 Brady Eidson 2016-08-10 17:05:09 PDT
Created attachment 285786 [details]
Patch v1
Comment 2 Tim Horton 2016-08-10 17:13:38 PDT
Comment on attachment 285786 [details]
Patch v1

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

> Source/WebKit2/UIProcess/Cocoa/WebViewImpl.mm:1061
> +        UIGamepadProvider::singleton().webPageBecameActive(m_page.get());

Don't know if this is the best name for this. It's not about a "web page".

> Source/WebKit2/UIProcess/Gamepad/UIGamepadProvider.cpp:160
> +void UIGamepadProvider::webPageBecameActive(WebPageProxy& page)

Don't you want windowIsKey && webViewIsFirstResponder? Or is the first-responderiness covered by m_processPoolsUsingGamepads?

> Source/WebKit2/UIProcess/Gamepad/UIGamepadProvider.h:83
> +    WebCore::Timer m_gamepadSyncTimer;
> +    WebCore::Timer m_disableMonitoringTimer;

Isn't using WebCore::Timer in the UI process a big no-no? (because of coexistence with the Web thread) I think you should be using the runloop timer thingy.
Comment 3 Brady Eidson 2016-08-11 17:49:32 PDT
(In reply to comment #2)
> Comment on attachment 285786 [details]
> Patch v1
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=285786&action=review
> 
> > Source/WebKit2/UIProcess/Gamepad/UIGamepadProvider.cpp:160
> > +void UIGamepadProvider::webPageBecameActive(WebPageProxy& page)
> 
> Don't you want windowIsKey && webViewIsFirstResponder? Or is the
> first-responderiness covered by m_processPoolsUsingGamepads?
> 

As we need it both at registration time and "event firing" time, first responderiness is covered by platformWebPageProxyForGamepadInput,
Comment 4 Brady Eidson 2016-08-11 17:49:58 PDT
Fixing the other stuff and building ATM
Comment 5 Brady Eidson 2016-08-11 18:01:14 PDT
Created attachment 285877 [details]
PFL
Comment 6 WebKit Commit Bot 2016-08-11 18:31:22 PDT
Comment on attachment 285877 [details]
PFL

Clearing flags on attachment: 285877

Committed r204401: <http://trac.webkit.org/changeset/204401>
Comment 7 Brady Eidson 2016-08-24 17:33:14 PDT
This works.