Bug 146179 - [WK2][iOS] Avoid synchronous IPC on view state change when the content is not visible
Summary: [WK2][iOS] Avoid synchronous IPC on view state change when the content is not...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit2 (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Chris Dumez
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2015-06-19 22:11 PDT by Chris Dumez
Modified: 2015-06-20 13:51 PDT (History)
6 users (show)

See Also:


Attachments
Patch (6.52 KB, patch)
2015-06-19 22:18 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff
Patch (6.52 KB, patch)
2015-06-20 13:49 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Chris Dumez 2015-06-19 22:11:25 PDT
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>
Comment 1 Chris Dumez 2015-06-19 22:18:18 PDT
Created attachment 255278 [details]
Patch
Comment 2 Chris Dumez 2015-06-19 22:29:59 PDT
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.
Comment 3 Tim Horton 2015-06-19 22:53:30 PDT
(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).
Comment 4 Tim Horton 2015-06-20 00:14:53 PDT
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).
Comment 5 Chris Dumez 2015-06-20 09:40:59 PDT
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?
Comment 6 Tim Horton 2015-06-20 11:27:27 PDT
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.
Comment 7 Babak Shafiei 2015-06-20 11:39:48 PDT
rdar://problem/20923432
Comment 8 Chris Dumez 2015-06-20 13:49:53 PDT
Created attachment 255296 [details]
Patch
Comment 9 Chris Dumez 2015-06-20 13:51:18 PDT
Comment on attachment 255296 [details]
Patch

Clearing flags on attachment: 255296

Committed r185799: <http://trac.webkit.org/changeset/185799>
Comment 10 Chris Dumez 2015-06-20 13:51:26 PDT
All reviewed patches have been landed.  Closing bug.