[iOS][WK2] Make the state restore from HistoryItem more precise and reliable
Created attachment 233506 [details] Patch
<rdar://problem/16031704>
Comment on attachment 233506 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=233506&action=review Too many things to take in right now, but some minor comments: > Source/WebKit2/Shared/VisibleContentRectUpdateInfo.h:77 > + uint64_t lastLayerTreeTransactionId() const { return m_lastLayerTreeTransactionId; } it's ID, not Id, everywhere else > Source/WebKit2/WebProcess/WebPage/ios/WebPageIOS.mm:2361 > + // Skip any VisibleContentRectUpdate that have been queued before DidCommitLoad suppress the updates on the UIProcess. there's at least one word missing in this sentence > Source/WebKit2/WebProcess/WebPage/ios/WebPageIOS.mm:2362 > + if (visibleContentRectUpdateInfo.lastLayerTreeTransactionId() <= m_lastLayerTreeTransactionBeforeDidCommitLoad) shouldn't m_lastLayerTreeTransactionBeforeDidCommitLoad have "ID" in it somewhere? > Source/WebKit2/WebProcess/WebPage/mac/RemoteLayerTreeDrawingArea.h:56 > + uint64_t currentTransactionID() { return m_currentTransactionID; } const
Comment on attachment 233506 [details] Patch Let's not add more things to HistoryItem; I'm working on a better way to do state restoration, and I'm planning to have something landed early next week. Maybe we can revisit this patch then?
(In reply to comment #4) > (From update of attachment 233506 [details]) > Let's not add more things to HistoryItem; I'm working on a better way to do state restoration, and I'm planning to have something landed early next week. Maybe we can revisit this patch then? Are you kidding? I have been debugging this crap for a week. Why didn't you mention it last monday?
Created attachment 233626 [details] Patch
Comment on attachment 233626 [details] Patch 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())->nextLayerTreeTransactionID(); 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*
Committed r170325: <http://trac.webkit.org/changeset/170325>