Bug 134009 – [iOS][WK2] Re-sync didCommitLoadForMainFrame with its corresponding tile update

• Component: New Bugs
• Status: RESOLVED FIXED

[Benjamin Poulain]

[iOS][WK2] Re-sync didCommitLoadForMainFrame with its corresponding tile update

[Benjamin Poulain]

Created attachment 233274 [details]
Patch

[Benjamin Poulain]

<rdar://problem/17352039>

[WebKit Commit Bot]

Thanks for the patch. If this patch contains new public API please make sure it follows the guidelines for new WebKit2 GTK+ API. See http://trac.webkit.org/wiki/WebKitGTK/AddingNewWebKit2API

[Tim Horton]

Comment on attachment 233274 [details]
Patch

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

> Source/WebKit2/UIProcess/API/Cocoa/WKWebView.mm:705
> +- (void)_didCommitLoadForMainFrameWithNextDrawingTransactionID:(uint64_t)nextDrawingTransactionID

The transaction IDs are for the whole commit, which includes layer tree updates, scrolling updates, render tree size, scale factors, etc. Maybe just "TransactionID", instead of Drawing-?

> Source/WebKit2/UIProcess/API/gtk/PageClientImpl.cpp:281
> +void PageClientImpl::didCommitLoadForMainFrame(const String& /* mimeType */, bool /* useCustomContentProvider */, uint64_t /* nextDrawingTransactionID */)

I really don't think we should propagate this RemoteLayerTreeDrawingArea-specific thing through everywhere, but I'm not sure how else to do this.

[Benjamin Poulain]

Created attachment 233284 [details]
Patch

[Tim Horton]

Comment on attachment 233284 [details]
Patch

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

> Source/WebKit2/ChangeLog:8
> +        WKWebView assumed  the first _didCommitLayerTree: after _didCommitLoadForMainFrame

extra space between 'assumed' and 'the'

> Source/WebKit2/ChangeLog:11
> +        This is not always true. Sometimes, a set of tile can be rendered in the CoreAnimation thread

tile*s*, and it's not the CoreAnimation thread, it's our own layer flush thread (com.apple.WebKit.WebContent.RemoteLayerTreeDrawingArea.CommitQueue).

> Source/WebKit2/UIProcess/API/Cocoa/WKWebView.mm:708
> +    _firstPaintAfterCommitLoadTransactionID = static_cast<WebKit::RemoteLayerTreeDrawingAreaProxy*>(_page->drawingArea())->nextLayerTreeIdTransactionID();

Don't we have toRemoteLayerTreeDrawingAreaProxy()? If not, maybe add the macros so we do?

> Source/WebKit2/UIProcess/mac/RemoteLayerTreeDrawingAreaProxy.h:52
> +    uint64_t nextLayerTreeIdTransactionID() const { return m_lastWillCommitLayerTreeTransactionID + 1; }

what's with the Id in the middle of this name? should just be nextLayerTreeTransactionID I think.

> Source/WebKit2/UIProcess/mac/RemoteLayerTreeDrawingAreaProxy.h:96
> +    uint64_t m_lastWillCommitLayerTreeTransactionID;

maybe this is m_pendingLayerTreeTransactionID?

[Benjamin Poulain]

Committed r170115: <http://trac.webkit.org/changeset/170115>