diff options
author | gavinp@chromium.org <gavinp@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2013-04-23 20:19:46 +0000 |
---|---|---|
committer | gavinp@chromium.org <gavinp@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2013-04-23 20:19:46 +0000 |
commit | d61799b42c305390648543e1c0483e72f6d684df (patch) | |
tree | 0847fc4995598f53cb3ee56640e61a787c73369c /net | |
parent | fe053f9301b7003d0bbe82baf9001d5b95564b9e (diff) | |
download | chromium_src-d61799b42c305390648543e1c0483e72f6d684df.zip chromium_src-d61799b42c305390648543e1c0483e72f6d684df.tar.gz chromium_src-d61799b42c305390648543e1c0483e72f6d684df.tar.bz2 |
Simplify cross thread messaging in SimpleCache.
The message passing model was needless complicated before. With this
update, we switched to PostTaskAndReply to send all messages.
Our old model forced us to consider cross thread destruction, to pass
MessageLoopProxy objects into SimpleSynchronousEntry etc...
By using PostTaskAndReply, the SimpleSynchronousEntry now has no
threading code in it whatsoever, and the SimpleEntryImpl no longer is
RefCountedThreadSafe. Since the base::Closure for the task
is destructed before passing the reply, we also now pass the previously
flaky DiskCacheEntryTest.SimpleCacheReleaseBuffer.
**REMOVEBEFORECOMMIT** This change is downstream of https://codereview.chromium.org/13221002/
R=pliard,pasko,felipeg1
BUG=233123
Review URL: https://chromiumcodereview.appspot.com/14028021
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@195898 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'net')
-rw-r--r-- | net/disk_cache/entry_unittest.cc | 4 | ||||
-rw-r--r-- | net/disk_cache/simple/simple_backend_impl.cc | 21 | ||||
-rw-r--r-- | net/disk_cache/simple/simple_entry_impl.cc | 193 | ||||
-rw-r--r-- | net/disk_cache/simple/simple_entry_impl.h | 23 | ||||
-rw-r--r-- | net/disk_cache/simple/simple_index.cc | 22 | ||||
-rw-r--r-- | net/disk_cache/simple/simple_index.h | 2 | ||||
-rw-r--r-- | net/disk_cache/simple/simple_synchronous_entry.cc | 87 | ||||
-rw-r--r-- | net/disk_cache/simple/simple_synchronous_entry.h | 48 |
8 files changed, 188 insertions, 212 deletions
diff --git a/net/disk_cache/entry_unittest.cc b/net/disk_cache/entry_unittest.cc index 6279666..8f85d8c4 100644 --- a/net/disk_cache/entry_unittest.cc +++ b/net/disk_cache/entry_unittest.cc @@ -2247,9 +2247,7 @@ TEST_F(DiskCacheEntryTest, SimpleCacheExternalAsyncIO) { ExternalAsyncIO(); } -// TODO(felipeg): flaky, failing to WritePlatformFile in -// simple_synchronous_entry.cc. It failed in linux_asan bot. -TEST_F(DiskCacheEntryTest, DISABLED_SimpleCacheReleaseBuffer) { +TEST_F(DiskCacheEntryTest, SimpleCacheReleaseBuffer) { SetSimpleCacheMode(); InitCache(); ReleaseBuffer(); diff --git a/net/disk_cache/simple/simple_backend_impl.cc b/net/disk_cache/simple/simple_backend_impl.cc index 9d4b43f..a8de799 100644 --- a/net/disk_cache/simple/simple_backend_impl.cc +++ b/net/disk_cache/simple/simple_backend_impl.cc @@ -16,6 +16,7 @@ #include "net/disk_cache/simple/simple_index.h" #include "net/disk_cache/simple/simple_synchronous_entry.h" +using base::Closure; using base::FilePath; using base::MessageLoopProxy; using base::SingleThreadTaskRunner; @@ -100,6 +101,13 @@ bool FileStructureConsistent(const base::FilePath& path) { } } +void CallCompletionCallback(const net::CompletionCallback& callback, + scoped_ptr<int> result) { + DCHECK(!callback.is_null()); + DCHECK(result); + callback.Run(*result); +} + } // namespace namespace disk_cache { @@ -179,13 +187,12 @@ void SimpleBackendImpl::IndexReadyForDoom(Time initial_time, } scoped_ptr<std::vector<uint64> > key_hashes( index_->RemoveEntriesBetween(initial_time, end_time).release()); - WorkerPool::PostTask(FROM_HERE, - base::Bind(&SimpleSynchronousEntry::DoomEntrySet, - base::Passed(&key_hashes), - path_, - MessageLoopProxy::current(), - callback), - true); + scoped_ptr<int> new_result(new int()); + Closure task = base::Bind(&SimpleSynchronousEntry::DoomEntrySet, + base::Passed(&key_hashes), path_, new_result.get()); + Closure reply = base::Bind(CallCompletionCallback, + callback, base::Passed(&new_result)); + WorkerPool::PostTaskAndReply(FROM_HERE, task, reply, true); } int SimpleBackendImpl::DoomEntriesBetween( diff --git a/net/disk_cache/simple/simple_entry_impl.cc b/net/disk_cache/simple/simple_entry_impl.cc index d3ec7b3..1db0a0c 100644 --- a/net/disk_cache/simple/simple_entry_impl.cc +++ b/net/disk_cache/simple/simple_entry_impl.cc @@ -23,18 +23,18 @@ namespace { -typedef disk_cache::Entry::CompletionCallback CompletionCallback; -typedef disk_cache::SimpleSynchronousEntry::SynchronousCreationCallback - SynchronousCreationCallback; -typedef disk_cache::SimpleSynchronousEntry::SynchronousReadCallback - SynchronousReadCallback; -typedef disk_cache::SimpleSynchronousEntry::SynchronousOperationCallback - SynchronousOperationCallback; +void CallCompletionCallback(const net::CompletionCallback& callback, + scoped_ptr<int> result) { + DCHECK(result); + if (!callback.is_null()) + callback.Run(*result); +} } // namespace namespace disk_cache { +using base::Closure; using base::FilePath; using base::MessageLoopProxy; using base::Time; @@ -44,7 +44,7 @@ using base::WorkerPool; int SimpleEntryImpl::OpenEntry(SimpleIndex* entry_index, const FilePath& path, const std::string& key, - Entry** entry, + Entry** out_entry, const CompletionCallback& callback) { // If entry is not known to the index, initiate fast failover to the network. if (entry_index && !entry_index->Has(key)) @@ -52,14 +52,16 @@ int SimpleEntryImpl::OpenEntry(SimpleIndex* entry_index, scoped_refptr<SimpleEntryImpl> new_entry = new SimpleEntryImpl(entry_index, path, key); - SynchronousCreationCallback sync_creation_callback = - base::Bind(&SimpleEntryImpl::CreationOperationComplete, - new_entry, entry, callback); - WorkerPool::PostTask(FROM_HERE, - base::Bind(&SimpleSynchronousEntry::OpenEntry, path, - key, MessageLoopProxy::current(), - sync_creation_callback), - true); + + typedef SimpleSynchronousEntry* PointerToSimpleSynchronousEntry; + scoped_ptr<PointerToSimpleSynchronousEntry> sync_entry( + new PointerToSimpleSynchronousEntry()); + Closure task = base::Bind(&SimpleSynchronousEntry::OpenEntry, path, key, + sync_entry.get()); + Closure reply = base::Bind(&SimpleEntryImpl::CreationOperationComplete, + new_entry, callback, base::Passed(&sync_entry), + out_entry); + WorkerPool::PostTaskAndReply(FROM_HERE, task, reply, true); return net::ERR_IO_PENDING; } @@ -67,7 +69,7 @@ int SimpleEntryImpl::OpenEntry(SimpleIndex* entry_index, int SimpleEntryImpl::CreateEntry(SimpleIndex* entry_index, const FilePath& path, const std::string& key, - Entry** entry, + Entry** out_entry, const CompletionCallback& callback) { // We insert the entry in the index before creating the entry files in the // SimpleSynchronousEntry, because this way the worse scenario is when we @@ -78,14 +80,16 @@ int SimpleEntryImpl::CreateEntry(SimpleIndex* entry_index, entry_index->Insert(key); scoped_refptr<SimpleEntryImpl> new_entry = new SimpleEntryImpl(entry_index, path, key); - SynchronousCreationCallback sync_creation_callback = - base::Bind(&SimpleEntryImpl::CreationOperationComplete, - new_entry, entry, callback); - WorkerPool::PostTask(FROM_HERE, - base::Bind(&SimpleSynchronousEntry::CreateEntry, path, - key, MessageLoopProxy::current(), - sync_creation_callback), - true); + + typedef SimpleSynchronousEntry* PointerToSimpleSynchronousEntry; + scoped_ptr<PointerToSimpleSynchronousEntry> sync_entry( + new PointerToSimpleSynchronousEntry()); + Closure task = base::Bind(&SimpleSynchronousEntry::CreateEntry, path, key, + sync_entry.get()); + Closure reply = base::Bind(&SimpleEntryImpl::CreationOperationComplete, + new_entry, callback, base::Passed(&sync_entry), + out_entry); + WorkerPool::PostTaskAndReply(FROM_HERE, task, reply, true); return net::ERR_IO_PENDING; } @@ -95,10 +99,12 @@ int SimpleEntryImpl::DoomEntry(SimpleIndex* entry_index, const std::string& key, const CompletionCallback& callback) { entry_index->Remove(key); - WorkerPool::PostTask(FROM_HERE, - base::Bind(&SimpleSynchronousEntry::DoomEntry, path, key, - MessageLoopProxy::current(), callback), - true); + scoped_ptr<int> result(new int()); + Closure task = base::Bind(&SimpleSynchronousEntry::DoomEntry, path, key, + result.get()); + Closure reply = base::Bind(&CallCompletionCallback, + callback, base::Passed(&result)); + WorkerPool::PostTaskAndReply(FROM_HERE, task, reply, true); return net::ERR_IO_PENDING; } @@ -266,12 +272,15 @@ SimpleEntryImpl::SimpleEntryImpl(SimpleIndex* entry_index, } SimpleEntryImpl::~SimpleEntryImpl() { + DCHECK(io_thread_checker_.CalledOnValidThread()); DCHECK_EQ(0U, pending_operations_.size()); DCHECK(!operation_running_); + DCHECK(!synchronous_entry_); } bool SimpleEntryImpl::RunNextOperationIfNeeded() { DCHECK(io_thread_checker_.CalledOnValidThread()); + if (pending_operations_.size() <= 0 || operation_running_) return false; base::Closure operation = pending_operations_.front(); @@ -303,6 +312,7 @@ void SimpleEntryImpl::CloseInternal() { base::Unretained(synchronous_entry_), base::Passed(&crc32s_to_write)), true); + synchronous_entry_ = NULL; // Entry::Close() is expected to delete this entry. See disk_cache.h for // details. Release(); // Balanced in CreationOperationComplete(). @@ -318,15 +328,17 @@ void SimpleEntryImpl::ReadDataInternal(int stream_index, operation_running_ = true; if (entry_index_) entry_index_->UseIfExists(key_); - SynchronousReadCallback sync_read_callback = - base::Bind(&SimpleEntryImpl::ReadOperationComplete, - this, stream_index, offset, callback); - WorkerPool::PostTask(FROM_HERE, - base::Bind(&SimpleSynchronousEntry::ReadData, - base::Unretained(synchronous_entry_), - stream_index, offset, make_scoped_refptr(buf), - buf_len, sync_read_callback), - true); + + scoped_ptr<uint32> read_crc32(new uint32()); + scoped_ptr<int> result(new int()); + Closure task = base::Bind(&SimpleSynchronousEntry::ReadData, + base::Unretained(synchronous_entry_), + stream_index, offset, make_scoped_refptr(buf), + buf_len, read_crc32.get(), result.get()); + Closure reply = base::Bind(&SimpleEntryImpl::ReadOperationComplete, this, + stream_index, offset, callback, + base::Passed(&read_crc32), base::Passed(&result)); + WorkerPool::PostTaskAndReply(FROM_HERE, task, reply, true); } void SimpleEntryImpl::WriteDataInternal(int stream_index, @@ -358,23 +370,24 @@ void SimpleEntryImpl::WriteDataInternal(int stream_index, have_written_[stream_index] = true; - SynchronousOperationCallback sync_operation_callback = - base::Bind(&SimpleEntryImpl::EntryOperationComplete, - this, callback, stream_index); - WorkerPool::PostTask(FROM_HERE, - base::Bind(&SimpleSynchronousEntry::WriteData, - base::Unretained(synchronous_entry_), - stream_index, offset, make_scoped_refptr(buf), - buf_len, sync_operation_callback, truncate), - true); + scoped_ptr<int> result(new int()); + Closure task = base::Bind(&SimpleSynchronousEntry::WriteData, + base::Unretained(synchronous_entry_), + stream_index, offset, make_scoped_refptr(buf), + buf_len, truncate, result.get()); + Closure reply = base::Bind(&SimpleEntryImpl::EntryOperationComplete, this, + stream_index, callback, base::Passed(&result)); + WorkerPool::PostTaskAndReply(FROM_HERE, task, reply, true); } void SimpleEntryImpl::CreationOperationComplete( - Entry** out_entry, const CompletionCallback& completion_callback, - SimpleSynchronousEntry* sync_entry) { + scoped_ptr<SimpleSynchronousEntry*> in_sync_entry, + Entry** out_entry) { DCHECK(io_thread_checker_.CalledOnValidThread()); - if (!sync_entry) { + DCHECK(in_sync_entry); + + if (!*in_sync_entry) { completion_callback.Run(net::ERR_FAILED); // If OpenEntry failed, we must remove it from our index. if (entry_index_) @@ -387,30 +400,31 @@ void SimpleEntryImpl::CreationOperationComplete( // alive until Entry::Close(). Adding a reference to self will keep |this| // alive after the scope of the Callback calling us is destroyed. AddRef(); // Balanced in CloseInternal(). - synchronous_entry_ = sync_entry; + synchronous_entry_ = *in_sync_entry; SetSynchronousData(); *out_entry = this; completion_callback.Run(net::OK); } void SimpleEntryImpl::EntryOperationComplete( - const CompletionCallback& completion_callback, int stream_index, - int result) { + const CompletionCallback& completion_callback, + scoped_ptr<int> result) { DCHECK(io_thread_checker_.CalledOnValidThread()); DCHECK(synchronous_entry_); DCHECK(operation_running_); + DCHECK(result); operation_running_ = false; - if (result < 0) { + if (*result < 0) { if (entry_index_) entry_index_->Remove(key_); entry_index_.reset(); crc32s_end_offset_[stream_index] = 0; } SetSynchronousData(); - completion_callback.Run(result); + completion_callback.Run(*result); RunNextOperationIfNeeded(); } @@ -418,56 +432,59 @@ void SimpleEntryImpl::ReadOperationComplete( int stream_index, int offset, const CompletionCallback& completion_callback, - int result, - uint32 crc) { + scoped_ptr<uint32> read_crc32, + scoped_ptr<int> result) { DCHECK(io_thread_checker_.CalledOnValidThread()); DCHECK(synchronous_entry_); DCHECK(operation_running_); + DCHECK(read_crc32); + DCHECK(result); - if (result > 0 && crc32s_end_offset_[stream_index] == offset) { + if (*result > 0 && crc32s_end_offset_[stream_index] == offset) { uint32 current_crc = offset == 0 ? crc32(0, Z_NULL, 0) : crc32s_[stream_index]; - crc32s_[stream_index] = crc32_combine(current_crc, crc, result); - crc32s_end_offset_[stream_index] += result; - if (!have_written_[stream_index]) { - if (data_size_[stream_index] == crc32s_end_offset_[stream_index]) { - // We have just read a file from start to finish, and so we have - // computed a crc of the entire file. We can check it now. If a cache - // entry has a single reader, the normal pattern is to read from start - // to finish. - - // Other cases are possible. In the case of two readers on the same - // entry, one reader can be behind the other. In this case we compute - // the crc as the most advanced reader progresses, and check it for - // both readers as they read the last byte. - - SynchronousOperationCallback sync_operation_callback = - base::Bind(&SimpleEntryImpl::ChecksumOperationComplete, - this, result, stream_index, completion_callback); - WorkerPool::PostTask(FROM_HERE, - base::Bind(&SimpleSynchronousEntry::CheckEOFRecord, - base::Unretained(synchronous_entry_), - stream_index, crc32s_[stream_index], - sync_operation_callback), - true); - return; - } + crc32s_[stream_index] = crc32_combine(current_crc, *read_crc32, *result); + crc32s_end_offset_[stream_index] += *result; + if (!have_written_[stream_index] && + data_size_[stream_index] == crc32s_end_offset_[stream_index]) { + // We have just read a file from start to finish, and so we have + // computed a crc of the entire file. We can check it now. If a cache + // entry has a single reader, the normal pattern is to read from start + // to finish. + + // Other cases are possible. In the case of two readers on the same + // entry, one reader can be behind the other. In this case we compute + // the crc as the most advanced reader progresses, and check it for + // both readers as they read the last byte. + + scoped_ptr<int> new_result(new int()); + Closure task = base::Bind(&SimpleSynchronousEntry::CheckEOFRecord, + base::Unretained(synchronous_entry_), + stream_index, crc32s_[stream_index], + new_result.get()); + Closure reply = base::Bind(&SimpleEntryImpl::ChecksumOperationComplete, + this, *result, stream_index, + completion_callback, + base::Passed(&new_result)); + WorkerPool::PostTaskAndReply(FROM_HERE, task, reply, true); + return; } } - EntryOperationComplete(completion_callback, stream_index, result); + EntryOperationComplete(stream_index, completion_callback, result.Pass()); } void SimpleEntryImpl::ChecksumOperationComplete( int orig_result, int stream_index, const CompletionCallback& completion_callback, - int result) { + scoped_ptr<int> result) { DCHECK(io_thread_checker_.CalledOnValidThread()); DCHECK(synchronous_entry_); DCHECK(operation_running_); - if (result == net::OK) - result = orig_result; - EntryOperationComplete(completion_callback, stream_index, result); + DCHECK(result); + if (*result == net::OK) + *result = orig_result; + EntryOperationComplete(stream_index, completion_callback, result.Pass()); } void SimpleEntryImpl::SetSynchronousData() { diff --git a/net/disk_cache/simple/simple_entry_impl.h b/net/disk_cache/simple/simple_entry_impl.h index 5e36c19..0a0d96a 100644 --- a/net/disk_cache/simple/simple_entry_impl.h +++ b/net/disk_cache/simple/simple_entry_impl.h @@ -10,6 +10,8 @@ #include "base/files/file_path.h" #include "base/memory/ref_counted.h" +#include "base/memory/scoped_ptr.h" +#include "base/memory/weak_ptr.h" #include "base/threading/thread_checker.h" #include "net/disk_cache/disk_cache.h" #include "net/disk_cache/simple/simple_entry_format.h" @@ -30,9 +32,8 @@ class SimpleSynchronousEntry; // SimpleEntryImpl is the IO thread interface to an entry in the very simple // disk cache. It proxies for the SimpleSynchronousEntry, which performs IO // on the worker thread. -class SimpleEntryImpl : public Entry, - public base::RefCountedThreadSafe<SimpleEntryImpl> { - friend class base::RefCountedThreadSafe<SimpleEntryImpl>; +class SimpleEntryImpl : public Entry, public base::RefCounted<SimpleEntryImpl> { + friend class base::RefCounted<SimpleEntryImpl>; public: static int OpenEntry(SimpleIndex* entry_index, const base::FilePath& path, @@ -112,28 +113,28 @@ class SimpleEntryImpl : public Entry, bool truncate); // Called after a SimpleSynchronousEntry has completed CreateEntry() or - // OpenEntry(). If |sync_entry| is non-NULL, creation is successful and we + // OpenEntry(). If |in_sync_entry| is non-NULL, creation is successful and we // can return |this| SimpleEntryImpl to |*out_entry|. Runs // |completion_callback|. void CreationOperationComplete( - Entry** out_entry, const CompletionCallback& completion_callback, - SimpleSynchronousEntry* sync_entry); + scoped_ptr<SimpleSynchronousEntry*> in_sync_entry, + Entry** out_entry); // Called after a SimpleSynchronousEntry has completed an asynchronous IO // operation, such as ReadData() or WriteData(). Calls |completion_callback|. void EntryOperationComplete( - const CompletionCallback& completion_callback, int stream_index, - int result); + const CompletionCallback& completion_callback, + scoped_ptr<int> result); // Called after an asynchronous read. Updates |crc32s_| if possible. void ReadOperationComplete( int stream_index, int offset, const CompletionCallback& completion_callback, - int result, - uint32 read_data_crc); + scoped_ptr<uint32> read_crc32, + scoped_ptr<int> result); // Called after validating the checksums on an entry. Passes through the // original result if successful, propogates the error if the checksum does @@ -142,7 +143,7 @@ class SimpleEntryImpl : public Entry, int stream_index, int orig_result, const CompletionCallback& completion_callback, - int result); + scoped_ptr<int> result); // Called on initialization and also after the completion of asynchronous IO // to initialize the IO thread copies of data returned by synchronous accessor diff --git a/net/disk_cache/simple/simple_index.cc b/net/disk_cache/simple/simple_index.cc index 697da4e..cfd878d 100644 --- a/net/disk_cache/simple/simple_index.cc +++ b/net/disk_cache/simple/simple_index.cc @@ -338,15 +338,13 @@ void SimpleIndex::StartEvictionIfNeeded() { // Take out the rest of hashes from the eviction list. entry_hashes->erase(it, entry_hashes->end()); - net::CompletionCallback callback = - base::Bind(&SimpleIndex::EvictionDone, AsWeakPtr()); - base::WorkerPool::PostTask(FROM_HERE, - base::Bind(&SimpleSynchronousEntry::DoomEntrySet, - base::Passed(&entry_hashes), - index_filename_.DirName(), - io_thread_, - callback), - true); + scoped_ptr<int> result(new int()); + base::Closure task = base::Bind(&SimpleSynchronousEntry::DoomEntrySet, + base::Passed(&entry_hashes), + index_filename_.DirName(), result.get()); + base::Closure reply = base::Bind(&SimpleIndex::EvictionDone, AsWeakPtr(), + base::Passed(&result)); + base::WorkerPool::PostTaskAndReply(FROM_HERE, task, reply, true); } bool SimpleIndex::UpdateEntrySize(const std::string& key, uint64 entry_size) { @@ -365,10 +363,12 @@ bool SimpleIndex::UpdateEntrySize(const std::string& key, uint64 entry_size) { return true; } -void SimpleIndex::EvictionDone(int result) { +void SimpleIndex::EvictionDone(scoped_ptr<int> result) { DCHECK(io_thread_checker_.CalledOnValidThread()); + DCHECK(result); + // Ignore the result of eviction. We did our best. - UMA_HISTOGRAM_BOOLEAN("SimpleCache.EvictionSuccess", result == net::OK); + UMA_HISTOGRAM_BOOLEAN("SimpleCache.EvictionSuccess", *result == net::OK); eviction_in_progress_ = false; } diff --git a/net/disk_cache/simple/simple_index.h b/net/disk_cache/simple/simple_index.h index 29a58aa..4323b30 100644 --- a/net/disk_cache/simple/simple_index.h +++ b/net/disk_cache/simple/simple_index.h @@ -135,7 +135,7 @@ class NET_EXPORT_PRIVATE SimpleIndex IndexCompletionCallback; void StartEvictionIfNeeded(); - void EvictionDone(int result); + void EvictionDone(scoped_ptr<int> result); void PostponeWritingToDisk(); diff --git a/net/disk_cache/simple/simple_synchronous_entry.cc b/net/disk_cache/simple/simple_synchronous_entry.cc index 6a1696b..31962a7 100644 --- a/net/disk_cache/simple/simple_synchronous_entry.cc +++ b/net/disk_cache/simple/simple_synchronous_entry.cc @@ -13,11 +13,8 @@ #include "base/file_util.h" #include "base/hash.h" #include "base/location.h" -#include "base/memory/scoped_ptr.h" -#include "base/message_loop_proxy.h" #include "base/sha1.h" #include "base/stringprintf.h" -#include "base/task_runner.h" #include "net/base/io_buffer.h" #include "net/base/net_errors.h" #include "net/disk_cache/simple/simple_util.h" @@ -34,7 +31,6 @@ using base::PLATFORM_FILE_OPEN; using base::PLATFORM_FILE_READ; using base::PLATFORM_FILE_WRITE; using base::ReadPlatformFile; -using base::SingleThreadTaskRunner; using base::Time; using base::TruncatePlatformFile; using base::WritePlatformFile; @@ -66,36 +62,34 @@ SimpleSynchronousEntry::CRCRecord::CRCRecord(int index_p, void SimpleSynchronousEntry::OpenEntry( const FilePath& path, const std::string& key, - SingleThreadTaskRunner* callback_runner, - const SynchronousCreationCallback& callback) { - SimpleSynchronousEntry* sync_entry = - new SimpleSynchronousEntry(callback_runner, path, key); + SimpleSynchronousEntry** out_entry) { + SimpleSynchronousEntry* sync_entry = new SimpleSynchronousEntry(path, key); int rv = sync_entry->InitializeForOpen(); if (rv != net::OK) { sync_entry->Doom(); delete sync_entry; - sync_entry = NULL; + *out_entry = NULL; + return; } - callback_runner->PostTask(FROM_HERE, base::Bind(callback, sync_entry)); + *out_entry = sync_entry; } // static void SimpleSynchronousEntry::CreateEntry( const FilePath& path, const std::string& key, - SingleThreadTaskRunner* callback_runner, - const SynchronousCreationCallback& callback) { - SimpleSynchronousEntry* sync_entry = - new SimpleSynchronousEntry(callback_runner, path, key); + SimpleSynchronousEntry** out_entry) { + SimpleSynchronousEntry* sync_entry = new SimpleSynchronousEntry(path, key); int rv = sync_entry->InitializeForCreate(); if (rv != net::OK) { if (rv != net::ERR_FILE_EXISTS) { sync_entry->Doom(); } delete sync_entry; - sync_entry = NULL; + *out_entry = NULL; + return; } - callback_runner->PostTask(FROM_HERE, base::Bind(callback, sync_entry)); + *out_entry = sync_entry; } // static @@ -117,29 +111,23 @@ bool SimpleSynchronousEntry::DeleteFilesForEntry(const FilePath& path, void SimpleSynchronousEntry::DoomEntry( const FilePath& path, const std::string& key, - SingleThreadTaskRunner* callback_runner, - const net::CompletionCallback& callback) { + int* out_result) { bool deleted_well = DeleteFilesForEntry(path, GetEntryHashKeyAsHexString(key)); - int result = deleted_well ? net::OK : net::ERR_FAILED; - if (!callback.is_null()) - callback_runner->PostTask(FROM_HERE, base::Bind(callback, result)); + *out_result = deleted_well ? net::OK : net::ERR_FAILED; } // static void SimpleSynchronousEntry::DoomEntrySet( scoped_ptr<std::vector<uint64> > key_hashes, const FilePath& path, - SingleThreadTaskRunner* callback_runner, - const net::CompletionCallback& callback) { + int* out_result) { bool deleted_well = true; for (std::vector<uint64>::const_iterator it = key_hashes->begin(), end = key_hashes->end(); it != end; ++it) deleted_well &= DeleteFilesForEntry(path, ConvertEntryHashKeyToHexString((*it))); - int result = deleted_well ? net::OK : net::ERR_FAILED; - if (!callback.is_null()) - callback_runner->PostTask(FROM_HERE, base::Bind(callback, result)); + *out_result = deleted_well ? net::OK : net::ERR_FAILED; } void SimpleSynchronousEntry::ReadData( @@ -147,20 +135,23 @@ void SimpleSynchronousEntry::ReadData( int offset, net::IOBuffer* buf, int buf_len, - const SynchronousReadCallback& callback) { + uint32* out_crc32, + int* out_result) { DCHECK(initialized_); int64 file_offset = GetFileOffsetFromKeyAndDataOffset(key_, offset); int bytes_read = ReadPlatformFile(files_[index], file_offset, buf->data(), buf_len); - uint32 crc = crc32(0L, Z_NULL, 0); if (bytes_read > 0) { last_used_ = Time::Now(); - crc = crc32(crc, reinterpret_cast<const Bytef*>(buf->data()), bytes_read); + *out_crc32 = crc32(crc32(0L, Z_NULL, 0), + reinterpret_cast<const Bytef*>(buf->data()), bytes_read); } - int result = (bytes_read >= 0) ? bytes_read : net::ERR_FAILED; - if (result == net::ERR_FAILED) + if (bytes_read >= 0) { + *out_result = bytes_read; + } else { + *out_result = net::ERR_FAILED; Doom(); - callback_runner_->PostTask(FROM_HERE, base::Bind(callback, result, crc)); + } } void SimpleSynchronousEntry::WriteData( @@ -168,8 +159,8 @@ void SimpleSynchronousEntry::WriteData( int offset, net::IOBuffer* buf, int buf_len, - const SynchronousOperationCallback& callback, - bool truncate) { + bool truncate, + int* out_result) { DCHECK(initialized_); bool extending_by_write = offset + buf_len > data_size_[index]; @@ -179,8 +170,7 @@ void SimpleSynchronousEntry::WriteData( GetFileOffsetFromKeyAndDataOffset(key_, data_size_[index]); if (!TruncatePlatformFile(files_[index], file_eof_offset)) { Doom(); - callback_runner_->PostTask(FROM_HERE, - base::Bind(callback, net::ERR_FAILED)); + *out_result = net::ERR_FAILED; return; } } @@ -189,8 +179,7 @@ void SimpleSynchronousEntry::WriteData( if (WritePlatformFile(files_[index], file_offset, buf->data(), buf_len) != buf_len) { Doom(); - callback_runner_->PostTask(FROM_HERE, - base::Bind(callback, net::ERR_FAILED)); + *out_result = net::ERR_FAILED; return; } } @@ -199,21 +188,20 @@ void SimpleSynchronousEntry::WriteData( } else { if (!TruncatePlatformFile(files_[index], file_offset + buf_len)) { Doom(); - callback_runner_->PostTask(FROM_HERE, - base::Bind(callback, net::ERR_FAILED)); + *out_result = net::ERR_FAILED; return; } data_size_[index] = offset + buf_len; } last_modified_ = Time::Now(); - callback_runner_->PostTask(FROM_HERE, base::Bind(callback, buf_len)); + *out_result = buf_len; } void SimpleSynchronousEntry::CheckEOFRecord( int index, uint32 expected_crc32, - const SynchronousOperationCallback& callback) { + int* out_result) { DCHECK(initialized_); SimpleFileEOF eof_record; @@ -223,16 +211,14 @@ void SimpleSynchronousEntry::CheckEOFRecord( reinterpret_cast<char*>(&eof_record), sizeof(eof_record)) != sizeof(eof_record)) { Doom(); - callback_runner_->PostTask(FROM_HERE, - base::Bind(callback, net::ERR_FAILED)); + *out_result = net::ERR_FAILED; return; } if (eof_record.final_magic_number != kSimpleFinalMagicNumber) { DLOG(INFO) << "eof record had bad magic number."; Doom(); - callback_runner_->PostTask(FROM_HERE, - base::Bind(callback, net::ERR_FAILED)); + *out_result = net::ERR_FAILED; return; } @@ -240,12 +226,11 @@ void SimpleSynchronousEntry::CheckEOFRecord( eof_record.data_crc32 != expected_crc32) { DLOG(INFO) << "eof record had bad crc."; Doom(); - callback_runner_->PostTask(FROM_HERE, - base::Bind(callback, net::ERR_FAILED)); + *out_result = net::ERR_FAILED; return; } - callback_runner_->PostTask(FROM_HERE, base::Bind(callback, net::OK)); + *out_result = net::OK; } void SimpleSynchronousEntry::Close( @@ -278,11 +263,9 @@ void SimpleSynchronousEntry::Close( } SimpleSynchronousEntry::SimpleSynchronousEntry( - SingleThreadTaskRunner* callback_runner, const FilePath& path, const std::string& key) - : callback_runner_(callback_runner), - path_(path), + : path_(path), key_(key), initialized_(false) { } diff --git a/net/disk_cache/simple/simple_synchronous_entry.h b/net/disk_cache/simple/simple_synchronous_entry.h index c6d2ba2..eacf5d5 100644 --- a/net/disk_cache/simple/simple_synchronous_entry.h +++ b/net/disk_cache/simple/simple_synchronous_entry.h @@ -10,20 +10,12 @@ #include <utility> #include <vector> -#include "base/callback_forward.h" #include "base/files/file_path.h" -#include "base/memory/ref_counted.h" #include "base/memory/scoped_ptr.h" #include "base/platform_file.h" -#include "base/task_runner.h" #include "base/time.h" -#include "net/base/completion_callback.h" #include "net/disk_cache/simple/simple_entry_format.h" -namespace base { -class SingleThreadTaskRunner; -} - namespace net { class IOBuffer; } @@ -35,23 +27,6 @@ namespace disk_cache { // a single thread between synchronization points. class SimpleSynchronousEntry { public: - // Callback type for the completion of creation of SimpleSynchronousEntry - // objects, for instance after OpenEntry() or CreateEntry(). |new_entry| is - // the newly created SimpleSynchronousEntry, constructed on the worker pool - // thread. |new_entry| == NULL in the case of error. - typedef base::Callback<void(SimpleSynchronousEntry* new_entry)> - SynchronousCreationCallback; - - // Callback type for the completion of a read from an entry. - // |result| is the number of bytes read, or the net::Error if an error. |crc| - // is the crc32 of the data that was read on success, undefined otherwise. - typedef base::Callback<void(int result, uint32 read_data_crc)> - SynchronousReadCallback; - - // Callback type for IO operations on an entry not requiring special callback - // arguments (e.g. Write). |result| is a net::Error result code. - typedef base::Callback<void(int result)> SynchronousOperationCallback; - struct CRCRecord { CRCRecord(); CRCRecord(int index_p, bool has_crc32_p, uint32 data_crc32_p); @@ -64,14 +39,12 @@ class SimpleSynchronousEntry { static void OpenEntry( const base::FilePath& path, const std::string& key, - base::SingleThreadTaskRunner* callback_runner, - const SynchronousCreationCallback& callback); + SimpleSynchronousEntry** out_entry); static void CreateEntry( const base::FilePath& path, const std::string& key, - base::SingleThreadTaskRunner* callback_runner, - const SynchronousCreationCallback& callback); + SimpleSynchronousEntry** out_entry); // Deletes an entry without first Opening it. Does not check if there is // already an Entry object in memory holding the open files. Be careful! This @@ -79,31 +52,30 @@ class SimpleSynchronousEntry { // run by |callback_runner|. static void DoomEntry(const base::FilePath& path, const std::string& key, - base::SingleThreadTaskRunner* callback_runner, - const net::CompletionCallback& callback); + int* out_result); // Like |DoomEntry()| above. Deletes all entries corresponding to the // |key_hashes|. Succeeds only when all entries are deleted. static void DoomEntrySet(scoped_ptr<std::vector<uint64> > key_hashes, const base::FilePath& path, - base::SingleThreadTaskRunner* callback_runner, - const net::CompletionCallback& callback); + int* out_result); // N.B. ReadData(), WriteData(), CheckEOFRecord() and Close() may block on IO. void ReadData(int index, int offset, net::IOBuffer* buf, int buf_len, - const SynchronousReadCallback& callback); + uint32* out_crc32, + int* out_result); void WriteData(int index, int offset, net::IOBuffer* buf, int buf_len, - const SynchronousOperationCallback& callback, - bool truncate); + bool truncate, + int* out_result); void CheckEOFRecord(int index, uint32 expected_crc32, - const SynchronousOperationCallback& callback); + int* out_result); // Close all streams, and add write EOF records to streams indicated by the // CRCRecord entries in |crc32s_to_write|. @@ -119,7 +91,6 @@ class SimpleSynchronousEntry { private: SimpleSynchronousEntry( - base::SingleThreadTaskRunner* callback_runner, const base::FilePath& path, const std::string& key); @@ -141,7 +112,6 @@ class SimpleSynchronousEntry { static bool DeleteFilesForEntry(const base::FilePath& path, const std::string& key); - scoped_refptr<base::SingleThreadTaskRunner> callback_runner_; const base::FilePath path_; const std::string key_; |