[webkit-reviews] review granted: [Bug 134009] [iOS][WK2] Re-sync didCommitLoadForMainFrame with its corresponding tile update : [Attachment 233284] Patch

[bugzilla-daemon at webkit.org]

Tim Horton <thorton at apple.com> has granted Benjamin Poulain
<benjamin at webkit.org>'s request for review:
Bug 134009: [iOS][WK2] Re-sync didCommitLoadForMainFrame with its corresponding
tile update
https://bugs.webkit.org/show_bug.cgi?id=134009

Attachment 233284: Patch
https://bugs.webkit.org/attachment.cgi?id=233284&action=review

------- Additional Comments from Tim Horton <thorton at apple.com>
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())->ne
xtLayerTreeIdTransactionID();

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?