Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(58)

Unified Diff: net/base/directory_lister.cc

Issue 5710002: Create base::WorkerPoolJob. Use it for HostResolverImpl and DirectoryLister. (Closed) Base URL: svn://svn.chromium.org/chrome/trunk/src
Patch Set: Address comments. Created 10 years ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: net/base/directory_lister.cc
diff --git a/net/base/directory_lister.cc b/net/base/directory_lister.cc
index 61a2d6584db6029b371be0941004c417e4d8bf02..504806607a784161c95269a3b99985de56d1211d 100644
--- a/net/base/directory_lister.cc
+++ b/net/base/directory_lister.cc
@@ -9,41 +9,22 @@
#include "base/file_util.h"
#include "base/i18n/file_util_icu.h"
-#include "base/message_loop.h"
-#include "base/platform_thread.h"
-#include "base/thread_restrictions.h"
+#include "base/logging.h"
+#include "base/waitable_event.h"
+#include "base/worker_pool_job.h"
+#include "build/build_config.h"
#include "net/base/net_errors.h"
namespace net {
-static const int kFilesPerEvent = 8;
+namespace {
-// A task which is used to signal the delegate asynchronously.
-class DirectoryDataEvent : public Task {
-public:
- explicit DirectoryDataEvent(DirectoryLister* d) : lister(d), error(0) {
- // Allocations of the FindInfo aren't super cheap, so reserve space.
- data.reserve(64);
- }
-
- void Run() {
- if (data.empty()) {
- lister->OnDone(error);
- return;
- }
- lister->OnReceivedData(&data[0], static_cast<int>(data.size()));
- }
-
- scoped_refptr<DirectoryLister> lister;
- std::vector<DirectoryLister::DirectoryListerData> data;
- int error;
-};
+const int kFilesPerEvent = 8;
// Comparator for sorting lister results. This uses the locale aware filename
// comparison function on the filenames for sorting in the user's locale.
-// Static.
-bool DirectoryLister::CompareAlphaDirsFirst(const DirectoryListerData& a,
- const DirectoryListerData& b) {
+bool CompareAlphaDirsFirst(const DirectoryLister::Data& a,
+ const DirectoryLister::Data& b) {
// Parent directory before all else.
if (file_util::IsDotDot(file_util::FileEnumerator::GetFilename(a.info)))
return true;
@@ -61,9 +42,8 @@ bool DirectoryLister::CompareAlphaDirsFirst(const DirectoryListerData& a,
file_util::FileEnumerator::GetFilename(b.info));
}
-// Static.
-bool DirectoryLister::CompareDate(const DirectoryListerData& a,
- const DirectoryListerData& b) {
+bool CompareDate(const DirectoryLister::Data& a,
+ const DirectoryLister::Data& b) {
// Parent directory before all else.
if (file_util::IsDotDot(file_util::FileEnumerator::GetFilename(a.info)))
return true;
@@ -91,81 +71,63 @@ bool DirectoryLister::CompareDate(const DirectoryListerData& a,
// Comparator for sorting find result by paths. This uses the locale-aware
// comparison function on the filenames for sorting in the user's locale.
-// Static.
-bool DirectoryLister::CompareFullPath(const DirectoryListerData& a,
- const DirectoryListerData& b) {
+bool CompareFullPath(const DirectoryLister::Data& a,
+ const DirectoryLister::Data& b) {
return file_util::LocaleAwareCompareFilenames(a.path, b.path);
}
-DirectoryLister::DirectoryLister(const FilePath& dir,
- DirectoryListerDelegate* delegate)
- : dir_(dir),
- recursive_(false),
- delegate_(delegate),
- sort_(ALPHA_DIRS_FIRST),
- message_loop_(NULL),
- thread_(kNullThreadHandle) {
- DCHECK(!dir.value().empty());
-}
+} // namespace
-DirectoryLister::DirectoryLister(const FilePath& dir,
- bool recursive,
- SORT_TYPE sort,
- DirectoryListerDelegate* delegate)
- : dir_(dir),
- recursive_(recursive),
- delegate_(delegate),
- sort_(sort),
- message_loop_(NULL),
- thread_(kNullThreadHandle) {
- DCHECK(!dir.value().empty());
-}
+class DirectoryLister::Job : public base::WorkerPoolJob {
+ public:
+ Job(const FilePath& dir,
+ bool recursive,
+ SORT_TYPE sort,
+ Delegate* delegate);
-DirectoryLister::~DirectoryLister() {
- if (thread_) {
- // This is a bug and we should stop joining this thread.
- // http://crbug.com/65331
- base::ThreadRestrictions::ScopedAllowIO allow_io;
- PlatformThread::Join(thread_);
+ void Start() { StartJob(); }
+ void Cancel() {
+ CancelJob();
}
-}
-bool DirectoryLister::Start() {
- // spawn a thread to enumerate the specified directory
+ private:
+ friend class base::RefCountedThreadSafe<Job>;
- // pass events back to the current thread
- message_loop_ = MessageLoop::current();
- DCHECK(message_loop_) << "calling thread must have a message loop";
+ virtual ~Job();
- AddRef(); // the thread will release us when it is done
+ virtual void RunJob();
+ virtual void OnJobCompleted();
- if (!PlatformThread::Create(0, this, &thread_)) {
- Release();
- return false;
- }
+ // Construction parameters.
+ const FilePath dir_;
+ const bool recursive_;
+ const SORT_TYPE sort_;
+ Delegate* const delegate_;
- return true;
-}
+ // Job state to pass to delegate.
+ int error_;
+ std::vector<Data> data_;
+};
-void DirectoryLister::Cancel() {
- canceled_.Set();
-
- if (thread_) {
- // This is a bug and we should stop joining this thread.
- // http://crbug.com/65331
- base::ThreadRestrictions::ScopedAllowIO allow_io;
- PlatformThread::Join(thread_);
- thread_ = kNullThreadHandle;
- }
+DirectoryLister::Job::Job(const FilePath& dir,
+ bool recursive,
+ SORT_TYPE sort,
+ Delegate* delegate)
+ : dir_(dir),
+ recursive_(recursive),
+ sort_(sort),
+ delegate_(delegate),
+ error_(OK) {
+ DCHECK(!dir.value().empty());
+ // Allocations of the FindInfo aren't super cheap, so reserve space.
+ data_.reserve(64);
}
-void DirectoryLister::ThreadMain() {
- DirectoryDataEvent* e = new DirectoryDataEvent(this);
+DirectoryLister::Job::~Job() {}
+void DirectoryLister::Job::RunJob() {
if (!file_util::DirectoryExists(dir_)) {
- e->error = net::ERR_FILE_NOT_FOUND;
- message_loop_->PostTask(FROM_HERE, e);
- Release();
+ error_ = net::ERR_FILE_NOT_FOUND;
return;
}
@@ -178,11 +140,11 @@ void DirectoryLister::ThreadMain() {
static_cast<file_util::FileEnumerator::FILE_TYPE>(types));
FilePath path;
- while (!canceled_.IsSet() && !(path = file_enum.Next()).empty()) {
- DirectoryListerData data;
+ while (!canceled() && !(path = file_enum.Next()).empty()) {
+ Data data;
file_enum.GetFindInfo(&data.info);
data.path = path;
- e->data.push_back(data);
+ data_.push_back(data);
/* TODO(brettw) bug 24107: It would be nice to send incremental updates.
We gather them all so they can be sorted, but eventually the sorting
@@ -195,45 +157,48 @@ void DirectoryLister::ThreadMain() {
*/
}
- if (!e->data.empty()) {
+ if (!data_.empty()) {
// Sort the results. See the TODO above (this sort should be removed and we
// should do it from JS).
if (sort_ == DATE)
- std::sort(e->data.begin(), e->data.end(), CompareDate);
+ std::sort(data_.begin(), data_.end(), CompareDate);
else if (sort_ == FULL_PATH)
- std::sort(e->data.begin(), e->data.end(), CompareFullPath);
+ std::sort(data_.begin(), data_.end(), CompareFullPath);
else if (sort_ == ALPHA_DIRS_FIRST)
- std::sort(e->data.begin(), e->data.end(), CompareAlphaDirsFirst);
+ std::sort(data_.begin(), data_.end(), CompareAlphaDirsFirst);
else
DCHECK_EQ(NO_SORT, sort_);
+ }
+}
- message_loop_->PostTask(FROM_HERE, e);
- e = new DirectoryDataEvent(this);
+void DirectoryLister::Job::OnJobCompleted() {
+ for (size_t i = 0; i < data_.size(); ++i) {
+ delegate_->OnListFile(data_[i]);
+ if (canceled())
+ return;
}
- // Notify done
- Release();
- message_loop_->PostTask(FROM_HERE, e);
+ delegate_->OnListDone(error_);
}
-void DirectoryLister::OnReceivedData(const DirectoryListerData* data,
- int count) {
- // Since the delegate can clear itself during the OnListFile callback, we
- // need to null check it during each iteration of the loop. Similarly, it is
- // necessary to check the canceled_ flag to avoid sending data to a delegate
- // who doesn't want anymore.
- for (int i = 0; !canceled_.IsSet() && delegate_ && i < count; ++i)
- delegate_->OnListFile(data[i]);
-}
+DirectoryLister::DirectoryLister(const FilePath& dir,
+ Delegate* delegate)
+ : job_handle_(new Job(dir, false, ALPHA_DIRS_FIRST, delegate)) {}
-void DirectoryLister::OnDone(int error) {
- // If canceled is set, we need to report some kind of error,
- // but don't overwrite the error condition if it is already set.
- if (!error && canceled_.IsSet())
- error = net::ERR_ABORTED;
+DirectoryLister::DirectoryLister(const FilePath& dir,
+ bool recursive,
+ SORT_TYPE sort,
+ Delegate* delegate)
+ : job_handle_(new Job(dir, recursive, sort, delegate)) {}
+
+DirectoryLister::~DirectoryLister() {}
- if (delegate_)
- delegate_->OnListDone(error);
+void DirectoryLister::Start() {
+ job_handle_.job()->Start();
+}
+
+void DirectoryLister::Cancel() {
+ job_handle_.job()->Cancel();
}
} // namespace net

Powered by Google App Engine
This is Rietveld 408576698