Changeset 182059 in webkit

Timestamp:

Mar 27, 2015, 8:13:02 AM (11 years ago)

Author:

Antti Koivisto

Message:

Respect cache-control directives in request
​https://bugs.webkit.org/show_bug.cgi?id=143121
Source/WebCore:

rdar://problem/19714040

Reviewed by Chris Dumez.

Test: http/tests/cache/disk-cache/disk-cache-request-headers.html

• loader/cache/CacheValidation.cpp:

(WebCore::isCacheHeaderSeparator):
(WebCore::isControlCharacter):
(WebCore::trimToNextSeparator):
(WebCore::parseCacheHeader):
(WebCore::parseCacheControlDirectives):

Factor Cache-control parsing here so it can be used for both requests and responses.

• loader/cache/CacheValidation.h:
• platform/network/ResourceRequestBase.h:
• platform/network/ResourceResponseBase.cpp:

(WebCore::ResourceResponseBase::ResourceResponseBase):
(WebCore::ResourceResponseBase::parseCacheControlDirectives):
(WebCore::ResourceResponseBase::cacheControlContainsNoCache):
(WebCore::ResourceResponseBase::cacheControlContainsNoStore):
(WebCore::ResourceResponseBase::cacheControlContainsMustRevalidate):
(WebCore::ResourceResponseBase::cacheControlMaxAge):
(WebCore::isCacheHeaderSeparator): Deleted.
(WebCore::isControlCharacter): Deleted.
(WebCore::trimToNextSeparator): Deleted.
(WebCore::parseCacheHeader): Deleted.

• platform/network/ResourceResponseBase.h:

Source/WebKit2:

rdar://problem/19714040

Reviewed by Chris Dumez.

Better support for ​https://tools.ietf.org/html/rfc7234#section-5.2.1

• NetworkProcess/cache/NetworkCache.cpp:

(WebKit::NetworkCache::canUse):

Consider requests with Cache-control: no-cache and max-age=0 expired.

(WebKit::NetworkCache::canStore):

Don't store requests with Cache-control: no-store.

(WebKit::NetworkCache::Cache::store):

• NetworkProcess/cache/NetworkCache.h:
• NetworkProcess/cache/NetworkCacheStatisticsCocoa.mm:

(WebKit::NetworkCache::storeDecisionToDiagnosticKey):

LayoutTests:

Reviewed by Chris Dumez.

