Initial implementation of Chrome-Freshness header.

net/http/http_response_headers.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 // The rules for header parsing were borrowed from Firefox:
 // http://lxr.mozilla.org/seamonkey/source/netwerk/protocol/http/src/nsHttpResponseHead.cpp
 // The rules for parsing content-types were also borrowed from Firefox:
 // http://lxr.mozilla.org/mozilla/source/netwerk/base/src/nsURLHelper.cpp#834
 
 #include "net/http/http_response_headers.h"
@@ skipping 1074 matching lines @@
   TimeDelta corrected_received_age = std::max(apparent_age, age_value);
   TimeDelta response_delay = response_time - request_time;
   TimeDelta corrected_initial_age = corrected_received_age + response_delay;
   TimeDelta resident_time = current_time - response_time;
   TimeDelta current_age = corrected_initial_age + resident_time;
 
   return current_age;
 }
 
 bool HttpResponseHeaders::GetMaxAgeValue(TimeDelta* result) const {
-  std::string name = "cache-control";
-  std::string value;
-
-  const char kMaxAgePrefix[] = "max-age=";
-  const size_t kMaxAgePrefixLen = arraysize(kMaxAgePrefix) - 1;
-
-  void* iter = NULL;
-  while (EnumerateHeader(&iter, name, &value)) {
-    if (value.size() > kMaxAgePrefixLen) {
-      if (LowerCaseEqualsASCII(value.begin(),
-                               value.begin() + kMaxAgePrefixLen,
-                               kMaxAgePrefix)) {
-        int64 seconds;
-        base::StringToInt64(StringPiece(value.begin() + kMaxAgePrefixLen,
-                                        value.end()),
-                            &seconds);
-        *result = TimeDelta::FromSeconds(seconds);
-        return true;
-      }
-    }
-  }
-
-  return false;
+  return GetCacheControlDirective("max-age", result);
 }
 
 bool HttpResponseHeaders::GetAgeValue(TimeDelta* result) const {
   std::string value;
   if (!EnumerateHeader(NULL, "Age", &value))
     return false;
 
   int64 seconds;
   base::StringToInt64(value, &seconds);
   *result = TimeDelta::FromSeconds(seconds);
   return true;
 }
 
 bool HttpResponseHeaders::GetDateValue(Time* result) const {
   return GetTimeValuedHeader("Date", result);
 }
 
 bool HttpResponseHeaders::GetLastModifiedValue(Time* result) const {
   return GetTimeValuedHeader("Last-Modified", result);
 }
 
 bool HttpResponseHeaders::GetExpiresValue(Time* result) const {
   return GetTimeValuedHeader("Expires", result);
 }
 
+bool HttpResponseHeaders::GetStaleWhileRevalidateValue(

[rvargas (doing something else), 2014/07/28 19:24:00]

Please add HttpResponseHeaders unit tests to make sure this method behaves as
expected.

[Adam Rice, 2014/07/29 17:04:45]

Done. I didn't add as many tests as for max-age as there isn't any legacy
behaviour to worry about.

[rvargas (doing something else), 2014/07/29 19:21:22]

The tests look great, thanks.

+    TimeDelta* result) const {
+  return GetCacheControlDirective("stale-while-revalidate", result);
+}
+
+bool HttpResponseHeaders::GetCacheControlDirective(const StringPiece& directive,
+                                                   TimeDelta* result) const {
+  StringPiece name("cache-control");
+  std::string value;
+
+  const size_t directive_size = directive.size();

[rvargas (doing something else), 2014/07/28 19:24:00]

nit: no const

[Adam Rice, 2014/07/29 17:04:45]

May I ask why? const clarifies for the reader that this value will not change
and also provides a compiler guarantee of that invariant. It has no cost except
6 letters.

[rvargas (doing something else), 2014/07/29 19:21:22]

To me it goes with the overall spirit of not going crazy with using const from
the style guide (granted, this is not really excessive, it looks
straightforward). This is not a "real" constant, it is just a variable that
happens to retain the same value during the whole call (after this assignment),
in the current implementation, but there is nothing fundamentally wrong if the
code changes in the future and allows the variable to be updated. As for the
compiler, it should have no issue performing any optimization when the const is
omitted, as it should be able to see that there is no subsequent assignment.

In other words, if a local variable should be a constant (have const) it should
also be named kFoo (I don't think this line passes that test).

The cost is very small, but the value IMO is also very small, and anything that
I don't have to read when going over the code counts in my book (as in, ideally,
everything in the code should be important and provide valuable information, so
that my mind doesn't have to start masking out parts of what I read)

[Adam Rice, 2014/07/30 02:00:59]

Okay, you're the owner so I took it out. For me, anything that reduces the state
space I need to consider while reading the code is helpful.

+
+  void* iter = NULL;
+  while (EnumerateHeader(&iter, name, &value)) {
+    if (value.size() > directive_size + 1 &&

[rvargas (doing something else), 2014/07/28 19:24:01]

This '+ 1' is a slight change from the previous code, as before 'foo=' was
enough to trigger the second check and return true with result = 0, but now we
fail instead.

And it looks like GetMaxAgeValue doesn't have unit tests :(

[Adam Rice, 2014/07/29 17:04:45]

I added unit tests for GetMaxAgeValue() and verified the behaviour is the same
with both the new and old implementations. max-age= without a value returns
false on both (directive_size in the new implementation is one less than
kMaxAgePrefixLen in the old implementation).

+        LowerCaseEqualsASCII(value.begin(),
+                             value.begin() + directive_size,
+                             directive.begin()) &&
+        value[directive_size] == '=') {
+      int64 seconds;
+      base::StringToInt64(
+          StringPiece(value.begin() + directive_size + 1, value.end()),
+          &seconds);
+      *result = TimeDelta::FromSeconds(seconds);
+      return true;
+    }
+  }
+
+  return false;
+}
+
 bool HttpResponseHeaders::GetTimeValuedHeader(const std::string& name,
                                               Time* result) const {
   std::string value;
   if (!EnumerateHeader(NULL, name, &value))
     return false;
 
   // When parsing HTTP dates it's beneficial to default to GMT because:
   // 1. RFC2616 3.3.1 says times should always be specified in GMT
   // 2. Only counter-example incorrectly appended "UTC" (crbug.com/153759)
   // 3. When adjusting cookie expiration times for clock skew
@@ skipping 232 matching lines @@
   return true;
 }
 
 bool HttpResponseHeaders::IsChunkEncoded() const {
   // Ignore spurious chunked responses from HTTP/1.0 servers and proxies.
   return GetHttpVersion() >= HttpVersion(1, 1) &&
       HasHeaderValue("Transfer-Encoding", "chunked");
 }
 
 }  // namespace net