[webkit-reviews] review granted: [Bug 134150] [iOS][WK2] Make the state restore from HistoryItem more precise and reliable : [Attachment 233626] 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 134150: [iOS][WK2] Make the state restore from HistoryItem more precise and
reliable
https://bugs.webkit.org/show_bug.cgi?id=134150

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

------- Additional Comments from Tim Horton <thorton at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=233626&action=review


> Source/WebCore/history/CachedPage.cpp:93
> +	   bool hadProhibitsScrolling = false;

maybe explain this in the changelog?

> Source/WebCore/loader/HistoryController.cpp:82
> +    FrameView& frameView = *m_frame.view();

might as well move this up a little further and use it in more places

> Source/WebKit2/UIProcess/API/Cocoa/WKWebView.mm:176
> +    uint64_t _firstTransactionIdAfterPageRestore;

ID, not Id.

> Source/WebKit2/UIProcess/API/Cocoa/WKWebView.mm:785
> +    if (_needsToRestoreExposedRect && layerTreeTransaction.transactionID()
>= _firstTransactionIdAfterPageRestore) {

can we factor out the layerTreeTransaction.transactionID() >=
_firstTransactionIdAfterPageRestore from these two blocks

> Source/WebKit2/UIProcess/API/Cocoa/WKWebView.mm:832
> +- (void)_restorePageSateToExposedRect:(WebCore::FloatRect)exposedRect
scale:(double)scale

typo "Sate"

> Source/WebKit2/UIProcess/API/Cocoa/WKWebView.mm:847
> +- (void)_restorePageSateToUnobscuredCenter:(WebCore::FloatPoint)center
scale:(double)scale

typo "Sate"

> Source/WebKit2/UIProcess/API/Cocoa/WKWebView.mm:857
> +    _firstTransactionIdAfterPageRestore =
toRemoteLayerTreeDrawingAreaProxy(_page->drawingArea())->nextLayerTreeTransacti
onID();

ID

> Source/WebKit2/UIProcess/WebPageProxy.h:1201
> +    void restorePageSate(const WebCore::FloatRect&, double scale);

so many Sates

> Source/WebKit2/WebProcess/WebCoreSupport/ios/WebFrameLoaderClientIOS.mm:94
> +	   if (m_frame->isMainFrame())
> +	       m_frame->page()->restorePageState(*currentItem);

might as well use frame instead of m_frame

> Source/WebKit2/WebProcess/WebPage/WebPage.h:1220
> +    uint64_t m_lastLayerTreeTransactionIdBeforeDidCommitLoad;

ID

> Source/WebKit2/WebProcess/WebPage/ios/WebPageIOS.mm:171
> +	   // When the content size change, we keep the same relative
horizontal content width in view, otherwise we would

change*s*

> Source/WebKit2/WebProcess/WebPage/ios/WebPageIOS.mm:172
> +	   // end up zoom to far in landscape->portrait, and too close in
portrait->landscape.

zoom*ed*, to*o*