[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

Chris Dumez <cdumez at apple.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #272315|review?                     |review+, commit-queue-
              Flags|                            |

--- Comment #46 from Chris Dumez <cdumez at apple.com> ---
Comment on attachment 272315
  --> https://bugs.webkit.org/attachment.cgi?id=272315
Go back to initial approach

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

r=me with changes.

> Source/WebCore/history/HistoryItem.h:262
> +    // Item was created from a session restore.

I don't think this comment is useful.

> Source/WebCore/history/HistoryItem.h:263
> +    bool m_restoredFromSession { false };

we need a prefix for boolean members, I suggest m_wasRestoredFromSession.

Also, could you please move this new member next to the other boolean members above?

> Source/WebCore/loader/FrameLoader.cpp:3340
> +            if (!item.wasRestoredFromSession())

I would do the #ifdef iOS logic here.

> Source/WebKit2/WebProcess/WebPage/WebPage.cpp:2234
> +void WebPage::restoreSessionInternal(const Vector<BackForwardListItemState>& itemStates, bool restoredByAPIRequest)

Do we really need a new method? Maybe we could just add the extra parameter to restoreSession() ?

> Source/WebKit2/WebProcess/WebPage/WebPage.cpp:2238
> +#if !PLATFORM(IOS)

I would prefer if we moved the IOS ifdefing to FrameLoader::loadDifferentDocumentItem() and set the wasRestoredFromSession flag here no matter the platform. Otherwise, we end up with a boolean that has an inaccurate value on iOS.

> Source/WebKit2/WebProcess/WebPage/WebPage.cpp:2239
> +        // iOS always wants stale data after session restore.

s/wants/allows

> Source/WebKit2/WebProcess/WebPage/WebPage.cpp:2248
> +    restoreSessionInternal(itemStates, true);

Could you please make the boolean argument an enum for readability? e.g.
enum class WasRestoredByAPIRequest { No, Yes };

-- 
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/20160226/7e5d4165/attachment.html>