The networking process is allowed to app nap if all web pages are also currently in app nap.* In order to detect whether any page in any process currently requires the networking process to be active we: - maintains hash sets in every WebProcessProxy of pages that are okay with suppression. - if anything changes, the WebContext iterates every WebProcessProxy to recompute state. This is all crazy - all we actually need is a simple count of the number of pages that need to prevent the networking process from entering app nap. This patch gets us half way there - replace the HashSet with a RefCounter. Next step will be to hoist the RefCounters from the process proxies up to the context to do away with the iteration. (* Actually, more specifically the networking process will app nap if all pages are visually idle - that's probably a bug, we're failing to take media & page load state into account.)
Created attachment 242289 [details] Fix
Created attachment 243151 [details] Fix
Comment on attachment 243151 [details] Fix View in context: https://bugs.webkit.org/attachment.cgi?id=243151&action=review We should consider removing all the #ifdefs. The concepts seems useful for other platforms. Something I don't see: shouldn't there be code updating the counter when the settings change? > Source/WebKit2/UIProcess/WebPageProxy.cpp:391 > + updateProccessSuppressionState(); Why not make updateProccessSuppressionState() a step of updateActivityToken()? (and maybe rename updateActivityToken) > Source/WebKit2/UIProcess/WebPageProxy.cpp:1283 > + m_preventProcessSuppression.clear(); m_preventProcessSuppression = nullptr? > Source/WebKit2/UIProcess/WebPageProxy.cpp:4567 > + m_preventProcessSuppression.clear(); m_preventProcessSuppression = nullptr? > Source/WebKit2/UIProcess/WebProcessProxy.h:123 > + PassRefPtr<RefCounter::Count> preventProcessSuppressionForPage() The function name is an action, that is no good for a getter. Not sure what would be a better name. Maybe processSuppressionCounter()? > Source/WebKit2/UIProcess/WebProcessProxy.h:234 > + RefCounter m_pagesPreventingSuppression; m_pagesPreventingSuppression -> m_pagesPreventingSuppressionCount/m_pagesPreventingSuppressionCounter/m_processSupressionCounter?
(In reply to comment #3) > Comment on attachment 243151 [details] > Fix > > View in context: > https://bugs.webkit.org/attachment.cgi?id=243151&action=review > > We should consider removing all the #ifdefs. The concepts seems useful for > other platforms. Yep, that seems reasonable. > Something I don't see: shouldn't there be code updating the counter when the > settings change? That happens from WebPageProxy::preferencesDidChange(). (though this can probably go away after in the next patch). > > Source/WebKit2/UIProcess/WebPageProxy.cpp:391 > > + updateProccessSuppressionState(); > > Why not make updateProccessSuppressionState() a step of > updateActivityToken()? (and maybe rename updateActivityToken) Yeah, the repeated separate calls aren't great. :-( This should probably be merged out to a separate subclass, similar to PageThrotter in WebCore. I'm gonna leave them separate for now. > > Source/WebKit2/UIProcess/WebPageProxy.cpp:1283 > > + m_preventProcessSuppression.clear(); > > m_preventProcessSuppression = nullptr? > > > Source/WebKit2/UIProcess/WebPageProxy.cpp:4567 > > + m_preventProcessSuppression.clear(); > > m_preventProcessSuppression = nullptr? Done. > > Source/WebKit2/UIProcess/WebProcessProxy.h:123 > > + PassRefPtr<RefCounter::Count> preventProcessSuppressionForPage() > > The function name is an action, that is no good for a getter. > > Not sure what would be a better name. Maybe processSuppressionCounter()? Done. > > Source/WebKit2/UIProcess/WebProcessProxy.h:234 > > + RefCounter m_pagesPreventingSuppression; > > m_pagesPreventingSuppression -> > m_pagesPreventingSuppressionCount/m_pagesPreventingSuppressionCounter/ > m_processSupressionCounter? Done. https://trac.webkit.org/changeset/177209