Added HttpUtils::HasValidators and HttpResponse::HasValidators

net/http/http_util_unittest.cc

Index: net/http/http_util_unittest.cc
diff --git a/net/http/http_util_unittest.cc b/net/http/http_util_unittest.cc
index a5c984db66b1f5e46798036e8c1ac3758fab9f81..6579f93e9eb7ebcd34a345659e040fb77111d01e 100644
--- a/net/http/http_util_unittest.cc
+++ b/net/http/http_util_unittest.cc
@@ -1178,4 +1178,45 @@ TEST(HttpUtilTest, NameValuePairsIteratorMissingEndQuote) {
       &parser, false, true, std::string(), std::string()));
 }
 
+TEST(HttpUtilTest, HasValidatorsHttp09) {
+  std::string etags[] = {"", "\"strong\"", "W/\"weak\""};
+  std::string last_modified_values[] = {"", "Tue, 15 Nov 1994 12:45:26 GMT",
+                                        "invalid"};
+
+  for (const std::string& etag : etags) {
+    for (const std::string& last_modified : last_modified_values) {
+      EXPECT_FALSE(
+          HttpUtil::HasValidators(HttpVersion(0, 9), etag, last_modified));

[twifkak, 2016/02/22 23:44:44]

add `<< "with etag = " << etag << " and last_modified = "<< last_modified`, so
we know what loop iteration failed. (Ditto below.)

[jamartin, 2016/02/23 14:03:35]

I've applied your second suggestion, which obsoletes this.

+    }
+  }

[twifkak, 2016/02/22 23:44:44]

There are 3*3=9 etag/last-modified pairs.

Suggestion:
1. Make char[] constants for the five values you specify on lines 1182-3 (and
which you duplicate below!).
2. Just hard-code the 9 pairs without a loop, e.g.:
  EXPECT_FALSE(Http::HasValidators(HttpVersion(0, 9), kEmpty,
kLastModifiedInvalid));
  ...

This will:
1. Be easier to read, thanks to not having to use variables or boolean logic in
your expectations.
2. Not be much brittler, thanks to the constants.
3. Only be two lines longer than the current loop-based tests.

[jamartin, 2016/02/23 14:03:35]

Done.

There is one advantage of the loop approach, though: the condition for the
TRUE/FALSE evaluation is made explicit. But I do agree with you that it is now
as simple as it gets.

[twifkak, 2016/02/24 21:07:38]

LGTM. Some rambling thoughts: Agreed, that the logic is harder to read now. On
the plus side, you can read it in the prod code, and the test is much stronger
when prod and test are implemented differently. I see your point, though -- some
future coder might not know what to do with this wall of assertions. I lean
toward simpler still, but I'm not at all steadfast on that.

+}
+
+TEST(HttpUtilTest, HasValidatorsHttp10) {
+  std::string etags[] = {"", "\"strong\"", "W/\"weak\""};
+  std::string last_modified_values[] = {"", "Tue, 15 Nov 1994 12:45:26 GMT",
+                                        "invalid"};
+
+  for (const std::string& etag : etags) {
+    for (const std::string& last_modified : last_modified_values) {
+      EXPECT_EQ(
+          last_modified != "" && last_modified != "invalid",
+          HttpUtil::HasValidators(HttpVersion(1, 0), etag, last_modified));
+    }
+  }
+}
+
+TEST(HttpUtilTest, HasValidatorsHttp11) {
+  std::string etags[] = {"", "\"strong\"", "W/\"weak\""};
+  std::string last_modified_values[] = {"", "Tue, 15 Nov 1994 12:45:26 GMT",
+                                        "invalid"};
+
+  for (const std::string& etag : etags) {
+    for (const std::string& last_modified : last_modified_values) {
+      EXPECT_EQ(
+          etag != "" || (last_modified != "" && last_modified != "invalid"),
+          HttpUtil::HasValidators(HttpVersion(1, 1), etag, last_modified));
+    }
+  }
+}
+
 }  // namespace net