Clean up ProcessThrottler

• Component: WebKit2
• Status: RESOLVED FIXED

[Gavin Barraclough]

Piggybacking on the setViewState reply to synchronize is a little hacky, and we'll want a to inform the process that it will suspend in cases other than visibility change. Add an explicit handshake for this purpose.

[Gavin Barraclough]

Created attachment 230749 [details]
Fix

[WebKit Commit Bot]

Attachment 230749 [details] did not pass style-queue:


ERROR: Source/WebKit2/UIProcess/ios/ProcessThrottler.h:60:  The parameter name "process" adds no information, so it should be removed.  [readability/parameter_name] [5]
Total errors found: 1 in 15 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Geoffrey Garen]

Comment on attachment 230749 [details]
Fix

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

r=me, but I might have found a bug.

> Source/WebKit2/ChangeLog:21
> +        longer needs request a reply from SetViewState, and WebPageProxy can lose all the 'hiding'

"needs to"

> Source/WebKit2/UIProcess/WebPageProxy.cpp:1101
> +    else if (!m_activityToken)
> +        m_activityToken = std::make_unique<ProcessThrottler::ForegroundActivityToken>(m_process->throttler());

If I already have an activity token, but it is Background instead of Foreground, won't this !m_activityToken check prevent me from entering the Foreground state when I should?

Seems like you should check !m_activityToken || m_activityToken->isBackgroundActivityToken() or something.

[Gavin Barraclough]

(In reply to comment #3)
> (From update of attachment 230749 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=230749&action=review
> 
> r=me, but I might have found a bug.
> 
> > Source/WebKit2/ChangeLog:21
> > +        longer needs request a reply from SetViewState, and WebPageProxy can lose all the 'hiding'
> 
> "needs to"
> 
> > Source/WebKit2/UIProcess/WebPageProxy.cpp:1101
> > +    else if (!m_activityToken)
> > +        m_activityToken = std::make_unique<ProcessThrottler::ForegroundActivityToken>(m_process->throttler());
> 
> If I already have an activity token, but it is Background instead of Foreground, won't this !m_activityToken check prevent me from entering the Foreground state when I should?
> 
> Seems like you should check !m_activityToken || m_activityToken->isBackgroundActivityToken() or something.

No – the WebPageProxy no longer has to know about keeping the page running the background state – that is now all invisible to it. WebPageProxy simply takes a ForegroundActivityToken when the page is visible, and releases it when the page is hidden. The ProcessThrottler is now responsible for transitioning from Foreground to Suspended via a Background state.

[Gavin Barraclough]

Transmitting file data ...............
Committed revision 168312.