Bug 110652 - add to HistoryItem a way to know if its underlying CachedPage has expired
Summary: add to HistoryItem a way to know if its underlying CachedPage has expired
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: History (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Alice Liu
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-02-22 15:31 PST by Alice Liu
Modified: 2013-03-14 00:34 PDT (History)
5 users (show)

See Also:


Attachments
not for review, just running through bots (13.04 KB, patch)
2013-02-22 15:54 PST, Alice Liu
buildbot: commit-queue-
Details | Formatted Diff | Diff
patch (12.69 KB, patch)
2013-02-22 18:13 PST, Alice Liu
beidson: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Alice Liu 2013-02-22 15:31:48 PST
Add to HistoryItem a way to know if its underlying CachedPage has expired. 

This inevitably touches some existing code that has hard-coded the expiration time for a back/forward PageCached-item to 1800.  
It was okay to have hard-coded these because the FIXMEs made it okay.   Fixmes make everything okay.
Comment 1 Alice Liu 2013-02-22 15:54:49 PST
Created attachment 189853 [details]
not for review, just running through bots
Comment 2 Build Bot 2013-02-22 17:46:32 PST
Comment on attachment 189853 [details]
not for review, just running through bots

Attachment 189853 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://queues.webkit.org/results/16719342
Comment 3 Alice Liu 2013-02-22 18:13:41 PST
Created attachment 189888 [details]
patch
Comment 4 Brady Eidson 2013-02-26 11:17:30 PST
Comment on attachment 189888 [details]
patch

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

r+ with that change

> Source/WebCore/history/CachedPage.cpp:56
>      : m_timeStamp(currentTime())
> +    , m_expiration(m_timeStamp + page->settings()->backForwardCacheExpirationInterval())
>      , m_cachedMainFrame(CachedFrame::create(page->mainFrame()))

Could this be m_expirationTime instead?

> Source/WebCore/history/CachedPage.cpp:122
> +bool CachedPage::hasExpired() const
> +{
> +    return currentTime() > m_expiration;
> +}

If it was, this code would read more naturally.
Comment 5 Alice Liu 2013-03-14 00:34:04 PDT
committed r145789