diff options
Diffstat (limited to 'net')
-rw-r--r-- | net/http/http_cache_transaction.cc | 9 | ||||
-rw-r--r-- | net/http/http_cache_transaction.h | 1 | ||||
-rw-r--r-- | net/http/http_cache_unittest.cc | 34 | ||||
-rw-r--r-- | net/http/http_network_transaction.h | 1 | ||||
-rw-r--r-- | net/http/http_transaction.h | 6 | ||||
-rw-r--r-- | net/http/http_transaction_unittest.cc | 20 | ||||
-rw-r--r-- | net/http/http_transaction_unittest.h | 13 | ||||
-rw-r--r-- | net/net.gyp | 1 | ||||
-rw-r--r-- | net/url_request/url_request_http_job.cc | 6 | ||||
-rw-r--r-- | net/url_request/url_request_http_job.h | 47 | ||||
-rw-r--r-- | net/url_request/url_request_job.cc | 20 | ||||
-rw-r--r-- | net/url_request/url_request_job.h | 4 | ||||
-rw-r--r-- | net/url_request/url_request_job_unittest.cc | 81 |
13 files changed, 211 insertions, 32 deletions
diff --git a/net/http/http_cache_transaction.cc b/net/http/http_cache_transaction.cc index 1e83f06..4b51cd5 100644 --- a/net/http/http_cache_transaction.cc +++ b/net/http/http_cache_transaction.cc @@ -365,6 +365,15 @@ int HttpCache::Transaction::Read(IOBuffer* buf, int buf_len, void HttpCache::Transaction::StopCaching() { } +void HttpCache::Transaction::DoneReading() { + if (cache_ && entry_) { + DCHECK(reading_); + DCHECK_NE(mode_, UPDATE); + if (mode_ & WRITE) + DoneWritingToEntry(true); + } +} + const HttpResponseInfo* HttpCache::Transaction::GetResponseInfo() const { // Null headers means we encountered an error or haven't a response yet if (auth_response_.headers) diff --git a/net/http/http_cache_transaction.h b/net/http/http_cache_transaction.h index 31dec94..ce6e79e 100644 --- a/net/http/http_cache_transaction.h +++ b/net/http/http_cache_transaction.h @@ -109,6 +109,7 @@ class HttpCache::Transaction : public HttpTransaction { virtual bool IsReadyToRestartForAuth(); virtual int Read(IOBuffer* buf, int buf_len, CompletionCallback* callback); virtual void StopCaching(); + virtual void DoneReading(); virtual const HttpResponseInfo* GetResponseInfo() const; virtual LoadState GetLoadState() const; virtual uint64 GetUploadProgress(void) const; diff --git a/net/http/http_cache_unittest.cc b/net/http/http_cache_unittest.cc index b467dc5..9fbd49e 100644 --- a/net/http/http_cache_unittest.cc +++ b/net/http/http_cache_unittest.cc @@ -5101,3 +5101,37 @@ TEST(HttpCache, ReadMetadata) { EXPECT_EQ(4, cache.disk_cache()->open_count()); EXPECT_EQ(1, cache.disk_cache()->create_count()); } + +// Tests that we don't mark entries as truncated when a filter detects the end +// of the stream. +TEST(HttpCache, FilterCompletion) { + MockHttpCache cache; + TestCompletionCallback callback; + + { + scoped_ptr<net::HttpTransaction> trans; + int rv = cache.http_cache()->CreateTransaction(&trans); + EXPECT_EQ(net::OK, rv); + + MockHttpRequest request(kSimpleGET_Transaction); + rv = trans->Start(&request, &callback, net::BoundNetLog()); + EXPECT_EQ(net::OK, callback.GetResult(rv)); + + scoped_refptr<net::IOBuffer> buf(new net::IOBuffer(256)); + rv = trans->Read(buf, 256, &callback); + EXPECT_GT(callback.GetResult(rv), 0); + + // Now make sure that the entry is preserved. + trans->DoneReading(); + } + + // Make sure that teh ActiveEntry is gone. + MessageLoop::current()->RunAllPending(); + + // Read from the cache. + RunTransactionTest(cache.http_cache(), kSimpleGET_Transaction); + + EXPECT_EQ(1, cache.network_layer()->transaction_count()); + EXPECT_EQ(1, cache.disk_cache()->open_count()); + EXPECT_EQ(1, cache.disk_cache()->create_count()); +} diff --git a/net/http/http_network_transaction.h b/net/http/http_network_transaction.h index 6f6ba77..639d17d 100644 --- a/net/http/http_network_transaction.h +++ b/net/http/http_network_transaction.h @@ -54,6 +54,7 @@ class NET_EXPORT_PRIVATE HttpNetworkTransaction virtual int Read(IOBuffer* buf, int buf_len, CompletionCallback* callback); virtual void StopCaching() {} + virtual void DoneReading() {} virtual const HttpResponseInfo* GetResponseInfo() const; virtual LoadState GetLoadState() const; virtual uint64 GetUploadProgress() const; diff --git a/net/http/http_transaction.h b/net/http/http_transaction.h index 400c91f..3e702ea 100644 --- a/net/http/http_transaction.h +++ b/net/http/http_transaction.h @@ -96,6 +96,12 @@ class NET_EXPORT_PRIVATE HttpTransaction { // Stops further caching of this request by the HTTP cache, if there is any. virtual void StopCaching() = 0; + // Called to tell the transaction that we have successfully reached the end + // of the stream. This is equivalent to performing an extra Read() at the end + // that should return 0 bytes. This method should not be called if the + // transaction is busy processing a previous operation (like a pending Read). + virtual void DoneReading() = 0; + // Returns the response info for this transaction or NULL if the response // info is not available. virtual const HttpResponseInfo* GetResponseInfo() const = 0; diff --git a/net/http/http_transaction_unittest.cc b/net/http/http_transaction_unittest.cc index cec930d..38347e0 100644 --- a/net/http/http_transaction_unittest.cc +++ b/net/http/http_transaction_unittest.cc @@ -212,8 +212,10 @@ void TestTransactionConsumer::RunWithParams(const Tuple1<int>& params) { } -MockNetworkTransaction::MockNetworkTransaction() : - ALLOW_THIS_IN_INITIALIZER_LIST(task_factory_(this)), data_cursor_(0) { +MockNetworkTransaction::MockNetworkTransaction(MockNetworkLayer* factory) + : ALLOW_THIS_IN_INITIALIZER_LIST(task_factory_(this)), + data_cursor_(0), + transaction_factory_(factory->AsWeakPtr()) { } MockNetworkTransaction::~MockNetworkTransaction() {} @@ -295,6 +297,11 @@ int MockNetworkTransaction::Read(net::IOBuffer* buf, int buf_len, void MockNetworkTransaction::StopCaching() {} +void MockNetworkTransaction::DoneReading() { + if (transaction_factory_) + transaction_factory_->TransactionDoneReading(); +} + const net::HttpResponseInfo* MockNetworkTransaction::GetResponseInfo() const { return &response_; } @@ -320,14 +327,19 @@ void MockNetworkTransaction::RunCallback(net::CompletionCallback* callback, callback->Run(result); } -MockNetworkLayer::MockNetworkLayer() : transaction_count_(0) {} +MockNetworkLayer::MockNetworkLayer() + : transaction_count_(0), done_reading_called_(false) {} MockNetworkLayer::~MockNetworkLayer() {} +void MockNetworkLayer::TransactionDoneReading() { + done_reading_called_ = true; +} + int MockNetworkLayer::CreateTransaction( scoped_ptr<net::HttpTransaction>* trans) { transaction_count_++; - trans->reset(new MockNetworkTransaction()); + trans->reset(new MockNetworkTransaction(this)); return net::OK; } diff --git a/net/http/http_transaction_unittest.h b/net/http/http_transaction_unittest.h index 589133c..714f263 100644 --- a/net/http/http_transaction_unittest.h +++ b/net/http/http_transaction_unittest.h @@ -147,13 +147,15 @@ class TestTransactionConsumer : public CallbackRunner< Tuple1<int> > { //----------------------------------------------------------------------------- // mock network layer +class MockNetworkLayer; + // This transaction class inspects the available set of mock transactions to // find data for the request URL. It supports IO operations that complete // synchronously or asynchronously to help exercise different code paths in the // HttpCache implementation. class MockNetworkTransaction : public net::HttpTransaction { public: - MockNetworkTransaction(); + explicit MockNetworkTransaction(MockNetworkLayer* factory); virtual ~MockNetworkTransaction(); virtual int Start(const net::HttpRequestInfo* request, @@ -176,6 +178,8 @@ class MockNetworkTransaction : public net::HttpTransaction { virtual void StopCaching(); + virtual void DoneReading(); + virtual const net::HttpResponseInfo* GetResponseInfo() const; virtual net::LoadState GetLoadState() const; @@ -191,14 +195,18 @@ class MockNetworkTransaction : public net::HttpTransaction { std::string data_; int data_cursor_; int test_mode_; + base::WeakPtr<MockNetworkLayer> transaction_factory_; }; -class MockNetworkLayer : public net::HttpTransactionFactory { +class MockNetworkLayer : public net::HttpTransactionFactory, + public base::SupportsWeakPtr<MockNetworkLayer> { public: MockNetworkLayer(); virtual ~MockNetworkLayer(); int transaction_count() const { return transaction_count_; } + bool done_reading_called() const { return done_reading_called_; } + void TransactionDoneReading(); // net::HttpTransactionFactory: virtual int CreateTransaction(scoped_ptr<net::HttpTransaction>* trans); @@ -207,6 +215,7 @@ class MockNetworkLayer : public net::HttpTransactionFactory { private: int transaction_count_; + bool done_reading_called_; }; //----------------------------------------------------------------------------- diff --git a/net/net.gyp b/net/net.gyp index bdc568d..4b2259e 100644 --- a/net/net.gyp +++ b/net/net.gyp @@ -1030,6 +1030,7 @@ 'tools/dump_cache/url_utilities_unittest.cc', 'udp/udp_socket_unittest.cc', 'url_request/url_request_job_factory_unittest.cc', + 'url_request/url_request_job_unittest.cc', 'url_request/url_request_throttler_simulation_unittest.cc', 'url_request/url_request_throttler_test_support.cc', 'url_request/url_request_throttler_test_support.h', diff --git a/net/url_request/url_request_http_job.cc b/net/url_request/url_request_http_job.cc index 24ab078..b852622 100644 --- a/net/url_request/url_request_http_job.cc +++ b/net/url_request/url_request_http_job.cc @@ -1201,6 +1201,12 @@ void URLRequestHttpJob::StopCaching() { transaction_->StopCaching(); } +void URLRequestHttpJob::DoneReading() { + if (transaction_.get()) + transaction_->DoneReading(); + DoneWithRequest(FINISHED); +} + HostPortPair URLRequestHttpJob::GetSocketAddress() const { return response_info_ ? response_info_->socket_address : HostPortPair(); } diff --git a/net/url_request/url_request_http_job.h b/net/url_request/url_request_http_job.h index a148553..1957dfd 100644 --- a/net/url_request/url_request_http_job.h +++ b/net/url_request/url_request_http_job.h @@ -65,29 +65,32 @@ class URLRequestHttpJob : public URLRequestJob { const string16& password); // Overridden from URLRequestJob: - virtual void SetUpload(UploadData* upload); - virtual void SetExtraRequestHeaders(const HttpRequestHeaders& headers); - virtual void Start(); - virtual void Kill(); - virtual LoadState GetLoadState() const; - virtual uint64 GetUploadProgress() const; - virtual bool GetMimeType(std::string* mime_type) const; - virtual bool GetCharset(std::string* charset); - virtual void GetResponseInfo(HttpResponseInfo* info); - virtual bool GetResponseCookies(std::vector<std::string>* cookies); - virtual int GetResponseCode() const; - virtual Filter* SetupFilter() const; - virtual bool IsSafeRedirect(const GURL& location); - virtual bool NeedsAuth(); - virtual void GetAuthChallengeInfo(scoped_refptr<AuthChallengeInfo>*); + virtual void SetUpload(UploadData* upload) OVERRIDE; + virtual void SetExtraRequestHeaders( + const HttpRequestHeaders& headers) OVERRIDE; + virtual void Start() OVERRIDE; + virtual void Kill() OVERRIDE; + virtual LoadState GetLoadState() const OVERRIDE; + virtual uint64 GetUploadProgress() const OVERRIDE; + virtual bool GetMimeType(std::string* mime_type) const OVERRIDE; + virtual bool GetCharset(std::string* charset) OVERRIDE; + virtual void GetResponseInfo(HttpResponseInfo* info) OVERRIDE; + virtual bool GetResponseCookies(std::vector<std::string>* cookies) OVERRIDE; + virtual int GetResponseCode() const OVERRIDE; + virtual Filter* SetupFilter() const OVERRIDE; + virtual bool IsSafeRedirect(const GURL& location) OVERRIDE; + virtual bool NeedsAuth() OVERRIDE; + virtual void GetAuthChallengeInfo(scoped_refptr<AuthChallengeInfo>*) OVERRIDE; virtual void SetAuth(const string16& username, - const string16& password); - virtual void CancelAuth(); - virtual void ContinueWithCertificate(X509Certificate* client_cert); - virtual void ContinueDespiteLastError(); - virtual bool ReadRawData(IOBuffer* buf, int buf_size, int *bytes_read); - virtual void StopCaching(); - virtual HostPortPair GetSocketAddress() const; + const string16& password) OVERRIDE; + virtual void CancelAuth() OVERRIDE; + virtual void ContinueWithCertificate(X509Certificate* client_cert) OVERRIDE; + virtual void ContinueDespiteLastError() OVERRIDE; + virtual bool ReadRawData(IOBuffer* buf, int buf_size, + int *bytes_read) OVERRIDE; + virtual void StopCaching() OVERRIDE; + virtual void DoneReading() OVERRIDE; + virtual HostPortPair GetSocketAddress() const OVERRIDE; // Keep a reference to the url request context to be sure it's not deleted // before us. diff --git a/net/url_request/url_request_job.cc b/net/url_request/url_request_job.cc index 9ea5347..42c073d 100644 --- a/net/url_request/url_request_job.cc +++ b/net/url_request/url_request_job.cc @@ -61,7 +61,7 @@ void URLRequestJob::DetachRequest() { bool URLRequestJob::Read(IOBuffer* buf, int buf_size, int *bytes_read) { bool rv = false; - DCHECK_LT(buf_size, 1000000); // sanity check + DCHECK_LT(buf_size, 1000000); // Sanity check. DCHECK(buf); DCHECK(bytes_read); DCHECK(filtered_read_buffer_ == NULL); @@ -69,7 +69,7 @@ bool URLRequestJob::Read(IOBuffer* buf, int buf_size, int *bytes_read) { *bytes_read = 0; - // Skip Filter if not present + // Skip Filter if not present. if (!filter_.get()) { rv = ReadRawDataHelper(buf, buf_size, bytes_read); } else { @@ -79,9 +79,15 @@ bool URLRequestJob::Read(IOBuffer* buf, int buf_size, int *bytes_read) { filtered_read_buffer_len_ = buf_size; if (ReadFilteredData(bytes_read)) { - rv = true; // we have data to return + rv = true; // We have data to return. + + // It is fine to call DoneReading even if ReadFilteredData receives 0 + // bytes from the net, but we avoid making that call if we know for + // sure that's the case (ReadRawDataHelper path). + if (*bytes_read == 0) + DoneReading(); } else { - rv = false; // error, or a new IO is pending + rv = false; // Error, or a new IO is pending. } } if (rv && *bytes_read == 0) @@ -358,6 +364,8 @@ void URLRequestJob::NotifyReadComplete(int bytes_read) { // Filter the data. int filter_bytes_read = 0; if (ReadFilteredData(&filter_bytes_read)) { + if (!filter_bytes_read) + DoneReading(); request_->NotifyReadCompleted(filter_bytes_read); } } else { @@ -454,6 +462,10 @@ bool URLRequestJob::ReadRawData(IOBuffer* buf, int buf_size, return true; } +void URLRequestJob::DoneReading() { + // Do nothing. +} + void URLRequestJob::FilteredDataRead(int bytes_read) { DCHECK(filter_.get()); // don't add data if there is no filter filter_->FlushStreamBuffer(bytes_read); diff --git a/net/url_request/url_request_job.h b/net/url_request/url_request_job.h index afb5484..01547cc 100644 --- a/net/url_request/url_request_job.h +++ b/net/url_request/url_request_job.h @@ -251,6 +251,10 @@ class NET_EXPORT URLRequestJob : public base::RefCounted<URLRequestJob>, // info. virtual bool ReadRawData(IOBuffer* buf, int buf_size, int *bytes_read); + // Called to tell the job that a filter has successfully reached the end of + // the stream. + virtual void DoneReading(); + // Informs the filter that data has been read into its buffer void FilteredDataRead(int bytes_read); diff --git a/net/url_request/url_request_job_unittest.cc b/net/url_request/url_request_job_unittest.cc new file mode 100644 index 0000000..017b98b --- /dev/null +++ b/net/url_request/url_request_job_unittest.cc @@ -0,0 +1,81 @@ +// Copyright (c) 2011 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 "net/url_request/url_request_job.h" + +#include "net/http/http_transaction_unittest.h" +#include "net/url_request/url_request_test_util.h" +#include "testing/gtest/include/gtest/gtest.h" + +namespace { + +// This is a header that signals the end of the data. +const char kGzipGata[] = "\x1f\x08b\x08\0\0\0\0\0\0\3\3\0\0\0\0\0\0\0\0"; + +void GZipServer(const net::HttpRequestInfo* request, + std::string* response_status, std::string* response_headers, + std::string* response_data) { + response_data->assign(kGzipGata, sizeof(kGzipGata)); +} + +const MockTransaction kGZip_Transaction = { + "http://www.google.com/gzyp", + "GET", + base::Time(), + "", + net::LOAD_NORMAL, + "HTTP/1.1 200 OK", + "Cache-Control: max-age=10000\n" + "Content-Encoding: gzip\n" + "Content-Length: 30\n", // Intentionally wrong. + base::Time(), + "", + TEST_MODE_NORMAL, + &GZipServer, + 0 +}; + +} // namespace + +TEST(URLRequestJob, TransactionNotifiedWhenDone) { + TestDelegate d; + TestURLRequest req(GURL(kGZip_Transaction.url), &d); + MockNetworkLayer network_layer; + + AddMockTransaction(&kGZip_Transaction); + + scoped_refptr<TestURLRequestContext> context(new TestURLRequestContext()); + context->set_http_transaction_factory(&network_layer); + req.set_context(context); + req.set_method("GET"); + req.Start(); + + MessageLoop::current()->Run(); + + EXPECT_TRUE(network_layer.done_reading_called()); + + RemoveMockTransaction(&kGZip_Transaction); +} + +TEST(URLRequestJob, SyncTransactionNotifiedWhenDone) { + TestDelegate d; + TestURLRequest req(GURL(kGZip_Transaction.url), &d); + MockNetworkLayer network_layer; + + MockTransaction transaction(kGZip_Transaction); + transaction.test_mode = TEST_MODE_SYNC_ALL; + AddMockTransaction(&transaction); + + scoped_refptr<TestURLRequestContext> context(new TestURLRequestContext()); + context->set_http_transaction_factory(&network_layer); + req.set_context(context); + req.set_method("GET"); + req.Start(); + + MessageLoop::current()->Run(); + + EXPECT_TRUE(network_layer.done_reading_called()); + + RemoveMockTransaction(&transaction); +} |