• http/tests/cache/disk-cache/disk-cache-request-headers-expected.txt: Added.
• http/tests/cache/disk-cache/disk-cache-request-headers.html: Added.

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/http/tests/cache/disk-cache/disk-cache-request-headers-expected.txt (added)
• LayoutTests/http/tests/cache/disk-cache/disk-cache-request-headers.html (added)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/loader/cache/CacheValidation.cpp (modified) (2 diffs)
• Source/WebCore/loader/cache/CacheValidation.h (modified) (2 diffs)
• Source/WebCore/platform/network/ResourceRequestBase.h (modified) (1 diff)
• Source/WebCore/platform/network/ResourceResponseBase.cpp (modified) (12 diffs)
• Source/WebCore/platform/network/ResourceResponseBase.h (modified) (3 diffs)
• Source/WebKit2/ChangeLog (modified) (1 diff)
• Source/WebKit2/NetworkProcess/cache/NetworkCache.cpp (modified) (6 diffs)
• Source/WebKit2/NetworkProcess/cache/NetworkCache.h (modified) (1 diff)
• Source/WebKit2/NetworkProcess/cache/NetworkCacheStatisticsCocoa.mm (modified) (1 diff)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2015-03-26  Antti Koivisto  <antti@apple.com>
+
+        Respect cache-control directives in request
+        https://bugs.webkit.org/show_bug.cgi?id=143121
+
+        Reviewed by Chris Dumez.
+
+        * http/tests/cache/disk-cache/disk-cache-request-headers-expected.txt: Added.
+        * http/tests/cache/disk-cache/disk-cache-request-headers.html: Added.
+
 2015-03-27  Michael Saboff  <msaboff@apple.com>
 

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2015-03-26  Antti Koivisto  <antti@apple.com>
+
+        Respect cache-control directives in request
+        https://bugs.webkit.org/show_bug.cgi?id=143121
+        rdar://problem/19714040
+
+        Reviewed by Chris Dumez.
+
+        Test: http/tests/cache/disk-cache/disk-cache-request-headers.html
+
+        * loader/cache/CacheValidation.cpp:
+        (WebCore::isCacheHeaderSeparator):
+        (WebCore::isControlCharacter):
+        (WebCore::trimToNextSeparator):
+        (WebCore::parseCacheHeader):
+        (WebCore::parseCacheControlDirectives):
+
+            Factor Cache-control parsing here so it can be used for both requests and responses.
+
+        * loader/cache/CacheValidation.h:
+        * platform/network/ResourceRequestBase.h:
+        * platform/network/ResourceResponseBase.cpp:
+        (WebCore::ResourceResponseBase::ResourceResponseBase):
+        (WebCore::ResourceResponseBase::parseCacheControlDirectives):
+        (WebCore::ResourceResponseBase::cacheControlContainsNoCache):
+        (WebCore::ResourceResponseBase::cacheControlContainsNoStore):
+        (WebCore::ResourceResponseBase::cacheControlContainsMustRevalidate):
+        (WebCore::ResourceResponseBase::cacheControlMaxAge):
+        (WebCore::isCacheHeaderSeparator): Deleted.
+        (WebCore::isControlCharacter): Deleted.
+        (WebCore::trimToNextSeparator): Deleted.
+        (WebCore::parseCacheHeader): Deleted.
+        * platform/network/ResourceResponseBase.h:
+
 2015-03-27  Víctor Manuel Jáquez Leal  <vjaquez@igalia.com>
 

trunk/Source/WebCore/loader/cache/CacheValidation.cpp

@@ -27 +27 @@
 #include "CacheValidation.h"
 
+#include "HTTPHeaderMap.h"
 #include "ResourceResponse.h"
 #include <wtf/CurrentTime.h>
@@ -150 +151 @@
 }
 
