769797 - Stop storing expiration time for offline cache entries

Status: RESOLVED WONTFIX

(Core :: Networking: Cache, defect)

Description

[Brian Smith (:briansmith, :bsmith, use NEEDINFO?)]

+++ This bug was initially created as a clone of Bug #769796 +++

Offline cache entries never expire, and in nsHttpChannel (HttpCacheQuery), we don't check offline cache entries for expiration anyway, so we don't need to store an expiration time for them on disk.

Comment 1

[Brian Smith (:briansmith, :bsmith, use NEEDINFO?)]

I think the offline cache can just hard-code the expiration time it stores in nsCacheEntry and nsCacheEntryDescriptor objects to some "infinitely in the future" time. I don't know how to represent that time though. numeric_limits<PRUint32>::max()?

Comment 2

[Brian Smith (:briansmith, :bsmith, use NEEDINFO?)]

Note that besides hard-coding GetExpirationTime() and making SetExpirationTime() a no-op for offline cache entries, we should also remove the code in nsHttpChannel that attempts to update the offline cache entry's expiration time. (Search nsHttpChannel for "This entry's expiration time should match the main entry's").

Comment 3

[Honza Bambas (:mayhemer)]

We are using offline cache entry as a normal HTTP cache entry during offline cache updates.  So expiration time is needed to preserve performance and server load balance.

This is WONTFIX for me.

Sorry for confusing when I've said the expiration time on offline cache entries is non-sense.

Comment 4

[Thinker Li [:sinker]]

(In reply to Brian Smith (:bsmith) from comment #2)
> Note that besides hard-coding GetExpirationTime() and making
> SetExpirationTime() a no-op for offline cache entries, we should also remove
> the code in nsHttpChannel that attempts to update the offline cache entry's
> expiration time. (Search nsHttpChannel for "This entry's expiration time
> should match the main entry's").

It should not happen to update offline cache entry's expiration time.  Since we never checking updates for any offline cache entry, attempting to update an offline cache entry's expiration time is never occurred.

By the way, I found HttpCacheQuery::CheckCache() will check expiration time for any type of cache entry. It means we should return a expiration time or to change CheckCache() if I don't miss anything.

Comment 5

[Brian Smith (:briansmith, :bsmith, use NEEDINFO?)]

(In reply to Honza Bambas (:mayhemer) from comment #3)
> We are using offline cache entry as a normal HTTP cache entry during offline
> cache updates.  So expiration time is needed to preserve performance and
> server load balance.
> 
> This is WONTFIX for me.

OK.

Comment 6

[Honza Bambas (:mayhemer)]

For the record, expiration time is updated only when:
- processing 206
- processing 304
- when a "normal" cache entry is open for write
- when a response header has been modified on the channel

None of this is case for reading from offline cache.