media: Send chrome-cache: frfr on first request.

components/data_reduction_proxy/core/browser/data_reduction_proxy_network_delegate.cc

 // Copyright 2014 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 "components/data_reduction_proxy/core/browser/data_reduction_proxy_network_delegate.h"
 
 #include <limits>
 #include <utility>
 
 #include "base/memory/ptr_util.h"
@@ skipping 243 matching lines @@
   // Reset |request|'s DataReductionProxyData.
   DataReductionProxyData::ClearData(request);
 
   if (params::IsIncludedInHoldbackFieldTrial()) {
     if (!WasEligibleWithoutHoldback(*request, proxy_info, proxy_retry_info))
       return;
     // For the holdback field trial, still log UMA as if the proxy was used.
     data = DataReductionProxyData::GetDataAndCreateIfNecessary(request);
     if (data)
       data->set_used_data_reduction_proxy(true);
+
+    headers->RemoveHeader(chrome_proxy_header());

[bengr, 2017/05/03 21:37:42]

Please add a case to data_reduction_proxy_network_delegate_unittest.cc that
verifies that the holdback produces no chrome-proxy header even if frfr was set
previously.

[hubbe, 2017/05/05 20:51:45]

Is there a test that does something similar I can use as a template?

[bengr, 2017/05/08 21:27:38]

Shockingly there isn't. We rely on the DCHECKs in VerifyHttpRequestHeaders. To
write a test, which I won't require, you could follow the pattern of BuildSocket
(see:
https://cs.chromium.org/chromium/src/components/data_reduction_proxy/core/bro...)
but you'd need to write everything from scratch. If you don't want to do this,
please add a TODO(bengr). Thanks.

The test would go something like this:

BuildSocketForDirectConnection(...);
std::unique_ptr<net::URLRequest> request =
  context_.CreateRequest(GURL(kTestURL), net::IDLE, &delegate);

request->SetExtraRequestHeaderByName("chrome-proxy", "frfr", true);

request->Start();
base::RunLoop().RunUntilIdle();

net::HttpRequestHeaders sent_request_header;
request->GetFulLRequestHeaders(&sent_request_headers)
EXPECT_FALSE(sent_request_headers.GetHeader(chrome_proxy_header(), &val);

[hubbe, 2017/05/11 21:19:05]

Added TODO

     VerifyHttpRequestHeaders(false, *headers);
     return;
   }
 
   bool using_data_reduction_proxy = true;
   // The following checks rule out direct, invalid, and other connection types.
   if (!proxy_info.is_http() && !proxy_info.is_https() &&
       !proxy_info.is_quic()) {
     using_data_reduction_proxy = false;
   } else if (proxy_info.proxy_server().host_port_pair().IsEmpty()) {
     using_data_reduction_proxy = false;
   } else if (!data_reduction_proxy_config_->IsDataReductionProxy(
                  proxy_info.proxy_server(), nullptr)) {
     using_data_reduction_proxy = false;
   }
 
   LoFiDecider* lofi_decider = nullptr;
   if (data_reduction_proxy_io_data_)
     lofi_decider = data_reduction_proxy_io_data_->lofi_decider();
 
   if (!using_data_reduction_proxy) {
     if (lofi_decider) {
       // If not using the data reduction proxy, strip the
       // Chrome-Proxy-Accept-Transform header.
       lofi_decider->RemoveAcceptTransformHeader(headers);
     }
     RemoveChromeProxyECTHeader(headers);
+    headers->RemoveHeader(chrome_proxy_header());

[bengr, 2017/05/03 21:37:42]

Please add a case to data_reduction_proxy_network_delegate_unittest.cc that
verifies that Chrome produces no chrome-proxy when a direct connection or other
proxty is in use, even if frfr was set previously.

     VerifyHttpRequestHeaders(false, *headers);
     return;
   }
 
   // Retrieves DataReductionProxyData from a request, creating a new instance
   // if needed.
   data = DataReductionProxyData::GetDataAndCreateIfNecessary(request);
   if (data) {
     data->set_used_data_reduction_proxy(true);
     // Only set GURL, NQE and session key string for main frame requests since
@@ skipping 30 matching lines @@
     if (!page_id) {
       page_id = data_reduction_proxy_request_options_->GeneratePageId();
     }
     data->set_page_id(page_id.value());
   }
 
   data_reduction_proxy_request_options_->AddRequestHeader(headers, page_id);
 
   if (lofi_decider)
     lofi_decider->MaybeSetIgnorePreviewsBlacklistDirective(headers);
   VerifyHttpRequestHeaders(true, *headers);

[bengr, 2017/05/03 21:37:42]

Does video not affect this case?

[hubbe, 2017/05/05 20:51:45]

This calls VerifyHttpRequestHeaders() with "true".
I assume that if we remove the header here, it will never make it anywhere
ever...

[bengr, 2017/05/08 21:27:38]

Acknowledged.

 }
 
 void DataReductionProxyNetworkDelegate::OnBeforeRedirectInternal(
     net::URLRequest* request,
     const GURL& new_location) {
   // Since this is after a redirect response, reset |request|'s
   // DataReductionProxyData, but keep page ID and session.
   // TODO(ryansturm): Change ClearData logic to have persistent and
   // non-persistent (WRT redirects) data.
   // crbug.com/709564
@@ skipping 289 matching lines @@
 }
 
 void DataReductionProxyNetworkDelegate::RemoveChromeProxyECTHeader(
     net::HttpRequestHeaders* request_headers) const {
   DCHECK(thread_checker_.CalledOnValidThread());
 
   request_headers->RemoveHeader(chrome_proxy_ect_header());
 }
 
 }  // namespace data_reduction_proxy