-}
+inline bool isCacheHeaderSeparator(UChar c)
+{
+    // See RFC 2616, Section 2.2
+    switch (c) {
+    case '(':
+    case ')':
+    case '<':
+    case '>':
+    case '@':
+    case ',':
+    case ';':
+    case ':':
+    case '\\':
+    case '"':
+    case '/':
+    case '[':
+    case ']':
+    case '?':
+    case '=':
+    case '{':
+    case '}':
+    case ' ':
+    case '\t':
+        return true;
+    default:
+        return false;
+    }
+}
+
+inline bool isControlCharacter(UChar c)
+{
+    return c < ' ' || c == 127;
+}
+
+inline String trimToNextSeparator(const String& str)
+{
+    return str.substring(0, str.find(isCacheHeaderSeparator));
+}
+
+static Vector<std::pair<String, String>> parseCacheHeader(const String& header)
+{
+    Vector<std::pair<String, String>> result;
+
+    const String safeHeader = header.removeCharacters(isControlCharacter);
+    unsigned max = safeHeader.length();
+    unsigned pos = 0;
+    while (pos < max) {
+        size_t nextCommaPosition = safeHeader.find(',', pos);
+        size_t nextEqualSignPosition = safeHeader.find('=', pos);
+        if (nextEqualSignPosition == notFound && nextCommaPosition == notFound) {
+            // Add last directive to map with empty string as value
+            result.append(std::make_pair(trimToNextSeparator(safeHeader.substring(pos, max - pos).stripWhiteSpace()), ""));
+            return result;
+        }
+        if (nextCommaPosition != notFound && (nextCommaPosition < nextEqualSignPosition || nextEqualSignPosition == notFound)) {
+            // Add directive to map with empty string as value
+            result.append(std::make_pair(trimToNextSeparator(safeHeader.substring(pos, nextCommaPosition - pos).stripWhiteSpace()), ""));
+            pos += nextCommaPosition - pos + 1;
+            continue;
+        }
+        // Get directive name, parse right hand side of equal sign, then add to map
+        String directive = trimToNextSeparator(safeHeader.substring(pos, nextEqualSignPosition - pos).stripWhiteSpace());
+        pos += nextEqualSignPosition - pos + 1;
+
+        String value = safeHeader.substring(pos, max - pos).stripWhiteSpace();
+        if (value[0] == '"') {
+            // The value is a quoted string
+            size_t nextDoubleQuotePosition = value.find('"', 1);
+            if (nextDoubleQuotePosition == notFound) {
+                // Parse error; just use the rest as the value
+                result.append(std::make_pair(directive, trimToNextSeparator(value.substring(1, value.length() - 1).stripWhiteSpace())));
+                return result;
+            }
+            // Store the value as a quoted string without quotes
+            result.append(std::make_pair(directive, value.substring(1, nextDoubleQuotePosition - 1).stripWhiteSpace()));
+            pos += (safeHeader.find('"', pos) - pos) + nextDoubleQuotePosition + 1;
+            // Move past next comma, if there is one
+            size_t nextCommaPosition2 = safeHeader.find(',', pos);
+            if (nextCommaPosition2 == notFound)
+                return result; // Parse error if there is anything left with no comma
+            pos += nextCommaPosition2 - pos + 1;
+            continue;
+        }
+        // The value is a token until the next comma
+        size_t nextCommaPosition2 = value.find(',');
+        if (nextCommaPosition2 == notFound) {
+            // The rest is the value; no change to value needed
+            result.append(std::make_pair(directive, trimToNextSeparator(value)));
+            return result;
+        }
+        // The value is delimited by the next comma
+        result.append(std::make_pair(directive, trimToNextSeparator(value.substring(0, nextCommaPosition2).stripWhiteSpace())));
+        pos += (safeHeader.find(',', pos) - pos) + 1;
+    }
+    return result;
+}
+
+CacheControlDirectives parseCacheControlDirectives(const HTTPHeaderMap& headers)
+{
+    CacheControlDirectives result;
+
+    String cacheControlValue = headers.get(HTTPHeaderName::CacheControl);
+    if (!cacheControlValue.isEmpty()) {
+        auto directives = parseCacheHeader(cacheControlValue);
+
+        size_t directivesSize = directives.size();
+        for (size_t i = 0; i < directivesSize; ++i) {
+            // RFC2616 14.9.1: A no-cache directive with a value is only meaningful for proxy caches.
+            // It should be ignored by a browser level cache.
+            if (equalIgnoringCase(directives[i].first, "no-cache") && directives[i].second.isEmpty())
+                result.noCache = true;
+            else if (equalIgnoringCase(directives[i].first, "no-store"))
+                result.noStore = true;
+            else if (equalIgnoringCase(directives[i].first, "must-revalidate"))
+                result.mustRevalidate = true;
+            else if (equalIgnoringCase(directives[i].first, "max-age")) {
+                if (!std::isnan(result.maxAge)) {
+                    // First max-age directive wins if there are multiple ones.
+                    continue;
+                }
+                bool ok;
+                double maxAge = directives[i].second.toDouble(&ok);
+                if (ok)
+                    result.maxAge = maxAge;
+            }
+        }
+    }
+
+    if (!result.noCache) {
+        // Handle Pragma: no-cache
+        // This is deprecated and equivalent to Cache-control: no-cache
+        // Don't bother tokenizing the value, it is not important
+        String pragmaValue = headers.get(HTTPHeaderName::Pragma);
+
+        result.noCache = pragmaValue.contains("no-cache", false);
+    }
+
+    return result;
+}
+
+}

trunk/Source/WebCore/loader/cache/CacheValidation.h

