diff options
author | adamk@chromium.org <adamk@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2011-03-08 19:24:40 +0000 |
---|---|---|
committer | adamk@chromium.org <adamk@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2011-03-08 19:24:40 +0000 |
commit | f74252d3754d1d58b6f97cb3cd89b47d151cd88d (patch) | |
tree | 487247718c1268c2061eafc24e132ed3eda333c6 | |
parent | eeb0af285a61c37c0fbcb046174bf29741324a08 (diff) | |
download | chromium_src-f74252d3754d1d58b6f97cb3cd89b47d151cd88d.zip chromium_src-f74252d3754d1d58b6f97cb3cd89b47d151cd88d.tar.gz chromium_src-f74252d3754d1d58b6f97cb3cd89b47d151cd88d.tar.bz2 |
Remove GetInputStreamBufferSize() method from FilterContext.
This virtual method, implemented only by URLRequestJob and MockFilterContext,
was only used for testing purposes. The kFilterBufSize constant now lives
in filter.cc (the only place it was used), and for the few tests that needed
to override the buffer size, I've added a test-only method in filter.h.
The result is a smaller interface surface in URLRequestJob and simpler tests for most cases in gzip_filter_unittest.cc and sdch_filter_unittest.cc. I've done some further refactoring of the former to remove redundancy (most of Filter's complexity is exercised only in the SDCH test).
Review URL: http://codereview.chromium.org/6516025
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@77315 0039d316-1c4b-4281-b951-d872f2087c98
-rw-r--r-- | net/base/filter.cc | 53 | ||||
-rw-r--r-- | net/base/filter.h | 26 | ||||
-rw-r--r-- | net/base/filter_unittest.cc | 18 | ||||
-rw-r--r-- | net/base/gzip_filter.h | 6 | ||||
-rw-r--r-- | net/base/gzip_filter_unittest.cc | 109 | ||||
-rw-r--r-- | net/base/mock_filter_context.cc | 7 | ||||
-rw-r--r-- | net/base/mock_filter_context.h | 6 | ||||
-rw-r--r-- | net/base/sdch_filter.h | 6 | ||||
-rw-r--r-- | net/base/sdch_filter_unittest.cc | 109 | ||||
-rw-r--r-- | net/url_request/url_request_job.cc | 8 | ||||
-rw-r--r-- | net/url_request/url_request_job.h | 4 |
11 files changed, 158 insertions, 194 deletions
diff --git a/net/base/filter.cc b/net/base/filter.cc index 38e6651..b229c45 100644 --- a/net/base/filter.cc +++ b/net/base/filter.cc @@ -34,6 +34,9 @@ const char kApplicationXCompress[] = "application/x-compress"; const char kApplicationCompress[] = "application/compress"; const char kTextHtml[] = "text/html"; +// Buffer size allocated when de-compressing data. +const int kFilterBufSize = 32 * 1024; + } // namespace FilterContext::~FilterContext() { @@ -43,15 +46,29 @@ Filter::~Filter() {} Filter* Filter::Factory(const std::vector<FilterType>& filter_types, const FilterContext& filter_context) { - DCHECK_GT(filter_context.GetInputStreamBufferSize(), 0); - if (filter_types.empty() || filter_context.GetInputStreamBufferSize() <= 0) + if (filter_types.empty()) return NULL; + Filter* filter_list = NULL; // Linked list of filters. + for (size_t i = 0; i < filter_types.size(); i++) { + filter_list = PrependNewFilter(filter_types[i], filter_context, + kFilterBufSize, filter_list); + if (!filter_list) + return NULL; + } + return filter_list; +} + +Filter* Filter::FactoryForTests(const std::vector<FilterType>& filter_types, + const FilterContext& filter_context, + int buffer_size) { + if (filter_types.empty()) + return NULL; Filter* filter_list = NULL; // Linked list of filters. for (size_t i = 0; i < filter_types.size(); i++) { filter_list = PrependNewFilter(filter_types[i], filter_context, - filter_list); + buffer_size, filter_list); if (!filter_list) return NULL; } @@ -329,6 +346,7 @@ Filter::FilterStatus Filter::CopyOut(char* dest_buffer, int* dest_len) { // static Filter* Filter::PrependNewFilter(FilterType type_id, const FilterContext& filter_context, + int buffer_size, Filter* filter_list) { Filter* first_filter = NULL; // Soon to be start of chain. switch (type_id) { @@ -337,10 +355,9 @@ Filter* Filter::PrependNewFilter(FilterType type_id, case FILTER_TYPE_GZIP: { scoped_ptr<net::GZipFilter> gz_filter( new net::GZipFilter(filter_context)); - if (gz_filter->InitBuffer()) { - if (gz_filter->InitDecoding(type_id)) { - first_filter = gz_filter.release(); - } + gz_filter->InitBuffer(buffer_size); + if (gz_filter->InitDecoding(type_id)) { + first_filter = gz_filter.release(); } break; } @@ -348,10 +365,9 @@ Filter* Filter::PrependNewFilter(FilterType type_id, case FILTER_TYPE_SDCH_POSSIBLE: { scoped_ptr<net::SdchFilter> sdch_filter( new net::SdchFilter(filter_context)); - if (sdch_filter->InitBuffer()) { - if (sdch_filter->InitDecoding(type_id)) { - first_filter = sdch_filter.release(); - } + sdch_filter->InitBuffer(buffer_size); + if (sdch_filter->InitDecoding(type_id)) { + first_filter = sdch_filter.release(); } break; } @@ -370,20 +386,11 @@ Filter* Filter::PrependNewFilter(FilterType type_id, return first_filter; } -bool Filter::InitBuffer() { - int buffer_size = filter_context_.GetInputStreamBufferSize(); +void Filter::InitBuffer(int buffer_size) { + DCHECK(!stream_buffer()); DCHECK_GT(buffer_size, 0); - if (buffer_size <= 0 || stream_buffer()) - return false; - stream_buffer_ = new net::IOBuffer(buffer_size); - - if (stream_buffer()) { - stream_buffer_size_ = buffer_size; - return true; - } - - return false; + stream_buffer_size_ = buffer_size; } void Filter::PushDataIntoNextFilter() { diff --git a/net/base/filter.h b/net/base/filter.h index c6004e9..f00e95e 100644 --- a/net/base/filter.h +++ b/net/base/filter.h @@ -43,6 +43,7 @@ class GURL; namespace net { class IOBuffer; +class SdchFilterChainingTest; } //------------------------------------------------------------------------------ @@ -94,13 +95,6 @@ class FilterContext { // For example: 200 is ok. 4xx are error codes. etc. virtual int GetResponseCode() const = 0; - // What is the desirable input buffer size for these filters? - // This value is currently supplied by the context, and is constant for all - // filters, even when they are part of a chain of filters. (i.e., we currently - // don't change the input buffer sizes for a linked chain of filters, and the - // buffer size for input to all filters in a chain is this one constant). - virtual int GetInputStreamBufferSize() const = 0; - // The following method forces the context to emit a specific set of // statistics as selected by the argument. virtual void RecordPacketStats(StatisticSelector statistic) const = 0; @@ -199,7 +193,8 @@ class Filter { std::vector<FilterType>* encoding_types); protected: - FRIEND_TEST_ALL_PREFIXES(SdchFilterTest, ContentTypeId); + friend class GZipUnitTest; + friend class net::SdchFilterChainingTest; explicit Filter(const FilterContext& filter_context); @@ -236,6 +231,9 @@ class Filter { int stream_data_len_; private: + // Allocates and initializes stream_buffer_ and stream_buffer_size_. + void InitBuffer(int size); + // A factory helper for creating filters for within a chain of potentially // multiple encodings. If a chain of filters is created, then this may be // called multiple times during the filter creation process. In most simple @@ -243,16 +241,18 @@ class Filter { // filter_list) if a new filter can't be constructed. static Filter* PrependNewFilter(FilterType type_id, const FilterContext& filter_context, + int buffer_size, Filter* filter_list); - // Allocates and initializes stream_buffer_ based on filter_context_. - // Establishes a buffer large enough to handle the amount specified in - // filter_context_.GetInputStreamBufferSize(). - bool InitBuffer(); - // Helper function to empty our output into the next filter's input. void PushDataIntoNextFilter(); + // Constructs a filter with an internal buffer of the given size. + // Only meant to be called by unit tests that need to control the buffer size. + static Filter* FactoryForTests(const std::vector<FilterType>& filter_types, + const FilterContext& filter_context, + int buffer_size); + // An optional filter to process output from this filter. scoped_ptr<Filter> next_filter_; // Remember what status or local filter last returned so we can better handle diff --git a/net/base/filter_unittest.cc b/net/base/filter_unittest.cc index c65c3de..9c20d69 100644 --- a/net/base/filter_unittest.cc +++ b/net/base/filter_unittest.cc @@ -35,8 +35,7 @@ TEST(FilterTest, ContentTypeId) { // Check various fixups that modify content encoding lists. TEST(FilterTest, ApacheGzip) { - const int kInputBufferSize(100); - net::MockFilterContext filter_context(kInputBufferSize); + net::MockFilterContext filter_context; filter_context.SetSdchResponse(false); // Check that redundant gzip mime type removes only solo gzip encoding. @@ -84,8 +83,7 @@ TEST(FilterTest, ApacheGzip) { TEST(FilterTest, SdchEncoding) { // Handle content encodings including SDCH. const std::string kTextHtmlMime("text/html"); - const int kInputBufferSize(100); - net::MockFilterContext filter_context(kInputBufferSize); + net::MockFilterContext filter_context; filter_context.SetSdchResponse(true); std::vector<Filter::FilterType> encoding_types; @@ -122,8 +120,7 @@ TEST(FilterTest, SdchEncoding) { TEST(FilterTest, MissingSdchEncoding) { // Handle interesting case where entire SDCH encoding assertion "got lost." const std::string kTextHtmlMime("text/html"); - const int kInputBufferSize(100); - net::MockFilterContext filter_context(kInputBufferSize); + net::MockFilterContext filter_context; filter_context.SetSdchResponse(true); std::vector<Filter::FilterType> encoding_types; @@ -158,8 +155,7 @@ TEST(FilterTest, MissingSdchEncoding) { } TEST(FilterTest, Svgz) { - const int kInputBufferSize(100); - net::MockFilterContext filter_context(kInputBufferSize); + net::MockFilterContext filter_context; // Check that svgz files are only decompressed when not downloading. const std::string kSvgzMime("image/svg+xml"); @@ -207,8 +203,7 @@ TEST(FilterTest, Svgz) { TEST(FilterTest, UnsupportedMimeGzip) { // From issue 8170 - handling files with Content-Encoding: x-gzip - const int kInputBufferSize(100); - net::MockFilterContext filter_context(kInputBufferSize); + net::MockFilterContext filter_context; std::vector<Filter::FilterType> encoding_types; const std::string kTarMime("application/x-tar"); const std::string kCpioMime("application/x-cpio"); @@ -295,8 +290,7 @@ TEST(FilterTest, UnsupportedMimeGzip) { TEST(FilterTest, SupportedMimeGzip) { // From issue 16430 - Files with supported mime types should be decompressed, // even though these files end in .gz/.tgz. - const int kInputBufferSize(100); - net::MockFilterContext filter_context(kInputBufferSize); + net::MockFilterContext filter_context; std::vector<Filter::FilterType> encoding_types; const std::string kGzUrl("http://ignore.com/foo.gz"); const std::string kUrl("http://ignore.com/foo"); diff --git a/net/base/gzip_filter.h b/net/base/gzip_filter.h index b37a5b1..7496be5 100644 --- a/net/base/gzip_filter.h +++ b/net/base/gzip_filter.h @@ -28,8 +28,6 @@ class GZipHeader; class GZipFilter : public Filter { public: - explicit GZipFilter(const FilterContext& filter_context); - virtual ~GZipFilter(); // Initializes filter decoding mode and internal control blocks. @@ -75,6 +73,10 @@ class GZipFilter : public Filter { static const int kGZipFooterSize = 8; + // Only to be instantiated by Filter::Factory. + explicit GZipFilter(const FilterContext& filter_context); + friend class Filter; + // Parses and verifies the GZip header. // Upon exit, the function updates gzip_header_status_ accordingly. // diff --git a/net/base/gzip_filter_unittest.cc b/net/base/gzip_filter_unittest.cc index 247e7da..d07c24d 100644 --- a/net/base/gzip_filter_unittest.cc +++ b/net/base/gzip_filter_unittest.cc @@ -24,7 +24,6 @@ namespace { const int kDefaultBufferSize = 4096; const int kSmallBufferSize = 128; -const int kMaxBufferSize = 1048576; // 1048576 == 2^20 == 1 MB const char kApplicationOctetStream[] = "application/octet-stream"; const char kApplicationXGzip[] = "application/x-gzip"; @@ -52,6 +51,8 @@ enum EncodeMode { ENCODE_DEFLATE // Raw deflate. }; +} // namespace + // These tests use the path service, which uses autoreleased objects on the // Mac, so this needs to be a PlatformTest. class GZipUnitTest : public PlatformTest { @@ -220,9 +221,27 @@ class GZipUnitTest : public PlatformTest { return filter->ReadData(dest, dest_len); } + void InitFilter(Filter::FilterType type) { + std::vector<Filter::FilterType> filter_types; + filter_types.push_back(type); + filter_.reset(Filter::Factory(filter_types, filter_context_)); + ASSERT_TRUE(filter_.get()); + ASSERT_GE(filter_->stream_buffer_size(), kDefaultBufferSize); + } + + void InitFilterWithBufferSize(Filter::FilterType type, int buffer_size) { + std::vector<Filter::FilterType> filter_types; + filter_types.push_back(type); + filter_.reset(Filter::FactoryForTests(filter_types, filter_context_, + buffer_size)); + ASSERT_TRUE(filter_.get()); + } + const char* source_buffer() const { return source_buffer_.data(); } int source_len() const { return static_cast<int>(source_buffer_.size()); } + scoped_ptr<Filter> filter_; + std::string source_buffer_; char* deflate_encode_buffer_; @@ -230,23 +249,22 @@ class GZipUnitTest : public PlatformTest { char* gzip_encode_buffer_; int gzip_encode_len_; + + private: + net::MockFilterContext filter_context_; }; // Basic scenario: decoding deflate data with big enough buffer. TEST_F(GZipUnitTest, DecodeDeflate) { // Decode the compressed data with filter - std::vector<Filter::FilterType> filter_types; - filter_types.push_back(Filter::FILTER_TYPE_DEFLATE); - net::MockFilterContext filter_context(kDefaultBufferSize); - scoped_ptr<Filter> filter(Filter::Factory(filter_types, filter_context)); - ASSERT_TRUE(filter.get()); - memcpy(filter->stream_buffer()->data(), deflate_encode_buffer_, + InitFilter(Filter::FILTER_TYPE_DEFLATE); + memcpy(filter_->stream_buffer()->data(), deflate_encode_buffer_, deflate_encode_len_); - filter->FlushStreamBuffer(deflate_encode_len_); + filter_->FlushStreamBuffer(deflate_encode_len_); char deflate_decode_buffer[kDefaultBufferSize]; int deflate_decode_size = kDefaultBufferSize; - filter->ReadData(deflate_decode_buffer, &deflate_decode_size); + filter_->ReadData(deflate_decode_buffer, &deflate_decode_size); // Compare the decoding result with source data EXPECT_TRUE(deflate_decode_size == source_len()); @@ -256,18 +274,14 @@ TEST_F(GZipUnitTest, DecodeDeflate) { // Basic scenario: decoding gzip data with big enough buffer. TEST_F(GZipUnitTest, DecodeGZip) { // Decode the compressed data with filter - std::vector<Filter::FilterType> filter_types; - filter_types.push_back(Filter::FILTER_TYPE_GZIP); - net::MockFilterContext filter_context(kDefaultBufferSize); - scoped_ptr<Filter> filter(Filter::Factory(filter_types, filter_context)); - ASSERT_TRUE(filter.get()); - memcpy(filter->stream_buffer()->data(), gzip_encode_buffer_, + InitFilter(Filter::FILTER_TYPE_GZIP); + memcpy(filter_->stream_buffer()->data(), gzip_encode_buffer_, gzip_encode_len_); - filter->FlushStreamBuffer(gzip_encode_len_); + filter_->FlushStreamBuffer(gzip_encode_len_); char gzip_decode_buffer[kDefaultBufferSize]; int gzip_decode_size = kDefaultBufferSize; - filter->ReadData(gzip_decode_buffer, &gzip_decode_size); + filter_->ReadData(gzip_decode_buffer, &gzip_decode_size); // Compare the decoding result with source data EXPECT_TRUE(gzip_decode_size == source_len()); @@ -278,12 +292,9 @@ TEST_F(GZipUnitTest, DecodeGZip) { // To do that, we create a filter with a small buffer that can not hold all // the input data. TEST_F(GZipUnitTest, DecodeWithSmallBuffer) { - std::vector<Filter::FilterType> filter_types; - filter_types.push_back(Filter::FILTER_TYPE_DEFLATE); - net::MockFilterContext filter_context(kSmallBufferSize); - scoped_ptr<Filter> filter(Filter::Factory(filter_types, filter_context)); - ASSERT_TRUE(filter.get()); - DecodeAndCompareWithFilter(filter.get(), source_buffer(), source_len(), + InitFilterWithBufferSize(Filter::FILTER_TYPE_DEFLATE, kSmallBufferSize); + EXPECT_EQ(kSmallBufferSize, filter_->stream_buffer_size()); + DecodeAndCompareWithFilter(filter_.get(), source_buffer(), source_len(), deflate_encode_buffer_, deflate_encode_len_, kDefaultBufferSize); } @@ -293,24 +304,17 @@ TEST_F(GZipUnitTest, DecodeWithSmallBuffer) { // header correctly. (2) Sometimes the filter will consume input without // generating output. Verify filter can handle it correctly. TEST_F(GZipUnitTest, DecodeWithOneByteBuffer) { - std::vector<Filter::FilterType> filter_types; - filter_types.push_back(Filter::FILTER_TYPE_GZIP); - net::MockFilterContext filter_context(1); - scoped_ptr<Filter> filter(Filter::Factory(filter_types, filter_context)); - ASSERT_TRUE(filter.get()); - DecodeAndCompareWithFilter(filter.get(), source_buffer(), source_len(), + InitFilterWithBufferSize(Filter::FILTER_TYPE_GZIP, 1); + EXPECT_EQ(1, filter_->stream_buffer_size()); + DecodeAndCompareWithFilter(filter_.get(), source_buffer(), source_len(), gzip_encode_buffer_, gzip_encode_len_, kDefaultBufferSize); } // Tests we can decode when caller has small buffer to read out from filter. TEST_F(GZipUnitTest, DecodeWithSmallOutputBuffer) { - std::vector<Filter::FilterType> filter_types; - filter_types.push_back(Filter::FILTER_TYPE_DEFLATE); - net::MockFilterContext filter_context(kDefaultBufferSize); - scoped_ptr<Filter> filter(Filter::Factory(filter_types, filter_context)); - ASSERT_TRUE(filter.get()); - DecodeAndCompareWithFilter(filter.get(), source_buffer(), source_len(), + InitFilter(Filter::FILTER_TYPE_DEFLATE); + DecodeAndCompareWithFilter(filter_.get(), source_buffer(), source_len(), deflate_encode_buffer_, deflate_encode_len_, kSmallBufferSize); } @@ -318,12 +322,9 @@ TEST_F(GZipUnitTest, DecodeWithSmallOutputBuffer) { // Tests we can still decode with just 1 byte buffer in the filter and just 1 // byte buffer in the caller. TEST_F(GZipUnitTest, DecodeWithOneByteInputAndOutputBuffer) { - std::vector<Filter::FilterType> filter_types; - filter_types.push_back(Filter::FILTER_TYPE_GZIP); - net::MockFilterContext filter_context(1); - scoped_ptr<Filter> filter(Filter::Factory(filter_types, filter_context)); - ASSERT_TRUE(filter.get()); - DecodeAndCompareWithFilter(filter.get(), source_buffer(), source_len(), + InitFilterWithBufferSize(Filter::FILTER_TYPE_GZIP, 1); + EXPECT_EQ(1, filter_->stream_buffer_size()); + DecodeAndCompareWithFilter(filter_.get(), source_buffer(), source_len(), gzip_encode_buffer_, gzip_encode_len_, 1); } @@ -337,15 +338,11 @@ TEST_F(GZipUnitTest, DecodeCorruptedData) { corrupt_data[pos] = !corrupt_data[pos]; // Decode the corrupted data with filter - std::vector<Filter::FilterType> filter_types; - filter_types.push_back(Filter::FILTER_TYPE_DEFLATE); - net::MockFilterContext filter_context(kDefaultBufferSize); - scoped_ptr<Filter> filter(Filter::Factory(filter_types, filter_context)); - ASSERT_TRUE(filter.get()); + InitFilter(Filter::FILTER_TYPE_DEFLATE); char corrupt_decode_buffer[kDefaultBufferSize]; int corrupt_decode_size = kDefaultBufferSize; - int code = DecodeAllWithFilter(filter.get(), corrupt_data, corrupt_data_len, + int code = DecodeAllWithFilter(filter_.get(), corrupt_data, corrupt_data_len, corrupt_decode_buffer, &corrupt_decode_size); // Expect failures @@ -364,15 +361,11 @@ TEST_F(GZipUnitTest, DecodeMissingData) { --corrupt_data_len; // Decode the corrupted data with filter - std::vector<Filter::FilterType> filter_types; - filter_types.push_back(Filter::FILTER_TYPE_DEFLATE); - net::MockFilterContext filter_context(kDefaultBufferSize); - scoped_ptr<Filter> filter(Filter::Factory(filter_types, filter_context)); - ASSERT_TRUE(filter.get()); + InitFilter(Filter::FILTER_TYPE_DEFLATE); char corrupt_decode_buffer[kDefaultBufferSize]; int corrupt_decode_size = kDefaultBufferSize; - int code = DecodeAllWithFilter(filter.get(), corrupt_data, corrupt_data_len, + int code = DecodeAllWithFilter(filter_.get(), corrupt_data, corrupt_data_len, corrupt_decode_buffer, &corrupt_decode_size); // Expect failures @@ -388,19 +381,13 @@ TEST_F(GZipUnitTest, DecodeCorruptedHeader) { corrupt_data[2] = !corrupt_data[2]; // Decode the corrupted data with filter - std::vector<Filter::FilterType> filter_types; - filter_types.push_back(Filter::FILTER_TYPE_GZIP); - net::MockFilterContext filter_context(kDefaultBufferSize); - scoped_ptr<Filter> filter(Filter::Factory(filter_types, filter_context)); - ASSERT_TRUE(filter.get()); + InitFilter(Filter::FILTER_TYPE_GZIP); char corrupt_decode_buffer[kDefaultBufferSize]; int corrupt_decode_size = kDefaultBufferSize; - int code = DecodeAllWithFilter(filter.get(), corrupt_data, corrupt_data_len, + int code = DecodeAllWithFilter(filter_.get(), corrupt_data, corrupt_data_len, corrupt_decode_buffer, &corrupt_decode_size); // Expect failures EXPECT_TRUE(code == Filter::FILTER_ERROR); } - -} // namespace diff --git a/net/base/mock_filter_context.cc b/net/base/mock_filter_context.cc index 6ca541a..7034589 100644 --- a/net/base/mock_filter_context.cc +++ b/net/base/mock_filter_context.cc @@ -6,9 +6,8 @@ namespace net { -MockFilterContext::MockFilterContext(int buffer_size) - : buffer_size_(buffer_size), - is_cached_content_(false), +MockFilterContext::MockFilterContext() + : is_cached_content_(false), is_download_(false), is_sdch_response_(false), response_code_(-1) { @@ -43,6 +42,4 @@ int64 MockFilterContext::GetByteReadCount() const { return 0; } int MockFilterContext::GetResponseCode() const { return response_code_; } -int MockFilterContext::GetInputStreamBufferSize() const { return buffer_size_; } - } // namespace net diff --git a/net/base/mock_filter_context.h b/net/base/mock_filter_context.h index 53c63ec..e04a551 100644 --- a/net/base/mock_filter_context.h +++ b/net/base/mock_filter_context.h @@ -15,10 +15,9 @@ namespace net { class MockFilterContext : public FilterContext { public: - explicit MockFilterContext(int buffer_size); + MockFilterContext(); virtual ~MockFilterContext(); - void SetBufferSize(int buffer_size) { buffer_size_ = buffer_size; } void SetMimeType(const std::string& mime_type) { mime_type_ = mime_type; } void SetURL(const GURL& gurl) { gurl_ = gurl; } void SetRequestTime(const base::Time time) { request_time_ = time; } @@ -52,9 +51,6 @@ class MockFilterContext : public FilterContext { virtual int GetResponseCode() const; - // What is the desirable input buffer size for these filters? - virtual int GetInputStreamBufferSize() const; - virtual void RecordPacketStats(StatisticSelector statistic) const {} private: diff --git a/net/base/sdch_filter.h b/net/base/sdch_filter.h index 9eac007..451e918 100644 --- a/net/base/sdch_filter.h +++ b/net/base/sdch_filter.h @@ -29,8 +29,6 @@ namespace net { class SdchFilter : public Filter { public: - explicit SdchFilter(const FilterContext& filter_context); - virtual ~SdchFilter(); // Initializes filter decoding mode and internal control blocks. @@ -55,6 +53,10 @@ class SdchFilter : public Filter { PASS_THROUGH, // Non-sdch content being passed without alteration. }; + // Only to be instantiated by Filter::Factory. + explicit SdchFilter(const FilterContext& filter_context); + friend class Filter; + // Identify the suggested dictionary, and initialize underlying decompressor. Filter::FilterStatus InitializeDictionary(); diff --git a/net/base/sdch_filter_unittest.cc b/net/base/sdch_filter_unittest.cc index 508365a..44b7f62 100644 --- a/net/base/sdch_filter_unittest.cc +++ b/net/base/sdch_filter_unittest.cc @@ -158,9 +158,8 @@ static std::string NewSdchDictionary(const std::string& domain) { TEST_F(SdchFilterTest, EmptyInputOk) { std::vector<Filter::FilterType> filter_types; filter_types.push_back(Filter::FILTER_TYPE_SDCH); - const int kInputBufferSize(30); char output_buffer[20]; - MockFilterContext filter_context(kInputBufferSize); + MockFilterContext filter_context; std::string url_string("http://ignore.com"); filter_context.SetURL(GURL(url_string)); scoped_ptr<Filter> filter(Filter::Factory(filter_types, filter_context)); @@ -179,9 +178,8 @@ TEST_F(SdchFilterTest, PassThroughWhenTentative) { std::vector<Filter::FilterType> filter_types; // Selective a tentative filter (which can fall back to pass through). filter_types.push_back(Filter::FILTER_TYPE_GZIP_HELPING_SDCH); - const int kInputBufferSize(30); char output_buffer[20]; - MockFilterContext filter_context(kInputBufferSize); + MockFilterContext filter_context; // Response code needs to be 200 to allow a pass through. filter_context.SetResponseCode(200); std::string url_string("http://ignore.com"); @@ -193,7 +191,6 @@ TEST_F(SdchFilterTest, PassThroughWhenTentative) { char* input_buffer = filter->stream_buffer()->data(); int input_buffer_size = filter->stream_buffer_size(); - EXPECT_EQ(kInputBufferSize, input_buffer_size); EXPECT_LT(static_cast<int>(non_gzip_content.size()), input_buffer_size); @@ -219,9 +216,8 @@ TEST_F(SdchFilterTest, RefreshBadReturnCode) { std::vector<Filter::FilterType> filter_types; // Selective a tentative filter (which can fall back to pass through). filter_types.push_back(Filter::FILTER_TYPE_SDCH_POSSIBLE); - const int kInputBufferSize(30); char output_buffer[20]; - MockFilterContext filter_context(kInputBufferSize); + MockFilterContext filter_context; // Response code needs to be 200 to allow a pass through. filter_context.SetResponseCode(403); // Meta refresh will only appear for html content @@ -236,7 +232,6 @@ TEST_F(SdchFilterTest, RefreshBadReturnCode) { char* input_buffer = filter->stream_buffer()->data(); int input_buffer_size = filter->stream_buffer_size(); - EXPECT_EQ(kInputBufferSize, input_buffer_size); EXPECT_LT(static_cast<int>(non_sdch_content.size()), input_buffer_size); @@ -262,9 +257,8 @@ TEST_F(SdchFilterTest, ErrorOnBadReturnCode) { std::vector<Filter::FilterType> filter_types; // Selective a tentative filter (which can fall back to pass through). filter_types.push_back(Filter::FILTER_TYPE_SDCH_POSSIBLE); - const int kInputBufferSize(30); char output_buffer[20]; - MockFilterContext filter_context(kInputBufferSize); + MockFilterContext filter_context; // Response code needs to be 200 to allow a pass through. filter_context.SetResponseCode(403); // Meta refresh will only appear for html content, so set to something else @@ -280,7 +274,6 @@ TEST_F(SdchFilterTest, ErrorOnBadReturnCode) { char* input_buffer = filter->stream_buffer()->data(); int input_buffer_size = filter->stream_buffer_size(); - EXPECT_EQ(kInputBufferSize, input_buffer_size); EXPECT_LT(static_cast<int>(non_sdch_content.size()), input_buffer_size); @@ -301,9 +294,8 @@ TEST_F(SdchFilterTest, ErrorOnBadReturnCodeWithHtml) { std::vector<Filter::FilterType> filter_types; // Selective a tentative filter (which can fall back to pass through). filter_types.push_back(Filter::FILTER_TYPE_SDCH_POSSIBLE); - const int kInputBufferSize(30); char output_buffer[20]; - MockFilterContext filter_context(kInputBufferSize); + MockFilterContext filter_context; // Response code needs to be 200 to allow a pass through. filter_context.SetResponseCode(403); // Meta refresh will only appear for html content @@ -318,7 +310,6 @@ TEST_F(SdchFilterTest, ErrorOnBadReturnCodeWithHtml) { char* input_buffer = filter->stream_buffer()->data(); int input_buffer_size = filter->stream_buffer_size(); - EXPECT_EQ(kInputBufferSize, input_buffer_size); EXPECT_LT(static_cast<int>(non_sdch_content.size()), input_buffer_size); @@ -344,9 +335,8 @@ TEST_F(SdchFilterTest, ErrorOnBadReturnCodeWithHtml) { TEST_F(SdchFilterTest, BasicBadDictionary) { std::vector<Filter::FilterType> filter_types; filter_types.push_back(Filter::FILTER_TYPE_SDCH); - const int kInputBufferSize(30); char output_buffer[20]; - MockFilterContext filter_context(kInputBufferSize); + MockFilterContext filter_context; std::string url_string("http://ignore.com"); filter_context.SetURL(GURL(url_string)); scoped_ptr<Filter> filter(Filter::Factory(filter_types, filter_context)); @@ -357,7 +347,6 @@ TEST_F(SdchFilterTest, BasicBadDictionary) { char* input_buffer = filter->stream_buffer()->data(); int input_buffer_size = filter->stream_buffer_size(); - EXPECT_EQ(kInputBufferSize, input_buffer_size); EXPECT_LT(static_cast<int>(dictionary_hash_prefix.size()), input_buffer_size); @@ -432,9 +421,7 @@ TEST_F(SdchFilterTest, BasicDictionary) { std::vector<Filter::FilterType> filter_types; filter_types.push_back(Filter::FILTER_TYPE_SDCH); - // Decode with a large buffer (larger than test input, or compressed data). - const int kInputBufferSize(100); - MockFilterContext filter_context(kInputBufferSize); + MockFilterContext filter_context; filter_context.SetURL(url); scoped_ptr<Filter> filter(Filter::Factory(filter_types, filter_context)); @@ -447,7 +434,7 @@ TEST_F(SdchFilterTest, BasicDictionary) { EXPECT_EQ(output, expanded_); // Decode with really small buffers (size 1) to check for edge effects. - filter.reset((Filter::Factory(filter_types, filter_context))); + filter.reset(Filter::Factory(filter_types, filter_context)); feed_block_size = 1; output_block_size = 1; @@ -472,8 +459,7 @@ TEST_F(SdchFilterTest, NoDecodeHttps) { std::vector<Filter::FilterType> filter_types; filter_types.push_back(Filter::FILTER_TYPE_SDCH); - const int kInputBufferSize(100); - MockFilterContext filter_context(kInputBufferSize); + MockFilterContext filter_context; filter_context.SetURL(GURL("https://" + kSampleDomain)); scoped_ptr<Filter> filter(Filter::Factory(filter_types, filter_context)); @@ -504,8 +490,7 @@ TEST_F(SdchFilterTest, NoDecodeFtp) { std::vector<Filter::FilterType> filter_types; filter_types.push_back(Filter::FILTER_TYPE_SDCH); - const int kInputBufferSize(100); - MockFilterContext filter_context(kInputBufferSize); + MockFilterContext filter_context; filter_context.SetURL(GURL("ftp://" + kSampleDomain)); scoped_ptr<Filter> filter(Filter::Factory(filter_types, filter_context)); @@ -532,8 +517,7 @@ TEST_F(SdchFilterTest, NoDecodeFileColon) { std::vector<Filter::FilterType> filter_types; filter_types.push_back(Filter::FILTER_TYPE_SDCH); - const int kInputBufferSize(100); - MockFilterContext filter_context(kInputBufferSize); + MockFilterContext filter_context; filter_context.SetURL(GURL("file://" + kSampleDomain)); scoped_ptr<Filter> filter(Filter::Factory(filter_types, filter_context)); @@ -560,8 +544,7 @@ TEST_F(SdchFilterTest, NoDecodeAboutColon) { std::vector<Filter::FilterType> filter_types; filter_types.push_back(Filter::FILTER_TYPE_SDCH); - const int kInputBufferSize(100); - MockFilterContext filter_context(kInputBufferSize); + MockFilterContext filter_context; filter_context.SetURL(GURL("about://" + kSampleDomain)); scoped_ptr<Filter> filter(Filter::Factory(filter_types, filter_context)); @@ -588,8 +571,7 @@ TEST_F(SdchFilterTest, NoDecodeJavaScript) { std::vector<Filter::FilterType> filter_types; filter_types.push_back(Filter::FILTER_TYPE_SDCH); - const int kInputBufferSize(100); - MockFilterContext filter_context(kInputBufferSize); + MockFilterContext filter_context; filter_context.SetURL(GURL("javascript://" + kSampleDomain)); scoped_ptr<Filter> filter(Filter::Factory(filter_types, filter_context)); @@ -616,8 +598,7 @@ TEST_F(SdchFilterTest, CanStillDecodeHttp) { std::vector<Filter::FilterType> filter_types; filter_types.push_back(Filter::FILTER_TYPE_SDCH); - const int kInputBufferSize(100); - MockFilterContext filter_context(kInputBufferSize); + MockFilterContext filter_context; filter_context.SetURL(GURL("http://" + kSampleDomain)); scoped_ptr<Filter> filter(Filter::Factory(filter_types, filter_context)); @@ -643,14 +624,13 @@ TEST_F(SdchFilterTest, CrossDomainDictionaryUse) { std::vector<Filter::FilterType> filter_types; filter_types.push_back(Filter::FILTER_TYPE_SDCH); - const int kInputBufferSize(100); // Decode with content arriving from the "wrong" domain. // This tests SdchManager::CanSet(). - MockFilterContext filter_context(kInputBufferSize); + MockFilterContext filter_context; GURL wrong_domain_url("http://www.wrongdomain.com"); filter_context.SetURL(wrong_domain_url); - scoped_ptr<Filter> filter((Filter::Factory(filter_types, filter_context))); + scoped_ptr<Filter> filter(Filter::Factory(filter_types, filter_context)); size_t feed_block_size = 100; size_t output_block_size = 100; @@ -685,12 +665,11 @@ TEST_F(SdchFilterTest, DictionaryPathValidation) { std::vector<Filter::FilterType> filter_types; filter_types.push_back(Filter::FILTER_TYPE_SDCH); - const int kInputBufferSize(100); // Test decode the path data, arriving from a valid path. - MockFilterContext filter_context(kInputBufferSize); + MockFilterContext filter_context; filter_context.SetURL(GURL(url_string + path)); - scoped_ptr<Filter> filter((Filter::Factory(filter_types, filter_context))); + scoped_ptr<Filter> filter(Filter::Factory(filter_types, filter_context)); size_t feed_block_size = 100; size_t output_block_size = 100; @@ -702,7 +681,7 @@ TEST_F(SdchFilterTest, DictionaryPathValidation) { // Test decode the path data, arriving from a invalid path. filter_context.SetURL(GURL(url_string)); - filter.reset((Filter::Factory(filter_types, filter_context))); + filter.reset(Filter::Factory(filter_types, filter_context)); feed_block_size = 100; output_block_size = 100; @@ -739,12 +718,11 @@ TEST_F(SdchFilterTest, DictionaryPortValidation) { std::vector<Filter::FilterType> filter_types; filter_types.push_back(Filter::FILTER_TYPE_SDCH); - const int kInputBufferSize(100); // Test decode the port data, arriving from a valid port. - MockFilterContext filter_context(kInputBufferSize); + MockFilterContext filter_context; filter_context.SetURL(GURL(url_string + ":" + port)); - scoped_ptr<Filter> filter((Filter::Factory(filter_types, filter_context))); + scoped_ptr<Filter> filter(Filter::Factory(filter_types, filter_context)); size_t feed_block_size = 100; size_t output_block_size = 100; @@ -755,7 +733,7 @@ TEST_F(SdchFilterTest, DictionaryPortValidation) { // Test decode the port data, arriving from a valid (default) port. filter_context.SetURL(GURL(url_string)); // Default port. - filter.reset((Filter::Factory(filter_types, filter_context))); + filter.reset(Filter::Factory(filter_types, filter_context)); feed_block_size = 100; output_block_size = 100; @@ -766,7 +744,7 @@ TEST_F(SdchFilterTest, DictionaryPortValidation) { // Test decode the port data, arriving from a invalid port. filter_context.SetURL(GURL(url_string + ":" + port + "1")); - filter.reset((Filter::Factory(filter_types, filter_context))); + filter.reset(Filter::Factory(filter_types, filter_context)); feed_block_size = 100; output_block_size = 100; @@ -836,6 +814,14 @@ static std::string gzip_compress(const std::string &input) { //------------------------------------------------------------------------------ +class SdchFilterChainingTest { + public: + static Filter* Factory(const std::vector<Filter::FilterType>& types, + const FilterContext& context, int size) { + return Filter::FactoryForTests(types, context, size); + } +}; + // Test that filters can be cascaded (chained) so that the output of one filter // is processed by the next one. This is most critical for SDCH, which is // routinely followed by gzip (during encoding). The filter we'll test for will @@ -865,9 +851,13 @@ TEST_F(SdchFilterTest, FilterChaining) { CHECK_GT(kLargeInputBufferSize, gzip_compressed_sdch.size()); CHECK_GT(kLargeInputBufferSize, sdch_compressed.size()); CHECK_GT(kLargeInputBufferSize, expanded_.size()); - MockFilterContext filter_context(kLargeInputBufferSize); + MockFilterContext filter_context; filter_context.SetURL(url); - scoped_ptr<Filter> filter(Filter::Factory(filter_types, filter_context)); + scoped_ptr<Filter> filter( + SdchFilterChainingTest::Factory(filter_types, filter_context, + kLargeInputBufferSize)); + EXPECT_EQ(static_cast<int>(kLargeInputBufferSize), + filter->stream_buffer_size()); // Verify that chained filter is waiting for data. char tiny_output_buffer[10]; @@ -891,9 +881,12 @@ TEST_F(SdchFilterTest, FilterChaining) { // We'll go even further, and force the chain to flush the buffer between the // two filters more than once (that is why we multiply by 2). CHECK_LT(kMidSizedInputBufferSize * 2, sdch_compressed.size()); - filter_context.SetBufferSize(kMidSizedInputBufferSize); filter_context.SetURL(url); - filter.reset(Filter::Factory(filter_types, filter_context)); + filter.reset( + SdchFilterChainingTest::Factory(filter_types, filter_context, + kMidSizedInputBufferSize)); + EXPECT_EQ(static_cast<int>(kMidSizedInputBufferSize), + filter->stream_buffer_size()); feed_block_size = kMidSizedInputBufferSize; output_block_size = kMidSizedInputBufferSize; @@ -903,8 +896,10 @@ TEST_F(SdchFilterTest, FilterChaining) { EXPECT_EQ(output, expanded_); // Next try with a tiny input and output buffer to cover edge effects. - filter_context.SetBufferSize(kLargeInputBufferSize); - filter.reset(Filter::Factory(filter_types, filter_context)); + filter.reset(SdchFilterChainingTest::Factory(filter_types, filter_context, + kLargeInputBufferSize)); + EXPECT_EQ(static_cast<int>(kLargeInputBufferSize), + filter->stream_buffer_size()); feed_block_size = 1; output_block_size = 1; @@ -934,8 +929,7 @@ TEST_F(SdchFilterTest, DefaultGzipIfSdch) { std::vector<Filter::FilterType> filter_types; filter_types.push_back(Filter::FILTER_TYPE_SDCH); - const int kInputBufferSize(100); - MockFilterContext filter_context(kInputBufferSize); + MockFilterContext filter_context; filter_context.SetMimeType("anything/mime"); filter_context.SetSdchResponse(true); Filter::FixupEncodingTypes(filter_context, &filter_types); @@ -994,8 +988,7 @@ TEST_F(SdchFilterTest, AcceptGzipSdchIfGzip) { std::vector<Filter::FilterType> filter_types; filter_types.push_back(Filter::FILTER_TYPE_GZIP); - const int kInputBufferSize(100); - MockFilterContext filter_context(kInputBufferSize); + MockFilterContext filter_context; filter_context.SetMimeType("anything/mime"); filter_context.SetSdchResponse(true); Filter::FixupEncodingTypes(filter_context, &filter_types); @@ -1053,8 +1046,7 @@ TEST_F(SdchFilterTest, DefaultSdchGzipIfEmpty) { // System should automatically add the missing (optional) sdch,gzip. std::vector<Filter::FilterType> filter_types; - const int kInputBufferSize(100); - MockFilterContext filter_context(kInputBufferSize); + MockFilterContext filter_context; filter_context.SetMimeType("anything/mime"); filter_context.SetSdchResponse(true); Filter::FixupEncodingTypes(filter_context, &filter_types); @@ -1116,8 +1108,7 @@ TEST_F(SdchFilterTest, AcceptGzipGzipSdchIfGzip) { std::vector<Filter::FilterType> filter_types; filter_types.push_back(Filter::FILTER_TYPE_GZIP); - const int kInputBufferSize(100); - MockFilterContext filter_context(kInputBufferSize); + MockFilterContext filter_context; filter_context.SetMimeType("anything/mime"); filter_context.SetSdchResponse(true); Filter::FixupEncodingTypes(filter_context, &filter_types); diff --git a/net/url_request/url_request_job.cc b/net/url_request/url_request_job.cc index c13b8f5..ed01f0e 100644 --- a/net/url_request/url_request_job.cc +++ b/net/url_request/url_request_job.cc @@ -26,10 +26,6 @@ using base::TimeTicks; namespace net { -// Buffer size allocated when de-compressing data. -// static -const int URLRequestJob::kFilterBufSize = 32 * 1024; - URLRequestJob::URLRequestJob(URLRequest* request) : request_(request), prefilter_bytes_read_(0), @@ -263,10 +259,6 @@ int URLRequestJob::GetResponseCode() const { return -1; } -int URLRequestJob::GetInputStreamBufferSize() const { - return kFilterBufSize; -} - void URLRequestJob::RecordPacketStats(StatisticSelector statistic) const { if (!packet_timing_enabled_ || (final_packet_time_ == base::Time())) return; diff --git a/net/url_request/url_request_job.h b/net/url_request/url_request_job.h index b6c286a..3d29201 100644 --- a/net/url_request/url_request_job.h +++ b/net/url_request/url_request_job.h @@ -207,7 +207,6 @@ class URLRequestJob : public base::RefCounted<URLRequestJob>, virtual bool IsCachedContent() const; virtual int64 GetByteReadCount() const; virtual int GetResponseCode() const; - virtual int GetInputStreamBufferSize() const; virtual void RecordPacketStats(StatisticSelector statistic) const; // Returns the socket address for the connection. @@ -304,9 +303,6 @@ class URLRequestJob : public base::RefCounted<URLRequestJob>, bool is_compressed_; private: - // Size of filter input buffers used by this class. - static const int kFilterBufSize; - // When data filtering is enabled, this function is used to read data // for the filter. Returns true if raw data was read. Returns false if // an error occurred (or we are waiting for IO to complete). |