Make URLRequestHttpJob the only URLRequestJob concerned with FilterContext.

Make URLRequestHttpJob the only URLRequestJob concerned with FilterContext.

In order to allow this, provide a new factory method in Filter to return a
GZipFilter (used by URLRequestFileJob and URLRequestJobTrackerTest), and
invert control so that URLRequestJobs are responsible for constructing filters.

This is one step away from removing FilterContext as a base class of URLRequestJob,
which will be tackled in a followup change.

BUG=none
TEST=net_unittests

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=78489

[willchan no longer on Chromium, 2011-03-16 23:13:16]

http://codereview.chromium.org/6674042/diff/1/net/base/filter.h
File net/base/filter.h (right):

http://codereview.chromium.org/6674042/diff/1/net/base/filter.h#newcode146
net/base/filter.h:146: // types[1] = FILTER_TYPE_GZIP will cause data to first
be gunizip filtered,
gunizip? Existing typo I think.

http://codereview.chromium.org/6674042/diff/1/net/base/filter.h#newcode249
net/base/filter.h:249: // Helper methods for PrependNewFilter. If initialization
is successful,
My stylistic preference is to move these private static functions to an
anonymous namespace in the .cc file. Up to you though.

http://codereview.chromium.org/6674042/diff/1/net/base/filter.h#newcode252
net/base/filter.h:252: static Filter* InitSdchFilter(FilterType tyep_id,
tyep_id => type_id

http://codereview.chromium.org/6674042/diff/1/net/url_request/url_request_job.h
File net/url_request/url_request_job.h (right):

http://codereview.chromium.org/6674042/diff/1/net/url_request/url_request_job...
net/url_request/url_request_job.h:124: virtual Filter* SetupFilter();
Offhand, it looks like this might be able to be a const member function. Is that
right?

[adamk, 2011-03-16 23:21:34]

http://codereview.chromium.org/6674042/diff/1/net/base/filter.h
File net/base/filter.h (right):

http://codereview.chromium.org/6674042/diff/1/net/base/filter.h#newcode146
net/base/filter.h:146: // types[1] = FILTER_TYPE_GZIP will cause data to first
be gunizip filtered,
On 2011/03/16 23:13:17, willchan wrote:
> gunizip? Existing typo I think.

Done.

http://codereview.chromium.org/6674042/diff/1/net/base/filter.h#newcode249
net/base/filter.h:249: // Helper methods for PrependNewFilter. If initialization
is successful,
On 2011/03/16 23:13:17, willchan wrote:
> My stylistic preference is to move these private static functions to an
> anonymous namespace in the .cc file. Up to you though.

Can't do that, as the GZipFilter and SdchFilter constructors are private and
Filter is a friend (their constructors are not sufficient for proper
initialization).

http://codereview.chromium.org/6674042/diff/1/net/base/filter.h#newcode252
net/base/filter.h:252: static Filter* InitSdchFilter(FilterType tyep_id,
On 2011/03/16 23:13:17, willchan wrote:
> tyep_id => type_id

Done.  I really like the WebKit style here, actually, where I wouldn't list a
name at all.

http://codereview.chromium.org/6674042/diff/1/net/url_request/url_request_job.h
File net/url_request/url_request_job.h (right):

http://codereview.chromium.org/6674042/diff/1/net/url_request/url_request_job...
net/url_request/url_request_job.h:124: virtual Filter* SetupFilter();
On 2011/03/16 23:13:17, willchan wrote:
> Offhand, it looks like this might be able to be a const member function. Is
that
> right?

Sure, let's make that const.  Done.

[willchan no longer on Chromium, 2011-03-16 23:27:58]

LGTM

[eustas, 2015-12-01 12:57:23]

eustas@chromium.org changed reviewers:
+ eustas@chromium.org

[eustas, 2015-12-01 12:57:23]

https://codereview.chromium.org/6674042/diff/4001/net/base/filter.cc
File net/base/filter.cc (left):

https://codereview.chromium.org/6674042/diff/4001/net/base/filter.cc#oldcode377
net/base/filter.cc:377: delete filter_list;
Won't this cause memleaks?
Usage pattern is:
Filter* filter_list = NULL;
...
filter_list = PrependNewFilter(..., filter_list);
...

filter_list is lost and there is no way to free the memory of already created
filters.

[adamk, 2015-12-01 18:29:47]

On 2015/12/01 12:57:23, eustas wrote:
> https://codereview.chromium.org/6674042/diff/4001/net/base/filter.cc
> File net/base/filter.cc (left):
> 
>
https://codereview.chromium.org/6674042/diff/4001/net/base/filter.cc#oldcode377
> net/base/filter.cc:377: delete filter_list;
> Won't this cause memleaks?
> Usage pattern is:
> Filter* filter_list = NULL;
> ...
> filter_list = PrependNewFilter(..., filter_list);
> ...
> 
> filter_list is lost and there is no way to free the memory of already created
> filters.

I haven't worked on net stuff since...about the time of this patch, so I'm
afraid I can't meaningfully answer your question.