Initial implementation of Chrome-Freshness header.

net/http/http_response_headers.cc

Index: net/http/http_response_headers.cc
diff --git a/net/http/http_response_headers.cc b/net/http/http_response_headers.cc
index b7ef98a5980633485902bdf522dbb77e2a5c0f02..1a2f2531ec0c14eff3afcc92200c36eef76fcbd2 100644
--- a/net/http/http_response_headers.cc
+++ b/net/http/http_response_headers.cc
@@ -1092,29 +1092,7 @@ TimeDelta HttpResponseHeaders::GetCurrentAge(const Time& request_time,
 }
 
 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 {
@@ -1140,6 +1118,37 @@ 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;