diff options
-rw-r--r-- | base/i18n/file_util_icu.cc | 6 | ||||
-rw-r--r-- | chrome/browser/file_select_helper.cc | 15 | ||||
-rw-r--r-- | chrome/browser/ui/webui/active_downloads_ui.cc | 26 | ||||
-rw-r--r-- | net/base/directory_lister.cc | 305 | ||||
-rw-r--r-- | net/base/directory_lister.h | 77 | ||||
-rw-r--r-- | net/base/directory_lister_unittest.cc | 54 | ||||
-rw-r--r-- | net/url_request/url_request_file_dir_job.cc | 46 | ||||
-rw-r--r-- | net/url_request/url_request_file_dir_job.h | 4 |
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_; |