[Webkit-unassigned] [Bug 17998] When a resource is cached locally, WebKit should follow RFC 2616 "Specific end-to-end revalidation" instead of "Unspecified end-to-end revalidation"

[bugzilla-daemon at webkit.org]

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


darin at apple.com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #25055|review?                     |review+
               Flag|                            |




------- Comment #35 from darin at apple.com  2008-11-24 13:56 PDT -------
(From update of attachment 25055)
> +    CachePolicy cachePolicy = this->cachePolicy();
> +    if (cachePolicy == CachePolicyVerify || cachePolicy == CachePolicyCache) {
> +        CachedResource* existing = cache()->resourceForURL(fullURL.string());
> +        if (existing && !existing->isPreloaded() && existing->mustRevalidate(cachePolicy)) {
> +            cache()->revalidateResource(existing, this);
> +            m_reloadedURLs.add(fullURL.string());
> +        }
> +        return;
> +    }
> +    if (cachePolicy == CachePolicyReload || cachePolicy == CachePolicyRevalidate) {
> +        CachedResource* existing = cache()->resourceForURL(fullURL.string());
> +        if (existing && !existing->isPreloaded()) {
> +            if (cachePolicy == CachePolicyReload)
> +                cache()->remove(existing);
> +            else
> +                cache()->revalidateResource(existing, this);
> +            m_reloadedURLs.add(fullURL.string());
> +        }
>      }

The above looks like it should be a switch statement.

> -    ScheduledRedirection(double redirectDelay, const String& redirectURL, bool redirectLockHistory, bool userGesture)
> +    ScheduledRedirection(double redirectDelay, const String& redirectURL, bool redirectLockHistory, bool userGesture, bool refresh)

It's a little confusing to have both the term "reload" and the term "refresh"
used.

> +    ResourceRequest request(url, referrer,  refresh ? ReloadIgnoringCacheData : UseProtocolCachePolicy);

Two spaces here after the comma.

> -        scheduleRedirection(new ScheduledRedirection(delay, url, delay <= 1, false));
> +        scheduleRedirection(new ScheduledRedirection(delay, url, delay <= 1, false, false));

Yuck, I hate booleans. We should really use enums instead so we can see what
"false" means.

> +    // FIXME: We don't have a mechanism to revalidate the main resource without reloadin at the moment.

Typo here. Need a "g" on the end of "reloading".

> -        void changeLocation(const String& url, const String& referrer, bool lockHistory = true, bool userGesture = false);
> -        void changeLocation(const KURL&, const String& referrer, bool lockHistory = true, bool userGesture = false);
> +        void changeLocation(const String& url, const String& referrer, bool lockHistory = true, bool userGesture = false, bool refresh = false);
> +        void changeLocation(const KURL&, const String& referrer, bool lockHistory = true, bool userGesture = false, bool refresh = false);

You added the "= false" default here, but you also added the false boolean
explicitly to some call sites. I think you should omit it at the call sites.

r=me


-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.