Bug 146179 – [WK2][iOS] Avoid synchronous IPC on view state change when the content is not visible

• Component: WebKit2
• Status: RESOLVED FIXED

[Chris Dumez]

After <http://trac.webkit.org/changeset/170787>, viewStateChange() would cause a sync IPC between the UIProcess and the WebProcess when the view becomes visible, to avoid painting empty/black tiles when unsuspending the WebProcess (on tab switch), in the event volatile IOSurfaces were purged. This sync IPC has performance implications and is not needed when the content is not visible (e.g. hideContentUntilNextUpdate() was called, or the tab was killed).

My proposal is to avoid the sync IPC when the content is hidden and to expose a private API on WKWebView so that the client can ask for the content to be hidden until the next update.
This would allow for clients to avoid the sync IPC if they don't need the content to be displayed synchronously.

Radar: <rdar://problem/20923432>

[Chris Dumez]

Created attachment 255278 [details]
Patch

[Chris Dumez]

Comment on attachment 255278 [details]
Patch

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

> Source/WebKit2/UIProcess/WebPageProxy.cpp:1376
>          m_viewStateChangeWantsSynchronousReply = true;

I wish we could just call "drawingArea->hideContentUntilNextUpdate()" here but it will display a grey tab until we get the IPC back, and I believe Tim told me this wasn't OK.

[Tim Horton]

(In reply to comment #2)
> Comment on attachment 255278 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=255278&action=review
> 
> > Source/WebKit2/UIProcess/WebPageProxy.cpp:1376
> >          m_viewStateChangeWantsSynchronousReply = true;
> 
> I wish we could just call "drawingArea->hideContentUntilNextUpdate()" here
> but it will display a grey tab until we get the IPC back, and I believe Tim
> told me this wasn't OK.

Right, tab switch isn't allowed to flash unless it's unavoidable (sync IPC in both directions, for example).

[Tim Horton]

Comment on attachment 255278 [details]
Patch

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

> Source/WebKit2/UIProcess/API/Cocoa/WKWebView.mm:2594
> +- (void)_hideContentUntilNextUpdate

Sad to expose this implementation detail, but it's not the worst thing.

> Source/WebKit2/UIProcess/mac/RemoteLayerTreeDrawingAreaProxy.mm:426
> +    return !m_remoteLayerTreeHost.rootLayer();

Technically this could also mean that we haven't gotten the first commit yet (making the name slightly but probably not meaningfully inaccurate).

[Chris Dumez]

Comment on attachment 255278 [details]
Patch

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

>> Source/WebKit2/UIProcess/mac/RemoteLayerTreeDrawingAreaProxy.mm:426
>> +    return !m_remoteLayerTreeHost.rootLayer();
> 
> Technically this could also mean that we haven't gotten the first commit yet (making the name slightly but probably not meaningfully inaccurate).

Right, would "hasVisibleContent()" be better?

[Tim Horton]

Comment on attachment 255278 [details]
Patch

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

>>> Source/WebKit2/UIProcess/mac/RemoteLayerTreeDrawingAreaProxy.mm:426
>>> +    return !m_remoteLayerTreeHost.rootLayer();
>> 
>> Technically this could also mean that we haven't gotten the first commit yet (making the name slightly but probably not meaningfully inaccurate).
> 
> Right, would "hasVisibleContent()" be better?

I think so.

[Babak Shafiei]

rdar://problem/20923432

[Chris Dumez]

Created attachment 255296 [details]
Patch

[Chris Dumez]

Comment on attachment 255296 [details]
Patch

Clearing flags on attachment: 255296

Committed r185799: <http://trac.webkit.org/changeset/185799>

[Chris Dumez]

All reviewed patches have been landed.  Closing bug.