diff options
author | rvargas@google.com <rvargas@google.com@0039d316-1c4b-4281-b951-d872f2087c98> | 2010-11-04 17:34:38 +0000 |
---|---|---|
committer | rvargas@google.com <rvargas@google.com@0039d316-1c4b-4281-b951-d872f2087c98> | 2010-11-04 17:34:38 +0000 |
commit | 42d5be693f7cc623caa3df6bfb2bcc4f6861cb36 (patch) | |
tree | e5defa1e3d5ba1f662aaebd6eaa70e0f08a76027 /net | |
parent | 05f06dfc585db57c544bca907e74d5e6d4ca6982 (diff) | |
download | chromium_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.cc | 32 | ||||
-rw-r--r-- | net/disk_cache/entry_unittest.cc | 12 |
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. |