DRP: Add check that ECT header is present when CPAT header is present.

Add a (d)check in data reduction proxy (DRP) code which verifies that
if the CPAT header is present in the request headers, then ECT header
must be present too.

Other checks added: e.g., chrome-proxy header should only
be sent when the resolved proxy is a data saver proxy.

Finally, the CL also modifies few unittests to avoid tripping the
DCHECKs.

BUG=706650

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

components/data_reduction_proxy/core/browser/data_reduction_proxy_network_delegate.cc

@@ -115,6 +115,21 @@ int64_t EstimateOriginalReceivedBytes(const net::URLRequest& request) {
          util::CalculateEffectiveOCL(request);
 }
 
+// Verifies that the chrome proxy related request headers are set correctly.
+// |via_chrome_proxy| is true if the request is being fetched via Chrome Data
+// Saver proxy.
+void VerifyHttpRequestHeaders(bool via_chrome_proxy,
+                              const net::HttpRequestHeaders& headers) {
+  if (via_chrome_proxy) {
+    DCHECK(headers.HasHeader(chrome_proxy_header()));
+    DCHECK(headers.HasHeader(chrome_proxy_ect_header()));
+  } else {
+    DCHECK(!headers.HasHeader(chrome_proxy_header()));
+    DCHECK(!headers.HasHeader(chrome_proxy_accept_transform_header()));
+    DCHECK(!headers.HasHeader(chrome_proxy_ect_header()));
+  }
+}
+
 }  // namespace
 
 DataReductionProxyNetworkDelegate::DataReductionProxyNetworkDelegate(
@@ -220,6 +235,7 @@ void DataReductionProxyNetworkDelegate::OnBeforeSendHeadersInternal(
         DataReductionProxyData::GetDataAndCreateIfNecessary(request);
     if (data)
       data->set_used_data_reduction_proxy(true);
+    VerifyHttpRequestHeaders(false, *headers);
     return;
   }
 
@@ -246,6 +262,7 @@ void DataReductionProxyNetworkDelegate::OnBeforeSendHeadersInternal(
       lofi_decider->RemoveAcceptTransformHeader(headers);
     }
     RemoveChromeProxyECTHeader(headers);
+    VerifyHttpRequestHeaders(false, *headers);
     return;
   }
 
@@ -282,6 +299,7 @@ void DataReductionProxyNetworkDelegate::OnBeforeSendHeadersInternal(
   data_reduction_proxy_request_options_->AddRequestHeader(headers);
   if (lofi_decider)
     lofi_decider->MaybeSetIgnorePreviewsBlacklistDirective(headers);
+  VerifyHttpRequestHeaders(true, *headers);
 }
 
 void DataReductionProxyNetworkDelegate::OnBeforeRedirectInternal(
@@ -514,7 +532,8 @@ void DataReductionProxyNetworkDelegate::MaybeAddChromeProxyECTHeader(
   DCHECK(!request.url().SchemeIsCryptographic());
   DCHECK(request.url().SchemeIsHTTPOrHTTPS());
 
-  DCHECK(!request_headers->HasHeader(chrome_proxy_ect_header()));
+  if (request_headers->HasHeader(chrome_proxy_ect_header()))
+    request_headers->RemoveHeader(chrome_proxy_ect_header());
 
   if (request.context()->network_quality_estimator()) {
     net::EffectiveConnectionType type = request.context()

components/data_reduction_proxy/core/browser/data_reduction_proxy_network_delegate_unittest.cc

@@ -633,6 +633,13 @@ TEST_F(DataReductionProxyNetworkDelegateTest, AuthenticationTest) {
   data_reduction_proxy_info.UseNamedProxy(data_reduction_proxy);
 
   net::HttpRequestHeaders headers;
+  // Call network delegate methods to ensure that appropriate chrome proxy
+  // headers get added/removed.
+  network_delegate()->NotifyBeforeStartTransaction(
+      fake_request.get(),
+      base::Bind(&DataReductionProxyNetworkDelegateTest::DelegateStageDone,
+                 base::Unretained(this)),
+      &headers);
   network_delegate()->NotifyBeforeSendHeaders(fake_request.get(),
                                               data_reduction_proxy_info,
                                               proxy_retry_info, &headers);
@@ -843,6 +850,14 @@ TEST_F(DataReductionProxyNetworkDelegateTest, RequestDataConfigurations) {
                                           : 0);
     lofi_decider()->SetIsUsingLoFi(test.lofi_on);
     io_data()->request_options()->SetSecureSession("fake-session");
+
+    // Call network delegate methods to ensure that appropriate chrome proxy
+    // headers get added/removed.
+    network_delegate()->NotifyBeforeStartTransaction(
+        request.get(),
+        base::Bind(&DataReductionProxyNetworkDelegateTest::DelegateStageDone,
+                   base::Unretained(this)),
+        &headers);
     network_delegate()->NotifyBeforeSendHeaders(
         request.get(), data_reduction_proxy_info, proxy_retry_info, &headers);
     DataReductionProxyData* data =
@@ -918,7 +933,7 @@ TEST_F(DataReductionProxyNetworkDelegateTest, RedirectRequestDataCleared) {
   data_reduction_proxy_info.UseNamedProxy(data_reduction_proxy);
 
   // Main frame loaded. Lo-Fi should be used.
-  net::HttpRequestHeaders headers;
+  net::HttpRequestHeaders headers_original;
   net::ProxyRetryInfoMap proxy_retry_info;
 
   test_network_quality_estimator()->set_effective_connection_type(
@@ -929,8 +944,17 @@ TEST_F(DataReductionProxyNetworkDelegateTest, RedirectRequestDataCleared) {
   request->SetLoadFlags(net::LOAD_MAIN_FRAME_DEPRECATED);
   lofi_decider()->SetIsUsingLoFi(true);
   io_data()->request_options()->SetSecureSession("fake-session");
+
+  // Call network delegate methods to ensure that appropriate chrome proxy
+  // headers get added/removed.
+  network_delegate()->NotifyBeforeStartTransaction(
+      request.get(),
+      base::Bind(&DataReductionProxyNetworkDelegateTest::DelegateStageDone,
+                 base::Unretained(this)),
+      &headers_original);
   network_delegate()->NotifyBeforeSendHeaders(
-      request.get(), data_reduction_proxy_info, proxy_retry_info, &headers);
+      request.get(), data_reduction_proxy_info, proxy_retry_info,
+      &headers_original);
   DataReductionProxyData* data =
       DataReductionProxyData::GetData(*request.get());
 
@@ -951,9 +975,19 @@ TEST_F(DataReductionProxyNetworkDelegateTest, RedirectRequestDataCleared) {
   EXPECT_FALSE(data);
 
   // Call NotifyBeforeSendHeaders again with different proxy info to check that
-  // new data isn't added.
+  // new data isn't added. Use a new set of headers since the redirected HTTP
+  // jobs do not reuse headers from the previous jobs. Also, call network
+  // delegate methods to ensure that appropriate chrome proxy headers get
+  // added/removed.
+  net::HttpRequestHeaders headers_redirect;
+  network_delegate()->NotifyBeforeStartTransaction(
+      request.get(),
+      base::Bind(&DataReductionProxyNetworkDelegateTest::DelegateStageDone,
+                 base::Unretained(this)),
+      &headers_redirect);
   network_delegate()->NotifyBeforeSendHeaders(
-      request.get(), data_reduction_proxy_info, proxy_retry_info, &headers);
+      request.get(), data_reduction_proxy_info, proxy_retry_info,
+      &headers_redirect);
   data = DataReductionProxyData::GetData(*request.get());
   EXPECT_FALSE(data);
 }