<html>
<head>
<base href="https://bugs.webkit.org/" />
</head>
<body>
<p>
<div>
<b><a class="bz_bug_link
bz_status_NEW "
title="NEW - Network cache: old pages returned by disk cache on history navigation after session is restored"
href="https://bugs.webkit.org/show_bug.cgi?id=153230#c25">Comment # 25</a>
on <a class="bz_bug_link
bz_status_NEW "
title="NEW - Network cache: old pages returned by disk cache on history navigation after session is restored"
href="https://bugs.webkit.org/show_bug.cgi?id=153230">bug 153230</a>
from <span class="vcard"><a class="email" href="mailto:cdumez@apple.com" title="Chris Dumez <cdumez@apple.com>"> <span class="fn">Chris Dumez</span></a>
</span></b>
<pre>(In reply to <a href="show_bug.cgi?id=153230#c24">comment #24</a>)
<span class="quote">> (In reply to <a href="show_bug.cgi?id=153230#c23">comment #23</a>)
> > Comment on <span class=""><a href="attachment.cgi?id=270743&action=diff" name="attach_270743" title="Try to fix mac builds">attachment 270743</a> <a href="attachment.cgi?id=270743&action=edit" title="Try to fix mac builds">[details]</a></span>
> > Try to fix mac builds
> >
> > View in context:
> > <a href="https://bugs.webkit.org/attachment.cgi?id=270743&action=review">https://bugs.webkit.org/attachment.cgi?id=270743&action=review</a>
> >
> > 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).</span >
I know we still want to use cached data if it is fresh ( does not need revalidation or revalidation succeeded). I still think my sentence is correct. Stale != cached. Stale data is data that has expired and is no longer fresh.
<span class="quote">> > > 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.</span ></pre>
</div>
</p>
<hr>
<span>You are receiving this mail because:</span>
<ul>
<li>You are the assignee for the bug.</li>
</ul>
</body>
</html>