Make request fail with ERR_CONTENT_DECODING_INIT_FAILED if sdch is not supported

Make request fail with ERR_CONTENT_DECODING_INIT_FAILED if sdch is not supported

When server sends us a sdch encoded response, but we never
advertised sdch support, this CL makes SetUpSourceStream
return null so that the request is failed with
ERR_CONTENT_DECODING_INIT_FAILED.

This old behavior in net::Filter passes through the raw
content without generating an error.

BUG=659363
Committed: https://crrev.com/9a328c6cd891d02c535c997abd330a05e10e2398
Cr-Commit-Position: refs/heads/master@{#427815}

[xunjieli, 2016-10-26 16:32:23]

Patchset #1 (id:1) has been deleted

[xunjieli, 2016-10-26 16:33:16]

Description was changed from

==========
Check SdchManager in URLRequestHttpJob::SetUpSourceStream

When server sends us a sdch encoded response, but we never advertised sdch
support, this CL makes SetUpSourceStream return null.

This old behavior passes through the raw content, we can match the behavior,
but I think it might mask serious errors.

BUG=659363
==========

to

==========
Check SdchManager in URLRequestHttpJob::SetUpSourceStream

When server sends us a sdch encoded response, but we never 
advertised sdch support, this CL makes SetUpSourceStream 
return null.

This old behavior passes through the raw content, we can 
match that behavior. But  I think it might mask serious 
errors.

BUG=659363
==========

[xunjieli, 2016-10-26 16:35:48]

xunjieli@chromium.org changed reviewers:
+ mmenke@chromium.org, rdsmith@chromium.org

[xunjieli, 2016-10-26 16:35:49]

Randy, Matt: PTAL.

The new behavior is slightly different than the old one. In the old code, if we
never advertise sdch, but receive a sdch response, we pass through the raw
content without applying any filters.

I think it's better not to mask this error. Let me know what you think.

[mmenke, 2016-10-26 16:45:43]

https://codereview.chromium.org/2451233002/diff/20001/net/url_request/url_req...
File net/url_request/url_request_http_job_unittest.cc (right):

https://codereview.chromium.org/2451233002/diff/20001/net/url_request/url_req...
net/url_request/url_request_http_job_unittest.cc:164:
SdchNotAdvertisedGotMalformedSdchResponse) {
Maybe add:

// This test expects TestURLRequestContexts to have no SdchManager.
DCHECK(!context_.sdch_manager());

[xunjieli, 2016-10-26 16:49:22]

https://codereview.chromium.org/2451233002/diff/20001/net/url_request/url_req...
File net/url_request/url_request_http_job_unittest.cc (right):

https://codereview.chromium.org/2451233002/diff/20001/net/url_request/url_req...
net/url_request/url_request_http_job_unittest.cc:164:
SdchNotAdvertisedGotMalformedSdchResponse) {
On 2016/10/26 16:45:43, mmenke wrote:
> Maybe add:
> 
> // This test expects TestURLRequestContexts to have no SdchManager.
> DCHECK(!context_.sdch_manager());

Done.

[mmenke, 2016-10-26 17:29:47]

LGTM

[Randy Smith (Not in Mondays), 2016-10-26 17:35:20]

I'd like to make sure the CL description describes clearly the user-visible
change, to make it easier for triagers in case it causes problems.  What will
making the source stream null do?  Fail the request?

(I'll do a review, just wanted to get that out there in case you were planning
to just go with Matt's review.)

[Randy Smith (Not in Mondays), 2016-10-26 17:36:49]

LGTM modulo CL description clearly describing the change in behavior (both old
and new behavior) from the consumers POV.

[xunjieli, 2016-10-26 17:46:29]

Description was changed from

==========
Check SdchManager in URLRequestHttpJob::SetUpSourceStream

When server sends us a sdch encoded response, but we never 
advertised sdch support, this CL makes SetUpSourceStream 
return null.

This old behavior passes through the raw content, we can 
match that behavior. But  I think it might mask serious 
errors.

BUG=659363
==========

to

==========
Make request fail with ERR_CONTENT_DECODING_INIT_FAILED if sdch is not supported

When server sends us a sdch encoded response, but we never 
advertised sdch support, this CL makes SetUpSourceStream 
return null so that the request is failed with 
ERR_CONTENT_DECODING_INIT_FAILED.

This old behavior in net::Filter passes through the raw content without
generating an error.

BUG=659363
==========

[xunjieli, 2016-10-26 17:46:37]

Description was changed from

==========
Make request fail with ERR_CONTENT_DECODING_INIT_FAILED if sdch is not supported

When server sends us a sdch encoded response, but we never 
advertised sdch support, this CL makes SetUpSourceStream 
return null so that the request is failed with 
ERR_CONTENT_DECODING_INIT_FAILED.

This old behavior in net::Filter passes through the raw content without
generating an error.

BUG=659363
==========

to

==========
Make request fail with ERR_CONTENT_DECODING_INIT_FAILED if sdch is not supported

When server sends us a sdch encoded response, but we never 
advertised sdch support, this CL makes SetUpSourceStream 
return null so that the request is failed with 
ERR_CONTENT_DECODING_INIT_FAILED.

This old behavior in net::Filter passes through the raw 
content without generating an error.

BUG=659363
==========

[xunjieli, 2016-10-26 17:46:59]

On 2016/10/26 17:36:49, Randy Smith - Not in Mondays wrote:
> LGTM modulo CL description clearly describing the change in behavior (both old
> and new behavior) from the consumers POV.

Done. Thanks!

[xunjieli, 2016-10-26 17:47:15]

The CQ bit was checked by xunjieli@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-10-26 17:47:47]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-26 19:01:31]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-10-26 19:01:33]

