summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
-rw-r--r--base/i18n/file_util_icu.cc6
-rw-r--r--chrome/browser/file_select_helper.cc15
-rw-r--r--chrome/browser/ui/webui/active_downloads_ui.cc26
-rw-r--r--net/base/directory_lister.cc305
-rw-r--r--net/base/directory_lister.h77
-rw-r--r--net/base/directory_lister_unittest.cc54
-rw-r--r--net/url_request/url_request_file_dir_job.cc46
-rw-r--r--net/url_request/url_request_file_dir_job.h4
8 files changed, 263 insertions, 270 deletions
diff --git a/base/i18n/file_util_icu.cc b/base/i18n/file_util_icu.cc
index 9d12097..0c7c09d 100644
--- a/base/i18n/file_util_icu.cc
+++ b/base/i18n/file_util_icu.cc
@@ -81,7 +81,8 @@ IllegalCharacters::IllegalCharacters() {
class LocaleAwareComparator {
public:
static LocaleAwareComparator* GetInstance() {
- return Singleton<LocaleAwareComparator>::get();
+ return Singleton<LocaleAwareComparator,
+ LeakySingletonTraits<LocaleAwareComparator> >::get();
}
// Note: A similar function is available in l10n_util.
@@ -104,6 +105,8 @@ class LocaleAwareComparator {
}
private:
+ friend struct DefaultSingletonTraits<LocaleAwareComparator>;
+
LocaleAwareComparator() {
UErrorCode error_code = U_ZERO_ERROR;
// Use the default collator. The default locale should have been properly
@@ -121,7 +124,6 @@ class LocaleAwareComparator {
scoped_ptr<icu::Collator> collator_;
base::Lock lock_;
- friend struct DefaultSingletonTraits<LocaleAwareComparator>;
DISALLOW_COPY_AND_ASSIGN(LocaleAwareComparator);
};
diff --git a/chrome/browser/file_select_helper.cc b/chrome/browser/file_select_helper.cc
index 3ee8c5a..985b669 100644
--- a/chrome/browser/file_select_helper.cc
+++ b/chrome/browser/file_select_helper.cc
@@ -38,7 +38,7 @@ struct FileSelectHelper::ActiveDirectoryEnumeration {
ActiveDirectoryEnumeration() {}
scoped_ptr<DirectoryListerDispatchDelegate> delegate_;
- scoped_refptr<net::DirectoryLister> lister_;
+ scoped_ptr<net::DirectoryLister> lister_;
RenderViewHost* rvh_;
std::vector<FilePath> results_;
};
@@ -62,10 +62,7 @@ FileSelectHelper::~FileSelectHelper() {
for (iter = directory_enumerations_.begin();
iter != directory_enumerations_.end();
++iter) {
- if (iter->second->lister_.get()) {
- iter->second->lister_->set_delegate(NULL);
- iter->second->lister_->Cancel();
- }
+ iter->second->lister_.reset();
delete iter->second;
}
}
@@ -119,10 +116,10 @@ void FileSelectHelper::StartNewEnumeration(const FilePath& path,
scoped_ptr<ActiveDirectoryEnumeration> entry(new ActiveDirectoryEnumeration);
entry->rvh_ = render_view_host;
entry->delegate_.reset(new DirectoryListerDispatchDelegate(this, request_id));
- entry->lister_ = new net::DirectoryLister(path,
- true,
- net::DirectoryLister::NO_SORT,
- entry->delegate_.get());
+ entry->lister_.reset(new net::DirectoryLister(path,
+ true,
+ net::DirectoryLister::NO_SORT,
+ entry->delegate_.get()));
if (!entry->lister_->Start()) {
if (request_id == kFileSelectEnumerationId)
FileSelectionCanceled(NULL);
diff --git a/chrome/browser/ui/webui/active_downloads_ui.cc b/chrome/browser/ui/webui/active_downloads_ui.cc
index cc6410d..2d516d7 100644
--- a/chrome/browser/ui/webui/active_downloads_ui.cc
+++ b/chrome/browser/ui/webui/active_downloads_ui.cc
@@ -15,6 +15,7 @@
#include "base/memory/weak_ptr.h"
#include "base/message_loop.h"
#include "base/path_service.h"
+#include "base/scoped_ptr.h"
#include "base/string_piece.h"
#include "base/string_util.h"
#include "base/threading/thread.h"
@@ -159,7 +160,7 @@ class ActiveDownloadsHandler
TabContents* tab_contents_;
std::string current_file_contents_;
TaskProxy* current_task_;
- scoped_refptr<net::DirectoryLister> lister_;
+ scoped_ptr<net::DirectoryLister> lister_;
bool is_refresh_;
DownloadManager* download_manager_;
@@ -310,15 +311,9 @@ ActiveDownloadsHandler::ActiveDownloadsHandler()
tab_contents_(NULL),
is_refresh_(false),
download_manager_(NULL) {
- lister_ = NULL;
}
ActiveDownloadsHandler::~ActiveDownloadsHandler() {
- if (lister_.get()) {
- lister_->Cancel();
- lister_->set_delegate(NULL);
- }
-
ClearDownloadItems();
download_manager_->RemoveObserver(this);
}
@@ -471,12 +466,7 @@ void ActiveDownloadsHandler::GetChildrenForPath(const FilePath& path,
filelist_value_.reset(new ListValue());
currentpath_ = path;
- if (lister_.get()) {
- lister_->Cancel();
- lister_->set_delegate(NULL);
- lister_ = NULL;
- }
-
+ lister_.reset();
is_refresh_ = is_refresh;
#if defined(OS_CHROMEOS)
@@ -492,12 +482,12 @@ void ActiveDownloadsHandler::GetChildrenForPath(const FilePath& path,
}
if (currentpath_ == default_download_path) {
- lister_ = new net::DirectoryLister(currentpath_,
- false,
- net::DirectoryLister::DATE,
- this);
+ lister_.reset(new net::DirectoryLister(currentpath_,
+ false,
+ net::DirectoryLister::DATE,
+ this));
} else {
- lister_ = new net::DirectoryLister(currentpath_, this);
+ lister_.reset(new net::DirectoryLister(currentpath_, this));
}
// lister_->Start();
OnListDone(0);
diff --git a/net/base/directory_lister.cc b/net/base/directory_lister.cc
index 621461f..dc41756 100644
--- a/net/base/directory_lister.cc
+++ b/net/base/directory_lister.cc
@@ -10,95 +10,183 @@
#include "base/file_util.h"
#include "base/i18n/file_util_icu.h"
#include "base/message_loop.h"
-#include "base/threading/platform_thread.h"
#include "base/threading/thread_restrictions.h"
+#include "base/threading/worker_pool.h"
#include "net/base/net_errors.h"
namespace net {
-static const int kFilesPerEvent = 8;
+namespace {
+
+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 CompareAlphaDirsFirst(const DirectoryLister::DirectoryListerData& a,
+ const DirectoryLister::DirectoryListerData& b) {
+ // Parent directory before all else.
+ if (file_util::IsDotDot(file_util::FileEnumerator::GetFilename(a.info)))
+ return true;
+ if (file_util::IsDotDot(file_util::FileEnumerator::GetFilename(b.info)))
+ return false;
+
+ // Directories before regular files.
+ bool a_is_directory = file_util::FileEnumerator::IsDirectory(a.info);
+ bool b_is_directory = file_util::FileEnumerator::IsDirectory(b.info);
+ if (a_is_directory != b_is_directory)
+ return a_is_directory;
+
+ return file_util::LocaleAwareCompareFilenames(
+ file_util::FileEnumerator::GetFilename(a.info),
+ file_util::FileEnumerator::GetFilename(b.info));
+}
+
+bool CompareDate(const DirectoryLister::DirectoryListerData& a,
+ const DirectoryLister::DirectoryListerData& b) {
+ // Parent directory before all else.
+ if (file_util::IsDotDot(file_util::FileEnumerator::GetFilename(a.info)))
+ return true;
+ if (file_util::IsDotDot(file_util::FileEnumerator::GetFilename(b.info)))
+ return false;
+
+ // Directories before regular files.
+ bool a_is_directory = file_util::FileEnumerator::IsDirectory(a.info);
+ bool b_is_directory = file_util::FileEnumerator::IsDirectory(b.info);
+ if (a_is_directory != b_is_directory)
+ return a_is_directory;
+#if defined(OS_POSIX)
+ return a.info.stat.st_mtime > b.info.stat.st_mtime;
+#elif defined(OS_WIN)
+ if (a.info.ftLastWriteTime.dwHighDateTime ==
+ b.info.ftLastWriteTime.dwHighDateTime) {
+ return a.info.ftLastWriteTime.dwLowDateTime >
+ b.info.ftLastWriteTime.dwLowDateTime;
+ } else {
+ return a.info.ftLastWriteTime.dwHighDateTime >
+ b.info.ftLastWriteTime.dwHighDateTime;
+ }
+#endif
+}
+
+// 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 CompareFullPath(const DirectoryLister::DirectoryListerData& a,
+ const DirectoryLister::DirectoryListerData& b) {
+ return file_util::LocaleAwareCompareFilenames(a.path, b.path);
+}
+
+} // namespace
// A task which is used to signal the delegate asynchronously.
-class DirectoryDataEvent : public Task {
+class DirectoryLister::Core::DataEvent : public Task {
public:
- explicit DirectoryDataEvent(DirectoryLister* d) : lister(d), error(0) {
+ explicit DataEvent(Core* core) : core_(core), error_(OK) {
// Allocations of the FindInfo aren't super cheap, so reserve space.
- data.reserve(64);
+ data_.reserve(64);
}
- void Run() {
- if (data.empty()) {
- lister->OnDone(error);
+ virtual void Run() {
+ DCHECK(core_->origin_loop_->BelongsToCurrentThread());
+ if (data_.empty()) {
+ core_->OnDone(error_);
return;
}
- lister->OnReceivedData(&data[0], static_cast<int>(data.size()));
+ core_->OnReceivedData(&data_[0], static_cast<int>(data_.size()));
}
- scoped_refptr<DirectoryLister> lister;
- std::vector<DirectoryLister::DirectoryListerData> data;
- int error;
+ void set_error(int error) { error_ = error; }
+
+ void AddData(const DirectoryListerData& data) {
+ data_.push_back(data);
+ }
+
+ bool HasData() const {
+ return !data_.empty();
+ }
+
+ void SortData(SortType sort_type) {
+ // Sort the results. See the TODO below (this sort should be removed and we
+ // should do it from JS).
+ if (sort_type == DATE)
+ std::sort(data_.begin(), data_.end(), CompareDate);
+ else if (sort_type == FULL_PATH)
+ std::sort(data_.begin(), data_.end(), CompareFullPath);
+ else if (sort_type == ALPHA_DIRS_FIRST)
+ std::sort(data_.begin(), data_.end(), CompareAlphaDirsFirst);
+ else
+ DCHECK_EQ(NO_SORT, sort_type);
+ }
+
+ private:
+ scoped_refptr<Core> core_;
+ std::vector<DirectoryListerData> data_;
+ int error_;
};
DirectoryLister::DirectoryLister(const FilePath& dir,
DirectoryListerDelegate* delegate)
- : dir_(dir),
- recursive_(false),
- delegate_(delegate),
- sort_(ALPHA_DIRS_FIRST),
- message_loop_(NULL),
- thread_(base::kNullThreadHandle) {
+ : ALLOW_THIS_IN_INITIALIZER_LIST(
+ core_(new Core(dir, false, ALPHA_DIRS_FIRST, this))),
+ delegate_(delegate) {
+ DCHECK(delegate_);
DCHECK(!dir.value().empty());
}
DirectoryLister::DirectoryLister(const FilePath& dir,
bool recursive,
- SORT_TYPE sort,
+ SortType sort,
DirectoryListerDelegate* delegate)
- : dir_(dir),
- recursive_(recursive),
- delegate_(delegate),
- sort_(sort),
- message_loop_(NULL),
- thread_(base::kNullThreadHandle) {
+ : ALLOW_THIS_IN_INITIALIZER_LIST(
+ core_(new Core(dir, recursive, sort, this))),
+ delegate_(delegate) {
+ DCHECK(delegate_);
DCHECK(!dir.value().empty());
}
+DirectoryLister::~DirectoryLister() {
+ Cancel();
+}
+
bool DirectoryLister::Start() {
- // spawn a thread to enumerate the specified directory
+ return core_->Start();
+}
- // pass events back to the current thread
- message_loop_ = MessageLoop::current();
- DCHECK(message_loop_) << "calling thread must have a message loop";
+void DirectoryLister::Cancel() {
+ return core_->Cancel();
+}
- AddRef(); // the thread will release us when it is done
+DirectoryLister::Core::Core(const FilePath& dir,
+ bool recursive,
+ SortType sort,
+ DirectoryLister* lister)
+ : dir_(dir),
+ recursive_(recursive),
+ sort_(sort),
+ lister_(lister) {
+ DCHECK(lister_);
+}
- if (!base::PlatformThread::Create(0, this, &thread_)) {
- Release();
- return false;
- }
+DirectoryLister::Core::~Core() {}
- return true;
+bool DirectoryLister::Core::Start() {
+ origin_loop_ = base::MessageLoopProxy::CreateForCurrentThread();
+
+ return base::WorkerPool::PostTask(
+ FROM_HERE, NewRunnableMethod(this, &Core::StartInternal), true);
}
-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;
- base::PlatformThread::Join(thread_);
- thread_ = base::kNullThreadHandle;
- }
+void DirectoryLister::Core::Cancel() {
+ lister_ = NULL;
}
-void DirectoryLister::ThreadMain() {
- DirectoryDataEvent* e = new DirectoryDataEvent(this);
+void DirectoryLister::Core::StartInternal() {
+ DataEvent* e = new DataEvent(this);
if (!file_util::DirectoryExists(dir_)) {
- e->error = ERR_FILE_NOT_FOUND;
- message_loop_->PostTask(FROM_HERE, e);
- Release();
+ e->set_error(ERR_FILE_NOT_FOUND);
+ origin_loop_->PostTask(FROM_HERE, e);
return;
}
@@ -111,11 +199,11 @@ void DirectoryLister::ThreadMain() {
static_cast<file_util::FileEnumerator::FILE_TYPE>(types));
FilePath path;
- while (!canceled_.IsSet() && !(path = file_enum.Next()).empty()) {
+ while (lister_ && !(path = file_enum.Next()).empty()) {
DirectoryListerData data;
file_enum.GetFindInfo(&data.info);
data.path = path;
- e->data.push_back(data);
+ e->AddData(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
@@ -123,117 +211,40 @@ void DirectoryLister::ThreadMain() {
that, we can uncomment this to send incremental updates to the page.
if (++e->count == kFilesPerEvent) {
message_loop_->PostTask(FROM_HERE, e);
- e = new DirectoryDataEvent(this);
+ e = new DataEvent(this);
}
*/
}
- if (!e->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);
- else if (sort_ == FULL_PATH)
- std::sort(e->data.begin(), e->data.end(), CompareFullPath);
- else if (sort_ == ALPHA_DIRS_FIRST)
- std::sort(e->data.begin(), e->data.end(), CompareAlphaDirsFirst);
- else
- DCHECK_EQ(NO_SORT, sort_);
-
- message_loop_->PostTask(FROM_HERE, e);
- e = new DirectoryDataEvent(this);
+ if (e->HasData()) {
+ e->SortData(sort_);
+ origin_loop_->PostTask(FROM_HERE, e);
+ e = new DataEvent(this);
}
// Notify done
- Release();
- message_loop_->PostTask(FROM_HERE, e);
-}
-
-DirectoryLister::~DirectoryLister() {
- if (thread_) {
- // This is a bug and we should stop joining this thread.
- // http://crbug.com/65331
- base::ThreadRestrictions::ScopedAllowIO allow_io;
- base::PlatformThread::Join(thread_);
- }
-}
-
-// 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) {
- // Parent directory before all else.
- if (file_util::IsDotDot(file_util::FileEnumerator::GetFilename(a.info)))
- return true;
- if (file_util::IsDotDot(file_util::FileEnumerator::GetFilename(b.info)))
- return false;
-
- // Directories before regular files.
- bool a_is_directory = file_util::FileEnumerator::IsDirectory(a.info);
- bool b_is_directory = file_util::FileEnumerator::IsDirectory(b.info);
- if (a_is_directory != b_is_directory)
- return a_is_directory;
-
- return file_util::LocaleAwareCompareFilenames(
- file_util::FileEnumerator::GetFilename(a.info),
- file_util::FileEnumerator::GetFilename(b.info));
+ origin_loop_->PostTask(FROM_HERE, e);
}
-// Static.
-bool DirectoryLister::CompareDate(const DirectoryListerData& a,
- const DirectoryListerData& b) {
- // Parent directory before all else.
- if (file_util::IsDotDot(file_util::FileEnumerator::GetFilename(a.info)))
- return true;
- if (file_util::IsDotDot(file_util::FileEnumerator::GetFilename(b.info)))
- return false;
-
- // Directories before regular files.
- bool a_is_directory = file_util::FileEnumerator::IsDirectory(a.info);
- bool b_is_directory = file_util::FileEnumerator::IsDirectory(b.info);
- if (a_is_directory != b_is_directory)
- return a_is_directory;
-#if defined(OS_POSIX)
- return a.info.stat.st_mtime > b.info.stat.st_mtime;
-#elif defined(OS_WIN)
- if (a.info.ftLastWriteTime.dwHighDateTime ==
- b.info.ftLastWriteTime.dwHighDateTime) {
- return a.info.ftLastWriteTime.dwLowDateTime >
- b.info.ftLastWriteTime.dwLowDateTime;
- } else {
- return a.info.ftLastWriteTime.dwHighDateTime >
- b.info.ftLastWriteTime.dwHighDateTime;
- }
-#endif
+void DirectoryLister::Core::OnReceivedData(const DirectoryListerData* data,
+ int count) {
+ // We need to check for cancellation (indicated by NULL'ing of |lister_|),
+ // which can happen during each callback.
+ for (int i = 0; lister_ && i < count; ++i)
+ lister_->OnReceivedData(data[i]);
}
-// 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) {
- return file_util::LocaleAwareCompareFilenames(a.path, b.path);
+void DirectoryLister::Core::OnDone(int error) {
+ if (lister_)
+ lister_->OnDone(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]);
+void DirectoryLister::OnReceivedData(const DirectoryListerData& data) {
+ delegate_->OnListFile(data);
}
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 = ERR_ABORTED;
-
- if (delegate_)
- delegate_->OnListDone(error);
+ delegate_->OnListDone(error);
}
} // namespace net
diff --git a/net/base/directory_lister.h b/net/base/directory_lister.h
index dbcb4f5..29d2691 100644
--- a/net/base/directory_lister.h
+++ b/net/base/directory_lister.h
@@ -10,12 +10,8 @@
#include "base/file_path.h"
#include "base/file_util.h"
+#include "base/message_loop_proxy.h"
#include "base/memory/ref_counted.h"
-#include "base/synchronization/cancellation_flag.h"
-#include "base/task.h"
-#include "base/threading/platform_thread.h"
-
-class MessageLoop;
namespace net {
@@ -26,8 +22,7 @@ namespace net {
// structs over to the main application thread. The consumer of this class
// is insulated from any of the multi-threading details.
//
-class DirectoryLister : public base::RefCountedThreadSafe<DirectoryLister>,
- public base::PlatformThread::Delegate {
+class DirectoryLister {
public:
// Represents one file found.
struct DirectoryListerData {
@@ -53,7 +48,7 @@ class DirectoryLister : public base::RefCountedThreadSafe<DirectoryLister>,
// directories first in name order, then files by name order
// FULL_PATH sorts by paths as strings, ignoring files v. directories
// DATE sorts by last modified date
- enum SORT_TYPE {
+ enum SortType {
NO_SORT,
DATE,
ALPHA_DIRS_FIRST,
@@ -65,49 +60,61 @@ class DirectoryLister : public base::RefCountedThreadSafe<DirectoryLister>,
DirectoryLister(const FilePath& dir,
bool recursive,
- SORT_TYPE sort,
+ SortType sort,
DirectoryListerDelegate* delegate);
+ // Will invoke Cancel().
+ ~DirectoryLister();
// Call this method to start the directory enumeration thread.
bool Start();
// Call this method to asynchronously stop directory enumeration. The
- // delegate will receive the OnListDone notification with an error code of
- // ERR_ABORTED.
+ // delegate will not be called back.
void Cancel();
- // The delegate pointer may be modified at any time.
- DirectoryListerDelegate* delegate() const { return delegate_; }
- void set_delegate(DirectoryListerDelegate* d) { delegate_ = d; }
+ private:
+ class Core : public base::RefCountedThreadSafe<Core> {
+ public:
+ Core(const FilePath& dir,
+ bool recursive,
+ SortType sort,
+ DirectoryLister* lister);
- // PlatformThread::Delegate implementation
- virtual void ThreadMain();
+ bool Start();
- private:
- friend class base::RefCountedThreadSafe<DirectoryLister>;
- friend class DirectoryDataEvent;
+ void Cancel();
- ~DirectoryLister();
+ private:
+ friend class base::RefCountedThreadSafe<Core>;
+ class DataEvent;
+
+ ~Core();
- // Comparison methods for sorting, chosen based on |sort_|.
- static bool CompareAlphaDirsFirst(const DirectoryListerData& a,
- const DirectoryListerData& b);
- static bool CompareDate(const DirectoryListerData& a,
- const DirectoryListerData& b);
- static bool CompareFullPath(const DirectoryListerData& a,
- const DirectoryListerData& b);
+ // Runs on a WorkerPool thread.
+ void StartInternal();
- void OnReceivedData(const DirectoryListerData* data, int count);
+ void OnReceivedData(const DirectoryListerData* data, int count);
+ void OnDone(int error);
+
+ FilePath dir_;
+ bool recursive_;
+ SortType sort_;
+ scoped_refptr<base::MessageLoopProxy> origin_loop_;
+
+ // |lister_| gets set to NULL when canceled.
+ DirectoryLister* lister_;
+
+ DISALLOW_COPY_AND_ASSIGN(Core);
+ };
+
+ void OnReceivedData(const DirectoryListerData& data);
void OnDone(int error);
- FilePath dir_;
- bool recursive_;
- DirectoryListerDelegate* delegate_;
- SORT_TYPE sort_;
- MessageLoop* message_loop_;
- base::PlatformThreadHandle thread_;
- base::CancellationFlag canceled_;
+ const scoped_refptr<Core> core_;
+ DirectoryListerDelegate* const delegate_;
+
+ DISALLOW_COPY_AND_ASSIGN(DirectoryLister);
};
} // namespace net
diff --git a/net/base/directory_lister_unittest.cc b/net/base/directory_lister_unittest.cc
index 91c8224..7f598a8 100644
--- a/net/base/directory_lister_unittest.cc
+++ b/net/base/directory_lister_unittest.cc
@@ -16,12 +16,20 @@ namespace net {
class ListerDelegate : public DirectoryLister::DirectoryListerDelegate {
public:
- explicit ListerDelegate(bool recursive) : error_(-1), recursive_(recursive) {
+ ListerDelegate(bool recursive,
+ bool quit_loop_after_each_file)
+ : error_(-1),
+ recursive_(recursive),
+ quit_loop_after_each_file_(quit_loop_after_each_file) {
}
+
void OnListFile(const DirectoryLister::DirectoryListerData& data) {
file_list_.push_back(data.info);
paths_.push_back(data.path);
+ if (quit_loop_after_each_file_)
+ MessageLoop::current()->Quit();
}
+
void OnListDone(int error) {
error_ = error;
MessageLoop::current()->Quit();
@@ -30,6 +38,7 @@ class ListerDelegate : public DirectoryLister::DirectoryListerDelegate {
else
CheckSort();
}
+
void CheckRecursiveSort() {
// Check that we got files in the right order.
if (!file_list_.empty()) {
@@ -41,6 +50,7 @@ class ListerDelegate : public DirectoryLister::DirectoryListerDelegate {
}
}
}
+
void CheckSort() {
// Check that we got files in the right order.
if (!file_list_.empty()) {
@@ -62,10 +72,15 @@ class ListerDelegate : public DirectoryLister::DirectoryListerDelegate {
}
}
}
+
int error() const { return error_; }
+
+ int num_files() const { return file_list_.size(); }
+
private:
int error_;
bool recursive_;
+ bool quit_loop_after_each_file_;
std::vector<file_util::FileEnumerator::FindInfo> file_list_;
std::vector<FilePath> paths_;
};
@@ -74,46 +89,45 @@ TEST(DirectoryListerTest, BigDirTest) {
FilePath path;
ASSERT_TRUE(PathService::Get(base::DIR_SOURCE_ROOT, &path));
- ListerDelegate delegate(false);
- scoped_refptr<DirectoryLister> lister(
- new DirectoryLister(path, &delegate));
-
- lister->Start();
+ ListerDelegate delegate(false, false);
+ DirectoryLister lister(path, &delegate);
+ lister.Start();
MessageLoop::current()->Run();
- EXPECT_EQ(delegate.error(), OK);
+ EXPECT_EQ(OK, delegate.error());
}
TEST(DirectoryListerTest, BigDirRecursiveTest) {
FilePath path;
ASSERT_TRUE(PathService::Get(base::DIR_EXE, &path));
- ListerDelegate delegate(true);
- scoped_refptr<DirectoryLister> lister(
- new DirectoryLister(path, true, DirectoryLister::FULL_PATH, &delegate));
-
- lister->Start();
+ ListerDelegate delegate(true, false);
+ DirectoryLister lister(path, true, DirectoryLister::FULL_PATH, &delegate);
+ lister.Start();
MessageLoop::current()->Run();
- EXPECT_EQ(delegate.error(), OK);
+ EXPECT_EQ(OK, delegate.error());
}
TEST(DirectoryListerTest, CancelTest) {
FilePath path;
ASSERT_TRUE(PathService::Get(base::DIR_SOURCE_ROOT, &path));
- ListerDelegate delegate(false);
- scoped_refptr<DirectoryLister> lister(
- new DirectoryLister(path, &delegate));
-
- lister->Start();
- lister->Cancel();
+ ListerDelegate delegate(false, true);
+ DirectoryLister lister(path, &delegate);
+ lister.Start();
MessageLoop::current()->Run();
- EXPECT_EQ(delegate.error(), ERR_ABORTED);
+ int num_files = delegate.num_files();
+
+ lister.Cancel();
+
+ MessageLoop::current()->RunAllPending();
+
+ EXPECT_EQ(num_files, delegate.num_files());
}
} // namespace net
diff --git a/net/url_request/url_request_file_dir_job.cc b/net/url_request/url_request_file_dir_job.cc
index 1cc15be..c47e800 100644
--- a/net/url_request/url_request_file_dir_job.cc
+++ b/net/url_request/url_request_file_dir_job.cc
@@ -1,4 +1,4 @@
-// Copyright (c) 2010 The Chromium Authors. All rights reserved.
+// 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.
@@ -12,6 +12,7 @@
#include "base/time.h"
#include "googleurl/src/gurl.h"
#include "net/base/io_buffer.h"
+#include "net/base/net_errors.h"
#include "net/base/net_util.h"
#include "net/url_request/url_request.h"
@@ -24,6 +25,7 @@ namespace net {
URLRequestFileDirJob::URLRequestFileDirJob(URLRequest* request,
const FilePath& dir_path)
: URLRequestJob(request),
+ ALLOW_THIS_IN_INITIALIZER_LIST(lister_(dir_path, this)),
dir_path_(dir_path),
canceled_(false),
list_complete_(false),
@@ -34,17 +36,7 @@ URLRequestFileDirJob::URLRequestFileDirJob(URLRequest* request,
}
void URLRequestFileDirJob::StartAsync() {
- DCHECK(!lister_);
-
- // TODO(willchan): This is stupid. We should tell |lister_| not to call us
- // back. Fix this stupidity.
-
- // AddRef so that *this* cannot be destroyed while the lister_
- // is trying to feed us data.
-
- AddRef();
- lister_ = new DirectoryLister(dir_path_, this);
- lister_->Start();
+ lister_.Start();
NotifyHeadersComplete();
}
@@ -64,11 +56,8 @@ void URLRequestFileDirJob::Kill() {
canceled_ = true;
- // Don't call CloseLister or dispatch an error to the URLRequest because
- // we want OnListDone to be called to also write the error to the output
- // stream. OnListDone will notify the URLRequest at this time.
- if (lister_)
- lister_->Cancel();
+ if (!list_complete_)
+ lister_.Cancel();
URLRequestJob::Kill();
@@ -153,34 +142,17 @@ void URLRequestFileDirJob::OnListFile(
}
void URLRequestFileDirJob::OnListDone(int error) {
- CloseLister();
-
- if (canceled_) {
- read_pending_ = false;
- // No need for NotifyCanceled() since canceled_ is set inside Kill().
- } else if (error) {
+ DCHECK(!canceled_);
+ if (error != OK) {
read_pending_ = false;
NotifyDone(URLRequestStatus(URLRequestStatus::FAILED, error));
} else {
list_complete_ = true;
CompleteRead();
}
-
- Release(); // The Lister is finished; may delete *this*
-}
-
-URLRequestFileDirJob::~URLRequestFileDirJob() {
- DCHECK(read_pending_ == false);
- DCHECK(lister_ == NULL);
}
-void URLRequestFileDirJob::CloseLister() {
- if (lister_) {
- lister_->Cancel();
- lister_->set_delegate(NULL);
- lister_ = NULL;
- }
-}
+URLRequestFileDirJob::~URLRequestFileDirJob() {}
void URLRequestFileDirJob::CompleteRead() {
if (read_pending_) {
diff --git a/net/url_request/url_request_file_dir_job.h b/net/url_request/url_request_file_dir_job.h
index f938417..ffc34bc 100644
--- a/net/url_request/url_request_file_dir_job.h
+++ b/net/url_request/url_request_file_dir_job.h
@@ -1,4 +1,4 @@
-// Copyright (c) 2010 The Chromium Authors. All rights reserved.
+// 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.
@@ -51,7 +51,7 @@ class URLRequestFileDirJob
// Fills a buffer with the output.
bool FillReadBuffer(char *buf, int buf_size, int *bytes_read);
- scoped_refptr<DirectoryLister> lister_;
+ DirectoryLister lister_;
FilePath dir_path_;
std::string data_;
bool canceled_;