Change the detection of preview request to via header.

chrome/renderer/page_load_histograms.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.
 
 #include "chrome/renderer/page_load_histograms.h"
 
 #include <string>
 
 #include "base/command_line.h"
 #include "base/logging.h"
 #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"
 #include "base/time/time.h"
 #include "chrome/common/chrome_switches.h"
 #include "chrome/renderer/chrome_content_renderer_client.h"
 #include "content/public/common/content_constants.h"
 #include "content/public/renderer/document_state.h"
 #include "content/public/renderer/render_thread.h"
 #include "content/public/renderer/render_view.h"
@@ skipping 142 matching lines @@
   return static_cast<URLPattern::SchemeMasks>(0);
 }
 
 // Returns true if the data reduction proxy was used. Note, this function will
 // produce a false positive if a page is fetched using SPDY and using a proxy,
 // and |kDatReductionProxyViaValue| is added to the Via header.
 // TODO(bengr): Plumb the hostname of the proxy from |HttpNetworkTransaction|
 // and check if it matches |SPDY_PROXY_AUTH_ORIGIN|.
 bool DataReductionProxyWasUsed(WebFrame* frame) {
 #if defined(SPDY_PROXY_AUTH_ORIGIN)
+  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.

[bengr, 2014/01/10 17:40:04]

"This is used only for requests fetched via the proxy" --> "Returns true if
|via_value| is one of the values listed in the Via header and the response was
fetched via a proxy."

[ksimbili, 2014/01/28 18:33:25]

Done.

+bool ViaHeaderContains(WebFrame* frame, const std::string& via_value) {

[bengr, 2014/01/10 17:40:04]

optional: you should check that |via_value| does not contain a comma.

   const char kViaHeaderName[] = "Via";
-  const char kDatReductionProxyViaValue[] = "1.1 Chrome Compression Proxy";
 
   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
-  return false;
+  std::vector<std::string> values;
+  // Multiple via headers have already been coalesced and hence each value
+  // seperated by a comma corresponds to a proxy. The value added by proxy is

[bengr, 2014/01/10 17:40:04]

seperated --> separated

by proxy --> by a proxy

[ksimbili, 2014/01/28 18:33:25]

Done.

+  // not expected to contain any commas.
+  // Example., Via: 1.0 Compression proxy, 1.1 Google promise preview
+  base::SplitString(
+      frame->dataSource()->response().httpHeaderField(kViaHeaderName).utf8(),
+      ',', &values);
+  return std::find(values.begin(), values.end(), via_value) != values.end();
 }
 
 // Returns true if the provided URL is a referrer string that came from
 // a Google Web Search results page. This is a little non-deterministic
 // because desktop and mobile websearch differ and sometimes just provide
 // http://www.google.com/ as the referrer. In the case of /url we can be sure
 // that it came from websearch but we will be generous and allow for cases
 // where a non-Google URL was provided a bare Google URL as a referrer.
 // The domain validation matches the code used by the prerenderer for similar
 // purposes.
@@ skipping 23 matching lines @@
     return kNoExperiment;
 
   int experiment_id;
   if (!base::StringToInt(value, &experiment_id))
     return kNoExperiment;
   if (0 < experiment_id && experiment_id <= kMaxExperimentID)
     return experiment_id;
   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,
                      int* preview_experiment_id) {
   // Conditions for GWS preview are,
   // 1. Data reduction proxy was used.
   // 2. URL is loaded from web search.

[bengr, 2014/01/10 17:40:04]

2. is misleading because you set state even when came_from_websearch is false.

Fix.

[ksimbili, 2014/01/28 18:33:25]

Done.

-  // If the URL contains "cerivrj_*' query parameter record under
-  // "Preview".
+  // Based on via header PREVIEW/PREVIEW_WAS_SHOWN/PREVIEW_NOT_USED is

[bengr, 2014/01/10 17:40:04]

// Determine the preview state (PREVIEW, PREVIEW_WAS_SHOWN, PREVIEW_NOT_USED) by
inspecting the Via header.

[ksimbili, 2014/01/28 18:33:25]

Done.

+  // 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;

[bengr, 2014/01/10 17:40:04]

I'd move "frame,..." down to the next line and indent 4, though I'd prefer:

if (ViaHeaderContains(frame, "1.1 Google Promise Preview")
  *preview_state = PREVIEW;
else
  *preview_state = PREVIEW_NOT_USED;

[ksimbili, 2014/01/28 18:33:25]

Done.

+    } 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()));
     }
   }
 }
 
 void DumpPerformanceTiming(const WebPerformance& performance,
                            DocumentState* document_state,
                            bool data_reduction_proxy_was_used,
                            bool came_from_websearch,
                            int websearch_chrome_joint_experiment_id,
                            GwsPreviewState preview_state,
@@ skipping 261 matching lines @@
       IsFromGoogleSearchResult(frame->document().url(),
                                GURL(frame->document().referrer()));
   int websearch_chrome_joint_experiment_id = kNoExperiment;
   if (came_from_websearch) {
     websearch_chrome_joint_experiment_id =
         GetQueryStringBasedExperiment(GURL(frame->document().referrer()));
   }
 
   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.
   // http://www.w3.org/TR/navigation-timing/
   // TODO(tonyg, jar): We are in the process of vetting these metrics against
   // the existing ones. Once we understand any differences, we will standardize
   // on a single set of metrics.
   DumpPerformanceTiming(frame->performance(), document_state,
                         data_reduction_proxy_was_used,
                         came_from_websearch,
@@ skipping 472 matching lines @@
 
   DCHECK(document_state);
   DCHECK(ds);
   GURL url(ds->request().url());
   Time start = document_state->start_load_time();
   Time finish = document_state->finish_load_time();
   // TODO(mbelshe): should we log more stats?
   VLOG(1) << "PLT: " << (finish - start).InMilliseconds() << "ms "
           << url.spec();
 }