[Webkit-unassigned] [Bug 13128] Safari not obeying cache header

[bugzilla-daemon at webkit.org]

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





------- Comment #19 from ddkilzer at webkit.org  2009-06-04 08:42 PDT -------
(From update of attachment 30930)
>Index: WebCore/ForwardingHeaders/runtime/DateMath.h
>===================================================================
>--- WebCore/ForwardingHeaders/runtime/DateMath.h	(revision 0)
>+++ WebCore/ForwardingHeaders/runtime/DateMath.h	(revision 0)
>@@ -0,0 +1,4 @@
>+#ifndef WebCore_FWD_DateMath_h
>+#define WebCore_FWD_DateMath_h
>+#include <JavaScriptCore/DateMath.h>
>+#endif

It would be nice to have this forwarding header added elsewhere for
consistency:

JavaScriptGlue/ForwardingHeaders/runtime
WebKit/mac/ForwardingHeaders/runtime

>Index: WebCore/loader/CachedResource.cpp
>===================================================================
>--- WebCore/loader/CachedResource.cpp	(revision 44396)
>+++ WebCore/loader/CachedResource.cpp	(working copy)
>@@ -26,12 +26,14 @@
> 
> #include "Cache.h"
> #include "CachedResourceHandle.h"
>+#include "CString.h"

Is this still needed?  It was probably added for debugging.

>Index: WebCore/platform/network/ResourceResponseBase.cpp
>===================================================================
>--- WebCore/platform/network/ResourceResponseBase.cpp	(revision 44396)
>+++ WebCore/platform/network/ResourceResponseBase.cpp	(working copy)
>@@ -35,6 +38,34 @@ namespace WebCore {
> 
> static void parseCacheHeader(const String& header, Vector<pair<String, String> >& result);
> 
>+ResourceResponseBase::ResourceResponseBase()  
>+    : m_expectedContentLength(0)
>+    , m_httpStatusCode(0)
>+    , m_isNull(true)
>+    , m_haveParsedCacheControlHeader(false)
>+    , m_haveParsedAgeHeader(false)
>+    , m_haveParsedDateHeader(false)
>+    , m_haveParsedExpiresHeader(false)
>+    , m_haveParsedLastModifiedHeader(false)
>+{
>+}
>+
>+ResourceResponseBase::ResourceResponseBase(const KURL& url, const String& mimeType, long long expectedLength, const String& textEncodingName, const String& filename)
>+    : m_url(url)
>+    , m_mimeType(mimeType)
>+    , m_expectedContentLength(expectedLength)
>+    , m_textEncodingName(textEncodingName)
>+    , m_suggestedFilename(filename)
>+    , m_httpStatusCode(0)
>+    , m_isNull(false)
>+    , m_haveParsedCacheControlHeader(false)
>+    , m_haveParsedAgeHeader(false)
>+    , m_haveParsedDateHeader(false)
>+    , m_haveParsedExpiresHeader(false)
>+    , m_haveParsedLastModifiedHeader(false)
>+{
>+}

Any particular reason the constructors were moved from the header into the
source file?  They won't be inlined any longer if they're defined here.

>Index: WebCore/platform/network/ResourceResponseBase.h
>===================================================================
>--- WebCore/platform/network/ResourceResponseBase.h	(revision 44396)
>+++ WebCore/platform/network/ResourceResponseBase.h	(working copy)
>@@ -147,16 +117,26 @@ protected:
>     int m_httpStatusCode;
>     String m_httpStatusText;
>     HTTPHeaderMap m_httpHeaderFields;
>-    time_t m_expirationDate;
>-    time_t m_lastModifiedDate;
>-    bool m_isNull : 1;
> 
>+    bool m_isNull : 1;
>+    
> private:
>     void parseCacheControlDirectives() const;
> 
>-    mutable bool m_haveParsedCacheControl : 1;
>-    mutable bool m_cacheControlContainsMustRevalidate : 1;
>+    mutable bool m_haveParsedCacheControlHeader : 1;
>+    mutable bool m_haveParsedAgeHeader : 1;
>+    mutable bool m_haveParsedDateHeader : 1;
>+    mutable bool m_haveParsedExpiresHeader : 1;
>+    mutable bool m_haveParsedLastModifiedHeader : 1;
>+
>     mutable bool m_cacheControlContainsNoCache : 1;
>+    mutable bool m_cacheControlContainsMustRevalidate : 1;
>+    mutable double m_cacheControlMaxAge;
>+
>+    mutable double m_age;
>+    mutable double m_date;
>+    mutable double m_expires;
>+    mutable double m_lastModified;
> };

Any 64-bit class size considerations here?  I think Simon was moving doubles
above bool bitfields in other classes.  Maybe it would be a good idea to group
the bool bitfields together even if you have to switch between private and
protected.

The only thing that I want to do is to compare the RFC language to the
implementation.  (Thanks for adding appropriate comments to the source!) 
Otherwise, this patch looks great!


-- 
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.