Fixing redirect through DataReductionProxy logic.

Redirects through DataReductionProxy were counting as a DRP
navigations/pageloads even when the new URL that was redirected to was
non-DRP. This affects DRP PageLoad.* UMA and the DRP pingback.

This introduces a reset_to_default method on DRPData as well as changing
using original_url from URLRequest to just URL (as DRP cares more about
the actual URL of the site rather than the original URL before
redirects).

BUG=565421

Review-Url: https://codereview.chromium.org/2326443004
Cr-Commit-Position: refs/heads/master@{#419514}

chrome/browser/page_load_metrics/observers/data_reduction_proxy_metrics_observer.cc

@@ -23,6 +23,7 @@
 #include "content/public/browser/navigation_data.h"
 #include "content/public/browser/navigation_handle.h"
 #include "content/public/browser/web_contents.h"
+#include "url/gurl.h"
 
 namespace data_reduction_proxy {
 
@@ -115,6 +116,9 @@ void DataReductionProxyMetricsObserver::OnCommit(
   if (!data)
     return;
   data_ = data->DeepCopy();
+  // DataReductionProxy page loads should only occur on HTTP navigations.
+  DCHECK(!data_->used_data_reduction_proxy() ||
+         !navigation_handle->GetURL().SchemeIsCryptographic());
 }
 
 void DataReductionProxyMetricsObserver::OnComplete(

chrome/browser/page_load_metrics/observers/data_reduction_proxy_metrics_observer_unittest.cc

@@ -25,8 +25,8 @@ namespace data_reduction_proxy {
 
 namespace {
 
-const char kDefaultTestUrl[] = "https://google.com";
-const char kDefaultTestUrl2[] = "https://example.com";
+const char kDefaultTestUrl[] = "http://google.com";
+const char kDefaultTestUrl2[] = "http://example.com";
 
 data_reduction_proxy::DataReductionProxyData* DataForNavigationHandle(
     content::WebContents* web_contents,

components/data_reduction_proxy/core/browser/data_reduction_proxy_data.cc

@@ -22,7 +22,7 @@ std::unique_ptr<DataReductionProxyData> DataReductionProxyData::DeepCopy()
   copy->used_data_reduction_proxy_ = used_data_reduction_proxy_;
   copy->lofi_requested_ = lofi_requested_;
   copy->session_key_ = session_key_;
-  copy->original_request_url_ = original_request_url_;
+  copy->request_url_ = request_url_;
   copy->effective_connection_type_ = effective_connection_type_;
   return copy;
 }
@@ -46,4 +46,8 @@ DataReductionProxyData* DataReductionProxyData::GetDataAndCreateIfNecessary(
   return data;
 }
 
+void DataReductionProxyData::ClearData(net::URLRequest* request) {
+  request->RemoveUserData(kDataReductionProxyUserDataKey);
+}
+
 }  // namespace data_reduction_proxy

components/data_reduction_proxy/core/browser/data_reduction_proxy_data.h

@@ -45,14 +45,13 @@ class DataReductionProxyData : public base::SupportsUserData::Data {
     session_key_ = session_key;
   }
 
-  // The URL of the request before redirects.
-  GURL original_request_url() const { return original_request_url_; }
-  void set_original_request_url(const GURL& original_request_url) {
-    original_request_url_ = original_request_url;
-  }
+  // The URL the frame is navigating to. This may change during the navigation
+  // when encountering a server redirect.
+  GURL request_url() const { return request_url_; }
+  void set_request_url(const GURL& request_url) { request_url_ = request_url; }
 
-  // The EffectiveConnectionType when the request starts. This is set for main
-  // frame requests only.
+  // The EffectiveConnectionType after the proxy is resolved. This is set for
+  // main frame requests only.
   net::EffectiveConnectionType effective_connection_type() const {
     return effective_connection_type_;
   }
@@ -61,6 +60,9 @@ class DataReductionProxyData : public base::SupportsUserData::Data {
     effective_connection_type_ = effective_connection_type;
   }
 
+  // Removes |this| from |request|.
+  static void ClearData(net::URLRequest* request);
+
   // Returns the Data from the URLRequest's UserData.
   static DataReductionProxyData* GetData(const net::URLRequest& request);
   // Returns the Data for a given URLRequest. If there is currently no
@@ -86,8 +88,9 @@ class DataReductionProxyData : public base::SupportsUserData::Data {
   // The session key used for this request or navigation.
   std::string session_key_;
 
-  // The URL of the request before redirects.
-  GURL original_request_url_;
+  // The URL the frame is navigating to. This may change during the navigation
+  // when encountering a server redirect.
+  GURL request_url_;
 
   // The EffectiveConnectionType when the request or navigation starts. This is
   // set for main frame requests only.

components/data_reduction_proxy/core/browser/data_reduction_proxy_data_unittest.cc

@@ -42,13 +42,13 @@ TEST_F(DataReductionProxyDataTest, BasicSettersAndGetters) {
   EXPECT_FALSE(data->lofi_requested());
 
   EXPECT_EQ(std::string(), data->session_key());
-  EXPECT_EQ(GURL(std::string()), data->original_request_url());
+  EXPECT_EQ(GURL(std::string()), data->request_url());
   std::string session_key = "test-key";
   data->set_session_key(session_key);
   EXPECT_EQ(session_key, data->session_key());
   GURL test_url("test-url");
-  data->set_original_request_url(test_url);
-  EXPECT_EQ(test_url, data->original_request_url());
+  data->set_request_url(test_url);
+  EXPECT_EQ(test_url, data->request_url());
   EXPECT_EQ(net::EFFECTIVE_CONNECTION_TYPE_UNKNOWN,
             data->effective_connection_type());
   data->set_effective_connection_type(net::EFFECTIVE_CONNECTION_TYPE_OFFLINE);
@@ -62,12 +62,12 @@ TEST_F(DataReductionProxyDataTest, AddToURLRequest) {
       GURL("http://www.google.com"), net::RequestPriority::IDLE, nullptr));
   DataReductionProxyData* data =
       DataReductionProxyData::GetData(*fake_request.get());
-  EXPECT_TRUE(!data);
+  EXPECT_FALSE(data);
   data =
       DataReductionProxyData::GetDataAndCreateIfNecessary(fake_request.get());
-  EXPECT_FALSE(!data);
+  EXPECT_TRUE(data);
   data = DataReductionProxyData::GetData(*fake_request.get());
-  EXPECT_FALSE(!data);
+  EXPECT_TRUE(data);
   DataReductionProxyData* data2 =
       DataReductionProxyData::GetDataAndCreateIfNecessary(fake_request.get());
   EXPECT_EQ(data, data2);
@@ -99,18 +99,31 @@ TEST_F(DataReductionProxyDataTest, DeepCopy) {
     data->set_used_data_reduction_proxy(tests[i].data_reduction_used);
     data->set_lofi_requested(tests[i].lofi_on);
     data->set_session_key(kSessionKey);
-    data->set_original_request_url(kTestURL);
+    data->set_request_url(kTestURL);
     data->set_effective_connection_type(net::EFFECTIVE_CONNECTION_TYPE_OFFLINE);
     std::unique_ptr<DataReductionProxyData> copy = data->DeepCopy();
     EXPECT_EQ(tests[i].lofi_on, copy->lofi_requested());
     EXPECT_EQ(tests[i].data_reduction_used, copy->used_data_reduction_proxy());
     EXPECT_EQ(kSessionKey, copy->session_key());
-    EXPECT_EQ(kTestURL, copy->original_request_url());
+    EXPECT_EQ(kTestURL, copy->request_url());
     EXPECT_EQ(net::EFFECTIVE_CONNECTION_TYPE_OFFLINE,
               copy->effective_connection_type());
   }
 }
 
+TEST_F(DataReductionProxyDataTest, ClearData) {
+  std::unique_ptr<net::URLRequestContext> context(new net::URLRequestContext());
+  std::unique_ptr<net::URLRequest> fake_request(context->CreateRequest(
+      GURL("http://www.google.com"), net::RequestPriority::IDLE, nullptr));
+
+  DataReductionProxyData* data =
+      DataReductionProxyData::GetDataAndCreateIfNecessary(fake_request.get());
+  EXPECT_TRUE(data);
+  DataReductionProxyData::ClearData(fake_request.get());
+  data = DataReductionProxyData::GetData(*fake_request.get());
+  EXPECT_FALSE(data);
+}
+
 }  // namespace
 
 }  // namespace data_reduction_proxy

components/data_reduction_proxy/core/browser/data_reduction_proxy_network_delegate.cc

@@ -237,6 +237,10 @@ void DataReductionProxyNetworkDelegate::OnBeforeSendHeadersInternal(
     net::HttpRequestHeaders* headers) {
   DCHECK(data_reduction_proxy_config_);
   DCHECK(request);
+
+  // If this is after a redirect, reset |request|'s DataReductionProxyData.
+  DataReductionProxyData::ClearData(request);
+
   if (params::IsIncludedInHoldbackFieldTrial()) {
     if (!WasEligibleWithoutHoldback(*request, proxy_info, proxy_retry_info))
       return;
@@ -266,7 +270,7 @@ void DataReductionProxyNetworkDelegate::OnBeforeSendHeadersInternal(
     data->set_used_data_reduction_proxy(true);
     data->set_session_key(
         data_reduction_proxy_request_options_->GetSecureSession());
-    data->set_original_request_url(request->original_url());
+    data->set_request_url(request->url());
     if ((request->load_flags() & net::LOAD_MAIN_FRAME_DEPRECATED) &&
         request->context()->network_quality_estimator()) {
       data->set_effective_connection_type(request->context()

components/data_reduction_proxy/core/browser/data_reduction_proxy_network_delegate_unittest.cc

@@ -556,7 +556,7 @@ TEST_F(DataReductionProxyNetworkDelegateTest, RequestDataConfigurations) {
                                 : net::EFFECTIVE_CONNECTION_TYPE_UNKNOWN,
                 data->effective_connection_type());
       EXPECT_TRUE(data->used_data_reduction_proxy());
-      EXPECT_EQ(GURL("http://www.google.com/"), data->original_request_url());
+      EXPECT_EQ(GURL("http://www.google.com/"), data->request_url());
       EXPECT_EQ("fake-session", data->session_key());
       EXPECT_EQ(test.lofi_on, data->lofi_requested());
     }
@@ -610,6 +610,49 @@ TEST_F(DataReductionProxyNetworkDelegateTest,
   }
 }
 
+TEST_F(DataReductionProxyNetworkDelegateTest, RedirectRequestDataCleared) {
+  net::ProxyInfo data_reduction_proxy_info;
+  std::string data_reduction_proxy;
+  base::TrimString(params()->DefaultOrigin(), "/", &data_reduction_proxy);
+  data_reduction_proxy_info.UseNamedProxy(data_reduction_proxy);
+
+  // Main frame loaded. Lo-Fi should be used.
+  net::HttpRequestHeaders headers;
+  net::ProxyRetryInfoMap proxy_retry_info;
+
+  std::map<std::string, std::string> network_quality_estimator_params;
+  TestNetworkQualityEstimator test_network_quality_estimator(
+      network_quality_estimator_params, net::EFFECTIVE_CONNECTION_TYPE_OFFLINE);
+  context()->set_network_quality_estimator(&test_network_quality_estimator);
+
+  std::unique_ptr<net::URLRequest> request = context()->CreateRequest(
+      GURL("http://www.google.com/"), net::RequestPriority::IDLE, nullptr);
+  request->SetLoadFlags(net::LOAD_MAIN_FRAME_DEPRECATED);
+  lofi_decider()->SetIsUsingLoFiMode(true);
+  io_data()->request_options()->SetSecureSession("fake-session");
+  network_delegate()->NotifyBeforeSendHeaders(
+      request.get(), data_reduction_proxy_info, proxy_retry_info, &headers);
+  DataReductionProxyData* data =
+      DataReductionProxyData::GetData(*request.get());
+
+  EXPECT_TRUE(data);
+  EXPECT_EQ(net::EFFECTIVE_CONNECTION_TYPE_OFFLINE,
+            data->effective_connection_type());
+  EXPECT_TRUE(data->used_data_reduction_proxy());
+  EXPECT_EQ(GURL("http://www.google.com/"), data->request_url());
+  EXPECT_EQ("fake-session", data->session_key());
+  EXPECT_TRUE(data->lofi_requested());
+
+  data_reduction_proxy_info.UseNamedProxy("port.of.other.proxy");
+
+  // Simulate a redirect by calling NotifyBeforeSendHeaders again with different
+  // proxy info.
+  network_delegate()->NotifyBeforeSendHeaders(
+      request.get(), data_reduction_proxy_info, proxy_retry_info, &headers);
+  data = DataReductionProxyData::GetData(*request.get());
+  EXPECT_FALSE(data);
+}
+
 TEST_F(DataReductionProxyNetworkDelegateTest, NetHistograms) {
   const std::string kReceivedValidOCLHistogramName =
       "Net.HttpContentLengthWithValidOCL";

components/data_reduction_proxy/core/browser/data_reduction_proxy_pingback_client.cc

@@ -40,8 +40,8 @@ void AddDataToPageloadMetrics(const DataReductionProxyData& request_data,
   request->set_allocated_first_request_time(
       protobuf_parser::CreateTimestampFromTime(timing.navigation_start)
           .release());
-  if (request_data.original_request_url().is_valid())
-    request->set_first_request_url(request_data.original_request_url().spec());
+  if (request_data.request_url().is_valid())
+    request->set_first_request_url(request_data.request_url().spec());
   if (timing.first_contentful_paint) {
     request->set_allocated_time_to_first_contentful_paint(
         protobuf_parser::CreateDurationFromTimeDelta(

components/data_reduction_proxy/core/browser/data_reduction_proxy_pingback_client_unittest.cc

@@ -113,7 +113,7 @@ class DataReductionProxyPingbackClientTest : public testing::Test {
   void CreateAndSendPingback() {
     DataReductionProxyData request_data;
     request_data.set_session_key(kSessionKey);
-    request_data.set_original_request_url(GURL(kFakeURL));
+    request_data.set_request_url(GURL(kFakeURL));
     request_data.set_effective_connection_type(
         net::EFFECTIVE_CONNECTION_TYPE_OFFLINE);
     factory()->set_remove_fetcher_on_delete(true);