@@ -29 +29 @@
 namespace WebCore {
 
+class HTTPHeaderMap;
 class ResourceResponse;
 
@@ -53 +54 @@
 WEBCORE_EXPORT bool redirectChainAllowsReuse(RedirectChainCacheStatus, ReuseExpiredRedirectionOrNot);
 
+struct CacheControlDirectives {
+    double maxAge { std::numeric_limits<double>::quiet_NaN() };
+    bool noCache { false };
+    bool noStore { false };
+    bool mustRevalidate { false };
+};
+WEBCORE_EXPORT CacheControlDirectives parseCacheControlDirectives(const HTTPHeaderMap&);
+
 }
 

trunk/Source/WebCore/platform/network/ResourceRequestBase.h

@@ -80 +80 @@
         WEBCORE_EXPORT void setHTTPMethod(const String& httpMethod);
         
-        const HTTPHeaderMap& httpHeaderFields() const;
+        WEBCORE_EXPORT const HTTPHeaderMap& httpHeaderFields() const;
         WEBCORE_EXPORT void setHTTPHeaderFields(HTTPHeaderMap);
 

trunk/Source/WebCore/platform/network/ResourceResponseBase.cpp

@@ -28 +28 @@
 #include "ResourceResponseBase.h"
 
+#include "CacheValidation.h"
 #include "HTTPHeaderNames.h"
 #include "HTTPParsers.h"
@@ -38 +39 @@
 namespace WebCore {
 
-static void parseCacheHeader(const String& header, Vector<std::pair<String, String>>& result);
-
 inline const ResourceResponse& ResourceResponseBase::asResourceResponse() const
 {
@@ -49 +48 @@
     , m_includesCertificateInfo(false)
     , m_httpStatusCode(0)
-    , m_cacheControlMaxAge(0)
     , m_age(0)
     , m_date(0)
@@ -60 +58 @@
     , m_haveParsedExpiresHeader(false)
     , m_haveParsedLastModifiedHeader(false)
-    , m_cacheControlContainsNoCache(false)
-    , m_cacheControlContainsNoStore(false)
-    , m_cacheControlContainsMustRevalidate(false)
     , m_source(Source::Unknown)
 {
@@ -74 +69 @@
     , m_includesCertificateInfo(true) // Empty but valid for synthetic responses.
     , m_httpStatusCode(0)
-    , m_cacheControlMaxAge(0)
     , m_age(0)
     , m_date(0)
@@ -85 +79 @@
     , m_haveParsedExpiresHeader(false)
     , m_haveParsedLastModifiedHeader(false)
-    , m_cacheControlContainsNoCache(false)
-    , m_cacheControlContainsNoStore(false)
-    , m_cacheControlContainsMustRevalidate(false)
     , m_source(Source::Unknown)
 {
@@ -359 +350 @@
     lazyInit(CommonFieldsOnly);
 
+    m_cacheControlDirectives = WebCore::parseCacheControlDirectives(m_httpHeaderFields);
     m_haveParsedCacheControlHeader = true;
-
-    m_cacheControlContainsMustRevalidate = false;
-    m_cacheControlContainsNoCache = false;
-    m_cacheControlMaxAge = std::numeric_limits<double>::quiet_NaN();
-
-    String cacheControlValue = m_httpHeaderFields.get(HTTPHeaderName::CacheControl);
-    if (!cacheControlValue.isEmpty()) {
-        Vector<std::pair<String, String>> directives;
-        parseCacheHeader(cacheControlValue, directives);
-
-        size_t directivesSize = directives.size();
-        for (size_t i = 0; i < directivesSize; ++i) {
-            // RFC2616 14.9.1: A no-cache directive with a value is only meaningful for proxy caches.
-            // It should be ignored by a browser level cache.
-            if (equalIgnoringCase(directives[i].first, "no-cache") && directives[i].second.isEmpty())
-                m_cacheControlContainsNoCache = true;
-            else if (equalIgnoringCase(directives[i].first, "no-store"))
-                m_cacheControlContainsNoStore = true;
-            else if (equalIgnoringCase(directives[i].first, "must-revalidate"))
-                m_cacheControlContainsMustRevalidate = true;
-            else if (equalIgnoringCase(directives[i].first, "max-age")) {
-                if (!std::isnan(m_cacheControlMaxAge)) {
-                    // First max-age directive wins if there are multiple ones.
-                    continue;
-                }
-                bool ok;
-                double maxAge = directives[i].second.toDouble(&ok);
-                if (ok)
-                    m_cacheControlMaxAge = maxAge;
-            }
-        }
-    }
-
-    if (!m_cacheControlContainsNoCache) {
-        // Handle Pragma: no-cache
-        // This is deprecated and equivalent to Cache-control: no-cache
-        // Don't bother tokenizing the value, it is not important
-        String pragmaValue = m_httpHeaderFields.get(HTTPHeaderName::Pragma);
-
-        m_cacheControlContainsNoCache = pragmaValue.contains("no-cache", false);
-    }
 }
     
@@ -407 +358 @@
     if (!m_haveParsedCacheControlHeader)
         parseCacheControlDirectives();
-    return m_cacheControlContainsNoCache;
+    return m_cacheControlDirectives.noCache;
 }
 
@@ -414 +365 @@
     if (!m_haveParsedCacheControlHeader)
         parseCacheControlDirectives();
-    return m_cacheControlContainsNoStore;
+    return m_cacheControlDirectives.noStore;
 }
 
@@ -421 +372 @@
     if (!m_haveParsedCacheControlHeader)
         parseCacheControlDirectives();
-    return m_cacheControlContainsMustRevalidate;
+    return m_cacheControlDirectives.mustRevalidate;
 }
 
