Change the detection of preview request to via header.

chrome/renderer/page_load_histograms.cc

Index: chrome/renderer/page_load_histograms.cc
diff --git a/chrome/renderer/page_load_histograms.cc b/chrome/renderer/page_load_histograms.cc
index 901b9324c1d2eec9208909436908b5e5c5a677ad..0436ee7b331769a76ddbb94a2d3a230305c0efc1 100644
--- a/chrome/renderer/page_load_histograms.cc
+++ b/chrome/renderer/page_load_histograms.cc
@@ -11,6 +11,7 @@
 #include "base/metrics/field_trial.h"
 #include "base/metrics/histogram.h"
 #include "base/strings/string_number_conversions.h"
+#include "base/strings/string_split.h"
 #include "base/strings/string_util.h"
 #include "base/strings/stringprintf.h"
 #include "base/strings/utf_string_conversions.h"
@@ -173,18 +174,33 @@ URLPattern::SchemeMasks GetSupportedSchemeType(const GURL& url) {
 // and check if it matches |SPDY_PROXY_AUTH_ORIGIN|.
 bool DataReductionProxyWasUsed(WebFrame* frame) {
 #if defined(SPDY_PROXY_AUTH_ORIGIN)
-  const char kViaHeaderName[] = "Via";
   const char kDatReductionProxyViaValue[] = "1.1 Chrome Compression Proxy";
+  return ViaHeaderContains(frame, kDatReductionProxyViaValue);
+#endif
+  return false;
+}
+
+// Helper function to check for string in 'via' header. This is used only for
+// requests fetched via the proxy.
+bool ViaHeaderContains(WebFrame* frame, const std::string& via_value) {
+  const char kViaHeaderName[] = "Via";
 
   DocumentState* document_state =
       DocumentState::FromDataSource(frame->dataSource());
   if (!document_state->was_fetched_via_proxy())
     return false;
 
-  std::string via_header(base::UTF16ToUTF8(
-      frame->dataSource()->response().httpHeaderField(kViaHeaderName)));
-  return via_header.find(kDatReductionProxyViaValue) != std::string::npos;
-#endif
+  std::vector<std::string> via_header_values;
+  // Multiple via headers have already been coalesced and hence each value
+  // seperated by ',' correspond to a proxy. Value added by proxy is not

[bengr, 2014/01/09 19:39:45]

seperated by ',' correspond --> that is separated by a comma corresponds

Value addded by --> The value added by a

any ',' --> any commas

[ksimbili, 2014/01/09 20:16:21]

Done.

+  // expected to contain any ','.
+  base::SplitString(base::UTF16ToUTF8(
+      frame->dataSource()->response().httpHeaderField(kViaHeaderName)),
+      ',', &via_header_values);
+  for (unsigned int i = 0; i < via_header_values.size(); ++i) {
+    if (via_header_values[i].find(via_value) != std::string::npos)

[bengr, 2014/01/09 19:39:45]

I think you should test for equality here.

I.e., as written, you'll return true if you're searching for foo and you have
this header:
Via: bar, foobar, goo

[ksimbili, 2014/01/09 20:16:21]

Done.

+      return true;
+  }
   return false;
 }
 
@@ -229,40 +245,8 @@ int GetQueryStringBasedExperiment(const GURL& referrer) {
   return kNoExperiment;
 }
 
-// Appends "cerivrj_*" and "gcjeid" query parameters from preview_url to url.
-// Returns true if "cerivrj_*" query is found.
-// This will be used only for search results URLs.
-bool AppendPreviewQueryFromURL(const GURL& preview_url, GURL* url) {
-  bool preview_query_found = false;
-  for (net::QueryIterator it(preview_url); !it.IsAtEnd(); it.Advance()) {
-    const std::string param_name = it.GetKey();
-    bool is_preview = StartsWithASCII(param_name, "cerivrj_", true);
-    if (url && (is_preview || param_name == "gcjeid"))
-      net::AppendQueryParameter(*url, param_name, it.GetValue());
-    preview_query_found = preview_query_found || is_preview;
-  }
-  return preview_query_found;
-}
-
-// Returns true if the provided referrer URL is the preview URL of the current
-// URL. Preview URL differs from original only with "cerivrj_*", "gcjeid" query
-// parameters.
-bool IsReferrerPreviewOfURL(const GURL& url,
-                            const base::string16& referrer_str) {
-  GURL referrer(referrer_str);
-  if (referrer.is_valid()) {
-    GURL generated_preview_url(url);
-    // Now try to copy "cerivrj_*" and "gcjeid" paramters to url and check if
-    // they exactly match.
-    if (AppendPreviewQueryFromURL(referrer, &generated_preview_url))
-      return generated_preview_url == referrer;
-  }
-  return false;
-}
-
 // Returns preview state by looking at url and referer url.
-void GetPreviewState(const GURL& url,
-                     const base::string16& referrer,
+void GetPreviewState(WebFrame* frame,
                      bool came_from_websearch,
                      bool data_reduction_proxy_was_used,
                      GwsPreviewState* preview_state,
@@ -270,15 +254,17 @@ void GetPreviewState(const GURL& url,
   // Conditions for GWS preview are,
   // 1. Data reduction proxy was used.
   // 2. URL is loaded from web search.
-  // If the URL contains "cerivrj_*' query parameter record under
-  // "Preview".
+  // Based on via header PREVIEW/PREVIEW_WAS_SHOWN/PREVIEW_NOT_USED is
+  // determined.
   if (data_reduction_proxy_was_used) {
     if (came_from_websearch) {
       *preview_state =
-          AppendPreviewQueryFromURL(url, NULL) ? PREVIEW : PREVIEW_NOT_USED;
-    } else if (IsReferrerPreviewOfURL(url, referrer)) {
+          ViaHeaderContains(frame, "1.1 Google Promise Preview") ? PREVIEW :
+          PREVIEW_NOT_USED;
+    } else if (ViaHeaderContains(frame, "1.1 Google Promise Original")) {
       *preview_state = PREVIEW_WAS_SHOWN;
-      *preview_experiment_id = GetQueryStringBasedExperiment(GURL(referrer));
+      *preview_experiment_id = GetQueryStringBasedExperiment(
+          GURL(frame->document().referrer()));
     }
   }
 }
@@ -560,8 +546,7 @@ void PageLoadHistograms::Dump(WebFrame* frame) {
 
   GwsPreviewState preview_state = PREVIEW_NONE;
   int preview_experiment_id = websearch_chrome_joint_experiment_id;
-  GetPreviewState(frame->document().url(), frame->document().referrer(),
-                  came_from_websearch, data_reduction_proxy_was_used,
+  GetPreviewState(frame, came_from_websearch, data_reduction_proxy_was_used,
                   &preview_state, &preview_experiment_id);
 
   // Times based on the Web Timing metrics.