Bug 132307 – [iOS][WK2] Restore the scroll position and scale from the HistoryItem (mostly)

• Component: New Bugs
• Status: RESOLVED FIXED

[Benjamin Poulain]

[iOS][WK2] Restore the scroll position and scale from the HistoryItem (mostly)

[Benjamin Poulain]

Created attachment 230328 [details]
Patch

[Simon Fraser (smfr)]

Comment on attachment 230328 [details]
Patch

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

> Source/WebKit2/WebProcess/WebCoreSupport/ios/WebFrameLoaderClientIOS.mm:90
> +            if (page && m_frame->isMainFrame() && currentItem->pageScaleFactor())
> +                m_frame->page()->restorePageState(currentItem->pageScaleFactor(), currentItem->scrollPoint());

Is the currentItem->pageScaleFactor() test useful, or just trying to catch odd uninitialized cases?

> Source/WebKit2/WebProcess/WebPage/ios/WebPageIOS.mm:144
> +    FrameView& frameView = *m_page->mainFrame().view();
> +    frameView.setScrollPosition(scrollPosition);

Odd to put FrameView in a local when you only use it once.

> Source/WebKit2/WebProcess/WebPage/ios/WebPageIOS.mm:145
> +    m_userHasChangedPageScaleFactor = true;

Why? Add a comment?

[Benjamin Poulain]

(In reply to comment #2)
> (From update of attachment 230328 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=230328&action=review
> 
> > Source/WebKit2/WebProcess/WebCoreSupport/ios/WebFrameLoaderClientIOS.mm:90
> > +            if (page && m_frame->isMainFrame() && currentItem->pageScaleFactor())
> > +                m_frame->page()->restorePageState(currentItem->pageScaleFactor(), currentItem->scrollPoint());
> 
> Is the currentItem->pageScaleFactor() test useful, or just trying to catch odd uninitialized cases?

Zero is the default value of HistoryItem.

It is a weird API, I would prefer if HistoryItem would keep a separate state to know if values have been set. Relying on the default pageScaleFactor is odd.

[Benjamin Poulain]

Created attachment 230345 [details]
Patch

[Benjamin Poulain]

Committed r167916: <http://trac.webkit.org/changeset/167916>

[Benjamin Poulain]

Comment on attachment 230345 [details]
Patch

Uploaded this again by accident.

[Simon Fraser (smfr)]

<rdar://problem/16031704>