Dry run: This issue passed the CQ dry run.

[xunjieli, 2016-10-26 19:02:01]

The CQ bit was checked by xunjieli@chromium.org

[commit-bot: I haz the power, 2016-10-26 19:03:27]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-26 21:38:46]

Description was changed from

==========
Make request fail with ERR_CONTENT_DECODING_INIT_FAILED if sdch is not supported

When server sends us a sdch encoded response, but we never 
advertised sdch support, this CL makes SetUpSourceStream 
return null so that the request is failed with 
ERR_CONTENT_DECODING_INIT_FAILED.

This old behavior in net::Filter passes through the raw 
content without generating an error.

BUG=659363
==========

to

==========
Make request fail with ERR_CONTENT_DECODING_INIT_FAILED if sdch is not supported

When server sends us a sdch encoded response, but we never 
advertised sdch support, this CL makes SetUpSourceStream 
return null so that the request is failed with 
ERR_CONTENT_DECODING_INIT_FAILED.

This old behavior in net::Filter passes through the raw 
content without generating an error.

BUG=659363
==========

[commit-bot: I haz the power, 2016-10-26 21:38:47]

Committed patchset #2 (id:40001)

[commit-bot: I haz the power, 2016-10-26 21:42:19]

Description was changed from

==========
Make request fail with ERR_CONTENT_DECODING_INIT_FAILED if sdch is not supported

When server sends us a sdch encoded response, but we never 
advertised sdch support, this CL makes SetUpSourceStream 
return null so that the request is failed with 
ERR_CONTENT_DECODING_INIT_FAILED.

This old behavior in net::Filter passes through the raw 
content without generating an error.

BUG=659363
==========

to

==========
Make request fail with ERR_CONTENT_DECODING_INIT_FAILED if sdch is not supported

When server sends us a sdch encoded response, but we never
advertised sdch support, this CL makes SetUpSourceStream
return null so that the request is failed with
ERR_CONTENT_DECODING_INIT_FAILED.

This old behavior in net::Filter passes through the raw
content without generating an error.

BUG=659363

Committed: https://crrev.com/9a328c6cd891d02c535c997abd330a05e10e2398
Cr-Commit-Position: refs/heads/master@{#427815}
==========

[commit-bot: I haz the power, 2016-10-26 21:42:20]

Patchset 2 (id:??) landed as
https://crrev.com/9a328c6cd891d02c535c997abd330a05e10e2398
Cr-Commit-Position: refs/heads/master@{#427815}