summaryrefslogtreecommitdiffstats
path: root/net
diff options
context:
space:
mode:
authorrvargas@google.com <rvargas@google.com@0039d316-1c4b-4281-b951-d872f2087c98>2010-11-04 17:34:38 +0000
committerrvargas@google.com <rvargas@google.com@0039d316-1c4b-4281-b951-d872f2087c98>2010-11-04 17:34:38 +0000
commit42d5be693f7cc623caa3df6bfb2bcc4f6861cb36 (patch)
treee5defa1e3d5ba1f662aaebd6eaa70e0f08a76027 /net
parent05f06dfc585db57c544bca907e74d5e6d4ca6982 (diff)
downloadchromium_src-42d5be693f7cc623caa3df6bfb2bcc4f6861cb36.zip
chromium_src-42d5be693f7cc623caa3df6bfb2bcc4f6861cb36.tar.gz
chromium_src-42d5be693f7cc623caa3df6bfb2bcc4f6861cb36.tar.bz2
Disk cache: Don't return zeros when reading from an entry
that has a buffer with an offset beyond the start of the read. The read logic was incorrectly calculating the eof for an entry as the current size minus the unreported size, so it was assuming that a buffer with a large offset was always extending the file... so it was returning zeros for the data. Now we don't attempt to keep buffers that extend a file, and the eof computation was fixed to be just the current lenght. BUG=59275 TEST=net_unittests Review URL: http://codereview.chromium.org/4289004 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@65074 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'net')
-rw-r--r--net/disk_cache/entry_impl.cc32
-rw-r--r--net/disk_cache/entry_unittest.cc12
2 files changed, 34 insertions, 10 deletions
diff --git a/net/disk_cache/entry_impl.cc b/net/disk_cache/entry_impl.cc
index 7fabbe5..ce59270 100644
--- a/net/disk_cache/entry_impl.cc
+++ b/net/disk_cache/entry_impl.cc
@@ -148,6 +148,7 @@ bool EntryImpl::UserBuffer::PreWrite(int offset, int len) {
void EntryImpl::UserBuffer::Truncate(int offset) {
DCHECK_GE(offset, 0);
DCHECK_GE(offset, offset_);
+ DVLOG(3) << "Buffer truncate at " << offset << " current " << offset_;
offset -= offset_;
if (Size() >= offset)
@@ -159,6 +160,7 @@ void EntryImpl::UserBuffer::Write(int offset, net::IOBuffer* buf, int len) {
DCHECK_GE(len, 0);
DCHECK_GE(offset + len, 0);
DCHECK_GE(offset, offset_);
+ DVLOG(3) << "Buffer write at " << offset << " current " << offset_;
if (!Size() && offset > kMaxBlockSize)
offset_ = offset;
@@ -268,6 +270,8 @@ bool EntryImpl::UserBuffer::GrowBuffer(int required, int limit) {
if (!grow_allowed_)
return false;
+ DVLOG(3) << "Buffer grow to " << required;
+
buffer_.reserve(required);
return true;
}
@@ -480,6 +484,7 @@ void EntryImpl::DoomImpl() {
int EntryImpl::ReadDataImpl(int index, int offset, net::IOBuffer* buf,
int buf_len, CompletionCallback* callback) {
DCHECK(node_.Data()->dirty || read_only_);
+ DVLOG(2) << "Read from " << index << " at " << offset << " : " << buf_len;
if (index < 0 || index >= kNumStreams)
return net::ERR_INVALID_ARGUMENT;
@@ -500,8 +505,8 @@ int EntryImpl::ReadDataImpl(int index, int offset, net::IOBuffer* buf,
backend_->OnEvent(Stats::READ_DATA);
backend_->OnRead(buf_len);
- // We need the current size in disk.
- int eof = entry_size - unreported_size_[index];
+ Addr address(entry_.Data()->data_addr[index]);
+ int eof = address.is_initialized() ? entry_size : 0;
if (user_buffers_[index].get() &&
user_buffers_[index]->PreRead(eof, offset, &buf_len)) {
// Complete the operation locally.
@@ -510,7 +515,7 @@ int EntryImpl::ReadDataImpl(int index, int offset, net::IOBuffer* buf,
return buf_len;
}
- Addr address(entry_.Data()->data_addr[index]);
+ address.set_value(entry_.Data()->data_addr[index]);
DCHECK(address.is_initialized());
if (!address.is_initialized())
return net::ERR_FAILED;
@@ -548,6 +553,7 @@ int EntryImpl::WriteDataImpl(int index, int offset, net::IOBuffer* buf,
int buf_len, CompletionCallback* callback,
bool truncate) {
DCHECK(node_.Data()->dirty || read_only_);
+ DVLOG(2) << "Write to " << index << " at " << offset << " : " << buf_len;
if (index < 0 || index >= kNumStreams)
return net::ERR_INVALID_ARGUMENT;
@@ -1142,14 +1148,20 @@ bool EntryImpl::ImportSeparateFile(int index, int new_size) {
bool EntryImpl::PrepareBuffer(int index, int offset, int buf_len) {
DCHECK(user_buffers_[index].get());
- if (offset > user_buffers_[index]->End()) {
- // We are about to extend the buffer (with zeros), so make sure that we are
- // not overwriting anything.
+ if ((user_buffers_[index]->End() && offset > user_buffers_[index]->End()) ||
+ offset > entry_.Data()->data_size[index]) {
+ // We are about to extend the buffer or the file (with zeros), so make sure
+ // that we are not overwriting anything.
Addr address(entry_.Data()->data_addr[index]);
if (address.is_initialized() && address.is_separate_file()) {
- int eof = entry_.Data()->data_size[index];
- if (eof > user_buffers_[index]->Start() && !Flush(index, 0))
+ if (!Flush(index, 0))
return false;
+ // There is an actual file already, and we don't want to keep track of
+ // its length so we let this operation go straight to disk.
+ // The only case when a buffer is allowed to extend the file (as in fill
+ // with zeros before the start) is when there is no file yet to extend.
+ user_buffers_[index].reset();
+ return true;
}
}
@@ -1158,7 +1170,8 @@ bool EntryImpl::PrepareBuffer(int index, int offset, int buf_len) {
return false;
// Lets try again.
- if (!user_buffers_[index]->PreWrite(offset, buf_len)) {
+ if (offset > user_buffers_[index]->End() ||
+ !user_buffers_[index]->PreWrite(offset, buf_len)) {
// We cannot complete the operation with a buffer.
DCHECK(!user_buffers_[index]->Size());
DCHECK(!user_buffers_[index]->Start());
@@ -1172,6 +1185,7 @@ bool EntryImpl::Flush(int index, int min_len) {
Addr address(entry_.Data()->data_addr[index]);
DCHECK(user_buffers_[index].get());
DCHECK(!address.is_initialized() || address.is_separate_file());
+ DVLOG(3) << "Flush";
int size = std::max(entry_.Data()->data_size[index], min_len);
if (size && !address.is_initialized() && !CreateDataBlock(index, size))
diff --git a/net/disk_cache/entry_unittest.cc b/net/disk_cache/entry_unittest.cc
index 41f9077..84568e3 100644
--- a/net/disk_cache/entry_unittest.cc
+++ b/net/disk_cache/entry_unittest.cc
@@ -900,6 +900,16 @@ void DiskCacheEntryTest::Buffering() {
EXPECT_EQ(100, ReadData(entry, 1, 23100, buffer2, kSize));
EXPECT_TRUE(!memcmp(buffer2->data(), buffer1->data() + 100, 100));
+ // Extend the file again and read before without closing the entry.
+ EXPECT_EQ(kSize, WriteData(entry, 1, 25000, buffer1, kSize, false));
+ EXPECT_EQ(kSize, WriteData(entry, 1, 45000, buffer1, kSize, false));
+ CacheTestFillBuffer(buffer2->data(), kSize, true);
+ EXPECT_EQ(kSize, ReadData(entry, 1, 25000, buffer2, kSize));
+ EXPECT_TRUE(!memcmp(buffer2->data(), buffer1->data(), kSize));
+ CacheTestFillBuffer(buffer2->data(), kSize, true);
+ EXPECT_EQ(kSize, ReadData(entry, 1, 45000, buffer2, kSize));
+ EXPECT_TRUE(!memcmp(buffer2->data(), buffer1->data(), kSize));
+
entry->Close();
}
@@ -943,7 +953,7 @@ void DiskCacheEntryTest::SizeChanges() {
EXPECT_TRUE(!memcmp(buffer2->data(), zeros, kSize));
// Read at the end of the old file size.
- EXPECT_EQ(35, ReadData(entry, 1, 23000 + kSize - 35, buffer2, kSize));
+ EXPECT_EQ(kSize, ReadData(entry, 1, 23000 + kSize - 35, buffer2, kSize));
EXPECT_TRUE(!memcmp(buffer2->data(), buffer1->data() + kSize - 35, 35));
// Read slightly before the last write.