@@ -435 +386 @@
     if (!m_haveParsedCacheControlHeader)
         parseCacheControlDirectives();
-    return m_cacheControlMaxAge;
+    return m_cacheControlDirectives.maxAge;
 }
 
@@ -556 +507 @@
 }
 
-static bool isCacheHeaderSeparator(UChar c)
-{
-    // See RFC 2616, Section 2.2
-    switch (c) {
-        case '(':
-        case ')':
-        case '<':
-        case '>':
-        case '@':
-        case ',':
-        case ';':
-        case ':':
-        case '\\':
-        case '"':
-        case '/':
-        case '[':
-        case ']':
-        case '?':
-        case '=':
-        case '{':
-        case '}':
-        case ' ':
-        case '\t':
-            return true;
-        default:
-            return false;
-    }
-}
-
-static bool isControlCharacter(UChar c)
-{
-    return c < ' ' || c == 127;
-}
-
-static inline String trimToNextSeparator(const String& str)
-{
-    return str.substring(0, str.find(isCacheHeaderSeparator));
-}
-
-static void parseCacheHeader(const String& header, Vector<std::pair<String, String>>& result)
-{
-    const String safeHeader = header.removeCharacters(isControlCharacter);
-    unsigned max = safeHeader.length();
-    for (unsigned pos = 0; pos < max; /* pos incremented in loop */) {
-        size_t nextCommaPosition = safeHeader.find(',', pos);
-        size_t nextEqualSignPosition = safeHeader.find('=', pos);
-        if (nextEqualSignPosition != notFound && (nextEqualSignPosition < nextCommaPosition || nextCommaPosition == notFound)) {
-            // Get directive name, parse right hand side of equal sign, then add to map
-            String directive = trimToNextSeparator(safeHeader.substring(pos, nextEqualSignPosition - pos).stripWhiteSpace());
-            pos += nextEqualSignPosition - pos + 1;
-
-            String value = safeHeader.substring(pos, max - pos).stripWhiteSpace();
-            if (value[0] == '"') {
-                // The value is a quoted string
-                size_t nextDoubleQuotePosition = value.find('"', 1);
-                if (nextDoubleQuotePosition != notFound) {
-                    // Store the value as a quoted string without quotes
-                    result.append(std::pair<String, String>(directive, value.substring(1, nextDoubleQuotePosition - 1).stripWhiteSpace()));
-                    pos += (safeHeader.find('"', pos) - pos) + nextDoubleQuotePosition + 1;
-                    // Move past next comma, if there is one
-                    size_t nextCommaPosition2 = safeHeader.find(',', pos);
-                    if (nextCommaPosition2 != notFound)
-                        pos += nextCommaPosition2 - pos + 1;
-                    else
-                        return; // Parse error if there is anything left with no comma
-                } else {
-                    // Parse error; just use the rest as the value
-                    result.append(std::pair<String, String>(directive, trimToNextSeparator(value.substring(1, value.length() - 1).stripWhiteSpace())));
-                    return;
-                }
-            } else {
-                // The value is a token until the next comma
-                size_t nextCommaPosition2 = value.find(',');
-                if (nextCommaPosition2 != notFound) {
-                    // The value is delimited by the next comma
-                    result.append(std::pair<String, String>(directive, trimToNextSeparator(value.substring(0, nextCommaPosition2).stripWhiteSpace())));
-                    pos += (safeHeader.find(',', pos) - pos) + 1;
-                } else {
-                    // The rest is the value; no change to value needed
-                    result.append(std::pair<String, String>(directive, trimToNextSeparator(value)));
-                    return;
-                }
-            }
-        } else if (nextCommaPosition != notFound && (nextCommaPosition < nextEqualSignPosition || nextEqualSignPosition == notFound)) {
-            // Add directive to map with empty string as value
-            result.append(std::pair<String, String>(trimToNextSeparator(safeHeader.substring(pos, nextCommaPosition - pos).stripWhiteSpace()), ""));
-            pos += nextCommaPosition - pos + 1;
-        } else {
-            // Add last directive to map with empty string as value
-            result.append(std::pair<String, String>(trimToNextSeparator(safeHeader.substring(pos, max - pos).stripWhiteSpace()), ""));
-            return;
-        }
-    }
-}
-
-}
+}

