summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
-rw-r--r--net/http/http_cache_transaction.cc9
-rw-r--r--net/http/http_cache_transaction.h1
-rw-r--r--net/http/http_cache_unittest.cc34
-rw-r--r--net/http/http_network_transaction.h1
-rw-r--r--net/http/http_transaction.h6
-rw-r--r--net/http/http_transaction_unittest.cc20
-rw-r--r--net/http/http_transaction_unittest.h13
-rw-r--r--net/net.gyp1
-rw-r--r--net/url_request/url_request_http_job.cc6
-rw-r--r--net/url_request/url_request_http_job.h47
-rw-r--r--net/url_request/url_request_job.cc20
-rw-r--r--net/url_request/url_request_job.h4
-rw-r--r--net/url_request/url_request_job_unittest.cc81
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);
+}