Add brotli content-encoding filter.

net/url_request/url_request_http_job.cc

Index: net/url_request/url_request_http_job.cc
diff --git a/net/url_request/url_request_http_job.cc b/net/url_request/url_request_http_job.cc
index 5c4ddd11c499c3cd997133376c8e5407bab861cb..97e6e11fc21a39ae747b9c4829cebe9569f0f023 100644
--- a/net/url_request/url_request_http_job.cc
+++ b/net/url_request/url_request_http_job.cc
@@ -591,30 +591,38 @@ void URLRequestHttpJob::AddExtraHeaders() {
       }
     }
 
+    // Advertise "br" encoding only if transferred data is opaque to proxy.
+    bool advertise_brotli = false;
+    const HttpNetworkSession::Params* network_session_params =
+        request()->context()->GetNetworkSessionParams();
+    if (network_session_params && network_session_params->enable_brotli) {

[Randy Smith (Not in Mondays), 2015/12/04 16:07:52]

nit: No curly braces needed if both conditional and statement are single line
(and its more consistent with surrounding code not to have them).

[eustas, 2015/12/07 12:30:38]

Done.

+      advertise_brotli = !request()->url().SchemeIs(url::kHttpScheme);

[Randy Smith (Not in Mondays), 2015/12/04 16:07:52]

I *think* you want SchemeIsCryptographic(); that's what I've seen elsewhere for
this type of test.  If that doesn't look right to you, push back and I'll dig up
someone who'll know for sure.

[eustas, 2015/12/07 12:30:38]

I'm not sure. The reason of this check is to avoid problems with old proxy
servers.
If proxy server deals well, for example, with ws/wss protocols, then brotli
encoding doesn't seem to be a problem.

Even "http" scheme check is too strict, because spdy/http2 protocols almost
guarantee (IMHO) that nothing bad should happen to the data with non
gzip/deflate Content-Encoding.

[Randy Smith (Not in Mondays), 2015/12/07 22:27:28]

So I'm confused as to why ws: or ftp: (both of which the current check allows)
would be ok.  I would think that any cleartext protocol allows interference from
proxies, whereas any end-to-end cryptographic protocol does not.  Thus my
suggestion.
I'm confused by this comment; this check is saying that if it's *not* http
you'll advertise brotli, which should include spdy2/http2.

[Ryan Sleevi, 2015/12/08 18:58:50]

+1 - I agree with Randy here.

!SchemeIs(url::kHttpScheme) smells like an anti-pattern here, and security has
been working to remove such ad-hoc checks in favour of SchemeIsCryptographic

Further, when we consider what other user agents may look for with implementing,
it's far more problematic to suggest a scheme-by-scheme whitelist as opposed to
simply stating 'secure origins'.

+    }
+
     // Supply Accept-Encoding headers first so that it is more likely that they
     // will be in the first transmitted packet. This can sometimes make it
     // easier to filter and analyze the streams to assure that a proxy has not
     // damaged these headers. Some proxies deliberately corrupt Accept-Encoding
     // headers.
-    if (!advertise_sdch) {
-      // Tell the server what compression formats we support (other than SDCH).
-      request_info_.extra_headers.SetHeader(
-          HttpRequestHeaders::kAcceptEncoding, "gzip, deflate");
-    } else {
-      // Include SDCH in acceptable list.
+    std::string advertised_encodings = "gzip, deflate";
+    if (advertise_sdch)
+      advertised_encodings += ", sdch";
+    if (advertise_brotli)
+      advertised_encodings += ", br";
+    // Tell the server what compression formats we support.

[xunjieli, 2015/12/04 15:21:19]

nit: avoid using first person pronouns. Maybe change to "Tell the server what
compression formats are supported."

[eustas, 2015/12/04 16:14:31]

Nice! Fixed.

+    request_info_.extra_headers.SetHeader(HttpRequestHeaders::kAcceptEncoding,
+                                          advertised_encodings);
+
+    if (dictionaries_advertised_) {
       request_info_.extra_headers.SetHeader(
-          HttpRequestHeaders::kAcceptEncoding, "gzip, deflate, sdch");
-      if (dictionaries_advertised_) {
-        request_info_.extra_headers.SetHeader(
-            kAvailDictionaryHeader,
-            dictionaries_advertised_->GetDictionaryClientHashList());
-        // Since we're tagging this transaction as advertising a dictionary,
-        // we'll definitely employ an SDCH filter (or tentative sdch filter)
-        // when we get a response. When done, we'll record histograms via
-        // SDCH_DECODE or SDCH_PASSTHROUGH. Hence we need to record packet
-        // arrival times.
-        packet_timing_enabled_ = true;
-      }
+          kAvailDictionaryHeader,
+          dictionaries_advertised_->GetDictionaryClientHashList());
+      // Since we're tagging this transaction as advertising a dictionary,
+      // we'll definitely employ an SDCH filter (or tentative sdch filter)
+      // when we get a response. When done, we'll record histograms via
+      // SDCH_DECODE or SDCH_PASSTHROUGH. Hence we need to record packet
+      // arrival times.
+      packet_timing_enabled_ = true;
     }
   }