trunk/Source/WebCore/platform/network/ResourceResponseBase.h

@@ -28 +28 @@
 #define ResourceResponseBase_h
 
+#include "CacheValidation.h"
 #include "CertificateInfo.h"
 #include "HTTPHeaderMap.h"
@@ -161 +162 @@
 
 private:
-    mutable double m_cacheControlMaxAge;
     mutable double m_age;
     mutable double m_date;
     mutable double m_expires;
     mutable double m_lastModified;
+    mutable CacheControlDirectives m_cacheControlDirectives;
 
 public:
@@ -180 +181 @@
     mutable bool m_haveParsedExpiresHeader : 1;
     mutable bool m_haveParsedLastModifiedHeader : 1;
-
-    mutable bool m_cacheControlContainsNoCache : 1;
-    mutable bool m_cacheControlContainsNoStore : 1;
-    mutable bool m_cacheControlContainsMustRevalidate : 1;
 
     Source m_source;

trunk/Source/WebKit2/ChangeLog

@@ +1 @@
+2015-03-26  Antti Koivisto  <antti@apple.com>
+
+        Respect cache-control directives in request
+        https://bugs.webkit.org/show_bug.cgi?id=143121
+        rdar://problem/19714040
+
+        Reviewed by Chris Dumez.
+
+        Better support for https://tools.ietf.org/html/rfc7234#section-5.2.1
+
+        * NetworkProcess/cache/NetworkCache.cpp:
+        (WebKit::NetworkCache::canUse):
+
+            Consider requests with Cache-control: no-cache and max-age=0 expired.
+
+        (WebKit::NetworkCache::canStore):
+
+            Don't store requests with Cache-control: no-store.
+
+        (WebKit::NetworkCache::Cache::store):
+        * NetworkProcess/cache/NetworkCache.h:
+        * NetworkProcess/cache/NetworkCacheStatisticsCocoa.mm:
+        (WebKit::NetworkCache::storeDecisionToDiagnosticKey):
+
 2015-03-25  Jon Honeycutt  <jhoneycutt@apple.com>
 

trunk/Source/WebKit2/NetworkProcess/cache/NetworkCache.cpp

@@ -151 +151 @@
 }
 
-static inline bool responseHasExpired(const WebCore::ResourceResponse& response, std::chrono::milliseconds timestamp)
+static bool responseHasExpired(const WebCore::ResourceResponse& response, std::chrono::milliseconds timestamp)
 {
     if (response.cacheControlContainsNoCache())
@@ -170 +170 @@
 }
 
