diff options
Diffstat (limited to 'chromium/base/files')
34 files changed, 2565 insertions, 1362 deletions
diff --git a/chromium/base/files/OWNERS b/chromium/base/files/OWNERS index 7260260bb41..b99e8a2fc7a 100644 --- a/chromium/base/files/OWNERS +++ b/chromium/base/files/OWNERS @@ -1,2 +1,3 @@ -# for file_path_watcher* -mnissler@chromium.org +rvargas@chromium.org + +per-file file_path_watcher*=mnissler@chromium.org diff --git a/chromium/base/files/file.cc b/chromium/base/files/file.cc index 4902f15a2ff..bfe87bdcc4d 100644 --- a/chromium/base/files/file.cc +++ b/chromium/base/files/file.cc @@ -20,7 +20,7 @@ File::Info::~Info() { File::File() : file_(kInvalidPlatformFileValue), - error_(FILE_OK), + error_details_(FILE_ERROR_FAILED), created_(false), async_(false) { } @@ -28,25 +28,39 @@ File::File() #if !defined(OS_NACL) File::File(const FilePath& name, uint32 flags) : file_(kInvalidPlatformFileValue), - error_(FILE_OK), + error_details_(FILE_OK), created_(false), async_(false) { - if (name.ReferencesParent()) { - error_ = FILE_ERROR_ACCESS_DENIED; - return; - } - CreateBaseFileUnsafe(name, flags); + Initialize(name, flags); } #endif +File::File(PlatformFile platform_file) + : file_(platform_file), + error_details_(FILE_OK), + created_(false), + async_(false) { +#if defined(OS_POSIX) + DCHECK_GE(platform_file, -1); +#endif +} + +File::File(Error error_details) + : file_(kInvalidPlatformFileValue), + error_details_(error_details), + created_(false), + async_(false) { +} + File::File(RValue other) : file_(other.object->TakePlatformFile()), - error_(other.object->error()), + error_details_(other.object->error_details()), created_(other.object->created()), async_(other.object->async_) { } File::~File() { + // Go through the AssertIOAllowed logic. Close(); } @@ -54,11 +68,65 @@ File& File::operator=(RValue other) { if (this != other.object) { Close(); SetPlatformFile(other.object->TakePlatformFile()); - error_ = other.object->error(); + error_details_ = other.object->error_details(); created_ = other.object->created(); async_ = other.object->async_; } return *this; } +#if !defined(OS_NACL) +void File::Initialize(const FilePath& name, uint32 flags) { + if (name.ReferencesParent()) { + error_details_ = FILE_ERROR_ACCESS_DENIED; + return; + } + InitializeUnsafe(name, flags); +} +#endif + +std::string File::ErrorToString(Error error) { + switch (error) { + case FILE_OK: + return "FILE_OK"; + case FILE_ERROR_FAILED: + return "FILE_ERROR_FAILED"; + case FILE_ERROR_IN_USE: + return "FILE_ERROR_IN_USE"; + case FILE_ERROR_EXISTS: + return "FILE_ERROR_EXISTS"; + case FILE_ERROR_NOT_FOUND: + return "FILE_ERROR_NOT_FOUND"; + case FILE_ERROR_ACCESS_DENIED: + return "FILE_ERROR_ACCESS_DENIED"; + case FILE_ERROR_TOO_MANY_OPENED: + return "FILE_ERROR_TOO_MANY_OPENED"; + case FILE_ERROR_NO_MEMORY: + return "FILE_ERROR_NO_MEMORY"; + case FILE_ERROR_NO_SPACE: + return "FILE_ERROR_NO_SPACE"; + case FILE_ERROR_NOT_A_DIRECTORY: + return "FILE_ERROR_NOT_A_DIRECTORY"; + case FILE_ERROR_INVALID_OPERATION: + return "FILE_ERROR_INVALID_OPERATION"; + case FILE_ERROR_SECURITY: + return "FILE_ERROR_SECURITY"; + case FILE_ERROR_ABORT: + return "FILE_ERROR_ABORT"; + case FILE_ERROR_NOT_A_FILE: + return "FILE_ERROR_NOT_A_FILE"; + case FILE_ERROR_NOT_EMPTY: + return "FILE_ERROR_NOT_EMPTY"; + case FILE_ERROR_INVALID_URL: + return "FILE_ERROR_INVALID_URL"; + case FILE_ERROR_IO: + return "FILE_ERROR_IO"; + case FILE_ERROR_MAX: + break; + } + + NOTREACHED(); + return ""; +} + } // namespace base diff --git a/chromium/base/files/file.h b/chromium/base/files/file.h index d1e0e8ca587..1913bc7f6c4 100644 --- a/chromium/base/files/file.h +++ b/chromium/base/files/file.h @@ -10,11 +10,15 @@ #include <windows.h> #endif +#if defined(OS_POSIX) +#include <sys/stat.h> +#endif + #include <string> #include "base/base_export.h" #include "base/basictypes.h" -#include "base/files/file_path.h" +#include "base/files/scoped_file.h" #include "base/move.h" #include "base/time/time.h" @@ -24,12 +28,19 @@ namespace base { +class FilePath; + #if defined(OS_WIN) typedef HANDLE PlatformFile; #elif defined(OS_POSIX) typedef int PlatformFile; -#endif +#if defined(OS_BSD) || defined(OS_MACOSX) || defined(OS_NACL) +typedef struct stat stat_wrapper_t; +#else +typedef struct stat64 stat_wrapper_t; +#endif +#endif // defined(OS_POSIX) // Thin wrapper around an OS-level file. // Note that this class does not provide any support for asynchronous IO, other @@ -120,6 +131,10 @@ class BASE_EXPORT File { struct BASE_EXPORT Info { Info(); ~Info(); +#if defined(OS_POSIX) + // Fills this struct with values from |stat_info|. + void FromStat(const stat_wrapper_t& stat_info); +#endif // The size of the file in bytes. Undefined when is_directory is true. int64 size; @@ -149,6 +164,9 @@ class BASE_EXPORT File { // Takes ownership of |platform_file|. explicit File(PlatformFile platform_file); + // Creates an object with a specific error_details code. + explicit File(Error error_details); + // Move constructor for C++03 move emulation of this type. File(RValue other); @@ -157,9 +175,12 @@ class BASE_EXPORT File { // Move operator= for C++03 move emulation of this type. File& operator=(RValue other); + // Creates or opens the given file. + void Initialize(const FilePath& name, uint32 flags); + // Creates or opens the given file, allowing paths with traversal ('..') // components. Use only with extreme care. - void CreateBaseFileUnsafe(const FilePath& name, uint32 flags); + void InitializeUnsafe(const FilePath& name, uint32 flags); bool IsValid() const; @@ -168,10 +189,14 @@ class BASE_EXPORT File { // FLAG_CREATE_ALWAYS), and false otherwise. bool created() const { return created_; } - // Returns the OS result of opening this file. - Error error() const { return error_; } + // Returns the OS result of opening this file. Note that the way to verify + // the success of the operation is to use IsValid(), not this method: + // File file(name, flags); + // if (!file.IsValid()) + // return; + Error error_details() const { return error_details_; } - PlatformFile GetPlatformFile() const { return file_; } + PlatformFile GetPlatformFile() const; PlatformFile TakePlatformFile(); // Destroying this object closes the file automatically. @@ -216,10 +241,13 @@ class BASE_EXPORT File { // platforms. Returns the number of bytes written, or -1 on error. int WriteAtCurrentPosNoBestEffort(const char* data, int size); + // Returns the current size of this file, or a negative number on failure. + int64 GetLength(); + // Truncates the file to the given length. If |length| is greater than the // current size of the file, the file is extended with zeros. If the file // doesn't exist, |false| is returned. - bool Truncate(int64 length); + bool SetLength(int64 length); // Flushes the buffers. bool Flush(); @@ -255,22 +283,27 @@ class BASE_EXPORT File { // Unlock a file previously locked. Error Unlock(); + bool async() const { return async_; } + #if defined(OS_WIN) static Error OSErrorToFileError(DWORD last_error); #elif defined(OS_POSIX) static Error OSErrorToFileError(int saved_errno); #endif + // Converts an error value to a human-readable form. Used for logging. + static std::string ErrorToString(Error error); + private: void SetPlatformFile(PlatformFile file); #if defined(OS_WIN) win::ScopedHandle file_; #elif defined(OS_POSIX) - PlatformFile file_; + ScopedFD file_; #endif - Error error_; + Error error_details_; bool created_; bool async_; }; diff --git a/chromium/base/files/file_path.cc b/chromium/base/files/file_path.cc index 3ea5856a0d5..a8b27139988 100644 --- a/chromium/base/files/file_path.cc +++ b/chromium/base/files/file_path.cc @@ -521,7 +521,7 @@ FilePath FilePath::Append(const FilePath& component) const { } FilePath FilePath::AppendASCII(const StringPiece& component) const { - DCHECK(IsStringASCII(component)); + DCHECK(base::IsStringASCII(component)); #if defined(OS_WIN) return Append(ASCIIToUTF16(component.as_string())); #elif defined(OS_POSIX) @@ -587,7 +587,7 @@ string16 FilePath::LossyDisplayName() const { } std::string FilePath::MaybeAsASCII() const { - if (IsStringASCII(path_)) + if (base::IsStringASCII(path_)) return path_; return std::string(); } @@ -632,9 +632,9 @@ string16 FilePath::LossyDisplayName() const { } std::string FilePath::MaybeAsASCII() const { - if (IsStringASCII(path_)) - return WideToASCII(path_); - return ""; + if (base::IsStringASCII(path_)) + return UTF16ToASCII(path_); + return std::string(); } std::string FilePath::AsUTF8Unsafe() const { @@ -1292,10 +1292,16 @@ void FilePath::StripTrailingSeparatorsInternal() { } FilePath FilePath::NormalizePathSeparators() const { + return NormalizePathSeparatorsTo(kSeparators[0]); +} + +FilePath FilePath::NormalizePathSeparatorsTo(CharType separator) const { #if defined(FILE_PATH_USES_WIN_SEPARATORS) + DCHECK_NE(kSeparators + kSeparatorsLength, + std::find(kSeparators, kSeparators + kSeparatorsLength, separator)); StringType copy = path_; - for (size_t i = 1; i < kSeparatorsLength; ++i) { - std::replace(copy.begin(), copy.end(), kSeparators[i], kSeparators[0]); + for (size_t i = 0; i < kSeparatorsLength; ++i) { + std::replace(copy.begin(), copy.end(), kSeparators[i], separator); } return FilePath(copy); #else diff --git a/chromium/base/files/file_path.h b/chromium/base/files/file_path.h index f4b8ff8ade8..008b9f5afc6 100644 --- a/chromium/base/files/file_path.h +++ b/chromium/base/files/file_path.h @@ -189,6 +189,13 @@ class BASE_EXPORT FilePath { // Returns a vector of all of the components of the provided path. It is // equivalent to calling DirName().value() on the path's root component, // and BaseName().value() on each child component. + // + // To make sure this is lossless so we can differentiate absolute and + // relative paths, the root slash will be included even though no other + // slashes will be. The precise behavior is: + // + // Posix: "/foo/bar" -> [ "/", "foo", "bar" ] + // Windows: "C:\foo\bar" -> [ "C:", "\\", "foo", "bar" ] void GetComponents(std::vector<FilePath::StringType>* components) const; // Returns true if this FilePath is a strict parent of the |child|. Absolute @@ -239,7 +246,7 @@ class BASE_EXPORT FilePath { // TODO(davidben): Check all our extension-sensitive code to see if // we can rename this to Extension() and the other to something like // LongExtension(), defaulting to short extensions and leaving the - // long "extensions" to logic like file_util::GetUniquePathNumber(). + // long "extensions" to logic like base::GetUniquePathNumber(). StringType FinalExtension() const; // Returns "C:\pics\jojo" for path "C:\pics\jojo.jpg" @@ -314,8 +321,8 @@ class BASE_EXPORT FilePath { // separator. FilePath StripTrailingSeparators() const WARN_UNUSED_RESULT; - // Returns true if this FilePath contains any attempt to reference a parent - // directory (i.e. has a path component that is ".." + // Returns true if this FilePath contains an attempt to reference a parent + // directory (e.g. has a path component that is ".."). bool ReferencesParent() const; // Return a Unicode human-readable version of this path. @@ -367,6 +374,10 @@ class BASE_EXPORT FilePath { // (if FILE_PATH_USES_WIN_SEPARATORS is true), or do nothing on POSIX systems. FilePath NormalizePathSeparators() const; + // Normalize all path separattors to given type on Windows + // (if FILE_PATH_USES_WIN_SEPARATORS is true), or do nothing on POSIX systems. + FilePath NormalizePathSeparatorsTo(CharType separator) const; + // Compare two strings in the same way the file system does. // Note that these always ignore case, even on file systems that are case- // sensitive. If case-sensitive comparison is ever needed, add corresponding diff --git a/chromium/base/files/file_path_watcher.cc b/chromium/base/files/file_path_watcher.cc index 49e0a237f69..b17354197d4 100644 --- a/chromium/base/files/file_path_watcher.cc +++ b/chromium/base/files/file_path_watcher.cc @@ -10,6 +10,10 @@ #include "base/logging.h" #include "base/message_loop/message_loop.h" +#if defined(OS_MACOSX) && !defined(OS_IOS) +#include "base/mac/mac_util.h" +#endif + namespace base { FilePathWatcher::~FilePathWatcher() { @@ -22,6 +26,19 @@ void FilePathWatcher::CancelWatch( delegate->CancelOnMessageLoopThread(); } +// static +bool FilePathWatcher::RecursiveWatchAvailable() { +#if defined(OS_MACOSX) && !defined(OS_IOS) + // FSEvents isn't available on iOS and is broken on OSX 10.6 and earlier. + // See http://crbug.com/54822#c31 + return mac::IsOSLionOrLater(); +#elif defined(OS_WIN) || defined(OS_LINUX) + return true; +#else + return false; +#endif +} + FilePathWatcher::PlatformDelegate::PlatformDelegate(): cancelled_(false) { } diff --git a/chromium/base/files/file_path_watcher.h b/chromium/base/files/file_path_watcher.h index 3c1941f012b..b90efa1800f 100644 --- a/chromium/base/files/file_path_watcher.h +++ b/chromium/base/files/file_path_watcher.h @@ -88,12 +88,15 @@ class BASE_EXPORT FilePathWatcher { // shutdown. static void CancelWatch(const scoped_refptr<PlatformDelegate>& delegate); + // Returns true if the platform and OS version support recursive watches. + static bool RecursiveWatchAvailable(); + // Invokes |callback| whenever updates to |path| are detected. This should be // called at most once, and from a MessageLoop of TYPE_IO. Set |recursive| to // true, to watch |path| and its children. The callback will be invoked on // the same loop. Returns true on success. // - // NOTE: Recursive watch is not supported on all platforms and file systems. + // Recursive watch is not supported on all platforms and file systems. // Watch() will return false in the case of failure. bool Watch(const FilePath& path, bool recursive, const Callback& callback); diff --git a/chromium/base/files/file_path_watcher_browsertest.cc b/chromium/base/files/file_path_watcher_browsertest.cc index aed409c7871..8f57cadda90 100644 --- a/chromium/base/files/file_path_watcher_browsertest.cc +++ b/chromium/base/files/file_path_watcher_browsertest.cc @@ -172,8 +172,7 @@ class FilePathWatcherTest : public testing::Test { // Write |content| to |file|. Returns true on success. bool WriteFile(const FilePath& file, const std::string& content) { - int write_size = file_util::WriteFile(file, content.c_str(), - content.length()); + int write_size = ::base::WriteFile(file, content.c_str(), content.length()); return write_size == static_cast<int>(content.length()); } @@ -206,9 +205,8 @@ bool FilePathWatcherTest::SetupWatch(const FilePath& target, bool result; file_thread_.message_loop_proxy()->PostTask( FROM_HERE, - base::Bind(SetupWatchCallback, - target, watcher, delegate, recursive_watch, &result, - &completion)); + base::Bind(SetupWatchCallback, target, watcher, delegate, recursive_watch, + &result, &completion)); completion.Wait(); return result; } @@ -484,12 +482,17 @@ TEST_F(FilePathWatcherTest, MoveParent) { DeleteDelegateOnFileThread(subdir_delegate.release()); } -#if defined(OS_WIN) TEST_F(FilePathWatcherTest, RecursiveWatch) { FilePathWatcher watcher; FilePath dir(temp_dir_.path().AppendASCII("dir")); scoped_ptr<TestDelegate> delegate(new TestDelegate(collector())); - ASSERT_TRUE(SetupWatch(dir, &watcher, delegate.get(), true)); + bool setup_result = SetupWatch(dir, &watcher, delegate.get(), true); + if (!FilePathWatcher::RecursiveWatchAvailable()) { + ASSERT_FALSE(setup_result); + DeleteDelegateOnFileThread(delegate.release()); + return; + } + ASSERT_TRUE(setup_result); // Main directory("dir") creation. ASSERT_TRUE(base::CreateDirectory(dir)); @@ -537,16 +540,48 @@ TEST_F(FilePathWatcherTest, RecursiveWatch) { ASSERT_TRUE(WaitForEvents()); DeleteDelegateOnFileThread(delegate.release()); } -#else -TEST_F(FilePathWatcherTest, RecursiveWatch) { + +#if defined(OS_POSIX) +TEST_F(FilePathWatcherTest, RecursiveWithSymLink) { + if (!FilePathWatcher::RecursiveWatchAvailable()) + return; + FilePathWatcher watcher; - FilePath dir(temp_dir_.path().AppendASCII("dir")); + FilePath test_dir(temp_dir_.path().AppendASCII("test_dir")); + ASSERT_TRUE(base::CreateDirectory(test_dir)); + FilePath symlink(test_dir.AppendASCII("symlink")); scoped_ptr<TestDelegate> delegate(new TestDelegate(collector())); - // Non-Windows implementaion does not support recursive watching. - ASSERT_FALSE(SetupWatch(dir, &watcher, delegate.get(), true)); + ASSERT_TRUE(SetupWatch(symlink, &watcher, delegate.get(), true)); + + // Link creation. + FilePath target1(temp_dir_.path().AppendASCII("target1")); + ASSERT_TRUE(base::CreateSymbolicLink(target1, symlink)); + ASSERT_TRUE(WaitForEvents()); + + // Target1 creation. + ASSERT_TRUE(base::CreateDirectory(target1)); + ASSERT_TRUE(WaitForEvents()); + + // Create a file in target1. + FilePath target1_file(target1.AppendASCII("file")); + ASSERT_TRUE(WriteFile(target1_file, "content")); + ASSERT_TRUE(WaitForEvents()); + + // Link change. + FilePath target2(temp_dir_.path().AppendASCII("target2")); + ASSERT_TRUE(base::CreateDirectory(target2)); + ASSERT_TRUE(base::DeleteFile(symlink, false)); + ASSERT_TRUE(base::CreateSymbolicLink(target2, symlink)); + ASSERT_TRUE(WaitForEvents()); + + // Create a file in target2. + FilePath target2_file(target2.AppendASCII("file")); + ASSERT_TRUE(WriteFile(target2_file, "content")); + ASSERT_TRUE(WaitForEvents()); + DeleteDelegateOnFileThread(delegate.release()); } -#endif +#endif // OS_POSIX TEST_F(FilePathWatcherTest, MoveChild) { FilePathWatcher file_watcher; @@ -575,10 +610,6 @@ TEST_F(FilePathWatcherTest, MoveChild) { DeleteDelegateOnFileThread(subdir_delegate.release()); } -#if !defined(OS_LINUX) -// Linux implementation of FilePathWatcher doesn't catch attribute changes. -// http://crbug.com/78043 - // Verify that changing attributes on a file is caught TEST_F(FilePathWatcherTest, FileAttributesChanged) { ASSERT_TRUE(WriteFile(test_file(), "content")); @@ -592,8 +623,6 @@ TEST_F(FilePathWatcherTest, FileAttributesChanged) { DeleteDelegateOnFileThread(delegate.release()); } -#endif // !OS_LINUX - #if defined(OS_LINUX) // Verify that creating a symlink is caught. diff --git a/chromium/base/files/file_path_watcher_fsevents.cc b/chromium/base/files/file_path_watcher_fsevents.cc new file mode 100644 index 00000000000..edf4d239465 --- /dev/null +++ b/chromium/base/files/file_path_watcher_fsevents.cc @@ -0,0 +1,263 @@ +// Copyright 2014 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. + +#include "base/files/file_path_watcher_fsevents.h" + +#include <list> + +#include "base/bind.h" +#include "base/file_util.h" +#include "base/lazy_instance.h" +#include "base/logging.h" +#include "base/mac/libdispatch_task_runner.h" +#include "base/mac/scoped_cftyperef.h" +#include "base/message_loop/message_loop.h" + +namespace base { + +namespace { + +// The latency parameter passed to FSEventsStreamCreate(). +const CFAbsoluteTime kEventLatencySeconds = 0.3; + +class FSEventsTaskRunner : public mac::LibDispatchTaskRunner { + public: + FSEventsTaskRunner() + : mac::LibDispatchTaskRunner("org.chromium.FilePathWatcherFSEvents") { + } + + protected: + virtual ~FSEventsTaskRunner() {} +}; + +static LazyInstance<FSEventsTaskRunner>::Leaky g_task_runner = + LAZY_INSTANCE_INITIALIZER; + +// Resolve any symlinks in the path. +FilePath ResolvePath(const FilePath& path) { + const unsigned kMaxLinksToResolve = 255; + + std::vector<FilePath::StringType> component_vector; + path.GetComponents(&component_vector); + std::list<FilePath::StringType> + components(component_vector.begin(), component_vector.end()); + + FilePath result; + unsigned resolve_count = 0; + while (resolve_count < kMaxLinksToResolve && !components.empty()) { + FilePath component(*components.begin()); + components.pop_front(); + + FilePath current; + if (component.IsAbsolute()) { + current = component; + } else { + current = result.Append(component); + } + + FilePath target; + if (ReadSymbolicLink(current, &target)) { + if (target.IsAbsolute()) + result.clear(); + std::vector<FilePath::StringType> target_components; + target.GetComponents(&target_components); + components.insert(components.begin(), target_components.begin(), + target_components.end()); + resolve_count++; + } else { + result = current; + } + } + + if (resolve_count >= kMaxLinksToResolve) + result.clear(); + return result; +} + +// The callback passed to FSEventStreamCreate(). +void FSEventsCallback(ConstFSEventStreamRef stream, + void* event_watcher, size_t num_events, + void* event_paths, const FSEventStreamEventFlags flags[], + const FSEventStreamEventId event_ids[]) { + FilePathWatcherFSEvents* watcher = + reinterpret_cast<FilePathWatcherFSEvents*>(event_watcher); + DCHECK(g_task_runner.Get().RunsTasksOnCurrentThread()); + + bool root_changed = watcher->ResolveTargetPath(); + std::vector<FilePath> paths; + FSEventStreamEventId root_change_at = FSEventStreamGetLatestEventId(stream); + for (size_t i = 0; i < num_events; i++) { + if (flags[i] & kFSEventStreamEventFlagRootChanged) + root_changed = true; + if (event_ids[i]) + root_change_at = std::min(root_change_at, event_ids[i]); + paths.push_back(FilePath( + reinterpret_cast<char**>(event_paths)[i]).StripTrailingSeparators()); + } + + // Reinitialize the event stream if we find changes to the root. This is + // necessary since FSEvents doesn't report any events for the subtree after + // the directory to be watched gets created. + if (root_changed) { + // Resetting the event stream from within the callback fails (FSEvents spews + // bad file descriptor errors), so post a task to do the reset. + g_task_runner.Get().PostTask( + FROM_HERE, + Bind(&FilePathWatcherFSEvents::UpdateEventStream, watcher, + root_change_at)); + } + + watcher->OnFilePathsChanged(paths); +} + +} // namespace + +FilePathWatcherFSEvents::FilePathWatcherFSEvents() : fsevent_stream_(NULL) { +} + +void FilePathWatcherFSEvents::OnFilePathsChanged( + const std::vector<FilePath>& paths) { + if (!message_loop()->BelongsToCurrentThread()) { + message_loop()->PostTask( + FROM_HERE, + Bind(&FilePathWatcherFSEvents::OnFilePathsChanged, this, paths)); + return; + } + + DCHECK(message_loop()->BelongsToCurrentThread()); + if (resolved_target_.empty()) + return; + + for (size_t i = 0; i < paths.size(); i++) { + if (resolved_target_.IsParent(paths[i]) || resolved_target_ == paths[i]) { + callback_.Run(target_, false); + return; + } + } +} + +bool FilePathWatcherFSEvents::Watch(const FilePath& path, + bool recursive, + const FilePathWatcher::Callback& callback) { + DCHECK(resolved_target_.empty()); + DCHECK(MessageLoopForIO::current()); + DCHECK(!callback.is_null()); + + // This class could support non-recursive watches, but that is currently + // left to FilePathWatcherKQueue. + if (!recursive) + return false; + + set_message_loop(MessageLoopProxy::current()); + callback_ = callback; + target_ = path; + + FSEventStreamEventId start_event = FSEventsGetCurrentEventId(); + g_task_runner.Get().PostTask( + FROM_HERE, + Bind(&FilePathWatcherFSEvents::StartEventStream, this, start_event)); + return true; +} + +void FilePathWatcherFSEvents::Cancel() { + if (callback_.is_null()) { + // Watch was never called, so exit. + set_cancelled(); + return; + } + + // Switch to the dispatch queue thread if necessary, so we can tear down + // the event stream. + if (!g_task_runner.Get().RunsTasksOnCurrentThread()) { + g_task_runner.Get().PostTask( + FROM_HERE, + Bind(&FilePathWatcherFSEvents::CancelOnMessageLoopThread, this)); + } else { + CancelOnMessageLoopThread(); + } +} + +void FilePathWatcherFSEvents::CancelOnMessageLoopThread() { + // For all other implementations, the "message loop thread" is the IO thread, + // as returned by message_loop(). This implementation, however, needs to + // cancel pending work on the Dipatch Queue thread. + DCHECK(g_task_runner.Get().RunsTasksOnCurrentThread()); + + set_cancelled(); + if (fsevent_stream_) { + DestroyEventStream(); + callback_.Reset(); + target_.clear(); + resolved_target_.clear(); + } +} + +void FilePathWatcherFSEvents::UpdateEventStream( + FSEventStreamEventId start_event) { + DCHECK(g_task_runner.Get().RunsTasksOnCurrentThread()); + + // It can happen that the watcher gets canceled while tasks that call this + // function are still in flight, so abort if this situation is detected. + if (is_cancelled() || resolved_target_.empty()) + return; + + if (fsevent_stream_) + DestroyEventStream(); + + ScopedCFTypeRef<CFStringRef> cf_path(CFStringCreateWithCString( + NULL, resolved_target_.value().c_str(), kCFStringEncodingMacHFS)); + ScopedCFTypeRef<CFStringRef> cf_dir_path(CFStringCreateWithCString( + NULL, resolved_target_.DirName().value().c_str(), + kCFStringEncodingMacHFS)); + CFStringRef paths_array[] = { cf_path.get(), cf_dir_path.get() }; + ScopedCFTypeRef<CFArrayRef> watched_paths(CFArrayCreate( + NULL, reinterpret_cast<const void**>(paths_array), arraysize(paths_array), + &kCFTypeArrayCallBacks)); + + FSEventStreamContext context; + context.version = 0; + context.info = this; + context.retain = NULL; + context.release = NULL; + context.copyDescription = NULL; + + fsevent_stream_ = FSEventStreamCreate(NULL, &FSEventsCallback, &context, + watched_paths, + start_event, + kEventLatencySeconds, + kFSEventStreamCreateFlagWatchRoot); + FSEventStreamSetDispatchQueue(fsevent_stream_, + g_task_runner.Get().GetDispatchQueue()); + + if (!FSEventStreamStart(fsevent_stream_)) + message_loop()->PostTask(FROM_HERE, Bind(callback_, target_, true)); +} + +bool FilePathWatcherFSEvents::ResolveTargetPath() { + DCHECK(g_task_runner.Get().RunsTasksOnCurrentThread()); + FilePath resolved = ResolvePath(target_).StripTrailingSeparators(); + bool changed = resolved != resolved_target_; + resolved_target_ = resolved; + if (resolved_target_.empty()) + message_loop()->PostTask(FROM_HERE, Bind(callback_, target_, true)); + return changed; +} + +void FilePathWatcherFSEvents::DestroyEventStream() { + FSEventStreamStop(fsevent_stream_); + FSEventStreamInvalidate(fsevent_stream_); + FSEventStreamRelease(fsevent_stream_); + fsevent_stream_ = NULL; +} + +void FilePathWatcherFSEvents::StartEventStream( + FSEventStreamEventId start_event) { + DCHECK(g_task_runner.Get().RunsTasksOnCurrentThread()); + ResolveTargetPath(); + UpdateEventStream(start_event); +} + +FilePathWatcherFSEvents::~FilePathWatcherFSEvents() {} + +} // namespace base diff --git a/chromium/base/files/file_path_watcher_fsevents.h b/chromium/base/files/file_path_watcher_fsevents.h new file mode 100644 index 00000000000..5640b4d5497 --- /dev/null +++ b/chromium/base/files/file_path_watcher_fsevents.h @@ -0,0 +1,73 @@ +// Copyright 2014 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. + +#ifndef BASE_FILES_FILE_PATH_WATCHER_FSEVENTS_H_ +#define BASE_FILES_FILE_PATH_WATCHER_FSEVENTS_H_ + +#include <CoreServices/CoreServices.h> + +#include <vector> + +#include "base/files/file_path.h" +#include "base/files/file_path_watcher.h" + +namespace base { + +// Mac-specific file watcher implementation based on FSEvents. +// There are trade-offs between the FSEvents implementation and a kqueue +// implementation. The biggest issues are that FSEvents on 10.6 sometimes drops +// events and kqueue does not trigger for modifications to a file in a watched +// directory. See file_path_watcher_mac.cc for the code that decides when to +// use which one. +class FilePathWatcherFSEvents : public FilePathWatcher::PlatformDelegate { + public: + FilePathWatcherFSEvents(); + + // Called from the FSEvents callback whenever there is a change to the paths. + void OnFilePathsChanged(const std::vector<FilePath>& paths); + + // (Re-)Initialize the event stream to start reporting events from + // |start_event|. + void UpdateEventStream(FSEventStreamEventId start_event); + + // Returns true if resolving the target path got a different result than + // last time it was done. + bool ResolveTargetPath(); + + // FilePathWatcher::PlatformDelegate overrides. + virtual bool Watch(const FilePath& path, + bool recursive, + const FilePathWatcher::Callback& callback) OVERRIDE; + virtual void Cancel() OVERRIDE; + + private: + virtual ~FilePathWatcherFSEvents(); + + // Destroy the event stream. + void DestroyEventStream(); + + // Start watching the FSEventStream. + void StartEventStream(FSEventStreamEventId start_event); + + // Cleans up and stops the event stream. + virtual void CancelOnMessageLoopThread() OVERRIDE; + + // Callback to notify upon changes. + FilePathWatcher::Callback callback_; + + // Target path to watch (passed to callback). + FilePath target_; + + // Target path with all symbolic links resolved. + FilePath resolved_target_; + + // Backend stream we receive event callbacks from (strong reference). + FSEventStreamRef fsevent_stream_; + + DISALLOW_COPY_AND_ASSIGN(FilePathWatcherFSEvents); +}; + +} // namespace base + +#endif // BASE_FILES_FILE_PATH_WATCHER_FSEVENTS_H_ diff --git a/chromium/base/files/file_path_watcher_kqueue.cc b/chromium/base/files/file_path_watcher_kqueue.cc index e035f22caa5..c38e3448e73 100644 --- a/chromium/base/files/file_path_watcher_kqueue.cc +++ b/chromium/base/files/file_path_watcher_kqueue.cc @@ -2,19 +2,14 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -#include "base/files/file_path_watcher.h" +#include "base/files/file_path_watcher_kqueue.h" #include <fcntl.h> -#include <sys/event.h> #include <sys/param.h> -#include <vector> - #include "base/bind.h" #include "base/file_util.h" #include "base/logging.h" -#include "base/message_loop/message_loop.h" -#include "base/message_loop/message_loop_proxy.h" #include "base/strings/stringprintf.h" // On some platforms these are not defined. @@ -27,136 +22,18 @@ namespace base { -namespace { - -// Mac-specific file watcher implementation based on kqueue. -// Originally it was based on FSEvents so that the semantics were equivalent -// on Linux, OSX and Windows where it was able to detect: -// - file creation/deletion/modification in a watched directory -// - file creation/deletion/modification for a watched file -// - modifications to the paths to a watched object that would affect the -// object such as renaming/attibute changes etc. -// The FSEvents version did all of the above except handling attribute changes -// to path components. Unfortunately FSEvents appears to have an issue where the -// current implementation (Mac OS X 10.6.7) sometimes drops events and doesn't -// send notifications. See -// http://code.google.com/p/chromium/issues/detail?id=54822#c31 for source that -// will reproduce the problem. FSEvents also required having a CFRunLoop -// backing the thread that it was running on, that caused added complexity -// in the interfaces. -// The kqueue implementation will handle all of the items in the list above -// except for detecting modifications to files in a watched directory. It will -// detect the creation and deletion of files, just not the modification of -// files. It does however detect the attribute changes that the FSEvents impl -// would miss. -class FilePathWatcherImpl : public FilePathWatcher::PlatformDelegate, - public MessageLoopForIO::Watcher, - public MessageLoop::DestructionObserver { - public: - FilePathWatcherImpl() : kqueue_(-1) {} - - // MessageLoopForIO::Watcher overrides. - virtual void OnFileCanReadWithoutBlocking(int fd) OVERRIDE; - virtual void OnFileCanWriteWithoutBlocking(int fd) OVERRIDE; - - // MessageLoop::DestructionObserver overrides. - virtual void WillDestroyCurrentMessageLoop() OVERRIDE; - - // FilePathWatcher::PlatformDelegate overrides. - virtual bool Watch(const FilePath& path, - bool recursive, - const FilePathWatcher::Callback& callback) OVERRIDE; - virtual void Cancel() OVERRIDE; - - protected: - virtual ~FilePathWatcherImpl() {} - - private: - class EventData { - public: - EventData(const FilePath& path, const FilePath::StringType& subdir) - : path_(path), subdir_(subdir) { } - FilePath path_; // Full path to this item. - FilePath::StringType subdir_; // Path to any sub item. - }; - typedef std::vector<struct kevent> EventVector; - - // Can only be called on |io_message_loop_|'s thread. - virtual void CancelOnMessageLoopThread() OVERRIDE; - - // Returns true if the kevent values are error free. - bool AreKeventValuesValid(struct kevent* kevents, int count); - - // Respond to a change of attributes of the path component represented by - // |event|. Sets |target_file_affected| to true if |target_| is affected. - // Sets |update_watches| to true if |events_| need to be updated. - void HandleAttributesChange(const EventVector::iterator& event, - bool* target_file_affected, - bool* update_watches); - - // Respond to a move or deletion of the path component represented by - // |event|. Sets |target_file_affected| to true if |target_| is affected. - // Sets |update_watches| to true if |events_| need to be updated. - void HandleDeleteOrMoveChange(const EventVector::iterator& event, - bool* target_file_affected, - bool* update_watches); - - // Respond to a creation of an item in the path component represented by - // |event|. Sets |target_file_affected| to true if |target_| is affected. - // Sets |update_watches| to true if |events_| need to be updated. - void HandleCreateItemChange(const EventVector::iterator& event, - bool* target_file_affected, - bool* update_watches); - - // Update |events_| with the current status of the system. - // Sets |target_file_affected| to true if |target_| is affected. - // Returns false if an error occurs. - bool UpdateWatches(bool* target_file_affected); - - // Fills |events| with one kevent per component in |path|. - // Returns the number of valid events created where a valid event is - // defined as one that has a ident (file descriptor) field != -1. - static int EventsForPath(FilePath path, EventVector *events); - - // Release a kevent generated by EventsForPath. - static void ReleaseEvent(struct kevent& event); - - // Returns a file descriptor that will not block the system from deleting - // the file it references. - static uintptr_t FileDescriptorForPath(const FilePath& path); - - static const uintptr_t kNoFileDescriptor = static_cast<uintptr_t>(-1); - - // Closes |*fd| and sets |*fd| to -1. - static void CloseFileDescriptor(uintptr_t* fd); - - // Returns true if kevent has open file descriptor. - static bool IsKeventFileDescriptorOpen(const struct kevent& event) { - return event.ident != kNoFileDescriptor; - } - - static EventData* EventDataForKevent(const struct kevent& event) { - return reinterpret_cast<EventData*>(event.udata); - } - - EventVector events_; - scoped_refptr<base::MessageLoopProxy> io_message_loop_; - MessageLoopForIO::FileDescriptorWatcher kqueue_watcher_; - FilePathWatcher::Callback callback_; - FilePath target_; - int kqueue_; +FilePathWatcherKQueue::FilePathWatcherKQueue() : kqueue_(-1) {} - DISALLOW_COPY_AND_ASSIGN(FilePathWatcherImpl); -}; +FilePathWatcherKQueue::~FilePathWatcherKQueue() {} -void FilePathWatcherImpl::ReleaseEvent(struct kevent& event) { +void FilePathWatcherKQueue::ReleaseEvent(struct kevent& event) { CloseFileDescriptor(&event.ident); EventData* entry = EventDataForKevent(event); delete entry; event.udata = NULL; } -int FilePathWatcherImpl::EventsForPath(FilePath path, EventVector* events) { +int FilePathWatcherKQueue::EventsForPath(FilePath path, EventVector* events) { DCHECK(MessageLoopForIO::current()); // Make sure that we are working with a clean slate. DCHECK(events->empty()); @@ -198,14 +75,14 @@ int FilePathWatcherImpl::EventsForPath(FilePath path, EventVector* events) { return last_existing_entry; } -uintptr_t FilePathWatcherImpl::FileDescriptorForPath(const FilePath& path) { +uintptr_t FilePathWatcherKQueue::FileDescriptorForPath(const FilePath& path) { int fd = HANDLE_EINTR(open(path.value().c_str(), O_EVTONLY)); if (fd == -1) return kNoFileDescriptor; return fd; } -void FilePathWatcherImpl::CloseFileDescriptor(uintptr_t* fd) { +void FilePathWatcherKQueue::CloseFileDescriptor(uintptr_t* fd) { if (*fd == kNoFileDescriptor) { return; } @@ -216,7 +93,7 @@ void FilePathWatcherImpl::CloseFileDescriptor(uintptr_t* fd) { *fd = kNoFileDescriptor; } -bool FilePathWatcherImpl::AreKeventValuesValid(struct kevent* kevents, +bool FilePathWatcherKQueue::AreKeventValuesValid(struct kevent* kevents, int count) { if (count < 0) { DPLOG(ERROR) << "kevent"; @@ -250,7 +127,7 @@ bool FilePathWatcherImpl::AreKeventValuesValid(struct kevent* kevents, return valid; } -void FilePathWatcherImpl::HandleAttributesChange( +void FilePathWatcherKQueue::HandleAttributesChange( const EventVector::iterator& event, bool* target_file_affected, bool* update_watches) { @@ -274,7 +151,7 @@ void FilePathWatcherImpl::HandleAttributesChange( } } -void FilePathWatcherImpl::HandleDeleteOrMoveChange( +void FilePathWatcherKQueue::HandleDeleteOrMoveChange( const EventVector::iterator& event, bool* target_file_affected, bool* update_watches) { @@ -290,7 +167,7 @@ void FilePathWatcherImpl::HandleDeleteOrMoveChange( } } -void FilePathWatcherImpl::HandleCreateItemChange( +void FilePathWatcherKQueue::HandleCreateItemChange( const EventVector::iterator& event, bool* target_file_affected, bool* update_watches) { @@ -310,7 +187,7 @@ void FilePathWatcherImpl::HandleCreateItemChange( } } -bool FilePathWatcherImpl::UpdateWatches(bool* target_file_affected) { +bool FilePathWatcherKQueue::UpdateWatches(bool* target_file_affected) { // Iterate over events adding kevents for items that exist to the kqueue. // Then check to see if new components in the path have been created. // Repeat until no new components in the path are detected. @@ -351,7 +228,7 @@ bool FilePathWatcherImpl::UpdateWatches(bool* target_file_affected) { return true; } -void FilePathWatcherImpl::OnFileCanReadWithoutBlocking(int fd) { +void FilePathWatcherKQueue::OnFileCanReadWithoutBlocking(int fd) { DCHECK(MessageLoopForIO::current()); DCHECK_EQ(fd, kqueue_); DCHECK(events_.size()); @@ -424,24 +301,24 @@ void FilePathWatcherImpl::OnFileCanReadWithoutBlocking(int fd) { } } -void FilePathWatcherImpl::OnFileCanWriteWithoutBlocking(int fd) { +void FilePathWatcherKQueue::OnFileCanWriteWithoutBlocking(int fd) { NOTREACHED(); } -void FilePathWatcherImpl::WillDestroyCurrentMessageLoop() { +void FilePathWatcherKQueue::WillDestroyCurrentMessageLoop() { CancelOnMessageLoopThread(); } -bool FilePathWatcherImpl::Watch(const FilePath& path, - bool recursive, - const FilePathWatcher::Callback& callback) { +bool FilePathWatcherKQueue::Watch(const FilePath& path, + bool recursive, + const FilePathWatcher::Callback& callback) { DCHECK(MessageLoopForIO::current()); DCHECK(target_.value().empty()); // Can only watch one path. DCHECK(!callback.is_null()); DCHECK_EQ(kqueue_, -1); if (recursive) { - // Recursive watch is not supported on this platform. + // Recursive watch is not supported using kqueue. NOTIMPLEMENTED(); return false; } @@ -478,7 +355,7 @@ bool FilePathWatcherImpl::Watch(const FilePath& path, kqueue_, true, MessageLoopForIO::WATCH_READ, &kqueue_watcher_, this); } -void FilePathWatcherImpl::Cancel() { +void FilePathWatcherKQueue::Cancel() { base::MessageLoopProxy* proxy = io_message_loop_.get(); if (!proxy) { set_cancelled(); @@ -486,13 +363,13 @@ void FilePathWatcherImpl::Cancel() { } if (!proxy->BelongsToCurrentThread()) { proxy->PostTask(FROM_HERE, - base::Bind(&FilePathWatcherImpl::Cancel, this)); + base::Bind(&FilePathWatcherKQueue::Cancel, this)); return; } CancelOnMessageLoopThread(); } -void FilePathWatcherImpl::CancelOnMessageLoopThread() { +void FilePathWatcherKQueue::CancelOnMessageLoopThread() { DCHECK(MessageLoopForIO::current()); if (!is_cancelled()) { set_cancelled(); @@ -509,10 +386,4 @@ void FilePathWatcherImpl::CancelOnMessageLoopThread() { } } -} // namespace - -FilePathWatcher::FilePathWatcher() { - impl_ = new FilePathWatcherImpl(); -} - } // namespace base diff --git a/chromium/base/files/file_path_watcher_kqueue.h b/chromium/base/files/file_path_watcher_kqueue.h new file mode 100644 index 00000000000..703fda67c4b --- /dev/null +++ b/chromium/base/files/file_path_watcher_kqueue.h @@ -0,0 +1,132 @@ +// Copyright 2014 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. + +#ifndef BASE_FILES_FILE_PATH_WATCHER_KQUEUE_H_ +#define BASE_FILES_FILE_PATH_WATCHER_KQUEUE_H_ + +#include <sys/event.h> +#include <vector> + +#include "base/files/file_path.h" +#include "base/files/file_path_watcher.h" +#include "base/message_loop/message_loop.h" +#include "base/message_loop/message_loop_proxy.h" + +namespace base { + +// Mac-specific file watcher implementation based on kqueue. +// The Linux and Windows versions are able to detect: +// - file creation/deletion/modification in a watched directory +// - file creation/deletion/modification for a watched file +// - modifications to the paths to a watched object that would affect the +// object such as renaming/attibute changes etc. +// The kqueue implementation will handle all of the items in the list above +// except for detecting modifications to files in a watched directory. It will +// detect the creation and deletion of files, just not the modification of +// files. It does however detect the attribute changes that the FSEvents impl +// would miss. +class FilePathWatcherKQueue : public FilePathWatcher::PlatformDelegate, + public MessageLoopForIO::Watcher, + public MessageLoop::DestructionObserver { + public: + FilePathWatcherKQueue(); + + // MessageLoopForIO::Watcher overrides. + virtual void OnFileCanReadWithoutBlocking(int fd) OVERRIDE; + virtual void OnFileCanWriteWithoutBlocking(int fd) OVERRIDE; + + // MessageLoop::DestructionObserver overrides. + virtual void WillDestroyCurrentMessageLoop() OVERRIDE; + + // FilePathWatcher::PlatformDelegate overrides. + virtual bool Watch(const FilePath& path, + bool recursive, + const FilePathWatcher::Callback& callback) OVERRIDE; + virtual void Cancel() OVERRIDE; + + protected: + virtual ~FilePathWatcherKQueue(); + + private: + class EventData { + public: + EventData(const FilePath& path, const FilePath::StringType& subdir) + : path_(path), subdir_(subdir) { } + FilePath path_; // Full path to this item. + FilePath::StringType subdir_; // Path to any sub item. + }; + + typedef std::vector<struct kevent> EventVector; + + // Can only be called on |io_message_loop_|'s thread. + virtual void CancelOnMessageLoopThread() OVERRIDE; + + // Returns true if the kevent values are error free. + bool AreKeventValuesValid(struct kevent* kevents, int count); + + // Respond to a change of attributes of the path component represented by + // |event|. Sets |target_file_affected| to true if |target_| is affected. + // Sets |update_watches| to true if |events_| need to be updated. + void HandleAttributesChange(const EventVector::iterator& event, + bool* target_file_affected, + bool* update_watches); + + // Respond to a move or deletion of the path component represented by + // |event|. Sets |target_file_affected| to true if |target_| is affected. + // Sets |update_watches| to true if |events_| need to be updated. + void HandleDeleteOrMoveChange(const EventVector::iterator& event, + bool* target_file_affected, + bool* update_watches); + + // Respond to a creation of an item in the path component represented by + // |event|. Sets |target_file_affected| to true if |target_| is affected. + // Sets |update_watches| to true if |events_| need to be updated. + void HandleCreateItemChange(const EventVector::iterator& event, + bool* target_file_affected, + bool* update_watches); + + // Update |events_| with the current status of the system. + // Sets |target_file_affected| to true if |target_| is affected. + // Returns false if an error occurs. + bool UpdateWatches(bool* target_file_affected); + + // Fills |events| with one kevent per component in |path|. + // Returns the number of valid events created where a valid event is + // defined as one that has a ident (file descriptor) field != -1. + static int EventsForPath(FilePath path, EventVector *events); + + // Release a kevent generated by EventsForPath. + static void ReleaseEvent(struct kevent& event); + + // Returns a file descriptor that will not block the system from deleting + // the file it references. + static uintptr_t FileDescriptorForPath(const FilePath& path); + + static const uintptr_t kNoFileDescriptor = static_cast<uintptr_t>(-1); + + // Closes |*fd| and sets |*fd| to -1. + static void CloseFileDescriptor(uintptr_t* fd); + + // Returns true if kevent has open file descriptor. + static bool IsKeventFileDescriptorOpen(const struct kevent& event) { + return event.ident != kNoFileDescriptor; + } + + static EventData* EventDataForKevent(const struct kevent& event) { + return reinterpret_cast<EventData*>(event.udata); + } + + EventVector events_; + scoped_refptr<base::MessageLoopProxy> io_message_loop_; + MessageLoopForIO::FileDescriptorWatcher kqueue_watcher_; + FilePathWatcher::Callback callback_; + FilePath target_; + int kqueue_; + + DISALLOW_COPY_AND_ASSIGN(FilePathWatcherKQueue); +}; + +} // namespace base + +#endif // BASE_FILES_FILE_PATH_WATCHER_KQUEUE_H_ diff --git a/chromium/base/files/file_path_watcher_linux.cc b/chromium/base/files/file_path_watcher_linux.cc index d5052e2e512..915ad50abb2 100644 --- a/chromium/base/files/file_path_watcher_linux.cc +++ b/chromium/base/files/file_path_watcher_linux.cc @@ -12,6 +12,7 @@ #include <unistd.h> #include <algorithm> +#include <map> #include <set> #include <utility> #include <vector> @@ -20,6 +21,7 @@ #include "base/containers/hash_tables.h" #include "base/debug/trace_event.h" #include "base/file_util.h" +#include "base/files/file_enumerator.h" #include "base/files/file_path.h" #include "base/lazy_instance.h" #include "base/location.h" @@ -49,8 +51,8 @@ class InotifyReader { // change. Returns kInvalidWatch on failure. Watch AddWatch(const FilePath& path, FilePathWatcherImpl* watcher); - // Remove |watch|. Returns true on success. - bool RemoveWatch(Watch watch, FilePathWatcherImpl* watcher); + // Remove |watch| if it's valid. + void RemoveWatch(Watch watch, FilePathWatcherImpl* watcher); // Callback for InotifyReaderTask. void OnInotifyEvent(const inotify_event* event); @@ -91,12 +93,21 @@ class FilePathWatcherImpl : public FilePathWatcher::PlatformDelegate, // Called for each event coming from the watch. |fired_watch| identifies the // watch that fired, |child| indicates what has changed, and is relative to - // the currently watched path for |fired_watch|. The flag |created| is true if - // the object appears. + // the currently watched path for |fired_watch|. + // + // |created| is true if the object appears. + // |deleted| is true if the object disappears. + // |is_dir| is true if the object is a directory. void OnFilePathChanged(InotifyReader::Watch fired_watch, const FilePath::StringType& child, - bool created); + bool created, + bool deleted, + bool is_dir); + protected: + virtual ~FilePathWatcherImpl() {} + + private: // Start watching |path| for changes and notify |delegate| on each change. // Returns true if watch for |path| has been added successfully. virtual bool Watch(const FilePath& path, @@ -106,36 +117,58 @@ class FilePathWatcherImpl : public FilePathWatcher::PlatformDelegate, // Cancel the watch. This unregisters the instance with InotifyReader. virtual void Cancel() OVERRIDE; + // Cleans up and stops observing the message_loop() thread. + virtual void CancelOnMessageLoopThread() OVERRIDE; + // Deletion of the FilePathWatcher will call Cancel() to dispose of this // object in the right thread. This also observes destruction of the required // cleanup thread, in case it quits before Cancel() is called. virtual void WillDestroyCurrentMessageLoop() OVERRIDE; - protected: - virtual ~FilePathWatcherImpl() {} - - private: - // Cleans up and stops observing the |message_loop_| thread. - virtual void CancelOnMessageLoopThread() OVERRIDE; - // Inotify watches are installed for all directory components of |target_|. A // WatchEntry instance holds the watch descriptor for a component and the // subdirectory for that identifies the next component. If a symbolic link // is being watched, the target of the link is also kept. struct WatchEntry { - WatchEntry(InotifyReader::Watch watch, const FilePath::StringType& subdir) - : watch_(watch), - subdir_(subdir) {} + explicit WatchEntry(const FilePath::StringType& dirname) + : watch(InotifyReader::kInvalidWatch), + subdir(dirname) {} - InotifyReader::Watch watch_; - FilePath::StringType subdir_; - FilePath::StringType linkname_; + InotifyReader::Watch watch; + FilePath::StringType subdir; + FilePath::StringType linkname; }; typedef std::vector<WatchEntry> WatchVector; // Reconfigure to watch for the most specific parent directory of |target_| - // that exists. Updates |watched_path_|. Returns true on success. - bool UpdateWatches() WARN_UNUSED_RESULT; + // that exists. Also calls UpdateRecursiveWatches() below. + void UpdateWatches(); + + // Reconfigure to recursively watch |target_| and all its sub-directories. + // - This is a no-op if the watch is not recursive. + // - If |target_| does not exist, then clear all the recursive watches. + // - Assuming |target_| exists, passing kInvalidWatch as |fired_watch| forces + // addition of recursive watches for |target_|. + // - Otherwise, only the directory associated with |fired_watch| and its + // sub-directories will be reconfigured. + void UpdateRecursiveWatches(InotifyReader::Watch fired_watch, bool is_dir); + + // Enumerate recursively through |path| and add / update watches. + void UpdateRecursiveWatchesForPath(const FilePath& path); + + // Do internal bookkeeping to update mappings between |watch| and its + // associated full path |path|. + void TrackWatchForRecursion(InotifyReader::Watch watch, const FilePath& path); + + // Remove all the recursive watches. + void RemoveRecursiveWatches(); + + // |path| is a symlink to a non-existent target. Attempt to add a watch to + // the link target's parent directory. Returns true and update |watch_entry| + // on success. + bool AddWatchForBrokenSymlink(const FilePath& path, WatchEntry* watch_entry); + + bool HasValidWatchVector() const; // Callback to notify upon changes. FilePathWatcher::Callback callback_; @@ -143,11 +176,16 @@ class FilePathWatcherImpl : public FilePathWatcher::PlatformDelegate, // The file or directory we're supposed to watch. FilePath target_; + bool recursive_; + // The vector of watches and next component names for all path components, // starting at the root directory. The last entry corresponds to the watch for - // |target_| and always stores an empty next component name in |subdir_|. + // |target_| and always stores an empty next component name in |subdir|. WatchVector watches_; + hash_map<InotifyReader::Watch, FilePath> recursive_paths_by_watch_; + std::map<FilePath, InotifyReader::Watch> recursive_watches_by_path_; + DISALLOW_COPY_AND_ASSIGN(FilePathWatcherImpl); }; @@ -224,8 +262,8 @@ InotifyReader::InotifyReader() shutdown_pipe_[1] = -1; if (inotify_fd_ >= 0 && pipe(shutdown_pipe_) == 0 && thread_.Start()) { thread_.message_loop()->PostTask( - FROM_HERE, Bind(&InotifyReaderCallback, this, inotify_fd_, - shutdown_pipe_[0])); + FROM_HERE, + Bind(&InotifyReaderCallback, this, inotify_fd_, shutdown_pipe_[0])); valid_ = true; } } @@ -255,7 +293,7 @@ InotifyReader::Watch InotifyReader::AddWatch( AutoLock auto_lock(lock_); Watch watch = inotify_add_watch(inotify_fd_, path.value().c_str(), - IN_CREATE | IN_DELETE | + IN_ATTRIB | IN_CREATE | IN_DELETE | IN_CLOSE_WRITE | IN_MOVE | IN_ONLYDIR); @@ -267,10 +305,9 @@ InotifyReader::Watch InotifyReader::AddWatch( return watch; } -bool InotifyReader::RemoveWatch(Watch watch, - FilePathWatcherImpl* watcher) { - if (!valid_) - return false; +void InotifyReader::RemoveWatch(Watch watch, FilePathWatcherImpl* watcher) { + if (!valid_ || (watch == kInvalidWatch)) + return; AutoLock auto_lock(lock_); @@ -278,10 +315,8 @@ bool InotifyReader::RemoveWatch(Watch watch, if (watchers_[watch].empty()) { watchers_.erase(watch); - return (inotify_rm_watch(inotify_fd_, watch) == 0); + inotify_rm_watch(inotify_fd_, watch); } - - return true; } void InotifyReader::OnInotifyEvent(const inotify_event* event) { @@ -296,73 +331,96 @@ void InotifyReader::OnInotifyEvent(const inotify_event* event) { ++watcher) { (*watcher)->OnFilePathChanged(event->wd, child, - event->mask & (IN_CREATE | IN_MOVED_TO)); + event->mask & (IN_CREATE | IN_MOVED_TO), + event->mask & (IN_DELETE | IN_MOVED_FROM), + event->mask & IN_ISDIR); } } -FilePathWatcherImpl::FilePathWatcherImpl() { +FilePathWatcherImpl::FilePathWatcherImpl() + : recursive_(false) { } void FilePathWatcherImpl::OnFilePathChanged(InotifyReader::Watch fired_watch, const FilePath::StringType& child, - bool created) { + bool created, + bool deleted, + bool is_dir) { if (!message_loop()->BelongsToCurrentThread()) { - // Switch to message_loop_ to access watches_ safely. - message_loop()->PostTask(FROM_HERE, - Bind(&FilePathWatcherImpl::OnFilePathChanged, - this, - fired_watch, - child, - created)); + // Switch to message_loop() to access |watches_| safely. + message_loop()->PostTask( + FROM_HERE, + Bind(&FilePathWatcherImpl::OnFilePathChanged, this, + fired_watch, child, created, deleted, is_dir)); + return; + } + + // Check to see if CancelOnMessageLoopThread() has already been called. + // May happen when code flow reaches here from the PostTask() above. + if (watches_.empty()) { + DCHECK(target_.empty()); return; } DCHECK(MessageLoopForIO::current()); + DCHECK(HasValidWatchVector()); + + // Used below to avoid multiple recursive updates. + bool did_update = false; // Find the entry in |watches_| that corresponds to |fired_watch|. - WatchVector::const_iterator watch_entry(watches_.begin()); - for ( ; watch_entry != watches_.end(); ++watch_entry) { - if (fired_watch == watch_entry->watch_) { - // Check whether a path component of |target_| changed. - bool change_on_target_path = child.empty() || - ((child == watch_entry->subdir_) && watch_entry->linkname_.empty()) || - (child == watch_entry->linkname_); - - // Check whether the change references |target_| or a direct child. - DCHECK(watch_entry->subdir_.empty() || - (watch_entry + 1) != watches_.end()); - bool target_changed = - (watch_entry->subdir_.empty() && (child == watch_entry->linkname_)) || - (watch_entry->subdir_.empty() && watch_entry->linkname_.empty()) || - (watch_entry->subdir_ == child && (watch_entry + 1)->subdir_.empty()); - - // Update watches if a directory component of the |target_| path - // (dis)appears. Note that we don't add the additional restriction - // of checking the event mask to see if it is for a directory here - // as changes to symlinks on the target path will not have - // IN_ISDIR set in the event masks. As a result we may sometimes - // call UpdateWatches() unnecessarily. - if (change_on_target_path && !UpdateWatches()) { - callback_.Run(target_, true /* error */); - return; - } + for (size_t i = 0; i < watches_.size(); ++i) { + const WatchEntry& watch_entry = watches_[i]; + if (fired_watch != watch_entry.watch) + continue; + + // Check whether a path component of |target_| changed. + bool change_on_target_path = + child.empty() || + (child == watch_entry.linkname) || + (child == watch_entry.subdir); + + // Check if the change references |target_| or a direct child of |target_|. + bool is_watch_for_target = watch_entry.subdir.empty(); + bool target_changed = + (is_watch_for_target && (child == watch_entry.linkname)) || + (is_watch_for_target && watch_entry.linkname.empty()) || + (watch_entry.subdir == child && watches_[i + 1].subdir.empty()); + + // Update watches if a directory component of the |target_| path + // (dis)appears. Note that we don't add the additional restriction of + // checking the event mask to see if it is for a directory here as changes + // to symlinks on the target path will not have IN_ISDIR set in the event + // masks. As a result we may sometimes call UpdateWatches() unnecessarily. + if (change_on_target_path && (created || deleted) && !did_update) { + UpdateWatches(); + did_update = true; + } - // Report the following events: - // - The target or a direct child of the target got changed (in case the - // watched path refers to a directory). - // - One of the parent directories got moved or deleted, since the target - // disappears in this case. - // - One of the parent directories appears. The event corresponding to - // the target appearing might have been missed in this case, so - // recheck. - if (target_changed || - (change_on_target_path && !created) || - (change_on_target_path && PathExists(target_))) { - callback_.Run(target_, false); - return; + // Report the following events: + // - The target or a direct child of the target got changed (in case the + // watched path refers to a directory). + // - One of the parent directories got moved or deleted, since the target + // disappears in this case. + // - One of the parent directories appears. The event corresponding to + // the target appearing might have been missed in this case, so recheck. + if (target_changed || + (change_on_target_path && deleted) || + (change_on_target_path && created && PathExists(target_))) { + if (!did_update) { + UpdateRecursiveWatches(fired_watch, is_dir); + did_update = true; } + callback_.Run(target_, false /* error */); + return; } } + + if (ContainsKey(recursive_paths_by_watch_, fired_watch)) { + if (!did_update) + UpdateRecursiveWatches(fired_watch, is_dir); + callback_.Run(target_, false /* error */); + } } bool FilePathWatcherImpl::Watch(const FilePath& path, @@ -370,120 +428,238 @@ bool FilePathWatcherImpl::Watch(const FilePath& path, const FilePathWatcher::Callback& callback) { DCHECK(target_.empty()); DCHECK(MessageLoopForIO::current()); - if (recursive) { - // Recursive watch is not supported on this platform. - NOTIMPLEMENTED(); - return false; - } set_message_loop(MessageLoopProxy::current().get()); callback_ = callback; target_ = path; + recursive_ = recursive; MessageLoop::current()->AddDestructionObserver(this); std::vector<FilePath::StringType> comps; target_.GetComponents(&comps); DCHECK(!comps.empty()); - std::vector<FilePath::StringType>::const_iterator comp = comps.begin(); - for (++comp; comp != comps.end(); ++comp) - watches_.push_back(WatchEntry(InotifyReader::kInvalidWatch, *comp)); - - watches_.push_back(WatchEntry(InotifyReader::kInvalidWatch, - FilePath::StringType())); - return UpdateWatches(); + for (size_t i = 1; i < comps.size(); ++i) + watches_.push_back(WatchEntry(comps[i])); + watches_.push_back(WatchEntry(FilePath::StringType())); + UpdateWatches(); + return true; } void FilePathWatcherImpl::Cancel() { if (callback_.is_null()) { - // Watch was never called, or the |message_loop_| thread is already gone. + // Watch was never called, or the message_loop() thread is already gone. set_cancelled(); return; } - // Switch to the message_loop_ if necessary so we can access |watches_|. + // Switch to the message_loop() if necessary so we can access |watches_|. if (!message_loop()->BelongsToCurrentThread()) { message_loop()->PostTask(FROM_HERE, Bind(&FilePathWatcher::CancelWatch, - make_scoped_refptr(this))); + make_scoped_refptr(this))); } else { CancelOnMessageLoopThread(); } } void FilePathWatcherImpl::CancelOnMessageLoopThread() { - if (!is_cancelled()) - set_cancelled(); + DCHECK(message_loop()->BelongsToCurrentThread()); + set_cancelled(); if (!callback_.is_null()) { MessageLoop::current()->RemoveDestructionObserver(this); callback_.Reset(); } - for (WatchVector::iterator watch_entry(watches_.begin()); - watch_entry != watches_.end(); ++watch_entry) { - if (watch_entry->watch_ != InotifyReader::kInvalidWatch) - g_inotify_reader.Get().RemoveWatch(watch_entry->watch_, this); - } + for (size_t i = 0; i < watches_.size(); ++i) + g_inotify_reader.Get().RemoveWatch(watches_[i].watch, this); watches_.clear(); target_.clear(); + + if (recursive_) + RemoveRecursiveWatches(); } void FilePathWatcherImpl::WillDestroyCurrentMessageLoop() { CancelOnMessageLoopThread(); } -bool FilePathWatcherImpl::UpdateWatches() { - // Ensure this runs on the |message_loop_| exclusively in order to avoid +void FilePathWatcherImpl::UpdateWatches() { + // Ensure this runs on the message_loop() exclusively in order to avoid // concurrency issues. DCHECK(message_loop()->BelongsToCurrentThread()); + DCHECK(HasValidWatchVector()); // Walk the list of watches and update them as we go. FilePath path(FILE_PATH_LITERAL("/")); bool path_valid = true; - for (WatchVector::iterator watch_entry(watches_.begin()); - watch_entry != watches_.end(); ++watch_entry) { - InotifyReader::Watch old_watch = watch_entry->watch_; + for (size_t i = 0; i < watches_.size(); ++i) { + WatchEntry& watch_entry = watches_[i]; + InotifyReader::Watch old_watch = watch_entry.watch; + watch_entry.watch = InotifyReader::kInvalidWatch; + watch_entry.linkname.clear(); if (path_valid) { - watch_entry->watch_ = g_inotify_reader.Get().AddWatch(path, this); - if ((watch_entry->watch_ == InotifyReader::kInvalidWatch) && - base::IsLink(path)) { - FilePath link; - if (ReadSymbolicLink(path, &link)) { - if (!link.IsAbsolute()) - link = path.DirName().Append(link); - // Try watching symlink target directory. If the link target is "/", - // then we shouldn't get here in normal situations and if we do, we'd - // watch "/" for changes to a component "/" which is harmless so no - // special treatment of this case is required. - watch_entry->watch_ = - g_inotify_reader.Get().AddWatch(link.DirName(), this); - if (watch_entry->watch_ != InotifyReader::kInvalidWatch) { - watch_entry->linkname_ = link.BaseName().value(); - } else { - DPLOG(WARNING) << "Watch failed for " << link.DirName().value(); - // TODO(craig) Symlinks only work if the parent directory - // for the target exist. Ideally we should make sure we've - // watched all the components of the symlink path for - // changes. See crbug.com/91561 for details. - } + watch_entry.watch = g_inotify_reader.Get().AddWatch(path, this); + if (watch_entry.watch == InotifyReader::kInvalidWatch) { + if (IsLink(path)) { + path_valid = AddWatchForBrokenSymlink(path, &watch_entry); + } else { + path_valid = false; } } - if (watch_entry->watch_ == InotifyReader::kInvalidWatch) { - path_valid = false; - } - } else { - watch_entry->watch_ = InotifyReader::kInvalidWatch; } - if (old_watch != InotifyReader::kInvalidWatch && - old_watch != watch_entry->watch_) { + if (old_watch != watch_entry.watch) g_inotify_reader.Get().RemoveWatch(old_watch, this); + path = path.Append(watch_entry.subdir); + } + + UpdateRecursiveWatches(InotifyReader::kInvalidWatch, + false /* is directory? */); +} + +void FilePathWatcherImpl::UpdateRecursiveWatches( + InotifyReader::Watch fired_watch, + bool is_dir) { + if (!recursive_) + return; + + if (!DirectoryExists(target_)) { + RemoveRecursiveWatches(); + return; + } + + // Check to see if this is a forced update or if some component of |target_| + // has changed. For these cases, redo the watches for |target_| and below. + if (!ContainsKey(recursive_paths_by_watch_, fired_watch)) { + UpdateRecursiveWatchesForPath(target_); + return; + } + + // Underneath |target_|, only directory changes trigger watch updates. + if (!is_dir) + return; + + const FilePath& changed_dir = recursive_paths_by_watch_[fired_watch]; + + std::map<FilePath, InotifyReader::Watch>::iterator start_it = + recursive_watches_by_path_.lower_bound(changed_dir); + std::map<FilePath, InotifyReader::Watch>::iterator end_it = start_it; + for (; end_it != recursive_watches_by_path_.end(); ++end_it) { + const FilePath& cur_path = end_it->first; + if (!changed_dir.IsParent(cur_path)) + break; + if (!DirectoryExists(cur_path)) + g_inotify_reader.Get().RemoveWatch(end_it->second, this); + } + recursive_watches_by_path_.erase(start_it, end_it); + UpdateRecursiveWatchesForPath(changed_dir); +} + +void FilePathWatcherImpl::UpdateRecursiveWatchesForPath(const FilePath& path) { + DCHECK(recursive_); + DCHECK(!path.empty()); + DCHECK(DirectoryExists(path)); + + // Note: SHOW_SYM_LINKS exposes symlinks as symlinks, so they are ignored + // rather than followed. Following symlinks can easily lead to the undesirable + // situation where the entire file system is being watched. + FileEnumerator enumerator( + path, + true /* recursive enumeration */, + FileEnumerator::DIRECTORIES | FileEnumerator::SHOW_SYM_LINKS); + for (FilePath current = enumerator.Next(); + !current.empty(); + current = enumerator.Next()) { + DCHECK(enumerator.GetInfo().IsDirectory()); + + if (!ContainsKey(recursive_watches_by_path_, current)) { + // Add new watches. + InotifyReader::Watch watch = + g_inotify_reader.Get().AddWatch(current, this); + TrackWatchForRecursion(watch, current); + } else { + // Update existing watches. + InotifyReader::Watch old_watch = recursive_watches_by_path_[current]; + DCHECK_NE(InotifyReader::kInvalidWatch, old_watch); + InotifyReader::Watch watch = + g_inotify_reader.Get().AddWatch(current, this); + if (watch != old_watch) { + g_inotify_reader.Get().RemoveWatch(old_watch, this); + recursive_paths_by_watch_.erase(old_watch); + recursive_watches_by_path_.erase(current); + TrackWatchForRecursion(watch, current); + } } - path = path.Append(watch_entry->subdir_); } +} + +void FilePathWatcherImpl::TrackWatchForRecursion(InotifyReader::Watch watch, + const FilePath& path) { + DCHECK(recursive_); + DCHECK(!path.empty()); + DCHECK(target_.IsParent(path)); + + if (watch == InotifyReader::kInvalidWatch) + return; + + DCHECK(!ContainsKey(recursive_paths_by_watch_, watch)); + DCHECK(!ContainsKey(recursive_watches_by_path_, path)); + recursive_paths_by_watch_[watch] = path; + recursive_watches_by_path_[path] = watch; +} + +void FilePathWatcherImpl::RemoveRecursiveWatches() { + if (!recursive_) + return; + + for (hash_map<InotifyReader::Watch, FilePath>::const_iterator it = + recursive_paths_by_watch_.begin(); + it != recursive_paths_by_watch_.end(); + ++it) { + g_inotify_reader.Get().RemoveWatch(it->first, this); + } + recursive_paths_by_watch_.clear(); + recursive_watches_by_path_.clear(); +} +bool FilePathWatcherImpl::AddWatchForBrokenSymlink(const FilePath& path, + WatchEntry* watch_entry) { + DCHECK_EQ(InotifyReader::kInvalidWatch, watch_entry->watch); + FilePath link; + if (!ReadSymbolicLink(path, &link)) + return false; + + if (!link.IsAbsolute()) + link = path.DirName().Append(link); + + // Try watching symlink target directory. If the link target is "/", then we + // shouldn't get here in normal situations and if we do, we'd watch "/" for + // changes to a component "/" which is harmless so no special treatment of + // this case is required. + InotifyReader::Watch watch = + g_inotify_reader.Get().AddWatch(link.DirName(), this); + if (watch == InotifyReader::kInvalidWatch) { + // TODO(craig) Symlinks only work if the parent directory for the target + // exist. Ideally we should make sure we've watched all the components of + // the symlink path for changes. See crbug.com/91561 for details. + DPLOG(WARNING) << "Watch failed for " << link.DirName().value(); + return false; + } + watch_entry->watch = watch; + watch_entry->linkname = link.BaseName().value(); return true; } +bool FilePathWatcherImpl::HasValidWatchVector() const { + if (watches_.empty()) + return false; + for (size_t i = 0; i < watches_.size() - 1; ++i) { + if (watches_[i].subdir.empty()) + return false; + } + return watches_[watches_.size() - 1].subdir.empty(); +} + } // namespace FilePathWatcher::FilePathWatcher() { diff --git a/chromium/base/files/file_path_watcher_mac.cc b/chromium/base/files/file_path_watcher_mac.cc new file mode 100644 index 00000000000..54ca46d12bb --- /dev/null +++ b/chromium/base/files/file_path_watcher_mac.cc @@ -0,0 +1,60 @@ +// Copyright 2014 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. + +#include "base/files/file_path_watcher.h" +#include "base/files/file_path_watcher_kqueue.h" + +#if !defined(OS_IOS) +#include "base/files/file_path_watcher_fsevents.h" +#endif + +namespace base { + +namespace { + +class FilePathWatcherImpl : public FilePathWatcher::PlatformDelegate { + public: + virtual bool Watch(const FilePath& path, + bool recursive, + const FilePathWatcher::Callback& callback) OVERRIDE { + // Use kqueue for non-recursive watches and FSEvents for recursive ones. + DCHECK(!impl_.get()); + if (recursive) { + if (!FilePathWatcher::RecursiveWatchAvailable()) + return false; +#if !defined(OS_IOS) + impl_ = new FilePathWatcherFSEvents(); +#endif // OS_IOS + } else { + impl_ = new FilePathWatcherKQueue(); + } + DCHECK(impl_.get()); + return impl_->Watch(path, recursive, callback); + } + + virtual void Cancel() OVERRIDE { + if (impl_) + impl_->Cancel(); + set_cancelled(); + } + + virtual void CancelOnMessageLoopThread() OVERRIDE { + if (impl_) + impl_->Cancel(); + set_cancelled(); + } + + protected: + virtual ~FilePathWatcherImpl() {} + + scoped_refptr<PlatformDelegate> impl_; +}; + +} // namespace + +FilePathWatcher::FilePathWatcher() { + impl_ = new FilePathWatcherImpl(); +} + +} // namespace base diff --git a/chromium/base/files/file_path_watcher_win.cc b/chromium/base/files/file_path_watcher_win.cc index 2abbceacd29..6c10c12d313 100644 --- a/chromium/base/files/file_path_watcher_win.cc +++ b/chromium/base/files/file_path_watcher_win.cc @@ -6,6 +6,7 @@ #include "base/bind.h" #include "base/file_util.h" +#include "base/files/file.h" #include "base/files/file_path.h" #include "base/logging.h" #include "base/memory/ref_counted.h" @@ -96,6 +97,12 @@ bool FilePathWatcherImpl::Watch(const FilePath& path, recursive_watch_ = recursive; MessageLoop::current()->AddDestructionObserver(this); + File::Info file_info; + if (GetFileInfo(target_, &file_info)) { + last_modified_ = file_info.last_modified; + first_notification_ = Time::Now(); + } + if (!UpdateWatch()) return false; @@ -122,6 +129,7 @@ void FilePathWatcherImpl::Cancel() { } void FilePathWatcherImpl::CancelOnMessageLoopThread() { + DCHECK(message_loop()->BelongsToCurrentThread()); set_cancelled(); if (handle_ != INVALID_HANDLE_VALUE) @@ -148,14 +156,23 @@ void FilePathWatcherImpl::OnObjectSignaled(HANDLE object) { } // Check whether the event applies to |target_| and notify the callback. - PlatformFileInfo file_info; + File::Info file_info; bool file_exists = GetFileInfo(target_, &file_info); - if (file_exists && (last_modified_.is_null() || - last_modified_ != file_info.last_modified)) { + if (recursive_watch_) { + // Only the mtime of |target_| is tracked but in a recursive watch, + // some other file or directory may have changed so all notifications + // are passed through. It is possible to figure out which file changed + // using ReadDirectoryChangesW() instead of FindFirstChangeNotification(), + // but that function is quite complicated: + // http://qualapps.blogspot.com/2010/05/understanding-readdirectorychangesw.html + callback_.Run(target_, false); + } else if (file_exists && (last_modified_.is_null() || + last_modified_ != file_info.last_modified)) { last_modified_ = file_info.last_modified; first_notification_ = Time::Now(); callback_.Run(target_, false); - } else if (file_exists && !first_notification_.is_null()) { + } else if (file_exists && last_modified_ == file_info.last_modified && + !first_notification_.is_null()) { // The target's last modification time is equal to what's on record. This // means that either an unrelated event occurred, or the target changed // again (file modification times only have a resolution of 1s). Comparing @@ -229,12 +246,6 @@ bool FilePathWatcherImpl::UpdateWatch() { if (handle_ != INVALID_HANDLE_VALUE) DestroyWatch(); - PlatformFileInfo file_info; - if (GetFileInfo(target_, &file_info)) { - last_modified_ = file_info.last_modified; - first_notification_ = Time::Now(); - } - // Start at the target and walk up the directory chain until we succesfully // create a watch handle in |handle_|. |child_dirs| keeps a stack of child // directories stripped from target, in reverse order. diff --git a/chromium/base/files/file_posix.cc b/chromium/base/files/file_posix.cc index 9d97c336aa6..0764ee98660 100644 --- a/chromium/base/files/file_posix.cc +++ b/chromium/base/files/file_posix.cc @@ -32,13 +32,11 @@ COMPILE_ASSERT(File::FROM_BEGIN == SEEK_SET && namespace { #if defined(OS_BSD) || defined(OS_MACOSX) || defined(OS_NACL) -typedef struct stat stat_wrapper_t; static int CallFstat(int fd, stat_wrapper_t *sb) { base::ThreadRestrictions::AssertIOAllowed(); return fstat(fd, sb); } #else -typedef struct stat64 stat_wrapper_t; static int CallFstat(int fd, stat_wrapper_t *sb) { base::ThreadRestrictions::AssertIOAllowed(); return fstat64(fd, sb); @@ -119,13 +117,63 @@ static File::Error CallFctnlFlock(PlatformFile file, bool do_lock) { } // namespace +void File::Info::FromStat(const stat_wrapper_t& stat_info) { + is_directory = S_ISDIR(stat_info.st_mode); + is_symbolic_link = S_ISLNK(stat_info.st_mode); + size = stat_info.st_size; + +#if defined(OS_LINUX) + time_t last_modified_sec = stat_info.st_mtim.tv_sec; + int64 last_modified_nsec = stat_info.st_mtim.tv_nsec; + time_t last_accessed_sec = stat_info.st_atim.tv_sec; + int64 last_accessed_nsec = stat_info.st_atim.tv_nsec; + time_t creation_time_sec = stat_info.st_ctim.tv_sec; + int64 creation_time_nsec = stat_info.st_ctim.tv_nsec; +#elif defined(OS_ANDROID) + time_t last_modified_sec = stat_info.st_mtime; + int64 last_modified_nsec = stat_info.st_mtime_nsec; + time_t last_accessed_sec = stat_info.st_atime; + int64 last_accessed_nsec = stat_info.st_atime_nsec; + time_t creation_time_sec = stat_info.st_ctime; + int64 creation_time_nsec = stat_info.st_ctime_nsec; +#elif defined(OS_MACOSX) || defined(OS_IOS) || defined(OS_BSD) + time_t last_modified_sec = stat_info.st_mtimespec.tv_sec; + int64 last_modified_nsec = stat_info.st_mtimespec.tv_nsec; + time_t last_accessed_sec = stat_info.st_atimespec.tv_sec; + int64 last_accessed_nsec = stat_info.st_atimespec.tv_nsec; + time_t creation_time_sec = stat_info.st_ctimespec.tv_sec; + int64 creation_time_nsec = stat_info.st_ctimespec.tv_nsec; +#else + time_t last_modified_sec = stat_info.st_mtime; + int64 last_modified_nsec = 0; + time_t last_accessed_sec = stat_info.st_atime; + int64 last_accessed_nsec = 0; + time_t creation_time_sec = stat_info.st_ctime; + int64 creation_time_nsec = 0; +#endif + + last_modified = + Time::FromTimeT(last_modified_sec) + + TimeDelta::FromMicroseconds(last_modified_nsec / + Time::kNanosecondsPerMicrosecond); + + last_accessed = + Time::FromTimeT(last_accessed_sec) + + TimeDelta::FromMicroseconds(last_accessed_nsec / + Time::kNanosecondsPerMicrosecond); + + creation_time = + Time::FromTimeT(creation_time_sec) + + TimeDelta::FromMicroseconds(creation_time_nsec / + Time::kNanosecondsPerMicrosecond); +} + // NaCl doesn't implement system calls to open files directly. #if !defined(OS_NACL) // TODO(erikkay): does it make sense to support FLAG_EXCLUSIVE_* here? -void File::CreateBaseFileUnsafe(const FilePath& name, uint32 flags) { +void File::InitializeUnsafe(const FilePath& name, uint32 flags) { base::ThreadRestrictions::AssertIOAllowed(); DCHECK(!IsValid()); - DCHECK(!(flags & FLAG_ASYNC)); int open_flags = 0; if (flags & FLAG_CREATE) @@ -135,6 +183,7 @@ void File::CreateBaseFileUnsafe(const FilePath& name, uint32 flags) { if (flags & FLAG_CREATE_ALWAYS) { DCHECK(!open_flags); + DCHECK(flags & FLAG_WRITE); open_flags = O_CREAT | O_TRUNC; } @@ -147,7 +196,7 @@ void File::CreateBaseFileUnsafe(const FilePath& name, uint32 flags) { if (!open_flags && !(flags & FLAG_OPEN) && !(flags & FLAG_OPEN_ALWAYS)) { NOTREACHED(); errno = EOPNOTSUPP; - error_ = FILE_ERROR_FAILED; + error_details_ = FILE_ERROR_FAILED; return; } @@ -191,47 +240,51 @@ void File::CreateBaseFileUnsafe(const FilePath& name, uint32 flags) { } } - if (descriptor >= 0 && (flags & (FLAG_CREATE_ALWAYS | FLAG_CREATE))) + if (descriptor < 0) { + error_details_ = File::OSErrorToFileError(errno); + return; + } + + if (flags & (FLAG_CREATE_ALWAYS | FLAG_CREATE)) created_ = true; - if ((descriptor >= 0) && (flags & FLAG_DELETE_ON_CLOSE)) + if (flags & FLAG_DELETE_ON_CLOSE) unlink(name.value().c_str()); - if (descriptor >= 0) - error_ = FILE_OK; - else - error_ = File::OSErrorToFileError(errno); - - file_ = descriptor; + async_ = ((flags & FLAG_ASYNC) == FLAG_ASYNC); + error_details_ = FILE_OK; + file_.reset(descriptor); } #endif // !defined(OS_NACL) bool File::IsValid() const { - return file_ >= 0; + return file_.is_valid(); +} + +PlatformFile File::GetPlatformFile() const { + return file_.get(); } PlatformFile File::TakePlatformFile() { - PlatformFile file = file_; - file_ = kInvalidPlatformFileValue; - return file; + return file_.release(); } void File::Close() { - base::ThreadRestrictions::AssertIOAllowed(); if (!IsValid()) return; - if (!IGNORE_EINTR(close(file_))) - file_ = kInvalidPlatformFileValue; + base::ThreadRestrictions::AssertIOAllowed(); + file_.reset(); } int64 File::Seek(Whence whence, int64 offset) { base::ThreadRestrictions::AssertIOAllowed(); DCHECK(IsValid()); - if (file_ < 0 || offset < 0) + if (offset < 0) return -1; - return lseek(file_, static_cast<off_t>(offset), static_cast<int>(whence)); + return lseek(file_.get(), static_cast<off_t>(offset), + static_cast<int>(whence)); } int File::Read(int64 offset, char* data, int size) { @@ -243,7 +296,7 @@ int File::Read(int64 offset, char* data, int size) { int bytes_read = 0; int rv; do { - rv = HANDLE_EINTR(pread(file_, data + bytes_read, + rv = HANDLE_EINTR(pread(file_.get(), data + bytes_read, size - bytes_read, offset + bytes_read)); if (rv <= 0) break; @@ -263,7 +316,7 @@ int File::ReadAtCurrentPos(char* data, int size) { int bytes_read = 0; int rv; do { - rv = HANDLE_EINTR(read(file_, data, size)); + rv = HANDLE_EINTR(read(file_.get(), data + bytes_read, size - bytes_read)); if (rv <= 0) break; @@ -277,7 +330,7 @@ int File::ReadNoBestEffort(int64 offset, char* data, int size) { base::ThreadRestrictions::AssertIOAllowed(); DCHECK(IsValid()); - return HANDLE_EINTR(pread(file_, data, size, offset)); + return HANDLE_EINTR(pread(file_.get(), data, size, offset)); } int File::ReadAtCurrentPosNoBestEffort(char* data, int size) { @@ -286,13 +339,13 @@ int File::ReadAtCurrentPosNoBestEffort(char* data, int size) { if (size < 0) return -1; - return HANDLE_EINTR(read(file_, data, size)); + return HANDLE_EINTR(read(file_.get(), data, size)); } int File::Write(int64 offset, const char* data, int size) { base::ThreadRestrictions::AssertIOAllowed(); - if (IsOpenAppend(file_)) + if (IsOpenAppend(file_.get())) return WriteAtCurrentPos(data, size); DCHECK(IsValid()); @@ -302,7 +355,7 @@ int File::Write(int64 offset, const char* data, int size) { int bytes_written = 0; int rv; do { - rv = HANDLE_EINTR(pwrite(file_, data + bytes_written, + rv = HANDLE_EINTR(pwrite(file_.get(), data + bytes_written, size - bytes_written, offset + bytes_written)); if (rv <= 0) break; @@ -322,7 +375,8 @@ int File::WriteAtCurrentPos(const char* data, int size) { int bytes_written = 0; int rv; do { - rv = HANDLE_EINTR(write(file_, data, size)); + rv = HANDLE_EINTR(write(file_.get(), data + bytes_written, + size - bytes_written)); if (rv <= 0) break; @@ -338,19 +392,29 @@ int File::WriteAtCurrentPosNoBestEffort(const char* data, int size) { if (size < 0) return -1; - return HANDLE_EINTR(write(file_, data, size)); + return HANDLE_EINTR(write(file_.get(), data, size)); } -bool File::Truncate(int64 length) { +int64 File::GetLength() { + DCHECK(IsValid()); + + stat_wrapper_t file_info; + if (CallFstat(file_.get(), &file_info)) + return false; + + return file_info.st_size; +} + +bool File::SetLength(int64 length) { base::ThreadRestrictions::AssertIOAllowed(); DCHECK(IsValid()); - return !CallFtruncate(file_, length); + return !CallFtruncate(file_.get(), length); } bool File::Flush() { base::ThreadRestrictions::AssertIOAllowed(); DCHECK(IsValid()); - return !CallFsync(file_); + return !CallFsync(file_.get()); } bool File::SetTimes(Time last_access_time, Time last_modified_time) { @@ -361,72 +425,26 @@ bool File::SetTimes(Time last_access_time, Time last_modified_time) { times[0] = last_access_time.ToTimeVal(); times[1] = last_modified_time.ToTimeVal(); - return !CallFutimes(file_, times); + return !CallFutimes(file_.get(), times); } bool File::GetInfo(Info* info) { DCHECK(IsValid()); stat_wrapper_t file_info; - if (CallFstat(file_, &file_info)) + if (CallFstat(file_.get(), &file_info)) return false; - info->is_directory = S_ISDIR(file_info.st_mode); - info->is_symbolic_link = S_ISLNK(file_info.st_mode); - info->size = file_info.st_size; - -#if defined(OS_LINUX) - const time_t last_modified_sec = file_info.st_mtim.tv_sec; - const int64 last_modified_nsec = file_info.st_mtim.tv_nsec; - const time_t last_accessed_sec = file_info.st_atim.tv_sec; - const int64 last_accessed_nsec = file_info.st_atim.tv_nsec; - const time_t creation_time_sec = file_info.st_ctim.tv_sec; - const int64 creation_time_nsec = file_info.st_ctim.tv_nsec; -#elif defined(OS_ANDROID) - const time_t last_modified_sec = file_info.st_mtime; - const int64 last_modified_nsec = file_info.st_mtime_nsec; - const time_t last_accessed_sec = file_info.st_atime; - const int64 last_accessed_nsec = file_info.st_atime_nsec; - const time_t creation_time_sec = file_info.st_ctime; - const int64 creation_time_nsec = file_info.st_ctime_nsec; -#elif defined(OS_MACOSX) || defined(OS_IOS) || defined(OS_BSD) - const time_t last_modified_sec = file_info.st_mtimespec.tv_sec; - const int64 last_modified_nsec = file_info.st_mtimespec.tv_nsec; - const time_t last_accessed_sec = file_info.st_atimespec.tv_sec; - const int64 last_accessed_nsec = file_info.st_atimespec.tv_nsec; - const time_t creation_time_sec = file_info.st_ctimespec.tv_sec; - const int64 creation_time_nsec = file_info.st_ctimespec.tv_nsec; -#else - // TODO(gavinp): Investigate a good high resolution option for OS_NACL. - const time_t last_modified_sec = file_info.st_mtime; - const int64 last_modified_nsec = 0; - const time_t last_accessed_sec = file_info.st_atime; - const int64 last_accessed_nsec = 0; - const time_t creation_time_sec = file_info.st_ctime; - const int64 creation_time_nsec = 0; -#endif - - info->last_modified = - base::Time::FromTimeT(last_modified_sec) + - base::TimeDelta::FromMicroseconds(last_modified_nsec / - base::Time::kNanosecondsPerMicrosecond); - info->last_accessed = - base::Time::FromTimeT(last_accessed_sec) + - base::TimeDelta::FromMicroseconds(last_accessed_nsec / - base::Time::kNanosecondsPerMicrosecond); - info->creation_time = - base::Time::FromTimeT(creation_time_sec) + - base::TimeDelta::FromMicroseconds(creation_time_nsec / - base::Time::kNanosecondsPerMicrosecond); + info->FromStat(file_info); return true; } File::Error File::Lock() { - return CallFctnlFlock(file_, true); + return CallFctnlFlock(file_.get(), true); } File::Error File::Unlock() { - return CallFctnlFlock(file_, false); + return CallFctnlFlock(file_.get(), false); } // Static. @@ -463,8 +481,8 @@ File::Error File::OSErrorToFileError(int saved_errno) { } void File::SetPlatformFile(PlatformFile file) { - DCHECK_EQ(file_, kInvalidPlatformFileValue); - file_ = file; + DCHECK(!file_.is_valid()); + file_.reset(file); } } // namespace base diff --git a/chromium/base/files/file_proxy.cc b/chromium/base/files/file_proxy.cc new file mode 100644 index 00000000000..291b98d1117 --- /dev/null +++ b/chromium/base/files/file_proxy.cc @@ -0,0 +1,359 @@ +// Copyright 2014 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. + +#include "base/files/file_proxy.h" + +#include "base/bind.h" +#include "base/bind_helpers.h" +#include "base/file_util.h" +#include "base/files/file.h" +#include "base/location.h" +#include "base/message_loop/message_loop_proxy.h" +#include "base/task_runner.h" +#include "base/task_runner_util.h" + +namespace { + +void FileDeleter(base::File file) { +} + +} // namespace + +namespace base { + +class FileHelper { + public: + FileHelper(FileProxy* proxy, File file) + : file_(file.Pass()), + error_(File::FILE_ERROR_FAILED), + task_runner_(proxy->task_runner()), + proxy_(AsWeakPtr(proxy)) { + } + + void PassFile() { + if (proxy_) + proxy_->SetFile(file_.Pass()); + else if (file_.IsValid()) + task_runner_->PostTask(FROM_HERE, Bind(&FileDeleter, Passed(&file_))); + } + + protected: + File file_; + File::Error error_; + + private: + scoped_refptr<TaskRunner> task_runner_; + WeakPtr<FileProxy> proxy_; + DISALLOW_COPY_AND_ASSIGN(FileHelper); +}; + +namespace { + +class GenericFileHelper : public FileHelper { + public: + GenericFileHelper(FileProxy* proxy, File file) + : FileHelper(proxy, file.Pass()) { + } + + void Close() { + file_.Close(); + error_ = File::FILE_OK; + } + + void SetTimes(Time last_access_time, Time last_modified_time) { + bool rv = file_.SetTimes(last_access_time, last_modified_time); + error_ = rv ? File::FILE_OK : File::FILE_ERROR_FAILED; + } + + void SetLength(int64 length) { + if (file_.SetLength(length)) + error_ = File::FILE_OK; + } + + void Flush() { + if (file_.Flush()) + error_ = File::FILE_OK; + } + + void Reply(const FileProxy::StatusCallback& callback) { + PassFile(); + if (!callback.is_null()) + callback.Run(error_); + } + + private: + DISALLOW_COPY_AND_ASSIGN(GenericFileHelper); +}; + +class CreateOrOpenHelper : public FileHelper { + public: + CreateOrOpenHelper(FileProxy* proxy, File file) + : FileHelper(proxy, file.Pass()) { + } + + void RunWork(const FilePath& file_path, int file_flags) { + file_.Initialize(file_path, file_flags); + error_ = file_.IsValid() ? File::FILE_OK : file_.error_details(); + } + + void Reply(const FileProxy::StatusCallback& callback) { + DCHECK(!callback.is_null()); + PassFile(); + callback.Run(error_); + } + + private: + DISALLOW_COPY_AND_ASSIGN(CreateOrOpenHelper); +}; + +class CreateTemporaryHelper : public FileHelper { + public: + CreateTemporaryHelper(FileProxy* proxy, File file) + : FileHelper(proxy, file.Pass()) { + } + + void RunWork(uint32 additional_file_flags) { + // TODO(darin): file_util should have a variant of CreateTemporaryFile + // that returns a FilePath and a File. + if (!CreateTemporaryFile(&file_path_)) { + // TODO(davidben): base::CreateTemporaryFile should preserve the error + // code. + error_ = File::FILE_ERROR_FAILED; + return; + } + + uint32 file_flags = File::FLAG_WRITE | + File::FLAG_TEMPORARY | + File::FLAG_CREATE_ALWAYS | + additional_file_flags; + + file_.Initialize(file_path_, file_flags); + if (file_.IsValid()) { + error_ = File::FILE_OK; + } else { + error_ = file_.error_details(); + DeleteFile(file_path_, false); + file_path_.clear(); + } + } + + void Reply(const FileProxy::CreateTemporaryCallback& callback) { + DCHECK(!callback.is_null()); + PassFile(); + callback.Run(error_, file_path_); + } + + private: + FilePath file_path_; + DISALLOW_COPY_AND_ASSIGN(CreateTemporaryHelper); +}; + +class GetInfoHelper : public FileHelper { + public: + GetInfoHelper(FileProxy* proxy, File file) + : FileHelper(proxy, file.Pass()) { + } + + void RunWork() { + if (file_.GetInfo(&file_info_)) + error_ = File::FILE_OK; + } + + void Reply(const FileProxy::GetFileInfoCallback& callback) { + PassFile(); + DCHECK(!callback.is_null()); + callback.Run(error_, file_info_); + } + + private: + File::Info file_info_; + DISALLOW_COPY_AND_ASSIGN(GetInfoHelper); +}; + +class ReadHelper : public FileHelper { + public: + ReadHelper(FileProxy* proxy, File file, int bytes_to_read) + : FileHelper(proxy, file.Pass()), + buffer_(new char[bytes_to_read]), + bytes_to_read_(bytes_to_read), + bytes_read_(0) { + } + + void RunWork(int64 offset) { + bytes_read_ = file_.Read(offset, buffer_.get(), bytes_to_read_); + error_ = (bytes_read_ < 0) ? File::FILE_ERROR_FAILED : File::FILE_OK; + } + + void Reply(const FileProxy::ReadCallback& callback) { + PassFile(); + DCHECK(!callback.is_null()); + callback.Run(error_, buffer_.get(), bytes_read_); + } + + private: + scoped_ptr<char[]> buffer_; + int bytes_to_read_; + int bytes_read_; + DISALLOW_COPY_AND_ASSIGN(ReadHelper); +}; + +class WriteHelper : public FileHelper { + public: + WriteHelper(FileProxy* proxy, + File file, + const char* buffer, int bytes_to_write) + : FileHelper(proxy, file.Pass()), + buffer_(new char[bytes_to_write]), + bytes_to_write_(bytes_to_write), + bytes_written_(0) { + memcpy(buffer_.get(), buffer, bytes_to_write); + } + + void RunWork(int64 offset) { + bytes_written_ = file_.Write(offset, buffer_.get(), bytes_to_write_); + error_ = (bytes_written_ < 0) ? File::FILE_ERROR_FAILED : File::FILE_OK; + } + + void Reply(const FileProxy::WriteCallback& callback) { + PassFile(); + if (!callback.is_null()) + callback.Run(error_, bytes_written_); + } + + private: + scoped_ptr<char[]> buffer_; + int bytes_to_write_; + int bytes_written_; + DISALLOW_COPY_AND_ASSIGN(WriteHelper); +}; + +} // namespace + +FileProxy::FileProxy(TaskRunner* task_runner) : task_runner_(task_runner) { +} + +FileProxy::~FileProxy() { + if (file_.IsValid()) + task_runner_->PostTask(FROM_HERE, Bind(&FileDeleter, Passed(&file_))); +} + +bool FileProxy::CreateOrOpen(const FilePath& file_path, + uint32 file_flags, + const StatusCallback& callback) { + DCHECK(!file_.IsValid()); + CreateOrOpenHelper* helper = new CreateOrOpenHelper(this, File()); + return task_runner_->PostTaskAndReply( + FROM_HERE, + Bind(&CreateOrOpenHelper::RunWork, Unretained(helper), file_path, + file_flags), + Bind(&CreateOrOpenHelper::Reply, Owned(helper), callback)); +} + +bool FileProxy::CreateTemporary(uint32 additional_file_flags, + const CreateTemporaryCallback& callback) { + DCHECK(!file_.IsValid()); + CreateTemporaryHelper* helper = new CreateTemporaryHelper(this, File()); + return task_runner_->PostTaskAndReply( + FROM_HERE, + Bind(&CreateTemporaryHelper::RunWork, Unretained(helper), + additional_file_flags), + Bind(&CreateTemporaryHelper::Reply, Owned(helper), callback)); +} + +bool FileProxy::IsValid() const { + return file_.IsValid(); +} + +void FileProxy::SetFile(File file) { + DCHECK(!file_.IsValid()); + file_ = file.Pass(); +} + +File FileProxy::TakeFile() { + return file_.Pass(); +} + +PlatformFile FileProxy::GetPlatformFile() const { + return file_.GetPlatformFile(); +} + +bool FileProxy::Close(const StatusCallback& callback) { + DCHECK(file_.IsValid()); + GenericFileHelper* helper = new GenericFileHelper(this, file_.Pass()); + return task_runner_->PostTaskAndReply( + FROM_HERE, + Bind(&GenericFileHelper::Close, Unretained(helper)), + Bind(&GenericFileHelper::Reply, Owned(helper), callback)); +} + +bool FileProxy::GetInfo(const GetFileInfoCallback& callback) { + DCHECK(file_.IsValid()); + GetInfoHelper* helper = new GetInfoHelper(this, file_.Pass()); + return task_runner_->PostTaskAndReply( + FROM_HERE, + Bind(&GetInfoHelper::RunWork, Unretained(helper)), + Bind(&GetInfoHelper::Reply, Owned(helper), callback)); +} + +bool FileProxy::Read(int64 offset, + int bytes_to_read, + const ReadCallback& callback) { + DCHECK(file_.IsValid()); + if (bytes_to_read < 0) + return false; + + ReadHelper* helper = new ReadHelper(this, file_.Pass(), bytes_to_read); + return task_runner_->PostTaskAndReply( + FROM_HERE, + Bind(&ReadHelper::RunWork, Unretained(helper), offset), + Bind(&ReadHelper::Reply, Owned(helper), callback)); +} + +bool FileProxy::Write(int64 offset, + const char* buffer, + int bytes_to_write, + const WriteCallback& callback) { + DCHECK(file_.IsValid()); + if (bytes_to_write <= 0 || buffer == NULL) + return false; + + WriteHelper* helper = + new WriteHelper(this, file_.Pass(), buffer, bytes_to_write); + return task_runner_->PostTaskAndReply( + FROM_HERE, + Bind(&WriteHelper::RunWork, Unretained(helper), offset), + Bind(&WriteHelper::Reply, Owned(helper), callback)); +} + +bool FileProxy::SetTimes(Time last_access_time, + Time last_modified_time, + const StatusCallback& callback) { + DCHECK(file_.IsValid()); + GenericFileHelper* helper = new GenericFileHelper(this, file_.Pass()); + return task_runner_->PostTaskAndReply( + FROM_HERE, + Bind(&GenericFileHelper::SetTimes, Unretained(helper), last_access_time, + last_modified_time), + Bind(&GenericFileHelper::Reply, Owned(helper), callback)); +} + +bool FileProxy::SetLength(int64 length, const StatusCallback& callback) { + DCHECK(file_.IsValid()); + GenericFileHelper* helper = new GenericFileHelper(this, file_.Pass()); + return task_runner_->PostTaskAndReply( + FROM_HERE, + Bind(&GenericFileHelper::SetLength, Unretained(helper), length), + Bind(&GenericFileHelper::Reply, Owned(helper), callback)); +} + +bool FileProxy::Flush(const StatusCallback& callback) { + DCHECK(file_.IsValid()); + GenericFileHelper* helper = new GenericFileHelper(this, file_.Pass()); + return task_runner_->PostTaskAndReply( + FROM_HERE, + Bind(&GenericFileHelper::Flush, Unretained(helper)), + Bind(&GenericFileHelper::Reply, Owned(helper), callback)); +} + +} // namespace base diff --git a/chromium/base/files/file_proxy.h b/chromium/base/files/file_proxy.h new file mode 100644 index 00000000000..f990d044d37 --- /dev/null +++ b/chromium/base/files/file_proxy.h @@ -0,0 +1,142 @@ +// Copyright 2014 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. + +#ifndef BASE_FILES_FILE_PROXY_H_ +#define BASE_FILES_FILE_PROXY_H_ + +#include "base/base_export.h" +#include "base/callback_forward.h" +#include "base/files/file.h" +#include "base/files/file_path.h" +#include "base/memory/ref_counted.h" +#include "base/memory/weak_ptr.h" + +namespace tracked_objects { +class Location; +}; + +namespace base { + +class TaskRunner; +class Time; + +// This class provides asynchronous access to a File. All methods follow the +// same rules of the equivalent File method, as they are implemented by bouncing +// the operation to File using a TaskRunner. +// +// This class performs automatic proxying to close the underlying file at +// destruction. +// +// The TaskRunner is in charge of any sequencing of the operations, but a single +// operation can be proxied at a time, regardless of the use of a callback. +// In other words, having a sequence like +// +// proxy.Write(...); +// proxy.Write(...); +// +// means the second Write will always fail. +class BASE_EXPORT FileProxy : public SupportsWeakPtr<FileProxy> { + public: + // This callback is used by methods that report only an error code. It is + // valid to pass a null callback to some functions that takes a + // StatusCallback, in which case the operation will complete silently. + typedef Callback<void(File::Error)> StatusCallback; + + typedef Callback<void(File::Error, + const FilePath&)> CreateTemporaryCallback; + typedef Callback<void(File::Error, + const File::Info&)> GetFileInfoCallback; + typedef Callback<void(File::Error, + const char* data, + int bytes_read)> ReadCallback; + typedef Callback<void(File::Error, + int bytes_written)> WriteCallback; + + FileProxy(); + explicit FileProxy(TaskRunner* task_runner); + ~FileProxy(); + + // Creates or opens a file with the given flags. It is invalid to pass a null + // callback. If File::FLAG_CREATE is set in |file_flags| it always tries to + // create a new file at the given |file_path| and fails if the file already + // exists. + // + // This returns false if task posting to |task_runner| has failed. + bool CreateOrOpen(const FilePath& file_path, + uint32 file_flags, + const StatusCallback& callback); + + // Creates a temporary file for writing. The path and an open file are + // returned. It is invalid to pass a null callback. The additional file flags + // will be added on top of the default file flags which are: + // File::FLAG_CREATE_ALWAYS + // File::FLAG_WRITE + // File::FLAG_TEMPORARY. + // + // This returns false if task posting to |task_runner| has failed. + bool CreateTemporary(uint32 additional_file_flags, + const CreateTemporaryCallback& callback); + + // Returns true if the underlying |file_| is valid. + bool IsValid() const; + + // Returns true if a new file was created (or an old one truncated to zero + // length to simulate a new file), and false otherwise. + bool created() const { return file_.created(); } + + // Claims ownership of |file|. It is an error to call this method when + // IsValid() returns true. + void SetFile(File file); + + File TakeFile(); + + PlatformFile GetPlatformFile() const; + + // Proxies File::Close. The callback can be null. + // This returns false if task posting to |task_runner| has failed. + bool Close(const StatusCallback& callback); + + // Proxies File::GetInfo. The callback can't be null. + // This returns false if task posting to |task_runner| has failed. + bool GetInfo(const GetFileInfoCallback& callback); + + // Proxies File::Read. The callback can't be null. + // This returns false if |bytes_to_read| is less than zero, or + // if task posting to |task_runner| has failed. + bool Read(int64 offset, int bytes_to_read, const ReadCallback& callback); + + // Proxies File::Write. The callback can be null. + // This returns false if |bytes_to_write| is less than or equal to zero, + // if |buffer| is NULL, or if task posting to |task_runner| has failed. + bool Write(int64 offset, + const char* buffer, + int bytes_to_write, + const WriteCallback& callback); + + // Proxies File::SetTimes. The callback can be null. + // This returns false if task posting to |task_runner| has failed. + bool SetTimes(Time last_access_time, + Time last_modified_time, + const StatusCallback& callback); + + // Proxies File::SetLength. The callback can be null. + // This returns false if task posting to |task_runner| has failed. + bool SetLength(int64 length, const StatusCallback& callback); + + // Proxies File::Flush. The callback can be null. + // This returns false if task posting to |task_runner| has failed. + bool Flush(const StatusCallback& callback); + + private: + friend class FileHelper; + TaskRunner* task_runner() { return task_runner_.get(); } + + scoped_refptr<TaskRunner> task_runner_; + File file_; + DISALLOW_COPY_AND_ASSIGN(FileProxy); +}; + +} // namespace base + +#endif // BASE_FILES_FILE_PROXY_H_ diff --git a/chromium/base/files/file_proxy_unittest.cc b/chromium/base/files/file_proxy_unittest.cc new file mode 100644 index 00000000000..9be8abf67d2 --- /dev/null +++ b/chromium/base/files/file_proxy_unittest.cc @@ -0,0 +1,370 @@ +// Copyright 2014 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. + +#include "base/files/file_proxy.h" + +#include "base/bind.h" +#include "base/file_util.h" +#include "base/files/file.h" +#include "base/files/scoped_temp_dir.h" +#include "base/memory/weak_ptr.h" +#include "base/message_loop/message_loop.h" +#include "base/threading/thread.h" +#include "base/threading/thread_restrictions.h" +#include "testing/gtest/include/gtest/gtest.h" + +namespace base { + +class FileProxyTest : public testing::Test { + public: + FileProxyTest() + : file_thread_("FileProxyTestFileThread"), + error_(File::FILE_OK), + bytes_written_(-1), + weak_factory_(this) {} + + virtual void SetUp() OVERRIDE { + ASSERT_TRUE(dir_.CreateUniqueTempDir()); + ASSERT_TRUE(file_thread_.Start()); + } + + void DidFinish(File::Error error) { + error_ = error; + MessageLoop::current()->QuitWhenIdle(); + } + + void DidCreateOrOpen(File::Error error) { + error_ = error; + MessageLoop::current()->QuitWhenIdle(); + } + + void DidCreateTemporary(File::Error error, + const FilePath& path) { + error_ = error; + path_ = path; + MessageLoop::current()->QuitWhenIdle(); + } + + void DidGetFileInfo(File::Error error, + const File::Info& file_info) { + error_ = error; + file_info_ = file_info; + MessageLoop::current()->QuitWhenIdle(); + } + + void DidRead(File::Error error, + const char* data, + int bytes_read) { + error_ = error; + buffer_.resize(bytes_read); + memcpy(&buffer_[0], data, bytes_read); + MessageLoop::current()->QuitWhenIdle(); + } + + void DidWrite(File::Error error, + int bytes_written) { + error_ = error; + bytes_written_ = bytes_written; + MessageLoop::current()->QuitWhenIdle(); + } + + protected: + void CreateProxy(uint32 flags, FileProxy* proxy) { + proxy->CreateOrOpen( + test_path(), flags, + Bind(&FileProxyTest::DidCreateOrOpen, weak_factory_.GetWeakPtr())); + MessageLoop::current()->Run(); + EXPECT_TRUE(proxy->IsValid()); + } + + TaskRunner* file_task_runner() const { + return file_thread_.message_loop_proxy().get(); + } + const FilePath& test_dir_path() const { return dir_.path(); } + const FilePath test_path() const { return dir_.path().AppendASCII("test"); } + + MessageLoopForIO message_loop_; + Thread file_thread_; + + ScopedTempDir dir_; + File::Error error_; + FilePath path_; + File::Info file_info_; + std::vector<char> buffer_; + int bytes_written_; + WeakPtrFactory<FileProxyTest> weak_factory_; +}; + +TEST_F(FileProxyTest, CreateOrOpen_Create) { + FileProxy proxy(file_task_runner()); + proxy.CreateOrOpen( + test_path(), + File::FLAG_CREATE | File::FLAG_READ, + Bind(&FileProxyTest::DidCreateOrOpen, weak_factory_.GetWeakPtr())); + MessageLoop::current()->Run(); + + EXPECT_EQ(File::FILE_OK, error_); + EXPECT_TRUE(proxy.IsValid()); + EXPECT_TRUE(proxy.created()); + EXPECT_TRUE(PathExists(test_path())); +} + +TEST_F(FileProxyTest, CreateOrOpen_Open) { + // Creates a file. + base::WriteFile(test_path(), NULL, 0); + ASSERT_TRUE(PathExists(test_path())); + + // Opens the created file. + FileProxy proxy(file_task_runner()); + proxy.CreateOrOpen( + test_path(), + File::FLAG_OPEN | File::FLAG_READ, + Bind(&FileProxyTest::DidCreateOrOpen, weak_factory_.GetWeakPtr())); + MessageLoop::current()->Run(); + + EXPECT_EQ(File::FILE_OK, error_); + EXPECT_TRUE(proxy.IsValid()); + EXPECT_FALSE(proxy.created()); +} + +TEST_F(FileProxyTest, CreateOrOpen_OpenNonExistent) { + FileProxy proxy(file_task_runner()); + proxy.CreateOrOpen( + test_path(), + File::FLAG_OPEN | File::FLAG_READ, + Bind(&FileProxyTest::DidCreateOrOpen, weak_factory_.GetWeakPtr())); + MessageLoop::current()->Run(); + EXPECT_EQ(File::FILE_ERROR_NOT_FOUND, error_); + EXPECT_FALSE(proxy.IsValid()); + EXPECT_FALSE(proxy.created()); + EXPECT_FALSE(PathExists(test_path())); +} + +TEST_F(FileProxyTest, CreateOrOpen_AbandonedCreate) { + bool prev = ThreadRestrictions::SetIOAllowed(false); + { + FileProxy proxy(file_task_runner()); + proxy.CreateOrOpen( + test_path(), + File::FLAG_CREATE | File::FLAG_READ, + Bind(&FileProxyTest::DidCreateOrOpen, weak_factory_.GetWeakPtr())); + } + MessageLoop::current()->Run(); + ThreadRestrictions::SetIOAllowed(prev); + + EXPECT_TRUE(PathExists(test_path())); +} + +TEST_F(FileProxyTest, Close) { + // Creates a file. + FileProxy proxy(file_task_runner()); + CreateProxy(File::FLAG_CREATE | File::FLAG_WRITE, &proxy); + +#if defined(OS_WIN) + // This fails on Windows if the file is not closed. + EXPECT_FALSE(base::Move(test_path(), test_dir_path().AppendASCII("new"))); +#endif + + proxy.Close(Bind(&FileProxyTest::DidFinish, weak_factory_.GetWeakPtr())); + MessageLoop::current()->Run(); + EXPECT_EQ(File::FILE_OK, error_); + EXPECT_FALSE(proxy.IsValid()); + + // Now it should pass on all platforms. + EXPECT_TRUE(base::Move(test_path(), test_dir_path().AppendASCII("new"))); +} + +TEST_F(FileProxyTest, CreateTemporary) { + { + FileProxy proxy(file_task_runner()); + proxy.CreateTemporary( + 0 /* additional_file_flags */, + Bind(&FileProxyTest::DidCreateTemporary, weak_factory_.GetWeakPtr())); + MessageLoop::current()->Run(); + + EXPECT_TRUE(proxy.IsValid()); + EXPECT_EQ(File::FILE_OK, error_); + EXPECT_TRUE(PathExists(path_)); + + // The file should be writable. + proxy.Write(0, "test", 4, + Bind(&FileProxyTest::DidWrite, weak_factory_.GetWeakPtr())); + MessageLoop::current()->Run(); + EXPECT_EQ(File::FILE_OK, error_); + EXPECT_EQ(4, bytes_written_); + } + + // Make sure the written data can be read from the returned path. + std::string data; + EXPECT_TRUE(ReadFileToString(path_, &data)); + EXPECT_EQ("test", data); + + // Make sure we can & do delete the created file to prevent leaks on the bots. + EXPECT_TRUE(base::DeleteFile(path_, false)); +} + +TEST_F(FileProxyTest, SetAndTake) { + File file(test_path(), File::FLAG_CREATE | File::FLAG_READ); + ASSERT_TRUE(file.IsValid()); + FileProxy proxy(file_task_runner()); + EXPECT_FALSE(proxy.IsValid()); + proxy.SetFile(file.Pass()); + EXPECT_TRUE(proxy.IsValid()); + EXPECT_FALSE(file.IsValid()); + + file = proxy.TakeFile(); + EXPECT_FALSE(proxy.IsValid()); + EXPECT_TRUE(file.IsValid()); +} + +TEST_F(FileProxyTest, GetInfo) { + // Setup. + ASSERT_EQ(4, base::WriteFile(test_path(), "test", 4)); + File::Info expected_info; + GetFileInfo(test_path(), &expected_info); + + // Run. + FileProxy proxy(file_task_runner()); + CreateProxy(File::FLAG_OPEN | File::FLAG_READ, &proxy); + proxy.GetInfo( + Bind(&FileProxyTest::DidGetFileInfo, weak_factory_.GetWeakPtr())); + MessageLoop::current()->Run(); + + // Verify. + EXPECT_EQ(File::FILE_OK, error_); + EXPECT_EQ(expected_info.size, file_info_.size); + EXPECT_EQ(expected_info.is_directory, file_info_.is_directory); + EXPECT_EQ(expected_info.is_symbolic_link, file_info_.is_symbolic_link); + EXPECT_EQ(expected_info.last_modified, file_info_.last_modified); + EXPECT_EQ(expected_info.creation_time, file_info_.creation_time); +} + +TEST_F(FileProxyTest, Read) { + // Setup. + const char expected_data[] = "bleh"; + int expected_bytes = arraysize(expected_data); + ASSERT_EQ(expected_bytes, + base::WriteFile(test_path(), expected_data, expected_bytes)); + + // Run. + FileProxy proxy(file_task_runner()); + CreateProxy(File::FLAG_OPEN | File::FLAG_READ, &proxy); + + proxy.Read(0, 128, Bind(&FileProxyTest::DidRead, weak_factory_.GetWeakPtr())); + MessageLoop::current()->Run(); + + // Verify. + EXPECT_EQ(File::FILE_OK, error_); + EXPECT_EQ(expected_bytes, static_cast<int>(buffer_.size())); + for (size_t i = 0; i < buffer_.size(); ++i) { + EXPECT_EQ(expected_data[i], buffer_[i]); + } +} + +TEST_F(FileProxyTest, WriteAndFlush) { + FileProxy proxy(file_task_runner()); + CreateProxy(File::FLAG_CREATE | File::FLAG_WRITE, &proxy); + + const char data[] = "foo!"; + int data_bytes = ARRAYSIZE_UNSAFE(data); + proxy.Write(0, data, data_bytes, + Bind(&FileProxyTest::DidWrite, weak_factory_.GetWeakPtr())); + MessageLoop::current()->Run(); + EXPECT_EQ(File::FILE_OK, error_); + EXPECT_EQ(data_bytes, bytes_written_); + + // Flush the written data. (So that the following read should always + // succeed. On some platforms it may work with or without this flush.) + proxy.Flush(Bind(&FileProxyTest::DidFinish, weak_factory_.GetWeakPtr())); + MessageLoop::current()->Run(); + EXPECT_EQ(File::FILE_OK, error_); + + // Verify the written data. + char buffer[10]; + EXPECT_EQ(data_bytes, base::ReadFile(test_path(), buffer, data_bytes)); + for (int i = 0; i < data_bytes; ++i) { + EXPECT_EQ(data[i], buffer[i]); + } +} + +TEST_F(FileProxyTest, SetTimes) { + FileProxy proxy(file_task_runner()); + CreateProxy( + File::FLAG_CREATE | File::FLAG_WRITE | File::FLAG_WRITE_ATTRIBUTES, + &proxy); + + Time last_accessed_time = Time::Now() - TimeDelta::FromDays(12345); + Time last_modified_time = Time::Now() - TimeDelta::FromHours(98765); + + proxy.SetTimes(last_accessed_time, last_modified_time, + Bind(&FileProxyTest::DidFinish, weak_factory_.GetWeakPtr())); + MessageLoop::current()->Run(); + EXPECT_EQ(File::FILE_OK, error_); + + File::Info info; + GetFileInfo(test_path(), &info); + + // The returned values may only have the seconds precision, so we cast + // the double values to int here. + EXPECT_EQ(static_cast<int>(last_modified_time.ToDoubleT()), + static_cast<int>(info.last_modified.ToDoubleT())); + EXPECT_EQ(static_cast<int>(last_accessed_time.ToDoubleT()), + static_cast<int>(info.last_accessed.ToDoubleT())); +} + +TEST_F(FileProxyTest, SetLength_Shrink) { + // Setup. + const char kTestData[] = "0123456789"; + ASSERT_EQ(10, base::WriteFile(test_path(), kTestData, 10)); + File::Info info; + GetFileInfo(test_path(), &info); + ASSERT_EQ(10, info.size); + + // Run. + FileProxy proxy(file_task_runner()); + CreateProxy(File::FLAG_OPEN | File::FLAG_WRITE, &proxy); + proxy.SetLength(7, + Bind(&FileProxyTest::DidFinish, weak_factory_.GetWeakPtr())); + MessageLoop::current()->Run(); + + // Verify. + GetFileInfo(test_path(), &info); + ASSERT_EQ(7, info.size); + + char buffer[7]; + EXPECT_EQ(7, base::ReadFile(test_path(), buffer, 7)); + int i = 0; + for (; i < 7; ++i) + EXPECT_EQ(kTestData[i], buffer[i]); +} + +TEST_F(FileProxyTest, SetLength_Expand) { + // Setup. + const char kTestData[] = "9876543210"; + ASSERT_EQ(10, base::WriteFile(test_path(), kTestData, 10)); + File::Info info; + GetFileInfo(test_path(), &info); + ASSERT_EQ(10, info.size); + + // Run. + FileProxy proxy(file_task_runner()); + CreateProxy(File::FLAG_OPEN | File::FLAG_WRITE, &proxy); + proxy.SetLength(53, + Bind(&FileProxyTest::DidFinish, weak_factory_.GetWeakPtr())); + MessageLoop::current()->Run(); + + // Verify. + GetFileInfo(test_path(), &info); + ASSERT_EQ(53, info.size); + + char buffer[53]; + EXPECT_EQ(53, base::ReadFile(test_path(), buffer, 53)); + int i = 0; + for (; i < 10; ++i) + EXPECT_EQ(kTestData[i], buffer[i]); + for (; i < 53; ++i) + EXPECT_EQ(0, buffer[i]); +} + +} // namespace base diff --git a/chromium/base/files/file_unittest.cc b/chromium/base/files/file_unittest.cc index b2e855da1a0..cba043c332f 100644 --- a/chromium/base/files/file_unittest.cc +++ b/chromium/base/files/file_unittest.cc @@ -11,16 +11,27 @@ using base::File; using base::FilePath; -TEST(File, Create) { +TEST(FileTest, Create) { base::ScopedTempDir temp_dir; ASSERT_TRUE(temp_dir.CreateUniqueTempDir()); FilePath file_path = temp_dir.path().AppendASCII("create_file_1"); { + // Don't create a File at all. + File file; + EXPECT_FALSE(file.IsValid()); + EXPECT_EQ(base::File::FILE_ERROR_FAILED, file.error_details()); + + File file2(base::File::FILE_ERROR_TOO_MANY_OPENED); + EXPECT_FALSE(file2.IsValid()); + EXPECT_EQ(base::File::FILE_ERROR_TOO_MANY_OPENED, file2.error_details()); + } + + { // Open a file that doesn't exist. File file(file_path, base::File::FLAG_OPEN | base::File::FLAG_READ); EXPECT_FALSE(file.IsValid()); - EXPECT_EQ(base::File::FILE_ERROR_NOT_FOUND, file.error()); + EXPECT_EQ(base::File::FILE_ERROR_NOT_FOUND, file.error_details()); } { @@ -28,7 +39,7 @@ TEST(File, Create) { File file(file_path, base::File::FLAG_OPEN_ALWAYS | base::File::FLAG_READ); EXPECT_TRUE(file.IsValid()); EXPECT_TRUE(file.created()); - EXPECT_EQ(base::File::FILE_OK, file.error()); + EXPECT_EQ(base::File::FILE_OK, file.error_details()); } { @@ -36,7 +47,20 @@ TEST(File, Create) { File file(file_path, base::File::FLAG_OPEN | base::File::FLAG_READ); EXPECT_TRUE(file.IsValid()); EXPECT_FALSE(file.created()); - EXPECT_EQ(base::File::FILE_OK, file.error()); + EXPECT_EQ(base::File::FILE_OK, file.error_details()); + + // This time verify closing the file. + file.Close(); + EXPECT_FALSE(file.IsValid()); + } + + { + // Open an existing file through Initialize + File file; + file.Initialize(file_path, base::File::FLAG_OPEN | base::File::FLAG_READ); + EXPECT_TRUE(file.IsValid()); + EXPECT_FALSE(file.created()); + EXPECT_EQ(base::File::FILE_OK, file.error_details()); // This time verify closing the file. file.Close(); @@ -48,16 +72,16 @@ TEST(File, Create) { File file(file_path, base::File::FLAG_CREATE | base::File::FLAG_READ); EXPECT_FALSE(file.IsValid()); EXPECT_FALSE(file.created()); - EXPECT_EQ(base::File::FILE_ERROR_EXISTS, file.error()); + EXPECT_EQ(base::File::FILE_ERROR_EXISTS, file.error_details()); } { // Create or overwrite a file. File file(file_path, - base::File::FLAG_CREATE_ALWAYS | base::File::FLAG_READ); + base::File::FLAG_CREATE_ALWAYS | base::File::FLAG_WRITE); EXPECT_TRUE(file.IsValid()); EXPECT_TRUE(file.created()); - EXPECT_EQ(base::File::FILE_OK, file.error()); + EXPECT_EQ(base::File::FILE_OK, file.error_details()); } { @@ -68,13 +92,31 @@ TEST(File, Create) { base::File::FLAG_DELETE_ON_CLOSE); EXPECT_TRUE(file.IsValid()); EXPECT_TRUE(file.created()); - EXPECT_EQ(base::File::FILE_OK, file.error()); + EXPECT_EQ(base::File::FILE_OK, file.error_details()); } EXPECT_FALSE(base::PathExists(file_path)); } -TEST(File, DeleteOpenFile) { +TEST(FileTest, Async) { + base::ScopedTempDir temp_dir; + ASSERT_TRUE(temp_dir.CreateUniqueTempDir()); + FilePath file_path = temp_dir.path().AppendASCII("create_file"); + + { + File file(file_path, base::File::FLAG_OPEN_ALWAYS | base::File::FLAG_ASYNC); + EXPECT_TRUE(file.IsValid()); + EXPECT_TRUE(file.async()); + } + + { + File file(file_path, base::File::FLAG_OPEN_ALWAYS); + EXPECT_TRUE(file.IsValid()); + EXPECT_FALSE(file.async()); + } +} + +TEST(FileTest, DeleteOpenFile) { base::ScopedTempDir temp_dir; ASSERT_TRUE(temp_dir.CreateUniqueTempDir()); FilePath file_path = temp_dir.path().AppendASCII("create_file_1"); @@ -85,7 +127,7 @@ TEST(File, DeleteOpenFile) { base::File::FLAG_SHARE_DELETE); EXPECT_TRUE(file.IsValid()); EXPECT_TRUE(file.created()); - EXPECT_EQ(base::File::FILE_OK, file.error()); + EXPECT_EQ(base::File::FILE_OK, file.error_details()); // Open an existing file and mark it as delete on close. File same_file(file_path, @@ -93,7 +135,7 @@ TEST(File, DeleteOpenFile) { base::File::FLAG_READ); EXPECT_TRUE(file.IsValid()); EXPECT_FALSE(same_file.created()); - EXPECT_EQ(base::File::FILE_OK, same_file.error()); + EXPECT_EQ(base::File::FILE_OK, same_file.error_details()); // Close both handles and check that the file is gone. file.Close(); @@ -101,7 +143,7 @@ TEST(File, DeleteOpenFile) { EXPECT_FALSE(base::PathExists(file_path)); } -TEST(File, ReadWrite) { +TEST(FileTest, ReadWrite) { base::ScopedTempDir temp_dir; ASSERT_TRUE(temp_dir.CreateUniqueTempDir()); FilePath file_path = temp_dir.path().AppendASCII("read_write_file"); @@ -173,7 +215,7 @@ TEST(File, ReadWrite) { EXPECT_EQ(data_to_write[i - kOffsetBeyondEndOfFile], data_read_2[i]); } -TEST(File, Append) { +TEST(FileTest, Append) { base::ScopedTempDir temp_dir; ASSERT_TRUE(temp_dir.CreateUniqueTempDir()); FilePath file_path = temp_dir.path().AppendASCII("append_file"); @@ -221,7 +263,7 @@ TEST(File, Append) { } -TEST(File, Truncate) { +TEST(FileTest, Length) { base::ScopedTempDir temp_dir; ASSERT_TRUE(temp_dir.CreateUniqueTempDir()); FilePath file_path = temp_dir.path().AppendASCII("truncate_file"); @@ -229,6 +271,7 @@ TEST(File, Truncate) { base::File::FLAG_CREATE | base::File::FLAG_READ | base::File::FLAG_WRITE); ASSERT_TRUE(file.IsValid()); + EXPECT_EQ(0, file.GetLength()); // Write "test" to the file. char data_to_write[] = "test"; @@ -239,7 +282,8 @@ TEST(File, Truncate) { // Extend the file. const int kExtendedFileLength = 10; int64 file_size = 0; - EXPECT_TRUE(file.Truncate(kExtendedFileLength)); + EXPECT_TRUE(file.SetLength(kExtendedFileLength)); + EXPECT_EQ(kExtendedFileLength, file.GetLength()); EXPECT_TRUE(GetFileSize(file_path, &file_size)); EXPECT_EQ(kExtendedFileLength, file_size); @@ -254,7 +298,8 @@ TEST(File, Truncate) { // Truncate the file. const int kTruncatedFileLength = 2; - EXPECT_TRUE(file.Truncate(kTruncatedFileLength)); + EXPECT_TRUE(file.SetLength(kTruncatedFileLength)); + EXPECT_EQ(kTruncatedFileLength, file.GetLength()); EXPECT_TRUE(GetFileSize(file_path, &file_size)); EXPECT_EQ(kTruncatedFileLength, file_size); @@ -267,9 +312,9 @@ TEST(File, Truncate) { // Flakily fails: http://crbug.com/86494 #if defined(OS_ANDROID) -TEST(File, TouchGetInfo) { +TEST(FileTest, TouchGetInfo) { #else -TEST(File, DISABLED_TouchGetInfo) { +TEST(FileTest, DISABLED_TouchGetInfo) { #endif base::ScopedTempDir temp_dir; ASSERT_TRUE(temp_dir.CreateUniqueTempDir()); @@ -333,18 +378,17 @@ TEST(File, DISABLED_TouchGetInfo) { creation_time.ToInternalValue()); } -TEST(File, ReadFileAtCurrentPosition) { +TEST(FileTest, ReadAtCurrentPosition) { base::ScopedTempDir temp_dir; ASSERT_TRUE(temp_dir.CreateUniqueTempDir()); - FilePath file_path = - temp_dir.path().AppendASCII("read_file_at_current_position"); + FilePath file_path = temp_dir.path().AppendASCII("read_at_current_position"); File file(file_path, base::File::FLAG_CREATE | base::File::FLAG_READ | base::File::FLAG_WRITE); EXPECT_TRUE(file.IsValid()); const char kData[] = "test"; - const int kDataSize = arraysize(kData) - 1; + const int kDataSize = sizeof(kData) - 1; EXPECT_EQ(kDataSize, file.Write(0, kData, kDataSize)); EXPECT_EQ(0, file.Seek(base::File::FROM_BEGIN, 0)); @@ -355,6 +399,53 @@ TEST(File, ReadFileAtCurrentPosition) { EXPECT_EQ(kDataSize - first_chunk_size, file.ReadAtCurrentPos(buffer + first_chunk_size, kDataSize - first_chunk_size)); - EXPECT_EQ(std::string(buffer, buffer + kDataSize), - std::string(kData)); + EXPECT_EQ(std::string(buffer, buffer + kDataSize), std::string(kData)); +} + +TEST(FileTest, WriteAtCurrentPosition) { + base::ScopedTempDir temp_dir; + ASSERT_TRUE(temp_dir.CreateUniqueTempDir()); + FilePath file_path = temp_dir.path().AppendASCII("write_at_current_position"); + File file(file_path, + base::File::FLAG_CREATE | base::File::FLAG_READ | + base::File::FLAG_WRITE); + EXPECT_TRUE(file.IsValid()); + + const char kData[] = "test"; + const int kDataSize = sizeof(kData) - 1; + + int first_chunk_size = kDataSize / 2; + EXPECT_EQ(first_chunk_size, file.WriteAtCurrentPos(kData, first_chunk_size)); + EXPECT_EQ(kDataSize - first_chunk_size, + file.WriteAtCurrentPos(kData + first_chunk_size, + kDataSize - first_chunk_size)); + + char buffer[kDataSize]; + EXPECT_EQ(kDataSize, file.Read(0, buffer, kDataSize)); + EXPECT_EQ(std::string(buffer, buffer + kDataSize), std::string(kData)); +} + +#if defined(OS_WIN) +TEST(FileTest, GetInfoForDirectory) { + base::ScopedTempDir temp_dir; + ASSERT_TRUE(temp_dir.CreateUniqueTempDir()); + FilePath empty_dir = temp_dir.path().Append(FILE_PATH_LITERAL("gpfi_test")); + ASSERT_TRUE(CreateDirectory(empty_dir)); + + base::File dir( + ::CreateFile(empty_dir.value().c_str(), + FILE_ALL_ACCESS, + FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE, + NULL, + OPEN_EXISTING, + FILE_FLAG_BACKUP_SEMANTICS, // Needed to open a directory. + NULL)); + ASSERT_TRUE(dir.IsValid()); + + base::File::Info info; + EXPECT_TRUE(dir.GetInfo(&info)); + EXPECT_TRUE(info.is_directory); + EXPECT_FALSE(info.is_symbolic_link); + EXPECT_EQ(0, info.size); } +#endif // defined(OS_WIN) diff --git a/chromium/base/files/file_util_proxy.cc b/chromium/base/files/file_util_proxy.cc index 40cac112820..72d9436fa1f 100644 --- a/chromium/base/files/file_util_proxy.cc +++ b/chromium/base/files/file_util_proxy.cc @@ -8,7 +8,6 @@ #include "base/bind_helpers.h" #include "base/file_util.h" #include "base/location.h" -#include "base/message_loop/message_loop_proxy.h" #include "base/task_runner.h" #include "base/task_runner_util.h" @@ -19,105 +18,21 @@ namespace { void CallWithTranslatedParameter(const FileUtilProxy::StatusCallback& callback, bool value) { DCHECK(!callback.is_null()); - callback.Run(value ? PLATFORM_FILE_OK : PLATFORM_FILE_ERROR_FAILED); + callback.Run(value ? File::FILE_OK : File::FILE_ERROR_FAILED); } -// Helper classes or routines for individual methods. -class CreateOrOpenHelper { - public: - CreateOrOpenHelper(TaskRunner* task_runner, - const FileUtilProxy::CloseTask& close_task) - : task_runner_(task_runner), - close_task_(close_task), - file_handle_(kInvalidPlatformFileValue), - created_(false), - error_(PLATFORM_FILE_OK) {} - - ~CreateOrOpenHelper() { - if (file_handle_ != kInvalidPlatformFileValue) { - task_runner_->PostTask( - FROM_HERE, - base::Bind(base::IgnoreResult(close_task_), file_handle_)); - } - } - - void RunWork(const FileUtilProxy::CreateOrOpenTask& task) { - error_ = task.Run(&file_handle_, &created_); - } - - void Reply(const FileUtilProxy::CreateOrOpenCallback& callback) { - DCHECK(!callback.is_null()); - callback.Run(error_, PassPlatformFile(&file_handle_), created_); - } - - private: - scoped_refptr<TaskRunner> task_runner_; - FileUtilProxy::CloseTask close_task_; - PlatformFile file_handle_; - bool created_; - PlatformFileError error_; - DISALLOW_COPY_AND_ASSIGN(CreateOrOpenHelper); -}; - -class CreateTemporaryHelper { - public: - explicit CreateTemporaryHelper(TaskRunner* task_runner) - : task_runner_(task_runner), - file_handle_(kInvalidPlatformFileValue), - error_(PLATFORM_FILE_OK) {} - - ~CreateTemporaryHelper() { - if (file_handle_ != kInvalidPlatformFileValue) { - FileUtilProxy::Close( - task_runner_.get(), file_handle_, FileUtilProxy::StatusCallback()); - } - } - - void RunWork(int additional_file_flags) { - // TODO(darin): file_util should have a variant of CreateTemporaryFile - // that returns a FilePath and a PlatformFile. - base::CreateTemporaryFile(&file_path_); - - int file_flags = - PLATFORM_FILE_WRITE | - PLATFORM_FILE_TEMPORARY | - PLATFORM_FILE_CREATE_ALWAYS | - additional_file_flags; - - error_ = PLATFORM_FILE_OK; - file_handle_ = CreatePlatformFile(file_path_, file_flags, NULL, &error_); - } - - void Reply(const FileUtilProxy::CreateTemporaryCallback& callback) { - DCHECK(!callback.is_null()); - callback.Run(error_, PassPlatformFile(&file_handle_), file_path_); - } - - private: - scoped_refptr<TaskRunner> task_runner_; - PlatformFile file_handle_; - FilePath file_path_; - PlatformFileError error_; - DISALLOW_COPY_AND_ASSIGN(CreateTemporaryHelper); -}; - class GetFileInfoHelper { public: GetFileInfoHelper() - : error_(PLATFORM_FILE_OK) {} + : error_(File::FILE_OK) {} void RunWorkForFilePath(const FilePath& file_path) { if (!PathExists(file_path)) { - error_ = PLATFORM_FILE_ERROR_NOT_FOUND; + error_ = File::FILE_ERROR_NOT_FOUND; return; } if (!GetFileInfo(file_path, &file_info_)) - error_ = PLATFORM_FILE_ERROR_FAILED; - } - - void RunWorkForPlatformFile(PlatformFile file) { - if (!GetPlatformFileInfo(file, &file_info_)) - error_ = PLATFORM_FILE_ERROR_FAILED; + error_ = File::FILE_ERROR_FAILED; } void Reply(const FileUtilProxy::GetFileInfoCallback& callback) { @@ -127,138 +42,26 @@ class GetFileInfoHelper { } private: - PlatformFileError error_; - PlatformFileInfo file_info_; + File::Error error_; + File::Info file_info_; DISALLOW_COPY_AND_ASSIGN(GetFileInfoHelper); }; -class ReadHelper { - public: - explicit ReadHelper(int bytes_to_read) - : buffer_(new char[bytes_to_read]), - bytes_to_read_(bytes_to_read), - bytes_read_(0) {} - - void RunWork(PlatformFile file, int64 offset) { - bytes_read_ = ReadPlatformFile(file, offset, buffer_.get(), bytes_to_read_); - } - - void Reply(const FileUtilProxy::ReadCallback& callback) { - if (!callback.is_null()) { - PlatformFileError error = - (bytes_read_ < 0) ? PLATFORM_FILE_ERROR_FAILED : PLATFORM_FILE_OK; - callback.Run(error, buffer_.get(), bytes_read_); - } - } - - private: - scoped_ptr<char[]> buffer_; - int bytes_to_read_; - int bytes_read_; - DISALLOW_COPY_AND_ASSIGN(ReadHelper); -}; - -class WriteHelper { - public: - WriteHelper(const char* buffer, int bytes_to_write) - : buffer_(new char[bytes_to_write]), - bytes_to_write_(bytes_to_write), - bytes_written_(0) { - memcpy(buffer_.get(), buffer, bytes_to_write); - } - - void RunWork(PlatformFile file, int64 offset) { - bytes_written_ = WritePlatformFile(file, offset, buffer_.get(), - bytes_to_write_); - } - - void Reply(const FileUtilProxy::WriteCallback& callback) { - if (!callback.is_null()) { - PlatformFileError error = - (bytes_written_ < 0) ? PLATFORM_FILE_ERROR_FAILED : PLATFORM_FILE_OK; - callback.Run(error, bytes_written_); - } - } - - private: - scoped_ptr<char[]> buffer_; - int bytes_to_write_; - int bytes_written_; - DISALLOW_COPY_AND_ASSIGN(WriteHelper); -}; - -PlatformFileError CreateOrOpenAdapter( - const FilePath& file_path, int file_flags, - PlatformFile* file_handle, bool* created) { - DCHECK(file_handle); - DCHECK(created); - if (!DirectoryExists(file_path.DirName())) { - // If its parent does not exist, should return NOT_FOUND error. - return PLATFORM_FILE_ERROR_NOT_FOUND; - } - PlatformFileError error = PLATFORM_FILE_OK; - *file_handle = CreatePlatformFile(file_path, file_flags, created, &error); - return error; -} - -PlatformFileError CloseAdapter(PlatformFile file_handle) { - if (!ClosePlatformFile(file_handle)) { - return PLATFORM_FILE_ERROR_FAILED; - } - return PLATFORM_FILE_OK; -} - -PlatformFileError DeleteAdapter(const FilePath& file_path, bool recursive) { +File::Error DeleteAdapter(const FilePath& file_path, bool recursive) { if (!PathExists(file_path)) { - return PLATFORM_FILE_ERROR_NOT_FOUND; + return File::FILE_ERROR_NOT_FOUND; } if (!base::DeleteFile(file_path, recursive)) { if (!recursive && !base::IsDirectoryEmpty(file_path)) { - return PLATFORM_FILE_ERROR_NOT_EMPTY; + return File::FILE_ERROR_NOT_EMPTY; } - return PLATFORM_FILE_ERROR_FAILED; + return File::FILE_ERROR_FAILED; } - return PLATFORM_FILE_OK; + return File::FILE_OK; } } // namespace -// static -bool FileUtilProxy::CreateOrOpen( - TaskRunner* task_runner, - const FilePath& file_path, int file_flags, - const CreateOrOpenCallback& callback) { - return RelayCreateOrOpen( - task_runner, - base::Bind(&CreateOrOpenAdapter, file_path, file_flags), - base::Bind(&CloseAdapter), - callback); -} - -// static -bool FileUtilProxy::CreateTemporary( - TaskRunner* task_runner, - int additional_file_flags, - const CreateTemporaryCallback& callback) { - CreateTemporaryHelper* helper = new CreateTemporaryHelper(task_runner); - return task_runner->PostTaskAndReply( - FROM_HERE, - Bind(&CreateTemporaryHelper::RunWork, Unretained(helper), - additional_file_flags), - Bind(&CreateTemporaryHelper::Reply, Owned(helper), callback)); -} - -// static -bool FileUtilProxy::Close( - TaskRunner* task_runner, - base::PlatformFile file_handle, - const StatusCallback& callback) { - return RelayClose( - task_runner, - base::Bind(&CloseAdapter), - file_handle, callback); -} - // Retrieves the information about a file. It is invalid to pass NULL for the // callback. bool FileUtilProxy::GetFileInfo( @@ -274,19 +77,6 @@ bool FileUtilProxy::GetFileInfo( } // static -bool FileUtilProxy::GetFileInfoFromPlatformFile( - TaskRunner* task_runner, - PlatformFile file, - const GetFileInfoCallback& callback) { - GetFileInfoHelper* helper = new GetFileInfoHelper; - return task_runner->PostTaskAndReply( - FROM_HERE, - Bind(&GetFileInfoHelper::RunWorkForPlatformFile, - Unretained(helper), file), - Bind(&GetFileInfoHelper::Reply, Owned(helper), callback)); -} - -// static bool FileUtilProxy::DeleteFile(TaskRunner* task_runner, const FilePath& file_path, bool recursive, @@ -298,56 +88,6 @@ bool FileUtilProxy::DeleteFile(TaskRunner* task_runner, } // static -bool FileUtilProxy::Read( - TaskRunner* task_runner, - PlatformFile file, - int64 offset, - int bytes_to_read, - const ReadCallback& callback) { - if (bytes_to_read < 0) { - return false; - } - ReadHelper* helper = new ReadHelper(bytes_to_read); - return task_runner->PostTaskAndReply( - FROM_HERE, - Bind(&ReadHelper::RunWork, Unretained(helper), file, offset), - Bind(&ReadHelper::Reply, Owned(helper), callback)); -} - -// static -bool FileUtilProxy::Write( - TaskRunner* task_runner, - PlatformFile file, - int64 offset, - const char* buffer, - int bytes_to_write, - const WriteCallback& callback) { - if (bytes_to_write <= 0 || buffer == NULL) { - return false; - } - WriteHelper* helper = new WriteHelper(buffer, bytes_to_write); - return task_runner->PostTaskAndReply( - FROM_HERE, - Bind(&WriteHelper::RunWork, Unretained(helper), file, offset), - Bind(&WriteHelper::Reply, Owned(helper), callback)); -} - -// static -bool FileUtilProxy::Touch( - TaskRunner* task_runner, - PlatformFile file, - const Time& last_access_time, - const Time& last_modified_time, - const StatusCallback& callback) { - return base::PostTaskAndReplyWithResult( - task_runner, - FROM_HERE, - Bind(&TouchPlatformFile, file, - last_access_time, last_modified_time), - Bind(&CallWithTranslatedParameter, callback)); -} - -// static bool FileUtilProxy::Touch( TaskRunner* task_runner, const FilePath& file_path, @@ -361,53 +101,4 @@ bool FileUtilProxy::Touch( Bind(&CallWithTranslatedParameter, callback)); } -// static -bool FileUtilProxy::Truncate( - TaskRunner* task_runner, - PlatformFile file, - int64 length, - const StatusCallback& callback) { - return base::PostTaskAndReplyWithResult( - task_runner, - FROM_HERE, - Bind(&TruncatePlatformFile, file, length), - Bind(&CallWithTranslatedParameter, callback)); -} - -// static -bool FileUtilProxy::Flush( - TaskRunner* task_runner, - PlatformFile file, - const StatusCallback& callback) { - return base::PostTaskAndReplyWithResult( - task_runner, - FROM_HERE, - Bind(&FlushPlatformFile, file), - Bind(&CallWithTranslatedParameter, callback)); -} - -// static -bool FileUtilProxy::RelayCreateOrOpen( - TaskRunner* task_runner, - const CreateOrOpenTask& open_task, - const CloseTask& close_task, - const CreateOrOpenCallback& callback) { - CreateOrOpenHelper* helper = new CreateOrOpenHelper( - task_runner, close_task); - return task_runner->PostTaskAndReply( - FROM_HERE, - Bind(&CreateOrOpenHelper::RunWork, Unretained(helper), open_task), - Bind(&CreateOrOpenHelper::Reply, Owned(helper), callback)); -} - -// static -bool FileUtilProxy::RelayClose( - TaskRunner* task_runner, - const CloseTask& close_task, - PlatformFile file_handle, - const StatusCallback& callback) { - return base::PostTaskAndReplyWithResult( - task_runner, FROM_HERE, Bind(close_task, file_handle), callback); -} - } // namespace base diff --git a/chromium/base/files/file_util_proxy.h b/chromium/base/files/file_util_proxy.h index bded1612005..80688cfbb48 100644 --- a/chromium/base/files/file_util_proxy.h +++ b/chromium/base/files/file_util_proxy.h @@ -7,13 +7,8 @@ #include "base/base_export.h" #include "base/callback_forward.h" +#include "base/files/file.h" #include "base/files/file_path.h" -#include "base/memory/ref_counted.h" -#include "base/platform_file.h" - -namespace tracked_objects { -class Location; -}; namespace base { @@ -26,57 +21,10 @@ class BASE_EXPORT FileUtilProxy { // This callback is used by methods that report only an error code. It is // valid to pass a null callback to any function that takes a StatusCallback, // in which case the operation will complete silently. - typedef Callback<void(PlatformFileError)> StatusCallback; - - typedef Callback<void(PlatformFileError, - PassPlatformFile, - bool /* created */)> CreateOrOpenCallback; - typedef Callback<void(PlatformFileError, - PassPlatformFile, - const FilePath&)> CreateTemporaryCallback; - typedef Callback<void(PlatformFileError, - const PlatformFileInfo&)> GetFileInfoCallback; - typedef Callback<void(PlatformFileError, - const char* /* data */, - int /* bytes read */)> ReadCallback; - typedef Callback<void(PlatformFileError, - int /* bytes written */)> WriteCallback; + typedef Callback<void(File::Error)> StatusCallback; - typedef Callback<PlatformFileError(PlatformFile*, bool*)> CreateOrOpenTask; - typedef Callback<PlatformFileError(PlatformFile)> CloseTask; - typedef Callback<PlatformFileError(void)> FileTask; - - // Creates or opens a file with the given flags. It is invalid to pass a null - // callback. If PLATFORM_FILE_CREATE is set in |file_flags| it always tries to - // create a new file at the given |file_path| and calls back with - // PLATFORM_FILE_ERROR_FILE_EXISTS if the |file_path| already exists. - // - // This returns false if task posting to |task_runner| has failed. - static bool CreateOrOpen(TaskRunner* task_runner, - const FilePath& file_path, - int file_flags, - const CreateOrOpenCallback& callback); - - // Creates a temporary file for writing. The path and an open file handle are - // returned. It is invalid to pass a null callback. The additional file flags - // will be added on top of the default file flags which are: - // base::PLATFORM_FILE_CREATE_ALWAYS - // base::PLATFORM_FILE_WRITE - // base::PLATFORM_FILE_TEMPORARY. - // Set |additional_file_flags| to 0 for synchronous writes and set to - // base::PLATFORM_FILE_ASYNC to support asynchronous file operations. - // - // This returns false if task posting to |task_runner| has failed. - static bool CreateTemporary( - TaskRunner* task_runner, - int additional_file_flags, - const CreateTemporaryCallback& callback); - - // Close the given file handle. - // This returns false if task posting to |task_runner| has failed. - static bool Close(TaskRunner* task_runner, - PlatformFile, - const StatusCallback& callback); + typedef Callback<void(File::Error, + const File::Info&)> GetFileInfoCallback; // Retrieves the information about a file. It is invalid to pass a null // callback. @@ -86,13 +34,6 @@ class BASE_EXPORT FileUtilProxy { const FilePath& file_path, const GetFileInfoCallback& callback); - // Does the same as GetFileInfo but takes PlatformFile instead of FilePath. - // This returns false if task posting to |task_runner| has failed. - static bool GetFileInfoFromPlatformFile( - TaskRunner* task_runner, - PlatformFile file, - const GetFileInfoCallback& callback); - // Deletes a file or a directory. // It is an error to delete a non-empty directory with recursive=false. // This returns false if task posting to |task_runner| has failed. @@ -101,42 +42,6 @@ class BASE_EXPORT FileUtilProxy { bool recursive, const StatusCallback& callback); - // Reads from a file. On success, the file pointer is moved to position - // |offset + bytes_to_read| in the file. The callback can be null. - // - // This returns false if |bytes_to_read| is less than zero, or - // if task posting to |task_runner| has failed. - static bool Read( - TaskRunner* task_runner, - PlatformFile file, - int64 offset, - int bytes_to_read, - const ReadCallback& callback); - - // Writes to a file. If |offset| is greater than the length of the file, - // |false| is returned. On success, the file pointer is moved to position - // |offset + bytes_to_write| in the file. The callback can be null. - // |bytes_to_write| must be greater than zero. - // - // This returns false if |bytes_to_write| is less than or equal to zero, - // if |buffer| is NULL, or if task posting to |task_runner| has failed. - static bool Write( - TaskRunner* task_runner, - PlatformFile file, - int64 offset, - const char* buffer, - int bytes_to_write, - const WriteCallback& callback); - - // Touches a file. The callback can be null. - // This returns false if task posting to |task_runner| has failed. - static bool Touch( - TaskRunner* task_runner, - PlatformFile file, - const Time& last_access_time, - const Time& last_modified_time, - const StatusCallback& callback); - // Touches a file. The callback can be null. // This returns false if task posting to |task_runner| has failed. static bool Touch( @@ -146,46 +51,6 @@ class BASE_EXPORT FileUtilProxy { const Time& last_modified_time, const StatusCallback& callback); - // Truncates a file to the given length. If |length| is greater than the - // current length of the file, the file will be extended with zeroes. - // The callback can be null. - // This returns false if task posting to |task_runner| has failed. - static bool Truncate( - TaskRunner* task_runner, - PlatformFile file, - int64 length, - const StatusCallback& callback); - - // Truncates a file to the given length. If |length| is greater than the - // current length of the file, the file will be extended with zeroes. - // The callback can be null. - // This returns false if task posting to |task_runner| has failed. - static bool Truncate( - TaskRunner* task_runner, - const FilePath& path, - int64 length, - const StatusCallback& callback); - - // Flushes a file. The callback can be null. - // This returns false if task posting to |task_runner| has failed. - static bool Flush( - TaskRunner* task_runner, - PlatformFile file, - const StatusCallback& callback); - - // Relay helpers. - // They return false if posting a given task to |task_runner| has failed. - static bool RelayCreateOrOpen( - TaskRunner* task_runner, - const CreateOrOpenTask& open_task, - const CloseTask& close_task, - const CreateOrOpenCallback& callback); - static bool RelayClose( - TaskRunner* task_runner, - const CloseTask& close_task, - PlatformFile, - const StatusCallback& callback); - private: DISALLOW_IMPLICIT_CONSTRUCTORS(FileUtilProxy); }; diff --git a/chromium/base/files/file_util_proxy_unittest.cc b/chromium/base/files/file_util_proxy_unittest.cc index 17c7a3f7cf5..52073ae3b50 100644 --- a/chromium/base/files/file_util_proxy_unittest.cc +++ b/chromium/base/files/file_util_proxy_unittest.cc @@ -4,15 +4,11 @@ #include "base/files/file_util_proxy.h" -#include <map> - #include "base/bind.h" #include "base/file_util.h" #include "base/files/scoped_temp_dir.h" -#include "base/logging.h" #include "base/memory/weak_ptr.h" #include "base/message_loop/message_loop.h" -#include "base/platform_file.h" #include "base/threading/thread.h" #include "testing/gtest/include/gtest/gtest.h" @@ -21,11 +17,9 @@ namespace base { class FileUtilProxyTest : public testing::Test { public: FileUtilProxyTest() - : message_loop_(MessageLoop::TYPE_IO), - file_thread_("FileUtilProxyTestFileThread"), - error_(PLATFORM_FILE_OK), + : file_thread_("FileUtilProxyTestFileThread"), + error_(File::FILE_OK), created_(false), - file_(kInvalidPlatformFileValue), bytes_written_(-1), weak_factory_(this) {} @@ -34,198 +28,43 @@ class FileUtilProxyTest : public testing::Test { ASSERT_TRUE(file_thread_.Start()); } - virtual void TearDown() OVERRIDE { - if (file_ != kInvalidPlatformFileValue) - ClosePlatformFile(file_); - } - - void DidFinish(PlatformFileError error) { - error_ = error; - MessageLoop::current()->QuitWhenIdle(); - } - - void DidCreateOrOpen(PlatformFileError error, - PassPlatformFile file, - bool created) { + void DidFinish(File::Error error) { error_ = error; - file_ = file.ReleaseValue(); - created_ = created; MessageLoop::current()->QuitWhenIdle(); } - void DidCreateTemporary(PlatformFileError error, - PassPlatformFile file, - const FilePath& path) { - error_ = error; - file_ = file.ReleaseValue(); - path_ = path; - MessageLoop::current()->QuitWhenIdle(); - } - - void DidGetFileInfo(PlatformFileError error, - const PlatformFileInfo& file_info) { + void DidGetFileInfo(File::Error error, + const File::Info& file_info) { error_ = error; file_info_ = file_info; MessageLoop::current()->QuitWhenIdle(); } - void DidRead(PlatformFileError error, - const char* data, - int bytes_read) { - error_ = error; - buffer_.resize(bytes_read); - memcpy(&buffer_[0], data, bytes_read); - MessageLoop::current()->QuitWhenIdle(); - } - - void DidWrite(PlatformFileError error, - int bytes_written) { - error_ = error; - bytes_written_ = bytes_written; - MessageLoop::current()->QuitWhenIdle(); - } - protected: - PlatformFile GetTestPlatformFile(int flags) { - if (file_ != kInvalidPlatformFileValue) - return file_; - bool created; - PlatformFileError error; - file_ = CreatePlatformFile(test_path(), flags, &created, &error); - EXPECT_EQ(PLATFORM_FILE_OK, error); - EXPECT_NE(kInvalidPlatformFileValue, file_); - return file_; - } - TaskRunner* file_task_runner() const { return file_thread_.message_loop_proxy().get(); } const FilePath& test_dir_path() const { return dir_.path(); } const FilePath test_path() const { return dir_.path().AppendASCII("test"); } - MessageLoop message_loop_; + MessageLoopForIO message_loop_; Thread file_thread_; ScopedTempDir dir_; - PlatformFileError error_; + File::Error error_; bool created_; - PlatformFile file_; FilePath path_; - PlatformFileInfo file_info_; + File::Info file_info_; std::vector<char> buffer_; int bytes_written_; WeakPtrFactory<FileUtilProxyTest> weak_factory_; }; -TEST_F(FileUtilProxyTest, CreateOrOpen_Create) { - FileUtilProxy::CreateOrOpen( - file_task_runner(), - test_path(), - PLATFORM_FILE_CREATE | PLATFORM_FILE_READ, - Bind(&FileUtilProxyTest::DidCreateOrOpen, weak_factory_.GetWeakPtr())); - MessageLoop::current()->Run(); - - EXPECT_EQ(PLATFORM_FILE_OK, error_); - EXPECT_TRUE(created_); - EXPECT_NE(kInvalidPlatformFileValue, file_); - EXPECT_TRUE(PathExists(test_path())); -} - -TEST_F(FileUtilProxyTest, CreateOrOpen_Open) { - // Creates a file. - file_util::WriteFile(test_path(), NULL, 0); - ASSERT_TRUE(PathExists(test_path())); - - // Opens the created file. - FileUtilProxy::CreateOrOpen( - file_task_runner(), - test_path(), - PLATFORM_FILE_OPEN | PLATFORM_FILE_READ, - Bind(&FileUtilProxyTest::DidCreateOrOpen, weak_factory_.GetWeakPtr())); - MessageLoop::current()->Run(); - - EXPECT_EQ(PLATFORM_FILE_OK, error_); - EXPECT_FALSE(created_); - EXPECT_NE(kInvalidPlatformFileValue, file_); -} - -TEST_F(FileUtilProxyTest, CreateOrOpen_OpenNonExistent) { - FileUtilProxy::CreateOrOpen( - file_task_runner(), - test_path(), - PLATFORM_FILE_OPEN | PLATFORM_FILE_READ, - Bind(&FileUtilProxyTest::DidCreateOrOpen, weak_factory_.GetWeakPtr())); - MessageLoop::current()->Run(); - EXPECT_EQ(PLATFORM_FILE_ERROR_NOT_FOUND, error_); - EXPECT_FALSE(created_); - EXPECT_EQ(kInvalidPlatformFileValue, file_); - EXPECT_FALSE(PathExists(test_path())); -} - -TEST_F(FileUtilProxyTest, Close) { - // Creates a file. - PlatformFile file = GetTestPlatformFile( - PLATFORM_FILE_CREATE | PLATFORM_FILE_WRITE); - -#if defined(OS_WIN) - // This fails on Windows if the file is not closed. - EXPECT_FALSE(base::Move(test_path(), - test_dir_path().AppendASCII("new"))); -#endif - - FileUtilProxy::Close( - file_task_runner(), - file, - Bind(&FileUtilProxyTest::DidFinish, weak_factory_.GetWeakPtr())); - MessageLoop::current()->Run(); - EXPECT_EQ(PLATFORM_FILE_OK, error_); - - // Now it should pass on all platforms. - EXPECT_TRUE(base::Move(test_path(), test_dir_path().AppendASCII("new"))); -} - -TEST_F(FileUtilProxyTest, CreateTemporary) { - FileUtilProxy::CreateTemporary( - file_task_runner(), 0 /* additional_file_flags */, - Bind(&FileUtilProxyTest::DidCreateTemporary, weak_factory_.GetWeakPtr())); - MessageLoop::current()->Run(); - EXPECT_EQ(PLATFORM_FILE_OK, error_); - EXPECT_TRUE(PathExists(path_)); - EXPECT_NE(kInvalidPlatformFileValue, file_); - - // The file should be writable. -#if defined(OS_WIN) - HANDLE hEvent = CreateEvent(NULL, FALSE, FALSE, NULL); - OVERLAPPED overlapped = {0}; - overlapped.hEvent = hEvent; - DWORD bytes_written; - if (!::WriteFile(file_, "test", 4, &bytes_written, &overlapped)) { - // Temporary file is created with ASYNC flag, so WriteFile may return 0 - // with ERROR_IO_PENDING. - EXPECT_EQ(ERROR_IO_PENDING, GetLastError()); - GetOverlappedResult(file_, &overlapped, &bytes_written, TRUE); - } - EXPECT_EQ(4, bytes_written); -#else - // On POSIX ASYNC flag does not affect synchronous read/write behavior. - EXPECT_EQ(4, WritePlatformFile(file_, 0, "test", 4)); -#endif - EXPECT_TRUE(ClosePlatformFile(file_)); - file_ = kInvalidPlatformFileValue; - - // Make sure the written data can be read from the returned path. - std::string data; - EXPECT_TRUE(ReadFileToString(path_, &data)); - EXPECT_EQ("test", data); - - // Make sure we can & do delete the created file to prevent leaks on the bots. - EXPECT_TRUE(base::DeleteFile(path_, false)); -} TEST_F(FileUtilProxyTest, GetFileInfo_File) { // Setup. - ASSERT_EQ(4, file_util::WriteFile(test_path(), "test", 4)); - PlatformFileInfo expected_info; + ASSERT_EQ(4, WriteFile(test_path(), "test", 4)); + File::Info expected_info; GetFileInfo(test_path(), &expected_info); // Run. @@ -236,7 +75,7 @@ TEST_F(FileUtilProxyTest, GetFileInfo_File) { MessageLoop::current()->Run(); // Verify. - EXPECT_EQ(PLATFORM_FILE_OK, error_); + EXPECT_EQ(File::FILE_OK, error_); EXPECT_EQ(expected_info.size, file_info_.size); EXPECT_EQ(expected_info.is_directory, file_info_.is_directory); EXPECT_EQ(expected_info.is_symbolic_link, file_info_.is_symbolic_link); @@ -248,7 +87,7 @@ TEST_F(FileUtilProxyTest, GetFileInfo_File) { TEST_F(FileUtilProxyTest, GetFileInfo_Directory) { // Setup. ASSERT_TRUE(base::CreateDirectory(test_path())); - PlatformFileInfo expected_info; + File::Info expected_info; GetFileInfo(test_path(), &expected_info); // Run. @@ -259,7 +98,7 @@ TEST_F(FileUtilProxyTest, GetFileInfo_Directory) { MessageLoop::current()->Run(); // Verify. - EXPECT_EQ(PLATFORM_FILE_OK, error_); + EXPECT_EQ(File::FILE_OK, error_); EXPECT_EQ(expected_info.size, file_info_.size); EXPECT_EQ(expected_info.is_directory, file_info_.is_directory); EXPECT_EQ(expected_info.is_symbolic_link, file_info_.is_symbolic_link); @@ -268,80 +107,21 @@ TEST_F(FileUtilProxyTest, GetFileInfo_Directory) { EXPECT_EQ(expected_info.creation_time, file_info_.creation_time); } -TEST_F(FileUtilProxyTest, Read) { - // Setup. - const char expected_data[] = "bleh"; - int expected_bytes = arraysize(expected_data); - ASSERT_EQ(expected_bytes, - file_util::WriteFile(test_path(), expected_data, expected_bytes)); - - // Run. - FileUtilProxy::Read( - file_task_runner(), - GetTestPlatformFile(PLATFORM_FILE_OPEN | PLATFORM_FILE_READ), - 0, // offset - 128, - Bind(&FileUtilProxyTest::DidRead, weak_factory_.GetWeakPtr())); - MessageLoop::current()->Run(); - - // Verify. - EXPECT_EQ(PLATFORM_FILE_OK, error_); - EXPECT_EQ(expected_bytes, static_cast<int>(buffer_.size())); - for (size_t i = 0; i < buffer_.size(); ++i) { - EXPECT_EQ(expected_data[i], buffer_[i]); - } -} - -TEST_F(FileUtilProxyTest, WriteAndFlush) { - const char data[] = "foo!"; - int data_bytes = ARRAYSIZE_UNSAFE(data); - PlatformFile file = GetTestPlatformFile( - PLATFORM_FILE_CREATE | PLATFORM_FILE_WRITE); - - FileUtilProxy::Write( - file_task_runner(), - file, - 0, // offset - data, - data_bytes, - Bind(&FileUtilProxyTest::DidWrite, weak_factory_.GetWeakPtr())); - MessageLoop::current()->Run(); - EXPECT_EQ(PLATFORM_FILE_OK, error_); - EXPECT_EQ(data_bytes, bytes_written_); - - // Flush the written data. (So that the following read should always - // succeed. On some platforms it may work with or without this flush.) - FileUtilProxy::Flush( - file_task_runner(), - file, - Bind(&FileUtilProxyTest::DidFinish, weak_factory_.GetWeakPtr())); - MessageLoop::current()->Run(); - EXPECT_EQ(PLATFORM_FILE_OK, error_); - - // Verify the written data. - char buffer[10]; - EXPECT_EQ(data_bytes, base::ReadFile(test_path(), buffer, data_bytes)); - for (int i = 0; i < data_bytes; ++i) { - EXPECT_EQ(data[i], buffer[i]); - } -} - TEST_F(FileUtilProxyTest, Touch) { + ASSERT_EQ(4, WriteFile(test_path(), "test", 4)); Time last_accessed_time = Time::Now() - TimeDelta::FromDays(12345); Time last_modified_time = Time::Now() - TimeDelta::FromHours(98765); FileUtilProxy::Touch( file_task_runner(), - GetTestPlatformFile(PLATFORM_FILE_CREATE | - PLATFORM_FILE_WRITE | - PLATFORM_FILE_WRITE_ATTRIBUTES), + test_path(), last_accessed_time, last_modified_time, Bind(&FileUtilProxyTest::DidFinish, weak_factory_.GetWeakPtr())); MessageLoop::current()->Run(); - EXPECT_EQ(PLATFORM_FILE_OK, error_); + EXPECT_EQ(File::FILE_OK, error_); - PlatformFileInfo info; + File::Info info; GetFileInfo(test_path(), &info); // The returned values may only have the seconds precision, so we cast @@ -352,60 +132,4 @@ TEST_F(FileUtilProxyTest, Touch) { static_cast<int>(info.last_accessed.ToDoubleT())); } -TEST_F(FileUtilProxyTest, Truncate_Shrink) { - // Setup. - const char kTestData[] = "0123456789"; - ASSERT_EQ(10, file_util::WriteFile(test_path(), kTestData, 10)); - PlatformFileInfo info; - GetFileInfo(test_path(), &info); - ASSERT_EQ(10, info.size); - - // Run. - FileUtilProxy::Truncate( - file_task_runner(), - GetTestPlatformFile(PLATFORM_FILE_OPEN | PLATFORM_FILE_WRITE), - 7, - Bind(&FileUtilProxyTest::DidFinish, weak_factory_.GetWeakPtr())); - MessageLoop::current()->Run(); - - // Verify. - GetFileInfo(test_path(), &info); - ASSERT_EQ(7, info.size); - - char buffer[7]; - EXPECT_EQ(7, base::ReadFile(test_path(), buffer, 7)); - int i = 0; - for (; i < 7; ++i) - EXPECT_EQ(kTestData[i], buffer[i]); -} - -TEST_F(FileUtilProxyTest, Truncate_Expand) { - // Setup. - const char kTestData[] = "9876543210"; - ASSERT_EQ(10, file_util::WriteFile(test_path(), kTestData, 10)); - PlatformFileInfo info; - GetFileInfo(test_path(), &info); - ASSERT_EQ(10, info.size); - - // Run. - FileUtilProxy::Truncate( - file_task_runner(), - GetTestPlatformFile(PLATFORM_FILE_OPEN | PLATFORM_FILE_WRITE), - 53, - Bind(&FileUtilProxyTest::DidFinish, weak_factory_.GetWeakPtr())); - MessageLoop::current()->Run(); - - // Verify. - GetFileInfo(test_path(), &info); - ASSERT_EQ(53, info.size); - - char buffer[53]; - EXPECT_EQ(53, base::ReadFile(test_path(), buffer, 53)); - int i = 0; - for (; i < 10; ++i) - EXPECT_EQ(kTestData[i], buffer[i]); - for (; i < 53; ++i) - EXPECT_EQ(0, buffer[i]); -} - } // namespace base diff --git a/chromium/base/files/file_win.cc b/chromium/base/files/file_win.cc index 94f4d7f59c9..9e18bda70b1 100644 --- a/chromium/base/files/file_win.cc +++ b/chromium/base/files/file_win.cc @@ -13,7 +13,7 @@ namespace base { -void File::CreateBaseFileUnsafe(const FilePath& name, uint32 flags) { +void File::InitializeUnsafe(const FilePath& name, uint32 flags) { base::ThreadRestrictions::AssertIOAllowed(); DCHECK(!IsValid()); @@ -34,6 +34,7 @@ void File::CreateBaseFileUnsafe(const FilePath& name, uint32 flags) { if (flags & FLAG_CREATE_ALWAYS) { DCHECK(!disposition); + DCHECK(flags & FLAG_WRITE); disposition = CREATE_ALWAYS; } @@ -84,7 +85,7 @@ void File::CreateBaseFileUnsafe(const FilePath& name, uint32 flags) { disposition, create_flags, NULL)); if (file_.IsValid()) { - error_ = FILE_OK; + error_details_ = FILE_OK; async_ = ((flags & FLAG_ASYNC) == FLAG_ASYNC); if (flags & (FLAG_OPEN_ALWAYS)) @@ -92,20 +93,27 @@ void File::CreateBaseFileUnsafe(const FilePath& name, uint32 flags) { else if (flags & (FLAG_CREATE_ALWAYS | FLAG_CREATE)) created_ = true; } else { - error_ = OSErrorToFileError(GetLastError()); + error_details_ = OSErrorToFileError(GetLastError()); } } bool File::IsValid() const { return file_.IsValid(); } + +PlatformFile File::GetPlatformFile() const { + return file_; +} + PlatformFile File::TakePlatformFile() { return file_.Take(); } void File::Close() { - base::ThreadRestrictions::AssertIOAllowed(); - file_.Close(); + if (file_.IsValid()) { + base::ThreadRestrictions::AssertIOAllowed(); + file_.Close(); + } } int64 File::Seek(Whence whence, int64 offset) { @@ -137,7 +145,7 @@ int File::Read(int64 offset, char* data, int size) { overlapped.OffsetHigh = offset_li.HighPart; DWORD bytes_read; - if (::ReadFile(file_, data, size, &bytes_read, &overlapped) != 0) + if (::ReadFile(file_, data, size, &bytes_read, &overlapped)) return bytes_read; if (ERROR_HANDLE_EOF == GetLastError()) return 0; @@ -153,7 +161,7 @@ int File::ReadAtCurrentPos(char* data, int size) { return -1; DWORD bytes_read; - if (::ReadFile(file_, data, size, &bytes_read, NULL) != 0) + if (::ReadFile(file_, data, size, &bytes_read, NULL)) return bytes_read; if (ERROR_HANDLE_EOF == GetLastError()) return 0; @@ -182,14 +190,23 @@ int File::Write(int64 offset, const char* data, int size) { overlapped.OffsetHigh = offset_li.HighPart; DWORD bytes_written; - if (::WriteFile(file_, data, size, &bytes_written, &overlapped) != 0) + if (::WriteFile(file_, data, size, &bytes_written, &overlapped)) return bytes_written; return -1; } int File::WriteAtCurrentPos(const char* data, int size) { - NOTREACHED(); + base::ThreadRestrictions::AssertIOAllowed(); + DCHECK(IsValid()); + DCHECK(!async_); + if (size < 0) + return -1; + + DWORD bytes_written; + if (::WriteFile(file_, data, size, &bytes_written, NULL)) + return bytes_written; + return -1; } @@ -197,7 +214,17 @@ int File::WriteAtCurrentPosNoBestEffort(const char* data, int size) { return WriteAtCurrentPos(data, size); } -bool File::Truncate(int64 length) { +int64 File::GetLength() { + base::ThreadRestrictions::AssertIOAllowed(); + DCHECK(IsValid()); + LARGE_INTEGER size; + if (!::GetFileSizeEx(file_.Get(), &size)) + return -1; + + return static_cast<int64>(size.QuadPart); +} + +bool File::SetLength(int64 length) { base::ThreadRestrictions::AssertIOAllowed(); DCHECK(IsValid()); @@ -205,7 +232,7 @@ bool File::Truncate(int64 length) { LARGE_INTEGER file_pointer; LARGE_INTEGER zero; zero.QuadPart = 0; - if (::SetFilePointerEx(file_, zero, &file_pointer, FILE_CURRENT) == 0) + if (!::SetFilePointerEx(file_, zero, &file_pointer, FILE_CURRENT)) return false; LARGE_INTEGER length_li; @@ -218,8 +245,11 @@ bool File::Truncate(int64 length) { // Set the new file length and move the file pointer to its old position. // This is consistent with ftruncate()'s behavior, even when the file // pointer points to a location beyond the end of the file. - return ((::SetEndOfFile(file_) != 0) && - (::SetFilePointerEx(file_, file_pointer, NULL, FILE_BEGIN) != 0)); + // TODO(rvargas): Emulating ftruncate details seem suspicious and it is not + // promised by the interface (nor was promised by PlatformFile). See if this + // implementation detail can be removed. + return ((::SetEndOfFile(file_) != FALSE) && + (::SetFilePointerEx(file_, file_pointer, NULL, FILE_BEGIN) != FALSE)); } bool File::Flush() { @@ -235,7 +265,7 @@ bool File::SetTimes(Time last_access_time, Time last_modified_time) { FILETIME last_access_filetime = last_access_time.ToFileTime(); FILETIME last_modified_filetime = last_modified_time.ToFileTime(); return (::SetFileTime(file_, NULL, &last_access_filetime, - &last_modified_filetime) != 0); + &last_modified_filetime) != FALSE); } bool File::GetInfo(Info* info) { @@ -243,7 +273,7 @@ bool File::GetInfo(Info* info) { DCHECK(IsValid()); BY_HANDLE_FILE_INFORMATION file_info; - if (GetFileInformationByHandle(file_, &file_info) == 0) + if (!GetFileInformationByHandle(file_, &file_info)) return false; LARGE_INTEGER size; diff --git a/chromium/base/files/important_file_writer.cc b/chromium/base/files/important_file_writer.cc index 261c98772e5..bf4e0033e3a 100644 --- a/chromium/base/files/important_file_writer.cc +++ b/chromium/base/files/important_file_writer.cc @@ -2,12 +2,6 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -#if defined _MSC_VER && _MSC_VER == 1800 -// TODO(scottmg): Internal errors on VS2013 RC in LTCG. This should be removed -// after RTM. http://crbug.com/288948 -#pragma optimize("", off) -#endif - #include "base/files/important_file_writer.h" #include <stdio.h> @@ -17,11 +11,13 @@ #include "base/bind.h" #include "base/critical_closure.h" #include "base/file_util.h" +#include "base/files/file.h" #include "base/files/file_path.h" #include "base/logging.h" #include "base/metrics/histogram.h" #include "base/strings/string_number_conversions.h" #include "base/task_runner.h" +#include "base/task_runner_util.h" #include "base/threading/thread.h" #include "base/time/time.h" @@ -63,25 +59,18 @@ bool ImportantFileWriter::WriteFileAtomically(const FilePath& path, return false; } - int flags = PLATFORM_FILE_OPEN | PLATFORM_FILE_WRITE; - PlatformFile tmp_file = - CreatePlatformFile(tmp_file_path, flags, NULL, NULL); - if (tmp_file == kInvalidPlatformFileValue) { + File tmp_file(tmp_file_path, File::FLAG_OPEN | File::FLAG_WRITE); + if (!tmp_file.IsValid()) { LogFailure(path, FAILED_OPENING, "could not open temporary file"); return false; } // If this happens in the wild something really bad is going on. CHECK_LE(data.length(), static_cast<size_t>(kint32max)); - int bytes_written = WritePlatformFile( - tmp_file, 0, data.data(), static_cast<int>(data.length())); - FlushPlatformFile(tmp_file); // Ignore return value. - - if (!ClosePlatformFile(tmp_file)) { - LogFailure(path, FAILED_CLOSING, "failed to close temporary file"); - base::DeleteFile(tmp_file_path, false); - return false; - } + int bytes_written = tmp_file.Write(0, data.data(), + static_cast<int>(data.length())); + tmp_file.Flush(); // Ignore return value. + tmp_file.Close(); if (bytes_written < static_cast<int>(data.length())) { LogFailure(path, FAILED_WRITING, "error writing, bytes_written=" + @@ -99,13 +88,13 @@ bool ImportantFileWriter::WriteFileAtomically(const FilePath& path, return true; } -ImportantFileWriter::ImportantFileWriter( - const FilePath& path, base::SequencedTaskRunner* task_runner) - : path_(path), - task_runner_(task_runner), - serializer_(NULL), - commit_interval_(TimeDelta::FromMilliseconds( - kDefaultCommitIntervalMs)) { +ImportantFileWriter::ImportantFileWriter(const FilePath& path, + base::SequencedTaskRunner* task_runner) + : path_(path), + task_runner_(task_runner), + serializer_(NULL), + commit_interval_(TimeDelta::FromMilliseconds(kDefaultCommitIntervalMs)), + weak_factory_(this) { DCHECK(CalledOnValidThread()); DCHECK(task_runner_.get()); } @@ -132,11 +121,7 @@ void ImportantFileWriter::WriteNow(const std::string& data) { if (HasPendingWrite()) timer_.Stop(); - if (!task_runner_->PostTask( - FROM_HERE, - MakeCriticalClosure( - Bind(IgnoreResult(&ImportantFileWriter::WriteFileAtomically), - path_, data)))) { + if (!PostWriteTask(data)) { // Posting the task to background message loop is not expected // to fail, but if it does, avoid losing data and just hit the disk // on the current thread. @@ -170,4 +155,40 @@ void ImportantFileWriter::DoScheduledWrite() { serializer_ = NULL; } +void ImportantFileWriter::RegisterOnNextSuccessfulWriteCallback( + const base::Closure& on_next_successful_write) { + DCHECK(on_next_successful_write_.is_null()); + on_next_successful_write_ = on_next_successful_write; +} + +bool ImportantFileWriter::PostWriteTask(const std::string& data) { + // TODO(gab): This code could always use PostTaskAndReplyWithResult and let + // ForwardSuccessfulWrite() no-op if |on_next_successful_write_| is null, but + // PostTaskAndReply causes memory leaks in tests (crbug.com/371974) and + // suppressing all of those is unrealistic hence we avoid most of them by + // using PostTask() in the typical scenario below. + if (!on_next_successful_write_.is_null()) { + return base::PostTaskAndReplyWithResult( + task_runner_, + FROM_HERE, + MakeCriticalClosure( + Bind(&ImportantFileWriter::WriteFileAtomically, path_, data)), + Bind(&ImportantFileWriter::ForwardSuccessfulWrite, + weak_factory_.GetWeakPtr())); + } + return task_runner_->PostTask( + FROM_HERE, + MakeCriticalClosure( + Bind(IgnoreResult(&ImportantFileWriter::WriteFileAtomically), + path_, data))); +} + +void ImportantFileWriter::ForwardSuccessfulWrite(bool result) { + DCHECK(CalledOnValidThread()); + if (result && !on_next_successful_write_.is_null()) { + on_next_successful_write_.Run(); + on_next_successful_write_.Reset(); + } +} + } // namespace base diff --git a/chromium/base/files/important_file_writer.h b/chromium/base/files/important_file_writer.h index ba1c745a4ef..61a53b16503 100644 --- a/chromium/base/files/important_file_writer.h +++ b/chromium/base/files/important_file_writer.h @@ -9,6 +9,7 @@ #include "base/base_export.h" #include "base/basictypes.h" +#include "base/callback.h" #include "base/files/file_path.h" #include "base/memory/ref_counted.h" #include "base/threading/non_thread_safe.h" @@ -89,6 +90,11 @@ class BASE_EXPORT ImportantFileWriter : public NonThreadSafe { // Serialize data pending to be saved and execute write on backend thread. void DoScheduledWrite(); + // Registers |on_next_successful_write| to be called once, on the next + // successful write event. Only one callback can be set at once. + void RegisterOnNextSuccessfulWriteCallback( + const base::Closure& on_next_successful_write); + TimeDelta commit_interval() const { return commit_interval_; } @@ -98,6 +104,16 @@ class BASE_EXPORT ImportantFileWriter : public NonThreadSafe { } private: + // Helper method for WriteNow(). + bool PostWriteTask(const std::string& data); + + // If |result| is true and |on_next_successful_write_| is set, invokes + // |on_successful_write_| and then resets it; no-ops otherwise. + void ForwardSuccessfulWrite(bool result); + + // Invoked once and then reset on the next successful write event. + base::Closure on_next_successful_write_; + // Path being written to. const FilePath path_; @@ -113,6 +129,8 @@ class BASE_EXPORT ImportantFileWriter : public NonThreadSafe { // Time delta after which scheduled data will be written to disk. TimeDelta commit_interval_; + WeakPtrFactory<ImportantFileWriter> weak_factory_; + DISALLOW_COPY_AND_ASSIGN(ImportantFileWriter); }; diff --git a/chromium/base/files/important_file_writer_unittest.cc b/chromium/base/files/important_file_writer_unittest.cc index 02a5f76b2b6..3f62fe4953b 100644 --- a/chromium/base/files/important_file_writer_unittest.cc +++ b/chromium/base/files/important_file_writer_unittest.cc @@ -4,6 +4,7 @@ #include "base/files/important_file_writer.h" +#include "base/bind.h" #include "base/compiler_specific.h" #include "base/file_util.h" #include "base/files/file_path.h" @@ -41,6 +42,41 @@ class DataSerializer : public ImportantFileWriter::DataSerializer { const std::string data_; }; +class SuccessfulWriteObserver { + public: + SuccessfulWriteObserver() : successful_write_observed_(false) {} + + // Register on_successful_write() to be called on the next successful write + // of |writer|. + void ObserveNextSuccessfulWrite(ImportantFileWriter* writer); + + // Returns true if a successful write was observed via on_successful_write() + // and resets the observation state to false regardless. + bool GetAndResetObservationState(); + + private: + void on_successful_write() { + EXPECT_FALSE(successful_write_observed_); + successful_write_observed_ = true; + } + + bool successful_write_observed_; + + DISALLOW_COPY_AND_ASSIGN(SuccessfulWriteObserver); +}; + +void SuccessfulWriteObserver::ObserveNextSuccessfulWrite( + ImportantFileWriter* writer) { + writer->RegisterOnNextSuccessfulWriteCallback(base::Bind( + &SuccessfulWriteObserver::on_successful_write, base::Unretained(this))); +} + +bool SuccessfulWriteObserver::GetAndResetObservationState() { + bool was_successful_write_observed = successful_write_observed_; + successful_write_observed_ = false; + return was_successful_write_observed; +} + } // namespace class ImportantFileWriterTest : public testing::Test { @@ -52,6 +88,7 @@ class ImportantFileWriterTest : public testing::Test { } protected: + SuccessfulWriteObserver successful_write_observer_; FilePath file_; MessageLoop loop_; @@ -62,11 +99,47 @@ class ImportantFileWriterTest : public testing::Test { TEST_F(ImportantFileWriterTest, Basic) { ImportantFileWriter writer(file_, MessageLoopProxy::current().get()); EXPECT_FALSE(PathExists(writer.path())); + EXPECT_FALSE(successful_write_observer_.GetAndResetObservationState()); + writer.WriteNow("foo"); + RunLoop().RunUntilIdle(); + + EXPECT_FALSE(successful_write_observer_.GetAndResetObservationState()); + ASSERT_TRUE(PathExists(writer.path())); + EXPECT_EQ("foo", GetFileContent(writer.path())); +} + +TEST_F(ImportantFileWriterTest, BasicWithSuccessfulWriteObserver) { + ImportantFileWriter writer(file_, MessageLoopProxy::current().get()); + EXPECT_FALSE(PathExists(writer.path())); + EXPECT_FALSE(successful_write_observer_.GetAndResetObservationState()); + successful_write_observer_.ObserveNextSuccessfulWrite(&writer); writer.WriteNow("foo"); RunLoop().RunUntilIdle(); + // Confirm that the observer is invoked. + EXPECT_TRUE(successful_write_observer_.GetAndResetObservationState()); ASSERT_TRUE(PathExists(writer.path())); EXPECT_EQ("foo", GetFileContent(writer.path())); + + // Confirm that re-installing the observer works for another write. + EXPECT_FALSE(successful_write_observer_.GetAndResetObservationState()); + successful_write_observer_.ObserveNextSuccessfulWrite(&writer); + writer.WriteNow("bar"); + RunLoop().RunUntilIdle(); + + EXPECT_TRUE(successful_write_observer_.GetAndResetObservationState()); + ASSERT_TRUE(PathExists(writer.path())); + EXPECT_EQ("bar", GetFileContent(writer.path())); + + // Confirm that writing again without re-installing the observer doesn't + // result in a notification. + EXPECT_FALSE(successful_write_observer_.GetAndResetObservationState()); + writer.WriteNow("baz"); + RunLoop().RunUntilIdle(); + + EXPECT_FALSE(successful_write_observer_.GetAndResetObservationState()); + ASSERT_TRUE(PathExists(writer.path())); + EXPECT_EQ("baz", GetFileContent(writer.path())); } TEST_F(ImportantFileWriterTest, ScheduleWrite) { diff --git a/chromium/base/files/memory_mapped_file.cc b/chromium/base/files/memory_mapped_file.cc index a48ec0ceb2a..ace4e112628 100644 --- a/chromium/base/files/memory_mapped_file.cc +++ b/chromium/base/files/memory_mapped_file.cc @@ -17,7 +17,14 @@ bool MemoryMappedFile::Initialize(const FilePath& file_name) { if (IsValid()) return false; - if (!MapFileToMemory(file_name)) { + file_.Initialize(file_name, File::FLAG_OPEN | File::FLAG_READ); + + if (!file_.IsValid()) { + DLOG(ERROR) << "Couldn't open " << file_name.AsUTF8Unsafe(); + return false; + } + + if (!MapFileToMemory()) { CloseHandles(); return false; } @@ -25,13 +32,13 @@ bool MemoryMappedFile::Initialize(const FilePath& file_name) { return true; } -bool MemoryMappedFile::Initialize(PlatformFile file) { +bool MemoryMappedFile::Initialize(File file) { if (IsValid()) return false; - file_ = file; + file_ = file.Pass(); - if (!MapFileToMemoryInternal()) { + if (!MapFileToMemory()) { CloseHandles(); return false; } @@ -43,16 +50,4 @@ bool MemoryMappedFile::IsValid() const { return data_ != NULL; } -bool MemoryMappedFile::MapFileToMemory(const FilePath& file_name) { - file_ = CreatePlatformFile(file_name, PLATFORM_FILE_OPEN | PLATFORM_FILE_READ, - NULL, NULL); - - if (file_ == kInvalidPlatformFileValue) { - DLOG(ERROR) << "Couldn't open " << file_name.AsUTF8Unsafe(); - return false; - } - - return MapFileToMemoryInternal(); -} - } // namespace base diff --git a/chromium/base/files/memory_mapped_file.h b/chromium/base/files/memory_mapped_file.h index 6df1bad6359..b02d8cfbdae 100644 --- a/chromium/base/files/memory_mapped_file.h +++ b/chromium/base/files/memory_mapped_file.h @@ -7,7 +7,7 @@ #include "base/base_export.h" #include "base/basictypes.h" -#include "base/platform_file.h" +#include "base/files/file.h" #include "build/build_config.h" #if defined(OS_WIN) @@ -30,9 +30,10 @@ class BASE_EXPORT MemoryMappedFile { // the file does not exist, or the memory mapping fails, it will return false. // Later we may want to allow the user to specify access. bool Initialize(const FilePath& file_name); - // As above, but works with an already-opened file. MemoryMappedFile will take - // ownership of |file| and close it when done. - bool Initialize(PlatformFile file); + + // As above, but works with an already-opened file. MemoryMappedFile takes + // ownership of |file| and closes it when done. + bool Initialize(File file); #if defined(OS_WIN) // Opens an existing file and maps it as an image section. Please refer to @@ -47,27 +48,22 @@ class BASE_EXPORT MemoryMappedFile { bool IsValid() const; private: - // Open the given file and pass it to MapFileToMemoryInternal(). - bool MapFileToMemory(const FilePath& file_name); - // Map the file to memory, set data_ to that memory address. Return true on // success, false on any kind of failure. This is a helper for Initialize(). - bool MapFileToMemoryInternal(); + bool MapFileToMemory(); - // Closes all open handles. Later we may want to make this public. + // Closes all open handles. void CloseHandles(); -#if defined(OS_WIN) - // MapFileToMemoryInternal calls this function. It provides the ability to - // pass in flags which control the mapped section. - bool MapFileToMemoryInternalEx(int flags); - - HANDLE file_mapping_; -#endif - PlatformFile file_; + File file_; uint8* data_; size_t length_; +#if defined(OS_WIN) + win::ScopedHandle file_mapping_; + bool image_; // Map as an image. +#endif + DISALLOW_COPY_AND_ASSIGN(MemoryMappedFile); }; diff --git a/chromium/base/files/memory_mapped_file_posix.cc b/chromium/base/files/memory_mapped_file_posix.cc index c4c477a3fe7..5d7e0079992 100644 --- a/chromium/base/files/memory_mapped_file_posix.cc +++ b/chromium/base/files/memory_mapped_file_posix.cc @@ -13,26 +13,23 @@ namespace base { -MemoryMappedFile::MemoryMappedFile() - : file_(kInvalidPlatformFileValue), - data_(NULL), - length_(0) { +MemoryMappedFile::MemoryMappedFile() : data_(NULL), length_(0) { } -bool MemoryMappedFile::MapFileToMemoryInternal() { +bool MemoryMappedFile::MapFileToMemory() { ThreadRestrictions::AssertIOAllowed(); struct stat file_stat; - if (fstat(file_, &file_stat) == kInvalidPlatformFileValue) { - DPLOG(ERROR) << "fstat " << file_; + if (fstat(file_.GetPlatformFile(), &file_stat) == -1 ) { + DPLOG(ERROR) << "fstat " << file_.GetPlatformFile(); return false; } length_ = file_stat.st_size; data_ = static_cast<uint8*>( - mmap(NULL, length_, PROT_READ, MAP_SHARED, file_, 0)); + mmap(NULL, length_, PROT_READ, MAP_SHARED, file_.GetPlatformFile(), 0)); if (data_ == MAP_FAILED) - DPLOG(ERROR) << "mmap " << file_; + DPLOG(ERROR) << "mmap " << file_.GetPlatformFile(); return data_ != MAP_FAILED; } @@ -42,12 +39,10 @@ void MemoryMappedFile::CloseHandles() { if (data_ != NULL) munmap(data_, length_); - if (file_ != kInvalidPlatformFileValue) - close(file_); + file_.Close(); data_ = NULL; length_ = 0; - file_ = kInvalidPlatformFileValue; } } // namespace base diff --git a/chromium/base/files/memory_mapped_file_win.cc b/chromium/base/files/memory_mapped_file_win.cc index 694212950de..f3822873bfd 100644 --- a/chromium/base/files/memory_mapped_file_win.cc +++ b/chromium/base/files/memory_mapped_file_win.cc @@ -5,83 +5,52 @@ #include "base/files/memory_mapped_file.h" #include "base/files/file_path.h" -#include "base/logging.h" -#include "base/metrics/histogram.h" #include "base/strings/string16.h" #include "base/threading/thread_restrictions.h" namespace base { -MemoryMappedFile::MemoryMappedFile() - : file_(INVALID_HANDLE_VALUE), - file_mapping_(INVALID_HANDLE_VALUE), - data_(NULL), - length_(INVALID_FILE_SIZE) { +MemoryMappedFile::MemoryMappedFile() : data_(NULL), length_(0), image_(false) { } bool MemoryMappedFile::InitializeAsImageSection(const FilePath& file_name) { - if (IsValid()) - return false; - file_ = CreatePlatformFile(file_name, PLATFORM_FILE_OPEN | PLATFORM_FILE_READ, - NULL, NULL); - - if (file_ == kInvalidPlatformFileValue) { - DLOG(ERROR) << "Couldn't open " << file_name.AsUTF8Unsafe(); - return false; - } - - if (!MapFileToMemoryInternalEx(SEC_IMAGE)) { - CloseHandles(); - return false; - } - - return true; -} - -bool MemoryMappedFile::MapFileToMemoryInternal() { - return MapFileToMemoryInternalEx(0); + image_ = true; + return Initialize(file_name); } -bool MemoryMappedFile::MapFileToMemoryInternalEx(int flags) { +bool MemoryMappedFile::MapFileToMemory() { ThreadRestrictions::AssertIOAllowed(); - if (file_ == INVALID_HANDLE_VALUE) + if (!file_.IsValid()) return false; - length_ = ::GetFileSize(file_, NULL); - if (length_ == INVALID_FILE_SIZE) + int64 len = file_.GetLength(); + if (len <= 0 || len > kint32max) return false; + length_ = static_cast<size_t>(len); + + int flags = image_ ? SEC_IMAGE | PAGE_READONLY : PAGE_READONLY; - file_mapping_ = ::CreateFileMapping(file_, NULL, PAGE_READONLY | flags, - 0, 0, NULL); - if (!file_mapping_) { - // According to msdn, system error codes are only reserved up to 15999. - // http://msdn.microsoft.com/en-us/library/ms681381(v=VS.85).aspx. - UMA_HISTOGRAM_ENUMERATION("MemoryMappedFile.CreateFileMapping", - logging::GetLastSystemErrorCode(), 16000); + file_mapping_.Set(::CreateFileMapping(file_.GetPlatformFile(), NULL, + flags, 0, 0, NULL)); + if (!file_mapping_.IsValid()) return false; - } - data_ = static_cast<uint8*>( - ::MapViewOfFile(file_mapping_, FILE_MAP_READ, 0, 0, 0)); - if (!data_) { - UMA_HISTOGRAM_ENUMERATION("MemoryMappedFile.MapViewOfFile", - logging::GetLastSystemErrorCode(), 16000); - } + data_ = static_cast<uint8*>(::MapViewOfFile(file_mapping_.Get(), + FILE_MAP_READ, 0, 0, 0)); return data_ != NULL; } void MemoryMappedFile::CloseHandles() { if (data_) ::UnmapViewOfFile(data_); - if (file_mapping_ != INVALID_HANDLE_VALUE) - ::CloseHandle(file_mapping_); - if (file_ != INVALID_HANDLE_VALUE) - ::CloseHandle(file_); + if (file_mapping_.IsValid()) + file_mapping_.Close(); + if (file_.IsValid()) + file_.Close(); data_ = NULL; - file_mapping_ = file_ = INVALID_HANDLE_VALUE; - length_ = INVALID_FILE_SIZE; + length_ = 0; } } // namespace base diff --git a/chromium/base/files/scoped_file.cc b/chromium/base/files/scoped_file.cc new file mode 100644 index 00000000000..39f064de1c4 --- /dev/null +++ b/chromium/base/files/scoped_file.cc @@ -0,0 +1,35 @@ +// Copyright 2014 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. + +#include "base/files/scoped_file.h" + +#include "base/logging.h" + +#if defined(OS_POSIX) +#include <unistd.h> + +#include "base/posix/eintr_wrapper.h" +#endif + +namespace base { +namespace internal { + +#if defined(OS_POSIX) + +// static +void ScopedFDCloseTraits::Free(int fd) { + // It's important to crash here. + // There are security implications to not closing a file descriptor + // properly. As file descriptors are "capabilities", keeping them open + // would make the current process keep access to a resource. Much of + // Chrome relies on being able to "drop" such access. + // It's especially problematic on Linux with the setuid sandbox, where + // a single open directory would bypass the entire security model. + PCHECK(0 == IGNORE_EINTR(close(fd))); +} + +#endif // OS_POSIX + +} // namespace internal +} // namespace base diff --git a/chromium/base/files/scoped_file.h b/chromium/base/files/scoped_file.h new file mode 100644 index 00000000000..106f6ad94bf --- /dev/null +++ b/chromium/base/files/scoped_file.h @@ -0,0 +1,61 @@ +// Copyright 2014 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. + +#ifndef BASE_FILES_SCOPED_FILE_H_ +#define BASE_FILES_SCOPED_FILE_H_ + +#include <stdio.h> + +#include "base/base_export.h" +#include "base/logging.h" +#include "base/memory/scoped_ptr.h" +#include "base/scoped_generic.h" +#include "build/build_config.h" + +namespace base { + +namespace internal { + +#if defined(OS_POSIX) +struct BASE_EXPORT ScopedFDCloseTraits { + static int InvalidValue() { + return -1; + } + static void Free(int fd); +}; +#endif + +// Functor for |ScopedFILE| (below). +struct ScopedFILECloser { + inline void operator()(FILE* x) const { + if (x) + fclose(x); + } +}; + +} // namespace internal + +// ----------------------------------------------------------------------------- + +#if defined(OS_POSIX) +// A low-level Posix file descriptor closer class. Use this when writing +// platform-specific code, especially that does non-file-like things with the +// FD (like sockets). +// +// If you're writing low-level Windows code, see base/win/scoped_handle.h +// which provides some additional functionality. +// +// If you're writing cross-platform code that deals with actual files, you +// should generally use base::File instead which can be constructed with a +// handle, and in addition to handling ownership, has convenient cross-platform +// file manipulation functions on it. +typedef ScopedGeneric<int, internal::ScopedFDCloseTraits> ScopedFD; +#endif + +// Automatically closes |FILE*|s. +typedef scoped_ptr<FILE, internal::ScopedFILECloser> ScopedFILE; + +} // namespace base + +#endif // BASE_FILES_SCOPED_FILE_H_ diff --git a/chromium/base/files/scoped_temp_dir_unittest.cc b/chromium/base/files/scoped_temp_dir_unittest.cc index fe243ce2ee5..da222304a09 100644 --- a/chromium/base/files/scoped_temp_dir_unittest.cc +++ b/chromium/base/files/scoped_temp_dir_unittest.cc @@ -5,8 +5,8 @@ #include <string> #include "base/file_util.h" +#include "base/files/file.h" #include "base/files/scoped_temp_dir.h" -#include "base/platform_file.h" #include "testing/gtest/include/gtest/gtest.h" namespace base { @@ -98,17 +98,13 @@ TEST(ScopedTempDir, MultipleInvocations) { TEST(ScopedTempDir, LockedTempDir) { ScopedTempDir dir; EXPECT_TRUE(dir.CreateUniqueTempDir()); - int file_flags = base::PLATFORM_FILE_CREATE_ALWAYS | - base::PLATFORM_FILE_WRITE; - base::PlatformFileError error_code = base::PLATFORM_FILE_OK; - FilePath file_path(dir.path().Append(FILE_PATH_LITERAL("temp"))); - base::PlatformFile file = base::CreatePlatformFile(file_path, file_flags, - NULL, &error_code); - EXPECT_NE(base::kInvalidPlatformFileValue, file); - EXPECT_EQ(base::PLATFORM_FILE_OK, error_code); + base::File file(dir.path().Append(FILE_PATH_LITERAL("temp")), + base::File::FLAG_CREATE_ALWAYS | base::File::FLAG_WRITE); + EXPECT_TRUE(file.IsValid()); + EXPECT_EQ(base::File::FILE_OK, file.error_details()); EXPECT_FALSE(dir.Delete()); // We should not be able to delete. EXPECT_FALSE(dir.path().empty()); // We should still have a valid path. - EXPECT_TRUE(base::ClosePlatformFile(file)); + file.Close(); // Now, we should be able to delete. EXPECT_TRUE(dir.Delete()); } |