[Webkit-unassigned] [Bug 153230] Network cache: old pages returned by disk cache on history navigation after session is restored

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=153230

--- Comment #24 from Carlos Garcia Campos <cgarcia at igalia.com> ---
(In reply to comment #23)
> Comment on attachment 270743 [details]
> Try to fix mac builds
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=270743&action=review
> 
> I think Sam or Anders need to review this.
> 
> > Source/WebCore/ChangeLog:11
> > +        policy cache to ensure it's revalidated by the disk cache if needed.
> 
> I would say "to ensure we do not use stale cached data in this case". As
> this seems to be the intent.

Not really, we might still use the cached data if the revalidation is not needed, which is what happens very often. If you close the browser, and then open it, most of the resources are used from the disk cache without revalidation (responseNeedsRevalidation returns false).

> > Source/WebKit2/WebProcess/WebPage/WebPage.cpp:1199
> > +    // iOS always want stale data for history navigation.
> 
> 'wants'.
> 
> I don't quite get this comment because it is not just iOS, so does everyone
> else. The only case some platforms do not want stale data is for session
> restore (Which used to be considered as a history navigation but does not
> really have to).

Well, that's what I meant with "always", but I agree it's not clear enough. When a session is restored, the back forward list is also restored, and it contains the current item as well. So, if you go back/forward after a session restore, it's indeed a history navigation. And the fact that we do a history navigation for the current item too is because it's also restored in the back forward list, if we do a normal navigation we duplicate the entry in the bf list, so going back after session restore would load the same page again.

> > Source/WebKit2/WebProcess/WebPage/WebPage.cpp:1218
> > +    m_page->goToItem(*item, frameLoadTypeForBackForwardItem(backForwardItemID, FrameLoadType::Forward));
> 
> So we will use SessionRestoredBackForward on iOS even though this has
> nothing to do with session restore? This seems wrong.

No, on iOS frameLoadTypeForBackForwardItem always returns the proposed type, FrameLoadType::Forward in this case, so this patch doesn't affect iOS at all. For other ports SessionRestoredBackForward is only returned for history items created for a session restore. So, this has to do with session restore if the item we are about to navigate to was created from the session data.

> > Source/WebKit2/WebProcess/WebPage/WebPage.cpp:1233
> > +    m_page->goToItem(*item, frameLoadTypeForBackForwardItem(backForwardItemID, FrameLoadType::Back));
> 
> ditto.

iOS will use FrameLoadType::Back here, not SessionRestoredBackForward.

> > Source/WebKit2/WebProcess/WebPage/WebPage.cpp:1248
> > +    m_page->goToItem(*item, frameLoadTypeForBackForwardItem(backForwardItemID, FrameLoadType::IndexedBackForward));
> 
> ditto, this is not only used for session restore.

This is not about restoring the session, but about navigation to a history item that has been restored from the session. If we only care about the actual navigation that happens right after the session restore, we would show updated contents for the current page, but if then you go back, you could see very old contents again.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.webkit.org/pipermail/webkit-unassigned/attachments/20160206/3876d3a5/attachment.html>