+static bool requestNeedsRevalidation(const WebCore::ResourceRequest& request)
+{
+    auto requestDirectives = WebCore::parseCacheControlDirectives(request.httpHeaderFields());
+    if (requestDirectives.noCache)
+        return true;
+    // For requests we ignore max-age values other than zero.
+    if (requestDirectives.maxAge == 0)
+        return true;
+
+    return false;
+}
+
 static UseDecision canUse(const Entry& entry, const WebCore::ResourceRequest& request)
 {
@@ -177 +189 @@
     }
 
-    // We never revalidate in the case of a history navigation (i.e. cachePolicyAllowsExpired() returns true).
-    bool needsRevalidation = !cachePolicyAllowsExpired(request.cachePolicy()) && responseHasExpired(entry.response(), entry.timeStamp());
+    // We never revalidate in the case of a history navigation.
+    if (cachePolicyAllowsExpired(request.cachePolicy()))
+        return UseDecision::Use;
+
+    bool needsRevalidation = requestNeedsRevalidation(request) || responseHasExpired(entry.response(), entry.timeStamp());
     if (!needsRevalidation)
         return UseDecision::Use;
@@ -267 +282 @@
 static StoreDecision canStore(const WebCore::ResourceRequest& originalRequest, const WebCore::ResourceResponse& response)
 {
-    if (!originalRequest.url().protocolIsInHTTPFamily() || !response.isHTTP()) {
-        LOG(NetworkCache, "(NetworkProcess) not HTTP");
+    if (!originalRequest.url().protocolIsInHTTPFamily() || !response.isHTTP())
         return StoreDecision::NoDueToProtocol;
-    }
-    if (originalRequest.httpMethod() != "GET") {
-        LOG(NetworkCache, "(NetworkProcess) method %s", originalRequest.httpMethod().utf8().data());
+
+    if (originalRequest.httpMethod() != "GET")
         return StoreDecision::NoDueToHTTPMethod;
-    }
+
+    auto requestDirectives = WebCore::parseCacheControlDirectives(originalRequest.httpHeaderFields());
+    if (requestDirectives.noStore)
+        return StoreDecision::NoDueToNoStoreRequest;
+
+    if (response.cacheControlContainsNoStore())
+        return StoreDecision::NoDueToNoStoreResponse;
 
     switch (response.httpStatusCode()) {
@@ -285 +304 @@
     case 404: // Not Found
     case 410: // Gone
-        if (response.cacheControlContainsNoStore()) {
-            LOG(NetworkCache, "(NetworkProcess) Cache-control:no-store");
-            return StoreDecision::NoDueToNoStoreResponse;
-        }
         return StoreDecision::Yes;
     default:
@@ -306 +321 @@
     StoreDecision storeDecision = canStore(originalRequest, response);
     if (storeDecision != StoreDecision::Yes) {
-        LOG(NetworkCache, "(NetworkProcess) didn't store");
+        LOG(NetworkCache, "(NetworkProcess) didn't store, storeDecision=%d", storeDecision);
         if (m_statistics) {
             auto key = makeCacheKey(originalRequest);

trunk/Source/WebKit2/NetworkProcess/cache/NetworkCache.h

@@ -69 +69 @@
     NoDueToAttachmentResponse,
     NoDueToNoStoreResponse,
-    NoDueToHTTPStatusCode
+    NoDueToHTTPStatusCode,
+    NoDueToNoStoreRequest
 };
 

trunk/Source/WebKit2/NetworkProcess/cache/NetworkCacheStatisticsCocoa.mm

@@ -237 +237 @@
         return WebCore::DiagnosticLoggingKeys::isAttachmentKey();
     case StoreDecision::NoDueToNoStoreResponse:
+    case StoreDecision::NoDueToNoStoreRequest:
         return WebCore::DiagnosticLoggingKeys::cacheControlNoStoreKey();
     case StoreDecision::NoDueToHTTPStatusCode: