diff options
Diffstat (limited to 'chromium/base/files')
39 files changed, 1075 insertions, 668 deletions
diff --git a/chromium/base/files/dir_reader_linux.h b/chromium/base/files/dir_reader_linux.h index cb0cbd38e56..abf259530b3 100644 --- a/chromium/base/files/dir_reader_linux.h +++ b/chromium/base/files/dir_reader_linux.h @@ -95,4 +95,4 @@ class DirReaderLinux { } // namespace base -#endif // BASE_FILES_DIR_READER_LINUX_H_ +#endif // BASE_FILES_DIR_READER_LINUX_H_ diff --git a/chromium/base/files/dir_reader_posix.h b/chromium/base/files/dir_reader_posix.h index 6a20ced0223..6a32d9fd480 100644 --- a/chromium/base/files/dir_reader_posix.h +++ b/chromium/base/files/dir_reader_posix.h @@ -33,4 +33,4 @@ typedef DirReaderFallback DirReaderPosix; } // namespace base -#endif // BASE_FILES_DIR_READER_POSIX_H_ +#endif // BASE_FILES_DIR_READER_POSIX_H_ diff --git a/chromium/base/files/file.cc b/chromium/base/files/file.cc index ea8dbf27ac1..58f80c52322 100644 --- a/chromium/base/files/file.cc +++ b/chromium/base/files/file.cc @@ -4,6 +4,9 @@ #include "base/files/file.h" #include "base/files/file_path.h" +#include "base/files/file_tracing.h" +#include "base/metrics/histogram.h" +#include "base/timer/elapsed_timer.h" namespace base { @@ -23,11 +26,11 @@ File::File() } #if !defined(OS_NACL) -File::File(const FilePath& name, uint32 flags) +File::File(const FilePath& path, uint32 flags) : error_details_(FILE_OK), created_(false), async_(false) { - Initialize(name, flags); + Initialize(path, flags); } #endif @@ -49,6 +52,7 @@ File::File(Error error_details) File::File(RValue other) : file_(other.object->TakePlatformFile()), + path_(other.object->path_), error_details_(other.object->error_details()), created_(other.object->created()), async_(other.object->async_) { @@ -63,6 +67,7 @@ File& File::operator=(RValue other) { if (this != other.object) { Close(); SetPlatformFile(other.object->TakePlatformFile()); + path_ = other.object->path_; error_details_ = other.object->error_details(); created_ = other.object->created(); async_ = other.object->async_; @@ -71,12 +76,14 @@ File& File::operator=(RValue other) { } #if !defined(OS_NACL) -void File::Initialize(const FilePath& name, uint32 flags) { - if (name.ReferencesParent()) { +void File::Initialize(const FilePath& path, uint32 flags) { + if (path.ReferencesParent()) { error_details_ = FILE_ERROR_ACCESS_DENIED; return; } - InitializeUnsafe(name, flags); + path_ = path; + SCOPED_FILE_TRACE("Initialize"); + DoInitialize(flags); } #endif @@ -124,4 +131,12 @@ std::string File::ErrorToString(Error error) { return ""; } +bool File::Flush() { + ElapsedTimer timer; + SCOPED_FILE_TRACE("Flush"); + bool return_value = DoFlush(); + UMA_HISTOGRAM_TIMES("PlatformFile.FlushTime", timer.Elapsed()); + return return_value; +} + } // namespace base diff --git a/chromium/base/files/file.h b/chromium/base/files/file.h index 7b6366c1c2e..b21b15972bc 100644 --- a/chromium/base/files/file.h +++ b/chromium/base/files/file.h @@ -18,6 +18,8 @@ #include "base/base_export.h" #include "base/basictypes.h" +#include "base/files/file_path.h" +#include "base/files/file_tracing.h" #include "base/files/scoped_file.h" #include "base/gtest_prod_util.h" #include "base/move.h" @@ -31,8 +33,6 @@ FORWARD_DECLARE_TEST(FileTest, MemoryCorruption); namespace base { -class FilePath; - #if defined(OS_WIN) typedef HANDLE PlatformFile; #elif defined(OS_POSIX) @@ -128,9 +128,9 @@ class BASE_EXPORT File { // Used to hold information about a given file. // If you add more fields to this structure (platform-specific fields are OK), - // make sure to update all functions that use it in file_util_{win|posix}.cc - // too, and the ParamTraits<base::PlatformFileInfo> implementation in - // chrome/common/common_param_traits.cc. + // make sure to update all functions that use it in file_util_{win|posix}.cc, + // too, and the ParamTraits<base::File::Info> implementation in + // ipc/ipc_message_utils.cc. struct BASE_EXPORT Info { Info(); ~Info(); @@ -145,24 +145,25 @@ class BASE_EXPORT File { // True if the file corresponds to a directory. bool is_directory; - // True if the file corresponds to a symbolic link. + // True if the file corresponds to a symbolic link. For Windows currently + // not supported and thus always false. bool is_symbolic_link; // The last modified time of a file. - base::Time last_modified; + Time last_modified; // The last accessed time of a file. - base::Time last_accessed; + Time last_accessed; // The creation time of a file. - base::Time creation_time; + Time creation_time; }; File(); // Creates or opens the given file. This will fail with 'access denied' if the - // |name| contains path traversal ('..') components. - File(const FilePath& name, uint32 flags); + // |path| contains path traversal ('..') components. + File(const FilePath& path, uint32 flags); // Takes ownership of |platform_file|. explicit File(PlatformFile platform_file); @@ -179,11 +180,7 @@ class BASE_EXPORT File { 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 InitializeUnsafe(const FilePath& name, uint32 flags); + void Initialize(const FilePath& path, uint32 flags); bool IsValid() const; @@ -194,7 +191,7 @@ class BASE_EXPORT File { // 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); + // File file(path, flags); // if (!file.IsValid()) // return; Error error_details() const { return error_details_; } @@ -287,6 +284,13 @@ class BASE_EXPORT File { // Unlock a file previously locked. Error Unlock(); + // Returns a new object referencing this file for use within the current + // process. Handling of FLAG_DELETE_ON_CLOSE varies by OS. On POSIX, the File + // object that was created or initialized with this flag will have unlinked + // the underlying file when it was created or opened. On Windows, the + // underlying file is deleted when the last handle to it is closed. + File Duplicate(); + bool async() const { return async_; } #if defined(OS_WIN) @@ -301,6 +305,8 @@ class BASE_EXPORT File { private: FRIEND_TEST_ALL_PREFIXES(::FileTest, MemoryCorruption); + friend class FileTracing::ScopedTrace; + #if defined(OS_POSIX) // Encloses a single ScopedFD, saving a cheap tamper resistent memory checksum // alongside it. This checksum is validated at every access, allowing early @@ -346,6 +352,14 @@ class BASE_EXPORT File { }; #endif + // Creates or opens the given file. Only called if |path_| has no + // traversal ('..') components. + void DoInitialize(uint32 flags); + + // TODO(tnagel): Reintegrate into Flush() once histogram isn't needed anymore, + // cf. issue 473337. + bool DoFlush(); + void SetPlatformFile(PlatformFile file); #if defined(OS_WIN) @@ -354,6 +368,12 @@ class BASE_EXPORT File { MemoryCheckingScopedFD file_; #endif + // Path that |Initialize()| was called with. Only set if safe (i.e. no '..'). + FilePath path_; + + // Object tied to the lifetime of |this| that enables/disables tracing. + FileTracing::ScopedEnabler trace_enabler_; + Error error_details_; bool created_; bool async_; diff --git a/chromium/base/files/file_enumerator_win.cc b/chromium/base/files/file_enumerator_win.cc index 6da1667ed99..931d1549816 100644 --- a/chromium/base/files/file_enumerator_win.cc +++ b/chromium/base/files/file_enumerator_win.cc @@ -147,7 +147,8 @@ FilePath FileEnumerator::Next() { // add it to pending_paths_ so we scan it after we finish scanning this // directory. However, don't do recursion through reparse points or we // may end up with an infinite cycle. - if (!(find_data_.dwFileAttributes & FILE_ATTRIBUTE_REPARSE_POINT)) + DWORD attributes = GetFileAttributes(cur_file.value().c_str()); + if (!(attributes & FILE_ATTRIBUTE_REPARSE_POINT)) pending_paths_.push(cur_file); } if (file_type_ & FileEnumerator::DIRECTORIES) diff --git a/chromium/base/files/file_path.cc b/chromium/base/files/file_path.cc index bf37be615b1..33d5ca1b884 100644 --- a/chromium/base/files/file_path.cc +++ b/chromium/base/files/file_path.cc @@ -593,7 +593,7 @@ std::string FilePath::MaybeAsASCII() const { } std::string FilePath::AsUTF8Unsafe() const { -#if defined(OS_MACOSX) || defined(OS_CHROMEOS) +#if defined(SYSTEM_NATIVE_UTF8) return value(); #else return WideToUTF8(SysNativeMBToWide(value())); @@ -601,7 +601,7 @@ std::string FilePath::AsUTF8Unsafe() const { } string16 FilePath::AsUTF16Unsafe() const { -#if defined(OS_MACOSX) || defined(OS_CHROMEOS) +#if defined(SYSTEM_NATIVE_UTF8) return UTF8ToUTF16(value()); #else return WideToUTF16(SysNativeMBToWide(value())); @@ -610,7 +610,7 @@ string16 FilePath::AsUTF16Unsafe() const { // static FilePath FilePath::FromUTF8Unsafe(const std::string& utf8) { -#if defined(OS_MACOSX) || defined(OS_CHROMEOS) +#if defined(SYSTEM_NATIVE_UTF8) return FilePath(utf8); #else return FilePath(SysWideToNativeMB(UTF8ToWide(utf8))); @@ -619,7 +619,7 @@ FilePath FilePath::FromUTF8Unsafe(const std::string& utf8) { // static FilePath FilePath::FromUTF16Unsafe(const string16& utf16) { -#if defined(OS_MACOSX) || defined(OS_CHROMEOS) +#if defined(SYSTEM_NATIVE_UTF8) return FilePath(UTF16ToUTF8(utf16)); #else return FilePath(SysWideToNativeMB(UTF16ToWide(utf16))); diff --git a/chromium/base/files/file_path.h b/chromium/base/files/file_path.h index ad42b954927..5225b12ffd4 100644 --- a/chromium/base/files/file_path.h +++ b/chromium/base/files/file_path.h @@ -53,7 +53,7 @@ // between char[]-based pathnames on POSIX systems and wchar_t[]-based // pathnames on Windows. // -// Paths can't contain NULs as a precaution agaist premature truncation. +// As a precaution against premature truncation, paths can't contain NULs. // // Because a FilePath object should not be instantiated at the global scope, // instead, use a FilePath::CharType[] and initialize it with @@ -83,9 +83,9 @@ // in case it ever comes across such a system. FilePath needs this support // for Windows UNC paths, anyway. // References: -// The Open Group Base Specifications Issue 7, sections 3.266 ("Pathname") +// The Open Group Base Specifications Issue 7, sections 3.267 ("Pathname") // and 4.12 ("Pathname Resolution"), available at: -// http://www.opengroup.org/onlinepubs/9699919799/basedefs/V1_chap03.html#tag_03_266 +// http://www.opengroup.org/onlinepubs/9699919799/basedefs/V1_chap03.html#tag_03_267 // http://www.opengroup.org/onlinepubs/9699919799/basedefs/V1_chap04.html#tag_04_12 // // - Windows treats c:\\ the same way it treats \\. This was intended to @@ -107,6 +107,7 @@ #include <vector> #include "base/base_export.h" +#include "base/compiler_specific.h" #include "base/containers/hash_tables.h" #include "base/strings/string16.h" #include "base/strings/string_piece.h" // For implicit conversions. @@ -237,7 +238,7 @@ class BASE_EXPORT FilePath { // ASSERT(new_path == path.value()); // NOTE: this is different from the original file_util implementation which // returned the extension without a leading "." ("jpg" instead of ".jpg") - StringType Extension() const; + StringType Extension() const WARN_UNUSED_RESULT; // Returns the path's file extension, as in Extension(), but will // never return a double extension. @@ -246,7 +247,7 @@ class BASE_EXPORT FilePath { // 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 base::GetUniquePathNumber(). - StringType FinalExtension() const; + StringType FinalExtension() const WARN_UNUSED_RESULT; // Returns "C:\pics\jojo" for path "C:\pics\jojo.jpg" // NOTE: this is slightly different from the similar file_util implementation @@ -442,11 +443,9 @@ BASE_EXPORT extern void PrintTo(const base::FilePath& path, std::ostream* out); #if defined(OS_POSIX) #define FILE_PATH_LITERAL(x) x #define PRFilePath "s" -#define PRFilePathLiteral "%s" #elif defined(OS_WIN) #define FILE_PATH_LITERAL(x) L ## x #define PRFilePath "ls" -#define PRFilePathLiteral L"%ls" #endif // OS_WIN // Provide a hash function so that hash_sets and maps can contain FilePath diff --git a/chromium/base/files/file_path_unittest.cc b/chromium/base/files/file_path_unittest.cc index 956faea3218..c16293861ce 100644 --- a/chromium/base/files/file_path_unittest.cc +++ b/chromium/base/files/file_path_unittest.cc @@ -48,15 +48,7 @@ struct UTF8TestData { // file_util winds up using autoreleased objects on the Mac, so this needs // to be a PlatformTest -class FilePathTest : public PlatformTest { - protected: - virtual void SetUp() override { - PlatformTest::SetUp(); - } - virtual void TearDown() override { - PlatformTest::TearDown(); - } -}; +typedef PlatformTest FilePathTest; TEST_F(FilePathTest, DirName) { const struct UnaryTestData cases[] = { diff --git a/chromium/base/files/file_path_watcher.cc b/chromium/base/files/file_path_watcher.cc index b17354197d4..59ae7059caf 100644 --- a/chromium/base/files/file_path_watcher.cc +++ b/chromium/base/files/file_path_watcher.cc @@ -32,7 +32,7 @@ bool FilePathWatcher::RecursiveWatchAvailable() { // 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) +#elif defined(OS_WIN) || defined(OS_LINUX) || defined(OS_ANDROID) return true; #else return false; diff --git a/chromium/base/files/file_path_watcher.h b/chromium/base/files/file_path_watcher.h index d26fa060446..4f132aff78a 100644 --- a/chromium/base/files/file_path_watcher.h +++ b/chromium/base/files/file_path_watcher.h @@ -12,7 +12,7 @@ #include "base/callback.h" #include "base/files/file_path.h" #include "base/memory/ref_counted.h" -#include "base/message_loop/message_loop_proxy.h" +#include "base/single_thread_task_runner.h" namespace base { @@ -58,12 +58,13 @@ class BASE_EXPORT FilePathWatcher { // check |is_cancelled()| to avoid duplicate work. virtual void CancelOnMessageLoopThread() = 0; - scoped_refptr<base::MessageLoopProxy> message_loop() const { - return message_loop_; + scoped_refptr<base::SingleThreadTaskRunner> task_runner() const { + return task_runner_; } - void set_message_loop(const scoped_refptr<base::MessageLoopProxy>& loop) { - message_loop_ = loop; + void set_task_runner( + scoped_refptr<base::SingleThreadTaskRunner> task_runner) { + task_runner_ = task_runner.Pass(); } // Must be called before the PlatformDelegate is deleted. @@ -76,7 +77,7 @@ class BASE_EXPORT FilePathWatcher { } private: - scoped_refptr<base::MessageLoopProxy> message_loop_; + scoped_refptr<base::SingleThreadTaskRunner> task_runner_; bool cancelled_; }; diff --git a/chromium/base/files/file_path_watcher_fsevents.cc b/chromium/base/files/file_path_watcher_fsevents.cc index f240e33dd6a..da01c431bfe 100644 --- a/chromium/base/files/file_path_watcher_fsevents.cc +++ b/chromium/base/files/file_path_watcher_fsevents.cc @@ -13,6 +13,7 @@ #include "base/mac/libdispatch_task_runner.h" #include "base/mac/scoped_cftyperef.h" #include "base/message_loop/message_loop.h" +#include "base/thread_task_runner_handle.h" namespace base { @@ -75,11 +76,51 @@ FilePath ResolvePath(const FilePath& path) { 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[]) { +} // namespace + +FilePathWatcherFSEvents::FilePathWatcherFSEvents() : fsevent_stream_(NULL) { +} + +bool FilePathWatcherFSEvents::Watch(const FilePath& path, + bool recursive, + const FilePathWatcher::Callback& callback) { + DCHECK(MessageLoopForIO::current()); + DCHECK(!callback.is_null()); + DCHECK(callback_.is_null()); + + // This class could support non-recursive watches, but that is currently + // left to FilePathWatcherKQueue. + if (!recursive) + return false; + + set_task_runner(ThreadTaskRunnerHandle::Get()); + callback_ = callback; + + FSEventStreamEventId start_event = FSEventsGetCurrentEventId(); + g_task_runner.Get().PostTask( + FROM_HERE, Bind(&FilePathWatcherFSEvents::StartEventStream, this, + start_event, path)); + return true; +} + +void FilePathWatcherFSEvents::Cancel() { + set_cancelled(); + callback_.Reset(); + + // Switch to the dispatch queue thread to tear down the event stream. + g_task_runner.Get().PostTask( + FROM_HERE, + Bind(&FilePathWatcherFSEvents::CancelOnMessageLoopThread, this)); +} + +// static +void FilePathWatcherFSEvents::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()); @@ -111,83 +152,51 @@ void FSEventsCallback(ConstFSEventStreamRef stream, watcher->OnFilePathsChanged(paths); } -} // namespace - -FilePathWatcherFSEvents::FilePathWatcherFSEvents() : fsevent_stream_(NULL) { +FilePathWatcherFSEvents::~FilePathWatcherFSEvents() { + // This method may be called on either the libdispatch or task_runner() + // thread. Checking callback_ on the libdispatch thread here is safe because + // it is executing in a task posted by Cancel() which first reset callback_. + // PostTask forms a sufficient memory barrier to ensure that the value is + // consistent on the target thread. + DCHECK(callback_.is_null()) + << "Cancel() must be called before FilePathWatcher is destroyed."; } 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; - } - } + DCHECK(g_task_runner.Get().RunsTasksOnCurrentThread()); + DCHECK(!resolved_target_.empty()); + task_runner()->PostTask( + FROM_HERE, Bind(&FilePathWatcherFSEvents::DispatchEvents, this, paths, + target_, resolved_target_)); } -bool FilePathWatcherFSEvents::Watch(const FilePath& path, - bool recursive, - const FilePathWatcher::Callback& callback) { - DCHECK(resolved_target_.empty()); - DCHECK(MessageLoopForIO::current()); - DCHECK(!callback.is_null()); +void FilePathWatcherFSEvents::DispatchEvents(const std::vector<FilePath>& paths, + const FilePath& target, + const FilePath& resolved_target) { + DCHECK(task_runner()->RunsTasksOnCurrentThread()); - // 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(); + // Don't issue callbacks after Cancel() has been called. + if (is_cancelled() || callback_.is_null()) { 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(); + for (const FilePath& path : paths) { + if (resolved_target.IsParent(path) || resolved_target == path) { + callback_.Run(target, false); + return; + } } } 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. + // as returned by task_runner(). This implementation, however, needs to + // cancel pending work on the Dispatch Queue thread. DCHECK(g_task_runner.Get().RunsTasksOnCurrentThread()); - set_cancelled(); if (fsevent_stream_) { DestroyEventStream(); - callback_.Reset(); target_.clear(); resolved_target_.clear(); } @@ -199,7 +208,7 @@ void FilePathWatcherFSEvents::UpdateEventStream( // 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()) + if (resolved_target_.empty()) return; if (fsevent_stream_) @@ -230,8 +239,10 @@ void FilePathWatcherFSEvents::UpdateEventStream( FSEventStreamSetDispatchQueue(fsevent_stream_, g_task_runner.Get().GetDispatchQueue()); - if (!FSEventStreamStart(fsevent_stream_)) - message_loop()->PostTask(FROM_HERE, Bind(callback_, target_, true)); + if (!FSEventStreamStart(fsevent_stream_)) { + task_runner()->PostTask( + FROM_HERE, Bind(&FilePathWatcherFSEvents::ReportError, this, target_)); + } } bool FilePathWatcherFSEvents::ResolveTargetPath() { @@ -239,11 +250,20 @@ bool FilePathWatcherFSEvents::ResolveTargetPath() { 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)); + if (resolved_target_.empty()) { + task_runner()->PostTask( + FROM_HERE, Bind(&FilePathWatcherFSEvents::ReportError, this, target_)); + } return changed; } +void FilePathWatcherFSEvents::ReportError(const FilePath& target) { + DCHECK(task_runner()->RunsTasksOnCurrentThread()); + if (!callback_.is_null()) { + callback_.Run(target, true); + } +} + void FilePathWatcherFSEvents::DestroyEventStream() { FSEventStreamStop(fsevent_stream_); FSEventStreamInvalidate(fsevent_stream_); @@ -251,13 +271,14 @@ void FilePathWatcherFSEvents::DestroyEventStream() { fsevent_stream_ = NULL; } -void FilePathWatcherFSEvents::StartEventStream( - FSEventStreamEventId start_event) { +void FilePathWatcherFSEvents::StartEventStream(FSEventStreamEventId start_event, + const FilePath& path) { DCHECK(g_task_runner.Get().RunsTasksOnCurrentThread()); + DCHECK(resolved_target_.empty()); + + target_ = path; 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 index 800c5b43f68..300aa761dfa 100644 --- a/chromium/base/files/file_path_watcher_fsevents.h +++ b/chromium/base/files/file_path_watcher_fsevents.h @@ -24,9 +24,35 @@ class FilePathWatcherFSEvents : public FilePathWatcher::PlatformDelegate { public: FilePathWatcherFSEvents(); - // Called from the FSEvents callback whenever there is a change to the paths. + // FilePathWatcher::PlatformDelegate overrides. + bool Watch(const FilePath& path, + bool recursive, + const FilePathWatcher::Callback& callback) override; + void Cancel() override; + + private: + static void FSEventsCallback(ConstFSEventStreamRef stream, + void* event_watcher, + size_t num_events, + void* event_paths, + const FSEventStreamEventFlags flags[], + const FSEventStreamEventId event_ids[]); + + ~FilePathWatcherFSEvents() override; + + // Called from FSEventsCallback whenever there is a change to the paths. void OnFilePathsChanged(const std::vector<FilePath>& paths); + // Called on the message_loop() thread to dispatch path events. Can't access + // target_ and resolved_target_ directly as those are modified on the + // libdispatch thread. + void DispatchEvents(const std::vector<FilePath>& paths, + const FilePath& target, + const FilePath& resolved_target); + + // Cleans up and stops the event stream. + void CancelOnMessageLoopThread() override; + // (Re-)Initialize the event stream to start reporting events from // |start_event|. void UpdateEventStream(FSEventStreamEventId start_event); @@ -35,34 +61,29 @@ class FilePathWatcherFSEvents : public FilePathWatcher::PlatformDelegate { // last time it was done. bool ResolveTargetPath(); - // FilePathWatcher::PlatformDelegate overrides. - bool Watch(const FilePath& path, - bool recursive, - const FilePathWatcher::Callback& callback) override; - void Cancel() override; - - private: - ~FilePathWatcherFSEvents() override; + // Report an error watching the given target. + void ReportError(const FilePath& target); // Destroy the event stream. void DestroyEventStream(); // Start watching the FSEventStream. - void StartEventStream(FSEventStreamEventId start_event); - - // Cleans up and stops the event stream. - void CancelOnMessageLoopThread() override; + void StartEventStream(FSEventStreamEventId start_event, const FilePath& path); // Callback to notify upon changes. + // (Only accessed from the message_loop() thread.) FilePathWatcher::Callback callback_; // Target path to watch (passed to callback). + // (Only accessed from the libdispatch thread.) FilePath target_; // Target path with all symbolic links resolved. + // (Only accessed from the libdispatch thread.) FilePath resolved_target_; // Backend stream we receive event callbacks from (strong reference). + // (Only accessed from the libdispatch thread.) FSEventStreamRef fsevent_stream_; DISALLOW_COPY_AND_ASSIGN(FilePathWatcherFSEvents); diff --git a/chromium/base/files/file_path_watcher_kqueue.cc b/chromium/base/files/file_path_watcher_kqueue.cc index 8941d2e419f..e15cba7d341 100644 --- a/chromium/base/files/file_path_watcher_kqueue.cc +++ b/chromium/base/files/file_path_watcher_kqueue.cc @@ -11,6 +11,7 @@ #include "base/files/file_util.h" #include "base/logging.h" #include "base/strings/stringprintf.h" +#include "base/thread_task_runner_handle.h" // On some platforms these are not defined. #if !defined(EV_RECEIPT) @@ -327,7 +328,7 @@ bool FilePathWatcherKQueue::Watch(const FilePath& path, target_ = path; MessageLoop::current()->AddDestructionObserver(this); - io_message_loop_ = base::MessageLoopProxy::current(); + io_task_runner_ = ThreadTaskRunnerHandle::Get(); kqueue_ = kqueue(); if (kqueue_ == -1) { @@ -356,14 +357,14 @@ bool FilePathWatcherKQueue::Watch(const FilePath& path, } void FilePathWatcherKQueue::Cancel() { - base::MessageLoopProxy* proxy = io_message_loop_.get(); - if (!proxy) { + SingleThreadTaskRunner* task_runner = io_task_runner_.get(); + if (!task_runner) { set_cancelled(); return; } - if (!proxy->BelongsToCurrentThread()) { - proxy->PostTask(FROM_HERE, - base::Bind(&FilePathWatcherKQueue::Cancel, this)); + if (!task_runner->BelongsToCurrentThread()) { + task_runner->PostTask(FROM_HERE, + base::Bind(&FilePathWatcherKQueue::Cancel, this)); return; } CancelOnMessageLoopThread(); @@ -380,7 +381,7 @@ void FilePathWatcherKQueue::CancelOnMessageLoopThread() { kqueue_ = -1; std::for_each(events_.begin(), events_.end(), ReleaseEvent); events_.clear(); - io_message_loop_ = NULL; + io_task_runner_ = NULL; MessageLoop::current()->RemoveDestructionObserver(this); callback_.Reset(); } diff --git a/chromium/base/files/file_path_watcher_kqueue.h b/chromium/base/files/file_path_watcher_kqueue.h index 87af8913e55..69555a30032 100644 --- a/chromium/base/files/file_path_watcher_kqueue.h +++ b/chromium/base/files/file_path_watcher_kqueue.h @@ -11,7 +11,7 @@ #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" +#include "base/single_thread_task_runner.h" namespace base { @@ -59,7 +59,7 @@ class FilePathWatcherKQueue : public FilePathWatcher::PlatformDelegate, typedef std::vector<struct kevent> EventVector; - // Can only be called on |io_message_loop_|'s thread. + // Can only be called on |io_task_runner_|'s thread. void CancelOnMessageLoopThread() override; // Returns true if the kevent values are error free. @@ -118,7 +118,7 @@ class FilePathWatcherKQueue : public FilePathWatcher::PlatformDelegate, } EventVector events_; - scoped_refptr<base::MessageLoopProxy> io_message_loop_; + scoped_refptr<base::SingleThreadTaskRunner> io_task_runner_; MessageLoopForIO::FileDescriptorWatcher kqueue_watcher_; FilePathWatcher::Callback callback_; FilePath target_; diff --git a/chromium/base/files/file_path_watcher_linux.cc b/chromium/base/files/file_path_watcher_linux.cc index 4078920aac9..ba2f1d96c84 100644 --- a/chromium/base/files/file_path_watcher_linux.cc +++ b/chromium/base/files/file_path_watcher_linux.cc @@ -19,7 +19,6 @@ #include "base/bind.h" #include "base/containers/hash_tables.h" -#include "base/debug/trace_event.h" #include "base/files/file_enumerator.h" #include "base/files/file_path.h" #include "base/files/file_util.h" @@ -27,11 +26,12 @@ #include "base/location.h" #include "base/logging.h" #include "base/memory/scoped_ptr.h" -#include "base/message_loop/message_loop.h" -#include "base/message_loop/message_loop_proxy.h" #include "base/posix/eintr_wrapper.h" +#include "base/single_thread_task_runner.h" #include "base/synchronization/lock.h" +#include "base/thread_task_runner_handle.h" #include "base/threading/thread.h" +#include "base/trace_event/trace_event.h" namespace base { @@ -125,10 +125,13 @@ class FilePathWatcherImpl : public FilePathWatcher::PlatformDelegate, // cleanup thread, in case it quits before Cancel() is called. void WillDestroyCurrentMessageLoop() 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. + // Inotify watches are installed for all directory components of |target_|. + // A WatchEntry instance holds: + // - |watch|: the watch descriptor for a component. + // - |subdir|: the subdirectory that identifies the next component. + // - For the last component, there is no next component, so it is empty. + // - |linkname|: the target of the symlink. + // - Only if the target being watched is a symbolic link. struct WatchEntry { explicit WatchEntry(const FilePath::StringType& dirname) : watch(InotifyReader::kInvalidWatch), @@ -197,7 +200,7 @@ void InotifyReaderCallback(InotifyReader* reader, int inotify_fd, CHECK_LE(0, shutdown_fd); CHECK_GT(FD_SETSIZE, shutdown_fd); - debug::TraceLog::GetInstance()->SetCurrentThreadBlocksMessageLoop(); + trace_event::TraceLog::GetInstance()->SetCurrentThreadBlocksMessageLoop(); while (true) { fd_set rfds; @@ -261,7 +264,7 @@ InotifyReader::InotifyReader() shutdown_pipe_[0] = -1; shutdown_pipe_[1] = -1; if (inotify_fd_ >= 0 && pipe(shutdown_pipe_) == 0 && thread_.Start()) { - thread_.message_loop()->PostTask( + thread_.task_runner()->PostTask( FROM_HERE, Bind(&InotifyReaderCallback, this, inotify_fd_, shutdown_pipe_[0])); valid_ = true; @@ -346,12 +349,11 @@ void FilePathWatcherImpl::OnFilePathChanged(InotifyReader::Watch fired_watch, 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, deleted, is_dir)); + if (!task_runner()->BelongsToCurrentThread()) { + // Switch to task_runner() to access |watches_| safely. + task_runner()->PostTask(FROM_HERE, + Bind(&FilePathWatcherImpl::OnFilePathChanged, this, + fired_watch, child, created, deleted, is_dir)); return; } @@ -381,11 +383,31 @@ void FilePathWatcherImpl::OnFilePathChanged(InotifyReader::Watch fired_watch, (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()); + bool target_changed; + if (watch_entry.subdir.empty()) { + // The fired watch is for a WatchEntry without a subdir. Thus for a given + // |target_| = "/path/to/foo", this is for "foo". Here, check either: + // - the target has no symlink: it is the target and it changed. + // - the target has a symlink, and it matches |child|. + target_changed = (watch_entry.linkname.empty() || + child == watch_entry.linkname); + } else { + // The fired watch is for a WatchEntry with a subdir. Thus for a given + // |target_| = "/path/to/foo", this is for {"/", "/path", "/path/to"}. + // So we can safely access the next WatchEntry since we have not reached + // the end yet. Check |watch_entry| is for "/path/to", i.e. the next + // element is "foo". + bool next_watch_may_be_for_target = watches_[i + 1].subdir.empty(); + if (next_watch_may_be_for_target) { + // The current |watch_entry| is for "/path/to", so check if the |child| + // that changed is "foo". + target_changed = watch_entry.subdir == child; + } else { + // The current |watch_entry| is not for "/path/to", so the next entry + // cannot be "foo". Thus |target_| has not changed. + target_changed = false; + } + } // Update watches if a directory component of the |target_| path // (dis)appears. Note that we don't add the additional restriction of @@ -429,7 +451,7 @@ bool FilePathWatcherImpl::Watch(const FilePath& path, DCHECK(target_.empty()); DCHECK(MessageLoopForIO::current()); - set_message_loop(MessageLoopProxy::current()); + set_task_runner(ThreadTaskRunnerHandle::Get()); callback_ = callback; target_ = path; recursive_ = recursive; @@ -453,17 +475,16 @@ void FilePathWatcherImpl::Cancel() { } // 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))); + if (!task_runner()->BelongsToCurrentThread()) { + task_runner()->PostTask(FROM_HERE, Bind(&FilePathWatcher::CancelWatch, + make_scoped_refptr(this))); } else { CancelOnMessageLoopThread(); } } void FilePathWatcherImpl::CancelOnMessageLoopThread() { - DCHECK(message_loop()->BelongsToCurrentThread()); + DCHECK(task_runner()->BelongsToCurrentThread()); set_cancelled(); if (!callback_.is_null()) { @@ -487,7 +508,7 @@ void FilePathWatcherImpl::WillDestroyCurrentMessageLoop() { void FilePathWatcherImpl::UpdateWatches() { // Ensure this runs on the message_loop() exclusively in order to avoid // concurrency issues. - DCHECK(message_loop()->BelongsToCurrentThread()); + DCHECK(task_runner()->BelongsToCurrentThread()); DCHECK(HasValidWatchVector()); // Walk the list of watches and update them as we go. diff --git a/chromium/base/files/file_path_watcher_stub.cc b/chromium/base/files/file_path_watcher_stub.cc index d7ad2066aab..8138692ede1 100644 --- a/chromium/base/files/file_path_watcher_stub.cc +++ b/chromium/base/files/file_path_watcher_stub.cc @@ -13,18 +13,18 @@ namespace { class FilePathWatcherImpl : public FilePathWatcher::PlatformDelegate { public: - virtual bool Watch(const FilePath& path, - bool recursive, - const FilePathWatcher::Callback& callback) override { + bool Watch(const FilePath& path, + bool recursive, + const FilePathWatcher::Callback& callback) override { return false; } - virtual void Cancel() override {} + void Cancel() override {} - virtual void CancelOnMessageLoopThread() override {} + void CancelOnMessageLoopThread() override {} protected: - virtual ~FilePathWatcherImpl() {} + ~FilePathWatcherImpl() override {} }; } // namespace diff --git a/chromium/base/files/file_path_watcher_browsertest.cc b/chromium/base/files/file_path_watcher_unittest.cc index 0e19b2ed9ae..21e9dd137e2 100644 --- a/chromium/base/files/file_path_watcher_browsertest.cc +++ b/chromium/base/files/file_path_watcher_unittest.cc @@ -20,17 +20,22 @@ #include "base/files/file_path.h" #include "base/files/file_util.h" #include "base/files/scoped_temp_dir.h" -#include "base/message_loop/message_loop.h" -#include "base/message_loop/message_loop_proxy.h" +#include "base/location.h" #include "base/run_loop.h" +#include "base/single_thread_task_runner.h" #include "base/stl_util.h" #include "base/strings/stringprintf.h" #include "base/synchronization/waitable_event.h" #include "base/test/test_file_util.h" #include "base/test/test_timeouts.h" +#include "base/thread_task_runner_handle.h" #include "base/threading/thread.h" #include "testing/gtest/include/gtest/gtest.h" +#if defined(OS_ANDROID) +#include "base/android/path_utils.h" +#endif // defined(OS_ANDROID) + namespace base { namespace { @@ -42,14 +47,13 @@ class TestDelegate; class NotificationCollector : public base::RefCountedThreadSafe<NotificationCollector> { public: - NotificationCollector() - : loop_(base::MessageLoopProxy::current()) {} + NotificationCollector() : task_runner_(base::ThreadTaskRunnerHandle::Get()) {} // Called from the file thread by the delegates. void OnChange(TestDelegate* delegate) { - loop_->PostTask(FROM_HERE, - base::Bind(&NotificationCollector::RecordChange, this, - base::Unretained(delegate))); + task_runner_->PostTask( + FROM_HERE, base::Bind(&NotificationCollector::RecordChange, this, + base::Unretained(delegate))); } void Register(TestDelegate* delegate) { @@ -70,13 +74,13 @@ class NotificationCollector void RecordChange(TestDelegate* delegate) { // Warning: |delegate| is Unretained. Do not dereference. - ASSERT_TRUE(loop_->BelongsToCurrentThread()); + ASSERT_TRUE(task_runner_->BelongsToCurrentThread()); ASSERT_TRUE(delegates_.count(delegate)); signaled_.insert(delegate); // Check whether all delegates have been signaled. if (signaled_ == delegates_) - loop_->PostTask(FROM_HERE, MessageLoop::QuitWhenIdleClosure()); + task_runner_->PostTask(FROM_HERE, MessageLoop::QuitWhenIdleClosure()); } // Set of registered delegates. @@ -86,7 +90,7 @@ class NotificationCollector std::set<TestDelegate*> signaled_; // The loop we should break after all delegates signaled. - scoped_refptr<base::MessageLoopProxy> loop_; + scoped_refptr<base::SingleThreadTaskRunner> task_runner_; }; class TestDelegateBase : public SupportsWeakPtr<TestDelegateBase> { @@ -143,23 +147,31 @@ class FilePathWatcherTest : public testing::Test { FilePathWatcherTest() : file_thread_("FilePathWatcherTest") {} - virtual ~FilePathWatcherTest() {} + ~FilePathWatcherTest() override {} protected: - virtual void SetUp() override { + void SetUp() override { // Create a separate file thread in order to test proper thread usage. base::Thread::Options options(MessageLoop::TYPE_IO, 0); ASSERT_TRUE(file_thread_.StartWithOptions(options)); +#if defined(OS_ANDROID) + // Watching files is only permitted when all parent directories are + // accessible, which is not the case for the default temp directory + // on Android which is under /data/data. Use /sdcard instead. + // TODO(pauljensen): Remove this when crbug.com/475568 is fixed. + FilePath parent_dir; + ASSERT_TRUE(android::GetExternalStorageDirectory(&parent_dir)); + ASSERT_TRUE(temp_dir_.CreateUniqueTempDirUnderPath(parent_dir)); +#else // defined(OS_ANDROID) ASSERT_TRUE(temp_dir_.CreateUniqueTempDir()); +#endif // defined(OS_ANDROID) collector_ = new NotificationCollector(); } - virtual void TearDown() override { - RunLoop().RunUntilIdle(); - } + void TearDown() override { RunLoop().RunUntilIdle(); } void DeleteDelegateOnFileThread(TestDelegate* delegate) { - file_thread_.message_loop_proxy()->DeleteSoon(FROM_HERE, delegate); + file_thread_.task_runner()->DeleteSoon(FROM_HERE, delegate); } FilePath test_file() { @@ -194,6 +206,7 @@ class FilePathWatcherTest : public testing::Test { ScopedTempDir temp_dir_; scoped_refptr<NotificationCollector> collector_; + private: DISALLOW_COPY_AND_ASSIGN(FilePathWatcherTest); }; @@ -203,10 +216,9 @@ bool FilePathWatcherTest::SetupWatch(const FilePath& target, bool recursive_watch) { base::WaitableEvent completion(false, false); bool result; - file_thread_.message_loop_proxy()->PostTask( - FROM_HERE, - base::Bind(SetupWatchCallback, target, watcher, delegate, recursive_watch, - &result, &completion)); + file_thread_.task_runner()->PostTask( + FROM_HERE, base::Bind(SetupWatchCallback, target, watcher, delegate, + recursive_watch, &result, &completion)); completion.Wait(); return result; } @@ -276,7 +288,8 @@ class Deleter : public TestDelegateBase { void OnFileChanged(const FilePath&, bool) override { watcher_.reset(); - loop_->PostTask(FROM_HERE, MessageLoop::QuitWhenIdleClosure()); + loop_->task_runner()->PostTask(FROM_HERE, + MessageLoop::QuitWhenIdleClosure()); } FilePathWatcher* watcher() const { return watcher_.get(); } @@ -311,7 +324,7 @@ TEST_F(FilePathWatcherTest, DISABLED_DestroyWithPendingNotification) { FilePathWatcher* watcher = new FilePathWatcher; ASSERT_TRUE(SetupWatch(test_file(), watcher, delegate.get(), false)); ASSERT_TRUE(WriteFile(test_file(), "content")); - file_thread_.message_loop_proxy()->DeleteSoon(FROM_HERE, watcher); + file_thread_.task_runner()->DeleteSoon(FROM_HERE, watcher); DeleteDelegateOnFileThread(delegate.release()); } @@ -527,9 +540,16 @@ TEST_F(FilePathWatcherTest, RecursiveWatch) { ASSERT_TRUE(WriteFile(child_dir_file1, "content")); ASSERT_TRUE(WaitForEvents()); +// Apps cannot change file attributes on Android in /sdcard as /sdcard uses the +// "fuse" file system, while /data uses "ext4". Running these tests in /data +// would be preferable and allow testing file attributes and symlinks. +// TODO(pauljensen): Re-enable when crbug.com/475568 is fixed and SetUp() places +// the |temp_dir_| in /data. +#if !defined(OS_ANDROID) // Modify "$dir/subdir/subdir_child_dir/child_dir_file1" attributes. ASSERT_TRUE(base::MakeFileUnreadable(child_dir_file1)); ASSERT_TRUE(WaitForEvents()); +#endif // Delete "$dir/subdir/subdir_file1". ASSERT_TRUE(base::DeleteFile(subdir_file1, false)); @@ -542,6 +562,14 @@ TEST_F(FilePathWatcherTest, RecursiveWatch) { } #if defined(OS_POSIX) +#if defined(OS_ANDROID) +// Apps cannot create symlinks on Android in /sdcard as /sdcard uses the +// "fuse" file system, while /data uses "ext4". Running these tests in /data +// would be preferable and allow testing file attributes and symlinks. +// TODO(pauljensen): Re-enable when crbug.com/475568 is fixed and SetUp() places +// the |temp_dir_| in /data. +#define RecursiveWithSymLink DISABLED_RecursiveWithSymLink +#endif // defined(OS_ANDROID) TEST_F(FilePathWatcherTest, RecursiveWithSymLink) { if (!FilePathWatcher::RecursiveWatchAvailable()) return; @@ -611,6 +639,14 @@ TEST_F(FilePathWatcherTest, MoveChild) { } // Verify that changing attributes on a file is caught +#if defined(OS_ANDROID) +// Apps cannot change file attributes on Android in /sdcard as /sdcard uses the +// "fuse" file system, while /data uses "ext4". Running these tests in /data +// would be preferable and allow testing file attributes and symlinks. +// TODO(pauljensen): Re-enable when crbug.com/475568 is fixed and SetUp() places +// the |temp_dir_| in /data. +#define FileAttributesChanged DISABLED_FileAttributesChanged +#endif // defined(OS_ANDROID TEST_F(FilePathWatcherTest, FileAttributesChanged) { ASSERT_TRUE(WriteFile(test_file(), "content")); FilePathWatcher watcher; diff --git a/chromium/base/files/file_path_watcher_win.cc b/chromium/base/files/file_path_watcher_win.cc index 73f9cfb332a..081698f8df1 100644 --- a/chromium/base/files/file_path_watcher_win.cc +++ b/chromium/base/files/file_path_watcher_win.cc @@ -10,8 +10,7 @@ #include "base/files/file_util.h" #include "base/logging.h" #include "base/memory/ref_counted.h" -#include "base/message_loop/message_loop_proxy.h" -#include "base/profiler/scoped_tracker.h" +#include "base/thread_task_runner_handle.h" #include "base/time/time.h" #include "base/win/object_watcher.h" @@ -28,21 +27,21 @@ class FilePathWatcherImpl : public FilePathWatcher::PlatformDelegate, recursive_watch_(false) {} // FilePathWatcher::PlatformDelegate overrides. - virtual bool Watch(const FilePath& path, - bool recursive, - const FilePathWatcher::Callback& callback) override; - virtual void Cancel() override; + bool Watch(const FilePath& path, + bool recursive, + const FilePathWatcher::Callback& callback) override; + void Cancel() 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; + void WillDestroyCurrentMessageLoop() override; // Callback from MessageLoopForIO. - virtual void OnObjectSignaled(HANDLE object) override; + void OnObjectSignaled(HANDLE object) override; private: - virtual ~FilePathWatcherImpl() {} + ~FilePathWatcherImpl() override {} // Setup a watch handle for directory |dir|. Set |recursive| to true to watch // the directory sub trees. Returns true if no fatal error occurs. |handle| @@ -58,7 +57,7 @@ class FilePathWatcherImpl : public FilePathWatcher::PlatformDelegate, // Destroy the watch handle. void DestroyWatch(); - // Cleans up and stops observing the |message_loop_| thread. + // Cleans up and stops observing the |task_runner_| thread. void CancelOnMessageLoopThread() override; // Callback to notify upon changes. @@ -92,7 +91,7 @@ bool FilePathWatcherImpl::Watch(const FilePath& path, const FilePathWatcher::Callback& callback) { DCHECK(target_.value().empty()); // Can only watch one path. - set_message_loop(MessageLoopProxy::current()); + set_task_runner(ThreadTaskRunnerHandle::Get()); callback_ = callback; target_ = path; recursive_watch_ = recursive; @@ -114,23 +113,22 @@ bool FilePathWatcherImpl::Watch(const FilePath& path, void FilePathWatcherImpl::Cancel() { if (callback_.is_null()) { - // Watch was never called, or the |message_loop_| has already quit. + // Watch was never called, or the |task_runner_| has already quit. set_cancelled(); return; } // Switch to the file thread if necessary so we can stop |watcher_|. - if (!message_loop()->BelongsToCurrentThread()) { - message_loop()->PostTask(FROM_HERE, - Bind(&FilePathWatcher::CancelWatch, - make_scoped_refptr(this))); + if (!task_runner()->BelongsToCurrentThread()) { + task_runner()->PostTask(FROM_HERE, Bind(&FilePathWatcher::CancelWatch, + make_scoped_refptr(this))); } else { CancelOnMessageLoopThread(); } } void FilePathWatcherImpl::CancelOnMessageLoopThread() { - DCHECK(message_loop()->BelongsToCurrentThread()); + DCHECK(task_runner()->BelongsToCurrentThread()); set_cancelled(); if (handle_ != INVALID_HANDLE_VALUE) @@ -147,10 +145,6 @@ void FilePathWatcherImpl::WillDestroyCurrentMessageLoop() { } void FilePathWatcherImpl::OnObjectSignaled(HANDLE object) { - // TODO(vadimt): Remove ScopedTracker below once crbug.com/418183 is fixed. - tracked_objects::ScopedTracker tracking_profile( - FROM_HERE_WITH_EXPLICIT_FUNCTION("FilePathWatcherImpl_OnObjectSignaled")); - DCHECK(object == handle_); // Make sure we stay alive through the body of this function. scoped_refptr<FilePathWatcherImpl> keep_alive(this); diff --git a/chromium/base/files/file_posix.cc b/chromium/base/files/file_posix.cc index 3d229e4155e..bb49d2dd730 100644 --- a/chromium/base/files/file_posix.cc +++ b/chromium/base/files/file_posix.cc @@ -9,7 +9,6 @@ #include <sys/stat.h> #include <unistd.h> -#include "base/files/file_path.h" #include "base/logging.h" #include "base/metrics/sparse_histogram.h" #include "base/posix/eintr_wrapper.h" @@ -31,12 +30,12 @@ namespace { #if defined(OS_BSD) || defined(OS_MACOSX) || defined(OS_NACL) static int CallFstat(int fd, stat_wrapper_t *sb) { - base::ThreadRestrictions::AssertIOAllowed(); + ThreadRestrictions::AssertIOAllowed(); return fstat(fd, sb); } #else static int CallFstat(int fd, stat_wrapper_t *sb) { - base::ThreadRestrictions::AssertIOAllowed(); + ThreadRestrictions::AssertIOAllowed(); return fstat64(fd, sb); } #endif @@ -52,10 +51,6 @@ static int CallFtruncate(PlatformFile file, int64 length) { return HANDLE_EINTR(ftruncate(file, length)); } -static int CallFsync(PlatformFile file) { - return HANDLE_EINTR(fsync(file)); -} - static int CallFutimes(PlatformFile file, const struct timeval times[2]) { #ifdef __USE_XOPEN2K8 // futimens should be available, but futimes might not be @@ -97,11 +92,6 @@ static int CallFtruncate(PlatformFile file, int64 length) { return 0; } -static int CallFsync(PlatformFile file) { - NOTIMPLEMENTED(); // NaCl doesn't implement fsync. - return 0; -} - static int CallFutimes(PlatformFile file, const struct timeval times[2]) { NOTIMPLEMENTED(); // NaCl doesn't implement futimes. return 0; @@ -166,95 +156,6 @@ void File::Info::FromStat(const stat_wrapper_t& stat_info) { 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::InitializeUnsafe(const FilePath& name, uint32 flags) { - base::ThreadRestrictions::AssertIOAllowed(); - DCHECK(!IsValid()); - - int open_flags = 0; - if (flags & FLAG_CREATE) - open_flags = O_CREAT | O_EXCL; - - created_ = false; - - if (flags & FLAG_CREATE_ALWAYS) { - DCHECK(!open_flags); - DCHECK(flags & FLAG_WRITE); - open_flags = O_CREAT | O_TRUNC; - } - - if (flags & FLAG_OPEN_TRUNCATED) { - DCHECK(!open_flags); - DCHECK(flags & FLAG_WRITE); - open_flags = O_TRUNC; - } - - if (!open_flags && !(flags & FLAG_OPEN) && !(flags & FLAG_OPEN_ALWAYS)) { - NOTREACHED(); - errno = EOPNOTSUPP; - error_details_ = FILE_ERROR_FAILED; - return; - } - - if (flags & FLAG_WRITE && flags & FLAG_READ) { - open_flags |= O_RDWR; - } else if (flags & FLAG_WRITE) { - open_flags |= O_WRONLY; - } else if (!(flags & FLAG_READ) && - !(flags & FLAG_WRITE_ATTRIBUTES) && - !(flags & FLAG_APPEND) && - !(flags & FLAG_OPEN_ALWAYS)) { - NOTREACHED(); - } - - if (flags & FLAG_TERMINAL_DEVICE) - open_flags |= O_NOCTTY | O_NDELAY; - - if (flags & FLAG_APPEND && flags & FLAG_READ) - open_flags |= O_APPEND | O_RDWR; - else if (flags & FLAG_APPEND) - open_flags |= O_APPEND | O_WRONLY; - - COMPILE_ASSERT(O_RDONLY == 0, O_RDONLY_must_equal_zero); - - int mode = S_IRUSR | S_IWUSR; -#if defined(OS_CHROMEOS) - mode |= S_IRGRP | S_IROTH; -#endif - - int descriptor = HANDLE_EINTR(open(name.value().c_str(), open_flags, mode)); - - if (flags & FLAG_OPEN_ALWAYS) { - if (descriptor < 0) { - open_flags |= O_CREAT; - if (flags & FLAG_EXCLUSIVE_READ || flags & FLAG_EXCLUSIVE_WRITE) - open_flags |= O_EXCL; // together with O_CREAT implies O_NOFOLLOW - - descriptor = HANDLE_EINTR(open(name.value().c_str(), open_flags, mode)); - if (descriptor >= 0) - created_ = true; - } - } - - if (descriptor < 0) { - error_details_ = File::OSErrorToFileError(errno); - return; - } - - if (flags & (FLAG_CREATE_ALWAYS | FLAG_CREATE)) - created_ = true; - - if (flags & FLAG_DELETE_ON_CLOSE) - unlink(name.value().c_str()); - - async_ = ((flags & FLAG_ASYNC) == FLAG_ASYNC); - error_details_ = FILE_OK; - file_.reset(descriptor); -} -#endif // !defined(OS_NACL) - bool File::IsValid() const { return file_.is_valid(); } @@ -271,14 +172,17 @@ void File::Close() { if (!IsValid()) return; - base::ThreadRestrictions::AssertIOAllowed(); + SCOPED_FILE_TRACE("Close"); + ThreadRestrictions::AssertIOAllowed(); file_.reset(); } int64 File::Seek(Whence whence, int64 offset) { - base::ThreadRestrictions::AssertIOAllowed(); + ThreadRestrictions::AssertIOAllowed(); DCHECK(IsValid()); + SCOPED_FILE_TRACE_WITH_SIZE("Seek", offset); + #if defined(OS_ANDROID) COMPILE_ASSERT(sizeof(int64) == sizeof(off64_t), off64_t_64_bit); return lseek64(file_.get(), static_cast<off64_t>(offset), @@ -291,11 +195,13 @@ int64 File::Seek(Whence whence, int64 offset) { } int File::Read(int64 offset, char* data, int size) { - base::ThreadRestrictions::AssertIOAllowed(); + ThreadRestrictions::AssertIOAllowed(); DCHECK(IsValid()); if (size < 0) return -1; + SCOPED_FILE_TRACE_WITH_SIZE("Read", size); + int bytes_read = 0; int rv; do { @@ -311,11 +217,13 @@ int File::Read(int64 offset, char* data, int size) { } int File::ReadAtCurrentPos(char* data, int size) { - base::ThreadRestrictions::AssertIOAllowed(); + ThreadRestrictions::AssertIOAllowed(); DCHECK(IsValid()); if (size < 0) return -1; + SCOPED_FILE_TRACE_WITH_SIZE("ReadAtCurrentPos", size); + int bytes_read = 0; int rv; do { @@ -330,23 +238,24 @@ int File::ReadAtCurrentPos(char* data, int size) { } int File::ReadNoBestEffort(int64 offset, char* data, int size) { - base::ThreadRestrictions::AssertIOAllowed(); + ThreadRestrictions::AssertIOAllowed(); DCHECK(IsValid()); - + SCOPED_FILE_TRACE_WITH_SIZE("ReadNoBestEffort", size); return HANDLE_EINTR(pread(file_.get(), data, size, offset)); } int File::ReadAtCurrentPosNoBestEffort(char* data, int size) { - base::ThreadRestrictions::AssertIOAllowed(); + ThreadRestrictions::AssertIOAllowed(); DCHECK(IsValid()); if (size < 0) return -1; + SCOPED_FILE_TRACE_WITH_SIZE("ReadAtCurrentPosNoBestEffort", size); return HANDLE_EINTR(read(file_.get(), data, size)); } int File::Write(int64 offset, const char* data, int size) { - base::ThreadRestrictions::AssertIOAllowed(); + ThreadRestrictions::AssertIOAllowed(); if (IsOpenAppend(file_.get())) return WriteAtCurrentPos(data, size); @@ -355,6 +264,8 @@ int File::Write(int64 offset, const char* data, int size) { if (size < 0) return -1; + SCOPED_FILE_TRACE_WITH_SIZE("Write", size); + int bytes_written = 0; int rv; do { @@ -370,11 +281,13 @@ int File::Write(int64 offset, const char* data, int size) { } int File::WriteAtCurrentPos(const char* data, int size) { - base::ThreadRestrictions::AssertIOAllowed(); + ThreadRestrictions::AssertIOAllowed(); DCHECK(IsValid()); if (size < 0) return -1; + SCOPED_FILE_TRACE_WITH_SIZE("WriteAtCurrentPos", size); + int bytes_written = 0; int rv; do { @@ -390,17 +303,20 @@ int File::WriteAtCurrentPos(const char* data, int size) { } int File::WriteAtCurrentPosNoBestEffort(const char* data, int size) { - base::ThreadRestrictions::AssertIOAllowed(); + ThreadRestrictions::AssertIOAllowed(); DCHECK(IsValid()); if (size < 0) return -1; + SCOPED_FILE_TRACE_WITH_SIZE("WriteAtCurrentPosNoBestEffort", size); return HANDLE_EINTR(write(file_.get(), data, size)); } int64 File::GetLength() { DCHECK(IsValid()); + SCOPED_FILE_TRACE("GetLength"); + stat_wrapper_t file_info; if (CallFstat(file_.get(), &file_info)) return false; @@ -409,21 +325,19 @@ int64 File::GetLength() { } bool File::SetLength(int64 length) { - base::ThreadRestrictions::AssertIOAllowed(); + ThreadRestrictions::AssertIOAllowed(); DCHECK(IsValid()); - return !CallFtruncate(file_.get(), length); -} -bool File::Flush() { - base::ThreadRestrictions::AssertIOAllowed(); - DCHECK(IsValid()); - return !CallFsync(file_.get()); + SCOPED_FILE_TRACE_WITH_SIZE("SetLength", length); + return !CallFtruncate(file_.get(), length); } bool File::SetTimes(Time last_access_time, Time last_modified_time) { - base::ThreadRestrictions::AssertIOAllowed(); + ThreadRestrictions::AssertIOAllowed(); DCHECK(IsValid()); + SCOPED_FILE_TRACE("SetTimes"); + timeval times[2]; times[0] = last_access_time.ToTimeVal(); times[1] = last_modified_time.ToTimeVal(); @@ -434,6 +348,8 @@ bool File::SetTimes(Time last_access_time, Time last_modified_time) { bool File::GetInfo(Info* info) { DCHECK(IsValid()); + SCOPED_FILE_TRACE("GetInfo"); + stat_wrapper_t file_info; if (CallFstat(file_.get(), &file_info)) return false; @@ -443,13 +359,31 @@ bool File::GetInfo(Info* info) { } File::Error File::Lock() { + SCOPED_FILE_TRACE("Lock"); return CallFctnlFlock(file_.get(), true); } File::Error File::Unlock() { + SCOPED_FILE_TRACE("Unlock"); return CallFctnlFlock(file_.get(), false); } +File File::Duplicate() { + if (!IsValid()) + return File(); + + SCOPED_FILE_TRACE("Duplicate"); + + PlatformFile other_fd = dup(GetPlatformFile()); + if (other_fd == -1) + return File(OSErrorToFileError(errno)); + + File other(other_fd); + if (async()) + other.async_ = true; + return other.Pass(); +} + // Static. File::Error File::OSErrorToFileError(int saved_errno) { switch (saved_errno) { @@ -458,12 +392,15 @@ File::Error File::OSErrorToFileError(int saved_errno) { case EROFS: case EPERM: return FILE_ERROR_ACCESS_DENIED; + case EBUSY: #if !defined(OS_NACL) // ETXTBSY not defined by NaCl. case ETXTBSY: - return FILE_ERROR_IN_USE; #endif + return FILE_ERROR_IN_USE; case EEXIST: return FILE_ERROR_EXISTS; + case EIO: + return FILE_ERROR_IO; case ENOENT: return FILE_ERROR_NOT_FOUND; case EMFILE: @@ -526,6 +463,109 @@ void File::MemoryCheckingScopedFD::UpdateChecksum() { ComputeMemoryChecksum(&file_memory_checksum_); } +// 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::DoInitialize(uint32 flags) { + ThreadRestrictions::AssertIOAllowed(); + DCHECK(!IsValid()); + + int open_flags = 0; + if (flags & FLAG_CREATE) + open_flags = O_CREAT | O_EXCL; + + created_ = false; + + if (flags & FLAG_CREATE_ALWAYS) { + DCHECK(!open_flags); + DCHECK(flags & FLAG_WRITE); + open_flags = O_CREAT | O_TRUNC; + } + + if (flags & FLAG_OPEN_TRUNCATED) { + DCHECK(!open_flags); + DCHECK(flags & FLAG_WRITE); + open_flags = O_TRUNC; + } + + if (!open_flags && !(flags & FLAG_OPEN) && !(flags & FLAG_OPEN_ALWAYS)) { + NOTREACHED(); + errno = EOPNOTSUPP; + error_details_ = FILE_ERROR_FAILED; + return; + } + + if (flags & FLAG_WRITE && flags & FLAG_READ) { + open_flags |= O_RDWR; + } else if (flags & FLAG_WRITE) { + open_flags |= O_WRONLY; + } else if (!(flags & FLAG_READ) && + !(flags & FLAG_WRITE_ATTRIBUTES) && + !(flags & FLAG_APPEND) && + !(flags & FLAG_OPEN_ALWAYS)) { + NOTREACHED(); + } + + if (flags & FLAG_TERMINAL_DEVICE) + open_flags |= O_NOCTTY | O_NDELAY; + + if (flags & FLAG_APPEND && flags & FLAG_READ) + open_flags |= O_APPEND | O_RDWR; + else if (flags & FLAG_APPEND) + open_flags |= O_APPEND | O_WRONLY; + + COMPILE_ASSERT(O_RDONLY == 0, O_RDONLY_must_equal_zero); + + int mode = S_IRUSR | S_IWUSR; +#if defined(OS_CHROMEOS) + mode |= S_IRGRP | S_IROTH; +#endif + + int descriptor = HANDLE_EINTR(open(path_.value().c_str(), open_flags, mode)); + + if (flags & FLAG_OPEN_ALWAYS) { + if (descriptor < 0) { + open_flags |= O_CREAT; + if (flags & FLAG_EXCLUSIVE_READ || flags & FLAG_EXCLUSIVE_WRITE) + open_flags |= O_EXCL; // together with O_CREAT implies O_NOFOLLOW + + descriptor = HANDLE_EINTR(open(path_.value().c_str(), open_flags, mode)); + if (descriptor >= 0) + created_ = true; + } + } + + if (descriptor < 0) { + error_details_ = File::OSErrorToFileError(errno); + return; + } + + if (flags & (FLAG_CREATE_ALWAYS | FLAG_CREATE)) + created_ = true; + + if (flags & FLAG_DELETE_ON_CLOSE) + unlink(path_.value().c_str()); + + async_ = ((flags & FLAG_ASYNC) == FLAG_ASYNC); + error_details_ = FILE_OK; + file_.reset(descriptor); +} +#endif // !defined(OS_NACL) + +bool File::DoFlush() { + ThreadRestrictions::AssertIOAllowed(); + DCHECK(IsValid()); + +#if defined(OS_NACL) + NOTIMPLEMENTED(); // NaCl doesn't implement fsync. + return true; +#elif defined(OS_LINUX) || defined(OS_ANDROID) + return !HANDLE_EINTR(fdatasync(file_.get())); +#else + return !HANDLE_EINTR(fsync(file_.get())); +#endif +} + void File::SetPlatformFile(PlatformFile file) { DCHECK(!file_.is_valid()); file_.reset(file); diff --git a/chromium/base/files/file_proxy.cc b/chromium/base/files/file_proxy.cc index 53b14fef2fd..f995735d5bc 100644 --- a/chromium/base/files/file_proxy.cc +++ b/chromium/base/files/file_proxy.cc @@ -9,7 +9,6 @@ #include "base/files/file.h" #include "base/files/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" diff --git a/chromium/base/files/file_proxy_unittest.cc b/chromium/base/files/file_proxy_unittest.cc index b5ed3950a05..df0bbc869c2 100644 --- a/chromium/base/files/file_proxy_unittest.cc +++ b/chromium/base/files/file_proxy_unittest.cc @@ -9,7 +9,6 @@ #include "base/files/file_util.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" @@ -24,7 +23,7 @@ class FileProxyTest : public testing::Test { bytes_written_(-1), weak_factory_(this) {} - virtual void SetUp() override { + void SetUp() override { ASSERT_TRUE(dir_.CreateUniqueTempDir()); ASSERT_TRUE(file_thread_.Start()); } @@ -79,7 +78,7 @@ class FileProxyTest : public testing::Test { } TaskRunner* file_task_runner() const { - return file_thread_.message_loop_proxy().get(); + return file_thread_.task_runner().get(); } const FilePath& test_dir_path() const { return dir_.path(); } const FilePath test_path() const { return dir_.path().AppendASCII("test"); } diff --git a/chromium/base/files/file_tracing.cc b/chromium/base/files/file_tracing.cc new file mode 100644 index 00000000000..a1919c464bb --- /dev/null +++ b/chromium/base/files/file_tracing.cc @@ -0,0 +1,56 @@ +// Copyright 2015 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_tracing.h" + +#include "base/files/file.h" + +namespace base { + +namespace { +FileTracing::Provider* g_provider = nullptr; +} + +// static +void FileTracing::SetProvider(FileTracing::Provider* provider) { + g_provider = provider; +} + +FileTracing::ScopedEnabler::ScopedEnabler() { + if (g_provider) + g_provider->FileTracingEnable(this); +} + +FileTracing::ScopedEnabler::~ScopedEnabler() { + if (g_provider) + g_provider->FileTracingDisable(this); +} + +FileTracing::ScopedTrace::ScopedTrace() : initialized_(false) {} + +FileTracing::ScopedTrace::~ScopedTrace() { + if (initialized_ && g_provider) { + g_provider->FileTracingEventEnd( + name_, &file_->trace_enabler_, file_->path_, size_); + } +} + +bool FileTracing::ScopedTrace::ShouldInitialize() const { + return g_provider && g_provider->FileTracingCategoryIsEnabled(); +} + +void FileTracing::ScopedTrace::Initialize( + const char* name, File* file, int64 size) { + file_ = file; + name_ = name; + size_ = size; + initialized_ = true; + + if (g_provider) { + g_provider->FileTracingEventBegin( + name_, &file_->trace_enabler_, file_->path_, size_); + } +} + +} // namespace base diff --git a/chromium/base/files/file_tracing.h b/chromium/base/files/file_tracing.h new file mode 100644 index 00000000000..8452037790c --- /dev/null +++ b/chromium/base/files/file_tracing.h @@ -0,0 +1,95 @@ +// Copyright 2015 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_TRACING_H_ +#define BASE_FILES_FILE_TRACING_H_ + +#include "base/base_export.h" +#include "base/basictypes.h" +#include "base/macros.h" + +#define FILE_TRACING_PREFIX "File" + +#define SCOPED_FILE_TRACE_WITH_SIZE(name, size) \ + FileTracing::ScopedTrace scoped_file_trace; \ + if (scoped_file_trace.ShouldInitialize()) \ + scoped_file_trace.Initialize(FILE_TRACING_PREFIX "::" name, this, size) + +#define SCOPED_FILE_TRACE(name) SCOPED_FILE_TRACE_WITH_SIZE(name, 0) + +namespace base { + +class File; +class FilePath; + +class BASE_EXPORT FileTracing { + public: + class Provider { + public: + // Whether the file tracing category is currently enabled. + virtual bool FileTracingCategoryIsEnabled() const = 0; + + // Enables file tracing for |id|. Must be called before recording events. + virtual void FileTracingEnable(void* id) = 0; + + // Disables file tracing for |id|. + virtual void FileTracingDisable(void* id) = 0; + + // Begins an event for |id| with |name|. |path| tells where in the directory + // structure the event is happening (and may be blank). |size| is reported + // if not 0. + virtual void FileTracingEventBegin( + const char* name, void* id, const FilePath& path, int64 size) = 0; + + // Ends an event for |id| with |name|. |path| tells where in the directory + // structure the event is happening (and may be blank). |size| is reported + // if not 0. + virtual void FileTracingEventEnd( + const char* name, void* id, const FilePath& path, int64 size) = 0; + }; + + // Sets a global file tracing provider to query categories and record events. + static void SetProvider(Provider* provider); + + // Enables file tracing while in scope. + class ScopedEnabler { + public: + ScopedEnabler(); + ~ScopedEnabler(); + }; + + class ScopedTrace { + public: + ScopedTrace(); + ~ScopedTrace(); + + // Whether this trace should be initialized or not. + bool ShouldInitialize() const; + + // Called only if the tracing category is enabled. + void Initialize(const char* event, File* file, int64 size); + + private: + // True if |Initialize()| has been called. Don't touch |path_|, |event_|, + // or |bytes_| if |initialized_| is false. + bool initialized_; + + // The event name to trace (e.g. "Read", "Write"). Prefixed with "File". + const char* name_; + + // The file being traced. Must outlive this class. + File* file_; + + // The size (in bytes) of this trace. Not reported if 0. + int64 size_; + + DISALLOW_COPY_AND_ASSIGN(ScopedTrace); + }; + + DISALLOW_COPY_AND_ASSIGN(FileTracing); +}; + +} // namespace base + +#endif // BASE_FILES_FILE_TRACING_H_ diff --git a/chromium/base/files/file_unittest.cc b/chromium/base/files/file_unittest.cc index 3bc2db60f0e..5c594242bc8 100644 --- a/chromium/base/files/file_unittest.cc +++ b/chromium/base/files/file_unittest.cc @@ -443,6 +443,49 @@ TEST(FileTest, Seek) { EXPECT_EQ(kOffset, file.Seek(base::File::FROM_END, -kOffset)); } +TEST(FileTest, Duplicate) { + base::ScopedTempDir temp_dir; + ASSERT_TRUE(temp_dir.CreateUniqueTempDir()); + FilePath file_path = temp_dir.path().AppendASCII("file"); + File file(file_path,(base::File::FLAG_CREATE | + base::File::FLAG_READ | + base::File::FLAG_WRITE)); + ASSERT_TRUE(file.IsValid()); + + File file2(file.Duplicate()); + ASSERT_TRUE(file2.IsValid()); + + // Write through one handle, close it, read through the other. + static const char kData[] = "now is a good time."; + static const int kDataLen = sizeof(kData) - 1; + + ASSERT_EQ(0, file.Seek(base::File::FROM_CURRENT, 0)); + ASSERT_EQ(0, file2.Seek(base::File::FROM_CURRENT, 0)); + ASSERT_EQ(kDataLen, file.WriteAtCurrentPos(kData, kDataLen)); + ASSERT_EQ(kDataLen, file.Seek(base::File::FROM_CURRENT, 0)); + ASSERT_EQ(kDataLen, file2.Seek(base::File::FROM_CURRENT, 0)); + file.Close(); + char buf[kDataLen]; + ASSERT_EQ(kDataLen, file2.Read(0, &buf[0], kDataLen)); + ASSERT_EQ(std::string(kData, kDataLen), std::string(&buf[0], kDataLen)); +} + +TEST(FileTest, DuplicateDeleteOnClose) { + base::ScopedTempDir temp_dir; + ASSERT_TRUE(temp_dir.CreateUniqueTempDir()); + FilePath file_path = temp_dir.path().AppendASCII("file"); + File file(file_path,(base::File::FLAG_CREATE | + base::File::FLAG_READ | + base::File::FLAG_WRITE | + base::File::FLAG_DELETE_ON_CLOSE)); + ASSERT_TRUE(file.IsValid()); + File file2(file.Duplicate()); + ASSERT_TRUE(file2.IsValid()); + file.Close(); + file2.Close(); + ASSERT_FALSE(base::PathExists(file_path)); +} + #if defined(OS_WIN) TEST(FileTest, GetInfoForDirectory) { base::ScopedTempDir temp_dir; diff --git a/chromium/base/files/file_util.cc b/chromium/base/files/file_util.cc index e60dcd9d346..32dab6bd40d 100644 --- a/chromium/base/files/file_util.cc +++ b/chromium/base/files/file_util.cc @@ -47,12 +47,6 @@ bool Move(const FilePath& from_path, const FilePath& to_path) { return internal::MoveUnsafe(from_path, to_path); } -bool CopyFile(const FilePath& from_path, const FilePath& to_path) { - if (from_path.ReferencesParent() || to_path.ReferencesParent()) - return false; - return internal::CopyFileUnsafe(from_path, to_path); -} - bool ContentsEqual(const FilePath& filename1, const FilePath& filename2) { // We open the file in binary format even if they are text files because // we are just comparing that bytes are exactly same in both files and not diff --git a/chromium/base/files/file_util.h b/chromium/base/files/file_util.h index ecc0d581dab..7f169f1c54b 100644 --- a/chromium/base/files/file_util.h +++ b/chromium/base/files/file_util.h @@ -421,11 +421,6 @@ namespace internal { BASE_EXPORT bool MoveUnsafe(const FilePath& from_path, const FilePath& to_path); -// Same as CopyFile but allows paths with traversal components. -// Use only with extreme care. -BASE_EXPORT bool CopyFileUnsafe(const FilePath& from_path, - const FilePath& to_path); - #if defined(OS_WIN) // Copy from_path to to_path recursively and then delete from_path recursively. // Returns true if all operations succeed. diff --git a/chromium/base/files/file_util_linux.cc b/chromium/base/files/file_util_linux.cc index 532962f195c..b230fd96484 100644 --- a/chromium/base/files/file_util_linux.cc +++ b/chromium/base/files/file_util_linux.cc @@ -10,21 +10,6 @@ #include "base/files/file_path.h" -// Make sure some of the newer macros from magic.h are defined. -// TODO(mostynb@opera.com): remove this after 2014. -#ifndef BTRFS_SUPER_MAGIC -#define BTRFS_SUPER_MAGIC 0x9123683E -#endif -#ifndef HUGETLBFS_MAGIC -#define HUGETLBFS_MAGIC 0x958458f6 -#endif -#ifndef RAMFS_MAGIC -#define RAMFS_MAGIC 0x858458f6 -#endif -#ifndef TMPFS_MAGIC -#define TMPFS_MAGIC 0x01021994 -#endif - namespace base { bool GetFileSystemType(const FilePath& path, FileSystemType* type) { diff --git a/chromium/base/files/file_util_mac.mm b/chromium/base/files/file_util_mac.mm index 695935a2e31..acac8d738ea 100644 --- a/chromium/base/files/file_util_mac.mm +++ b/chromium/base/files/file_util_mac.mm @@ -14,16 +14,15 @@ #include "base/threading/thread_restrictions.h" namespace base { -namespace internal { -bool CopyFileUnsafe(const FilePath& from_path, const FilePath& to_path) { +bool CopyFile(const FilePath& from_path, const FilePath& to_path) { ThreadRestrictions::AssertIOAllowed(); + if (from_path.ReferencesParent() || to_path.ReferencesParent()) + return false; return (copyfile(from_path.value().c_str(), to_path.value().c_str(), NULL, COPYFILE_DATA) == 0); } -} // namespace internal - bool GetTempDir(base::FilePath* path) { NSString* tmp = NSTemporaryDirectory(); if (tmp == nil) diff --git a/chromium/base/files/file_util_posix.cc b/chromium/base/files/file_util_posix.cc index b8c0eeb94c0..b4a64ba9eee 100644 --- a/chromium/base/files/file_util_posix.cc +++ b/chromium/base/files/file_util_posix.cc @@ -28,8 +28,6 @@ #include <glib.h> // for g_get_home_dir() #endif -#include <fstream> - #include "base/basictypes.h" #include "base/files/file_enumerator.h" #include "base/files/file_path.h" @@ -59,7 +57,6 @@ namespace base { -#if !defined(OS_NACL_NONSFI) namespace { #if defined(OS_BSD) || defined(OS_MACOSX) || defined(OS_NACL) @@ -82,6 +79,7 @@ static int CallLstat(const char *path, stat_wrapper_t *sb) { } #endif // !(defined(OS_BSD) || defined(OS_MACOSX) || defined(OS_NACL)) +#if !defined(OS_NACL_NONSFI) // Helper for NormalizeFilePath(), defined below. bool RealPath(const FilePath& path, FilePath* real_path) { ThreadRestrictions::AssertIOAllowed(); // For realpath(). @@ -184,9 +182,11 @@ bool DetermineDevShmExecutable() { return result; } #endif // defined(OS_LINUX) +#endif // !defined(OS_NACL_NONSFI) } // namespace +#if !defined(OS_NACL_NONSFI) FilePath MakeAbsoluteFilePath(const FilePath& input) { ThreadRestrictions::AssertIOAllowed(); char full_path[PATH_MAX]; @@ -365,6 +365,7 @@ bool PathIsWritable(const FilePath& path) { ThreadRestrictions::AssertIOAllowed(); return access(path.value().c_str(), W_OK) == 0; } +#endif // !defined(OS_NACL_NONSFI) bool DirectoryExists(const FilePath& path) { ThreadRestrictions::AssertIOAllowed(); @@ -373,7 +374,6 @@ bool DirectoryExists(const FilePath& path) { return S_ISDIR(file_info.st_mode); return false; } -#endif // !defined(OS_NACL_NONSFI) bool ReadFromFD(int fd, char* buffer, size_t bytes) { size_t total_read = 0; @@ -428,7 +428,7 @@ bool GetPosixFilePermissions(const FilePath& path, int* mode) { bool SetPosixFilePermissions(const FilePath& path, int mode) { ThreadRestrictions::AssertIOAllowed(); - DCHECK((mode & ~FILE_PERMISSION_MASK) == 0); + DCHECK_EQ(mode & ~FILE_PERMISSION_MASK, 0); // Calls stat() so that we can preserve the higher bits like S_ISGID. stat_wrapper_t stat_buf; @@ -847,55 +847,33 @@ bool GetShmemTempDir(bool executable, FilePath* path) { } #endif // !defined(OS_ANDROID) -// ----------------------------------------------------------------------------- - -namespace internal { - -bool MoveUnsafe(const FilePath& from_path, const FilePath& to_path) { - ThreadRestrictions::AssertIOAllowed(); - // Windows compatibility: if to_path exists, from_path and to_path - // must be the same type, either both files, or both directories. - stat_wrapper_t to_file_info; - if (CallStat(to_path.value().c_str(), &to_file_info) == 0) { - stat_wrapper_t from_file_info; - if (CallStat(from_path.value().c_str(), &from_file_info) == 0) { - if (S_ISDIR(to_file_info.st_mode) != S_ISDIR(from_file_info.st_mode)) - return false; - } else { - return false; - } - } - - if (rename(from_path.value().c_str(), to_path.value().c_str()) == 0) - return true; - - if (!CopyDirectory(from_path, to_path, true)) - return false; - - DeleteFile(from_path, true); - return true; -} - #if !defined(OS_MACOSX) // Mac has its own implementation, this is for all other Posix systems. -bool CopyFileUnsafe(const FilePath& from_path, const FilePath& to_path) { +bool CopyFile(const FilePath& from_path, const FilePath& to_path) { ThreadRestrictions::AssertIOAllowed(); - int infile = HANDLE_EINTR(open(from_path.value().c_str(), O_RDONLY)); - if (infile < 0) + File infile; +#if defined(OS_ANDROID) + if (from_path.IsContentUri()) { + infile = OpenContentUriForRead(from_path); + } else { + infile = File(from_path, File::FLAG_OPEN | File::FLAG_READ); + } +#else + infile = File(from_path, File::FLAG_OPEN | File::FLAG_READ); +#endif + if (!infile.IsValid()) return false; - int outfile = HANDLE_EINTR(creat(to_path.value().c_str(), 0666)); - if (outfile < 0) { - close(infile); + File outfile(to_path, File::FLAG_WRITE | File::FLAG_CREATE_ALWAYS); + if (!outfile.IsValid()) return false; - } const size_t kBufferSize = 32768; std::vector<char> buffer(kBufferSize); bool result = true; while (result) { - ssize_t bytes_read = HANDLE_EINTR(read(infile, &buffer[0], buffer.size())); + ssize_t bytes_read = infile.ReadAtCurrentPos(&buffer[0], buffer.size()); if (bytes_read < 0) { result = false; break; @@ -905,10 +883,8 @@ bool CopyFileUnsafe(const FilePath& from_path, const FilePath& to_path) { // Allow for partial writes ssize_t bytes_written_per_read = 0; do { - ssize_t bytes_written_partial = HANDLE_EINTR(write( - outfile, - &buffer[bytes_written_per_read], - bytes_read - bytes_written_per_read)); + ssize_t bytes_written_partial = outfile.WriteAtCurrentPos( + &buffer[bytes_written_per_read], bytes_read - bytes_written_per_read); if (bytes_written_partial < 0) { result = false; break; @@ -917,15 +893,39 @@ bool CopyFileUnsafe(const FilePath& from_path, const FilePath& to_path) { } while (bytes_written_per_read < bytes_read); } - if (IGNORE_EINTR(close(infile)) < 0) - result = false; - if (IGNORE_EINTR(close(outfile)) < 0) - result = false; - return result; } #endif // !defined(OS_MACOSX) +// ----------------------------------------------------------------------------- + +namespace internal { + +bool MoveUnsafe(const FilePath& from_path, const FilePath& to_path) { + ThreadRestrictions::AssertIOAllowed(); + // Windows compatibility: if to_path exists, from_path and to_path + // must be the same type, either both files, or both directories. + stat_wrapper_t to_file_info; + if (CallStat(to_path.value().c_str(), &to_file_info) == 0) { + stat_wrapper_t from_file_info; + if (CallStat(from_path.value().c_str(), &from_file_info) == 0) { + if (S_ISDIR(to_file_info.st_mode) != S_ISDIR(from_file_info.st_mode)) + return false; + } else { + return false; + } + } + + if (rename(from_path.value().c_str(), to_path.value().c_str()) == 0) + return true; + + if (!CopyDirectory(from_path, to_path, true)) + return false; + + DeleteFile(from_path, true); + return true; +} + } // namespace internal #endif // !defined(OS_NACL_NONSFI) diff --git a/chromium/base/files/file_util_proxy_unittest.cc b/chromium/base/files/file_util_proxy_unittest.cc index 87ae66a6a0d..74083699f00 100644 --- a/chromium/base/files/file_util_proxy_unittest.cc +++ b/chromium/base/files/file_util_proxy_unittest.cc @@ -8,7 +8,6 @@ #include "base/files/file_util.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 "testing/gtest/include/gtest/gtest.h" @@ -19,11 +18,9 @@ class FileUtilProxyTest : public testing::Test { FileUtilProxyTest() : file_thread_("FileUtilProxyTestFileThread"), error_(File::FILE_OK), - created_(false), - bytes_written_(-1), weak_factory_(this) {} - virtual void SetUp() override { + void SetUp() override { ASSERT_TRUE(dir_.CreateUniqueTempDir()); ASSERT_TRUE(file_thread_.Start()); } @@ -42,7 +39,7 @@ class FileUtilProxyTest : public testing::Test { protected: TaskRunner* file_task_runner() const { - return file_thread_.message_loop_proxy().get(); + return file_thread_.task_runner().get(); } const FilePath& test_dir_path() const { return dir_.path(); } const FilePath test_path() const { return dir_.path().AppendASCII("test"); } @@ -52,11 +49,9 @@ class FileUtilProxyTest : public testing::Test { ScopedTempDir dir_; File::Error error_; - bool created_; FilePath path_; File::Info file_info_; std::vector<char> buffer_; - int bytes_written_; WeakPtrFactory<FileUtilProxyTest> weak_factory_; }; diff --git a/chromium/base/files/file_util_unittest.cc b/chromium/base/files/file_util_unittest.cc index 08c9587cc06..b107b0f9c89 100644 --- a/chromium/base/files/file_util_unittest.cc +++ b/chromium/base/files/file_util_unittest.cc @@ -30,6 +30,7 @@ #include "base/files/scoped_file.h" #include "base/files/scoped_temp_dir.h" #include "base/path_service.h" +#include "base/strings/string_util.h" #include "base/strings/utf_string_conversions.h" #include "base/test/test_file_util.h" #include "base/threading/platform_thread.h" @@ -184,7 +185,7 @@ const int FILES_AND_DIRECTORIES = // to be a PlatformTest class FileUtilTest : public PlatformTest { protected: - virtual void SetUp() override { + void SetUp() override { PlatformTest::SetUp(); ASSERT_TRUE(temp_dir_.CreateUniqueTempDir()); } @@ -196,9 +197,9 @@ class FileUtilTest : public PlatformTest { // interface to query whether a given file is present. class FindResultCollector { public: - explicit FindResultCollector(FileEnumerator& enumerator) { + explicit FindResultCollector(FileEnumerator* enumerator) { FilePath cur_file; - while (!(cur_file = enumerator.Next()).value().empty()) { + while (!(cur_file = enumerator->Next()).value().empty()) { FilePath::StringType path = cur_file.value(); // The file should not be returned twice. EXPECT_TRUE(files_.end() == files_.find(path)) @@ -326,6 +327,13 @@ TEST_F(FileUtilTest, NormalizeFilePathReparsePoints) { // |-> to_sub_long (reparse point to temp_dir\sub_a\long_name_\sub_long) FilePath base_a = temp_dir_.path().Append(FPL("base_a")); +#if defined(OS_WIN) + // TEMP can have a lower case drive letter. + string16 temp_base_a = base_a.value(); + ASSERT_FALSE(temp_base_a.empty()); + *temp_base_a.begin() = base::ToUpperASCII(*temp_base_a.begin()); + base_a = FilePath(temp_base_a); +#endif ASSERT_TRUE(CreateDirectory(base_a)); FilePath sub_a = base_a.Append(FPL("sub_a")); @@ -428,6 +436,7 @@ TEST_F(FileUtilTest, NormalizeFilePathReparsePoints) { TEST_F(FileUtilTest, DevicePathToDriveLetter) { // Get a drive letter. std::wstring real_drive_letter = temp_dir_.path().value().substr(0, 2); + StringToUpperASCII(&real_drive_letter); if (!isalpha(real_drive_letter[0]) || ':' != real_drive_letter[1]) { LOG(ERROR) << "Can't get a drive letter to test with."; return; @@ -815,7 +824,7 @@ TEST_F(FileUtilTest, ChangeDirectoryPermissionsAndEnumerate) { // Make sure the file in the directory can't be enumerated. FileEnumerator f1(subdir_path, true, FileEnumerator::FILES); EXPECT_TRUE(PathExists(subdir_path)); - FindResultCollector c1(f1); + FindResultCollector c1(&f1); EXPECT_EQ(0, c1.size()); EXPECT_FALSE(GetPosixFilePermissions(file_name, &mode)); @@ -826,7 +835,7 @@ TEST_F(FileUtilTest, ChangeDirectoryPermissionsAndEnumerate) { // Make sure the file in the directory can be enumerated. FileEnumerator f2(subdir_path, true, FileEnumerator::FILES); - FindResultCollector c2(f2); + FindResultCollector c2(&f2); EXPECT_TRUE(c2.HasFile(file_name)); EXPECT_EQ(1, c2.size()); @@ -1388,19 +1397,15 @@ void SetReadOnly(const FilePath& path, bool read_only) { path.value().c_str(), read_only ? (attrs | FILE_ATTRIBUTE_READONLY) : (attrs & ~FILE_ATTRIBUTE_READONLY))); - // Files in the temporary directory should not be indexed ever. If this - // assumption change, fix this unit test accordingly. - // FILE_ATTRIBUTE_NOT_CONTENT_INDEXED doesn't exist on XP. + DWORD expected = read_only ? ((attrs & (FILE_ATTRIBUTE_ARCHIVE | FILE_ATTRIBUTE_DIRECTORY)) | FILE_ATTRIBUTE_READONLY) : (attrs & (FILE_ATTRIBUTE_ARCHIVE | FILE_ATTRIBUTE_DIRECTORY)); - // TODO(ripp@yandex-team.ru): this seems out of place here. If we really think - // it is important to verify that temp files are not indexed there should be - // a dedicated test for that (create a file, inspect the attributes) - if (win::GetVersion() >= win::VERSION_VISTA) - expected |= FILE_ATTRIBUTE_NOT_CONTENT_INDEXED; - attrs = GetFileAttributes(path.value().c_str()); + + // Ignore FILE_ATTRIBUTE_NOT_CONTENT_INDEXED if present. + attrs = GetFileAttributes(path.value().c_str()) & + ~FILE_ATTRIBUTE_NOT_CONTENT_INDEXED; ASSERT_EQ(expected, attrs); #else // On all other platforms, it involves removing/setting the write bit. @@ -1471,30 +1476,29 @@ TEST_F(FileUtilTest, CopyFile) { FilePath dest_file = dir_name_from.Append(FILE_PATH_LITERAL("DestFile.txt")); ASSERT_TRUE(CopyFile(file_name_from, dest_file)); - // Copy the file to another location using '..' in the path. + // Try to copy the file to another location using '..' in the path. FilePath dest_file2(dir_name_from); dest_file2 = dest_file2.AppendASCII(".."); dest_file2 = dest_file2.AppendASCII("DestFile.txt"); ASSERT_FALSE(CopyFile(file_name_from, dest_file2)); - ASSERT_TRUE(internal::CopyFileUnsafe(file_name_from, dest_file2)); FilePath dest_file2_test(dir_name_from); dest_file2_test = dest_file2_test.DirName(); dest_file2_test = dest_file2_test.AppendASCII("DestFile.txt"); - // Check everything has been copied. + // Check expected copy results. EXPECT_TRUE(PathExists(file_name_from)); EXPECT_TRUE(PathExists(dest_file)); const std::wstring read_contents = ReadTextFile(dest_file); EXPECT_EQ(file_contents, read_contents); - EXPECT_TRUE(PathExists(dest_file2_test)); - EXPECT_TRUE(PathExists(dest_file2)); + EXPECT_FALSE(PathExists(dest_file2_test)); + EXPECT_FALSE(PathExists(dest_file2)); } TEST_F(FileUtilTest, CopyFileACL) { // While FileUtilTest.CopyFile asserts the content is correctly copied over, // this test case asserts the access control bits are meeting expectations in - // CopyFileUnsafe(). + // CopyFile(). FilePath src = temp_dir_.path().Append(FILE_PATH_LITERAL("src.txt")); const std::wstring file_contents(L"Gooooooooooooooooooooogle"); CreateTextFile(src, file_contents); @@ -1867,7 +1871,7 @@ TEST_F(FileUtilTest, FileEnumeratorTest) { // Only enumerate files. FileEnumerator f1(temp_dir_.path(), true, FileEnumerator::FILES); - FindResultCollector c1(f1); + FindResultCollector c1(&f1); EXPECT_TRUE(c1.HasFile(file1)); EXPECT_TRUE(c1.HasFile(file2_abs)); EXPECT_TRUE(c1.HasFile(dir2file)); @@ -1876,7 +1880,7 @@ TEST_F(FileUtilTest, FileEnumeratorTest) { // Only enumerate directories. FileEnumerator f2(temp_dir_.path(), true, FileEnumerator::DIRECTORIES); - FindResultCollector c2(f2); + FindResultCollector c2(&f2); EXPECT_TRUE(c2.HasFile(dir1)); EXPECT_TRUE(c2.HasFile(dir2)); EXPECT_TRUE(c2.HasFile(dir2inner)); @@ -1885,7 +1889,7 @@ TEST_F(FileUtilTest, FileEnumeratorTest) { // Only enumerate directories non-recursively. FileEnumerator f2_non_recursive( temp_dir_.path(), false, FileEnumerator::DIRECTORIES); - FindResultCollector c2_non_recursive(f2_non_recursive); + FindResultCollector c2_non_recursive(&f2_non_recursive); EXPECT_TRUE(c2_non_recursive.HasFile(dir1)); EXPECT_TRUE(c2_non_recursive.HasFile(dir2)); EXPECT_EQ(2, c2_non_recursive.size()); @@ -1894,7 +1898,7 @@ TEST_F(FileUtilTest, FileEnumeratorTest) { FileEnumerator f2_dotdot(temp_dir_.path(), false, FileEnumerator::DIRECTORIES | FileEnumerator::INCLUDE_DOT_DOT); - FindResultCollector c2_dotdot(f2_dotdot); + FindResultCollector c2_dotdot(&f2_dotdot); EXPECT_TRUE(c2_dotdot.HasFile(dir1)); EXPECT_TRUE(c2_dotdot.HasFile(dir2)); EXPECT_TRUE(c2_dotdot.HasFile(temp_dir_.path().Append(FPL("..")))); @@ -1902,7 +1906,7 @@ TEST_F(FileUtilTest, FileEnumeratorTest) { // Enumerate files and directories. FileEnumerator f3(temp_dir_.path(), true, FILES_AND_DIRECTORIES); - FindResultCollector c3(f3); + FindResultCollector c3(&f3); EXPECT_TRUE(c3.HasFile(dir1)); EXPECT_TRUE(c3.HasFile(dir2)); EXPECT_TRUE(c3.HasFile(file1)); @@ -1914,7 +1918,7 @@ TEST_F(FileUtilTest, FileEnumeratorTest) { // Non-recursive operation. FileEnumerator f4(temp_dir_.path(), false, FILES_AND_DIRECTORIES); - FindResultCollector c4(f4); + FindResultCollector c4(&f4); EXPECT_TRUE(c4.HasFile(dir2)); EXPECT_TRUE(c4.HasFile(dir2)); EXPECT_TRUE(c4.HasFile(file1)); @@ -1923,7 +1927,7 @@ TEST_F(FileUtilTest, FileEnumeratorTest) { // Enumerate with a pattern. FileEnumerator f5(temp_dir_.path(), true, FILES_AND_DIRECTORIES, FPL("dir*")); - FindResultCollector c5(f5); + FindResultCollector c5(&f5); EXPECT_TRUE(c5.HasFile(dir1)); EXPECT_TRUE(c5.HasFile(dir2)); EXPECT_TRUE(c5.HasFile(dir2file)); @@ -1942,7 +1946,7 @@ TEST_F(FileUtilTest, FileEnumeratorTest) { // the file system so skip this test for XP. // Enumerate the reparse point. FileEnumerator f6(dir1, true, FILES_AND_DIRECTORIES); - FindResultCollector c6(f6); + FindResultCollector c6(&f6); FilePath inner2 = dir1.Append(FPL("inner")); EXPECT_TRUE(c6.HasFile(inner2)); EXPECT_TRUE(c6.HasFile(inner2.Append(FPL("innerfile.txt")))); @@ -1952,7 +1956,7 @@ TEST_F(FileUtilTest, FileEnumeratorTest) { // No changes for non recursive operation. FileEnumerator f7(temp_dir_.path(), false, FILES_AND_DIRECTORIES); - FindResultCollector c7(f7); + FindResultCollector c7(&f7); EXPECT_TRUE(c7.HasFile(dir2)); EXPECT_TRUE(c7.HasFile(dir2)); EXPECT_TRUE(c7.HasFile(file1)); @@ -1961,7 +1965,7 @@ TEST_F(FileUtilTest, FileEnumeratorTest) { // Should not enumerate inside dir1 when using recursion. FileEnumerator f8(temp_dir_.path(), true, FILES_AND_DIRECTORIES); - FindResultCollector c8(f8); + FindResultCollector c8(&f8); EXPECT_TRUE(c8.HasFile(dir1)); EXPECT_TRUE(c8.HasFile(dir2)); EXPECT_TRUE(c8.HasFile(file1)); @@ -2171,7 +2175,7 @@ TEST_F(FileUtilTest, IsDirectoryEmpty) { // with a common SetUp() method. class VerifyPathControlledByUserTest : public FileUtilTest { protected: - virtual void SetUp() override { + void SetUp() override { FileUtilTest::SetUp(); // Create a basic structure used by each test. diff --git a/chromium/base/files/file_util_win.cc b/chromium/base/files/file_util_win.cc index 733c32c27da..e254232f663 100644 --- a/chromium/base/files/file_util_win.cc +++ b/chromium/base/files/file_util_win.cc @@ -210,7 +210,7 @@ bool CopyDirectory(const FilePath& from_path, const FilePath& to_path, << target_path.value().c_str(); success = false; } - } else if (!internal::CopyFileUnsafe(current, target_path)) { + } else if (!CopyFile(current, target_path)) { DLOG(ERROR) << "CopyDirectory() couldn't create file: " << target_path.value().c_str(); success = false; @@ -480,7 +480,7 @@ bool DevicePathToDriveLetterPath(const FilePath& nt_device_path, } // Move to the next drive letter string, which starts one // increment after the '\0' that terminates the current string. - while (*drive_map_ptr++); + while (*drive_map_ptr++) {} } // No drive matched. The path does not start with a device junction @@ -496,7 +496,7 @@ bool NormalizeToNativeFilePath(const FilePath& path, FilePath* nt_path) { // code below to a call to GetFinalPathNameByHandle(). The method this // function uses is explained in the following msdn article: // http://msdn.microsoft.com/en-us/library/aa366789(VS.85).aspx - base::win::ScopedHandle file_handle( + win::ScopedHandle file_handle( ::CreateFile(path.value().c_str(), GENERIC_READ, kFileShareAll, @@ -510,7 +510,7 @@ bool NormalizeToNativeFilePath(const FilePath& path, FilePath* nt_path) { // Create a file mapping object. Can't easily use MemoryMappedFile, because // we only map the first byte, and need direct access to the handle. You can // not map an empty file, this call fails in that case. - base::win::ScopedHandle file_map_handle( + win::ScopedHandle file_map_handle( ::CreateFileMapping(file_handle.Get(), NULL, PAGE_READONLY, @@ -575,7 +575,7 @@ bool GetFileInfo(const FilePath& file_path, File::Info* results) { FILE* OpenFile(const FilePath& filename, const char* mode) { ThreadRestrictions::AssertIOAllowed(); - std::wstring w_mode = ASCIIToWide(std::string(mode)); + string16 w_mode = ASCIIToUTF16(mode); return _wfsopen(filename.value().c_str(), w_mode.c_str(), _SH_DENYNO); } @@ -595,13 +595,13 @@ FILE* FileToFILE(File file, const char* mode) { int ReadFile(const FilePath& filename, char* data, int max_size) { ThreadRestrictions::AssertIOAllowed(); - base::win::ScopedHandle file(CreateFile(filename.value().c_str(), - GENERIC_READ, - FILE_SHARE_READ | FILE_SHARE_WRITE, - NULL, - OPEN_EXISTING, - FILE_FLAG_SEQUENTIAL_SCAN, - NULL)); + win::ScopedHandle file(CreateFile(filename.value().c_str(), + GENERIC_READ, + FILE_SHARE_READ | FILE_SHARE_WRITE, + NULL, + OPEN_EXISTING, + FILE_FLAG_SEQUENTIAL_SCAN, + NULL)); if (!file.IsValid()) return -1; @@ -614,13 +614,13 @@ int ReadFile(const FilePath& filename, char* data, int max_size) { int WriteFile(const FilePath& filename, const char* data, int size) { ThreadRestrictions::AssertIOAllowed(); - base::win::ScopedHandle file(CreateFile(filename.value().c_str(), - GENERIC_WRITE, - 0, - NULL, - CREATE_ALWAYS, - 0, - NULL)); + win::ScopedHandle file(CreateFile(filename.value().c_str(), + GENERIC_WRITE, + 0, + NULL, + CREATE_ALWAYS, + 0, + NULL)); if (!file.IsValid()) { DPLOG(WARNING) << "CreateFile failed for path " << UTF16ToUTF8(filename.value()); @@ -646,13 +646,13 @@ int WriteFile(const FilePath& filename, const char* data, int size) { bool AppendToFile(const FilePath& filename, const char* data, int size) { ThreadRestrictions::AssertIOAllowed(); - base::win::ScopedHandle file(CreateFile(filename.value().c_str(), - FILE_APPEND_DATA, - 0, - NULL, - OPEN_EXISTING, - 0, - NULL)); + win::ScopedHandle file(CreateFile(filename.value().c_str(), + FILE_APPEND_DATA, + 0, + NULL, + OPEN_EXISTING, + 0, + NULL)); if (!file.IsValid()) { VPLOG(1) << "CreateFile failed for path " << UTF16ToUTF8(filename.value()); return false; @@ -722,6 +722,37 @@ int GetMaximumPathComponentLength(const FilePath& path) { return std::min(whole_path_limit, static_cast<int>(max_length)); } +bool CopyFile(const FilePath& from_path, const FilePath& to_path) { + ThreadRestrictions::AssertIOAllowed(); + if (from_path.ReferencesParent() || to_path.ReferencesParent()) + return false; + + // NOTE: I suspect we could support longer paths, but that would involve + // analyzing all our usage of files. + if (from_path.value().length() >= MAX_PATH || + to_path.value().length() >= MAX_PATH) { + return false; + } + + // Unlike the posix implementation that copies the file manually and discards + // the ACL bits, CopyFile() copies the complete SECURITY_DESCRIPTOR and access + // bits, which is usually not what we want. We can't do much about the + // SECURITY_DESCRIPTOR but at least remove the read only bit. + const wchar_t* dest = to_path.value().c_str(); + if (!::CopyFile(from_path.value().c_str(), dest, false)) { + // Copy failed. + return false; + } + DWORD attrs = GetFileAttributes(dest); + if (attrs == INVALID_FILE_ATTRIBUTES) { + return false; + } + if (attrs & FILE_ATTRIBUTE_READONLY) { + SetFileAttributes(dest, attrs & ~FILE_ATTRIBUTE_READONLY); + } + return true; +} + // ----------------------------------------------------------------------------- namespace internal { @@ -760,35 +791,6 @@ bool MoveUnsafe(const FilePath& from_path, const FilePath& to_path) { return ret; } -bool CopyFileUnsafe(const FilePath& from_path, const FilePath& to_path) { - ThreadRestrictions::AssertIOAllowed(); - - // NOTE: I suspect we could support longer paths, but that would involve - // analyzing all our usage of files. - if (from_path.value().length() >= MAX_PATH || - to_path.value().length() >= MAX_PATH) { - return false; - } - - // Unlike the posix implementation that copies the file manually and discards - // the ACL bits, CopyFile() copies the complete SECURITY_DESCRIPTOR and access - // bits, which is usually not what we want. We can't do much about the - // SECURITY_DESCRIPTOR but at least remove the read only bit. - const wchar_t* dest = to_path.value().c_str(); - if (!::CopyFile(from_path.value().c_str(), dest, false)) { - // Copy failed. - return false; - } - DWORD attrs = GetFileAttributes(dest); - if (attrs == INVALID_FILE_ATTRIBUTES) { - return false; - } - if (attrs & FILE_ATTRIBUTE_READONLY) { - SetFileAttributes(dest, attrs & ~FILE_ATTRIBUTE_READONLY); - } - return true; -} - bool CopyAndDeleteDirectory(const FilePath& from_path, const FilePath& to_path) { ThreadRestrictions::AssertIOAllowed(); diff --git a/chromium/base/files/file_win.cc b/chromium/base/files/file_win.cc index 727b5ce1dbb..97928522f10 100644 --- a/chromium/base/files/file_win.cc +++ b/chromium/base/files/file_win.cc @@ -6,7 +6,6 @@ #include <io.h> -#include "base/files/file_path.h" #include "base/logging.h" #include "base/metrics/sparse_histogram.h" #include "base/threading/thread_restrictions.h" @@ -18,90 +17,6 @@ COMPILE_ASSERT(File::FROM_BEGIN == FILE_BEGIN && File::FROM_CURRENT == FILE_CURRENT && File::FROM_END == FILE_END, whence_matches_system); -void File::InitializeUnsafe(const FilePath& name, uint32 flags) { - base::ThreadRestrictions::AssertIOAllowed(); - DCHECK(!IsValid()); - - DWORD disposition = 0; - - if (flags & FLAG_OPEN) - disposition = OPEN_EXISTING; - - if (flags & FLAG_CREATE) { - DCHECK(!disposition); - disposition = CREATE_NEW; - } - - if (flags & FLAG_OPEN_ALWAYS) { - DCHECK(!disposition); - disposition = OPEN_ALWAYS; - } - - if (flags & FLAG_CREATE_ALWAYS) { - DCHECK(!disposition); - DCHECK(flags & FLAG_WRITE); - disposition = CREATE_ALWAYS; - } - - if (flags & FLAG_OPEN_TRUNCATED) { - DCHECK(!disposition); - DCHECK(flags & FLAG_WRITE); - disposition = TRUNCATE_EXISTING; - } - - if (!disposition) { - NOTREACHED(); - return; - } - - DWORD access = 0; - if (flags & FLAG_WRITE) - access = GENERIC_WRITE; - if (flags & FLAG_APPEND) { - DCHECK(!access); - access = FILE_APPEND_DATA; - } - if (flags & FLAG_READ) - access |= GENERIC_READ; - if (flags & FLAG_WRITE_ATTRIBUTES) - access |= FILE_WRITE_ATTRIBUTES; - if (flags & FLAG_EXECUTE) - access |= GENERIC_EXECUTE; - - DWORD sharing = (flags & FLAG_EXCLUSIVE_READ) ? 0 : FILE_SHARE_READ; - if (!(flags & FLAG_EXCLUSIVE_WRITE)) - sharing |= FILE_SHARE_WRITE; - if (flags & FLAG_SHARE_DELETE) - sharing |= FILE_SHARE_DELETE; - - DWORD create_flags = 0; - if (flags & FLAG_ASYNC) - create_flags |= FILE_FLAG_OVERLAPPED; - if (flags & FLAG_TEMPORARY) - create_flags |= FILE_ATTRIBUTE_TEMPORARY; - if (flags & FLAG_HIDDEN) - create_flags |= FILE_ATTRIBUTE_HIDDEN; - if (flags & FLAG_DELETE_ON_CLOSE) - create_flags |= FILE_FLAG_DELETE_ON_CLOSE; - if (flags & FLAG_BACKUP_SEMANTICS) - create_flags |= FILE_FLAG_BACKUP_SEMANTICS; - - file_.Set(CreateFile(name.value().c_str(), access, sharing, NULL, - disposition, create_flags, NULL)); - - if (file_.IsValid()) { - error_details_ = FILE_OK; - async_ = ((flags & FLAG_ASYNC) == FLAG_ASYNC); - - if (flags & (FLAG_OPEN_ALWAYS)) - created_ = (ERROR_ALREADY_EXISTS != GetLastError()); - else if (flags & (FLAG_CREATE_ALWAYS | FLAG_CREATE)) - created_ = true; - } else { - error_details_ = OSErrorToFileError(GetLastError()); - } -} - bool File::IsValid() const { return file_.IsValid(); } @@ -115,16 +30,20 @@ PlatformFile File::TakePlatformFile() { } void File::Close() { - if (file_.IsValid()) { - base::ThreadRestrictions::AssertIOAllowed(); - file_.Close(); - } + if (!file_.IsValid()) + return; + + ThreadRestrictions::AssertIOAllowed(); + SCOPED_FILE_TRACE("Close"); + file_.Close(); } int64 File::Seek(Whence whence, int64 offset) { - base::ThreadRestrictions::AssertIOAllowed(); + ThreadRestrictions::AssertIOAllowed(); DCHECK(IsValid()); + SCOPED_FILE_TRACE_WITH_SIZE("Seek", offset); + LARGE_INTEGER distance, res; distance.QuadPart = offset; DWORD move_method = static_cast<DWORD>(whence); @@ -134,12 +53,14 @@ int64 File::Seek(Whence whence, int64 offset) { } int File::Read(int64 offset, char* data, int size) { - base::ThreadRestrictions::AssertIOAllowed(); + ThreadRestrictions::AssertIOAllowed(); DCHECK(IsValid()); DCHECK(!async_); if (size < 0) return -1; + SCOPED_FILE_TRACE_WITH_SIZE("Read", size); + LARGE_INTEGER offset_li; offset_li.QuadPart = offset; @@ -157,12 +78,14 @@ int File::Read(int64 offset, char* data, int size) { } int File::ReadAtCurrentPos(char* data, int size) { - base::ThreadRestrictions::AssertIOAllowed(); + ThreadRestrictions::AssertIOAllowed(); DCHECK(IsValid()); DCHECK(!async_); if (size < 0) return -1; + SCOPED_FILE_TRACE_WITH_SIZE("ReadAtCurrentPos", size); + DWORD bytes_read; if (::ReadFile(file_.Get(), data, size, &bytes_read, NULL)) return bytes_read; @@ -173,18 +96,22 @@ int File::ReadAtCurrentPos(char* data, int size) { } int File::ReadNoBestEffort(int64 offset, char* data, int size) { + // TODO(dbeam): trace this separately? return Read(offset, data, size); } int File::ReadAtCurrentPosNoBestEffort(char* data, int size) { + // TODO(dbeam): trace this separately? return ReadAtCurrentPos(data, size); } int File::Write(int64 offset, const char* data, int size) { - base::ThreadRestrictions::AssertIOAllowed(); + ThreadRestrictions::AssertIOAllowed(); DCHECK(IsValid()); DCHECK(!async_); + SCOPED_FILE_TRACE_WITH_SIZE("Write", size); + LARGE_INTEGER offset_li; offset_li.QuadPart = offset; @@ -200,12 +127,14 @@ int File::Write(int64 offset, const char* data, int size) { } int File::WriteAtCurrentPos(const char* data, int size) { - base::ThreadRestrictions::AssertIOAllowed(); + ThreadRestrictions::AssertIOAllowed(); DCHECK(IsValid()); DCHECK(!async_); if (size < 0) return -1; + SCOPED_FILE_TRACE_WITH_SIZE("WriteAtCurrentPos", size); + DWORD bytes_written; if (::WriteFile(file_.Get(), data, size, &bytes_written, NULL)) return bytes_written; @@ -218,8 +147,11 @@ int File::WriteAtCurrentPosNoBestEffort(const char* data, int size) { } int64 File::GetLength() { - base::ThreadRestrictions::AssertIOAllowed(); + ThreadRestrictions::AssertIOAllowed(); DCHECK(IsValid()); + + SCOPED_FILE_TRACE("GetLength"); + LARGE_INTEGER size; if (!::GetFileSizeEx(file_.Get(), &size)) return -1; @@ -228,9 +160,11 @@ int64 File::GetLength() { } bool File::SetLength(int64 length) { - base::ThreadRestrictions::AssertIOAllowed(); + ThreadRestrictions::AssertIOAllowed(); DCHECK(IsValid()); + SCOPED_FILE_TRACE_WITH_SIZE("SetLength", length); + // Get the current file pointer. LARGE_INTEGER file_pointer; LARGE_INTEGER zero; @@ -256,16 +190,12 @@ bool File::SetLength(int64 length) { FALSE)); } -bool File::Flush() { - base::ThreadRestrictions::AssertIOAllowed(); - DCHECK(IsValid()); - return ::FlushFileBuffers(file_.Get()) != FALSE; -} - bool File::SetTimes(Time last_access_time, Time last_modified_time) { - base::ThreadRestrictions::AssertIOAllowed(); + ThreadRestrictions::AssertIOAllowed(); DCHECK(IsValid()); + SCOPED_FILE_TRACE("SetTimes"); + FILETIME last_access_filetime = last_access_time.ToFileTime(); FILETIME last_modified_filetime = last_modified_time.ToFileTime(); return (::SetFileTime(file_.Get(), NULL, &last_access_filetime, @@ -273,9 +203,11 @@ bool File::SetTimes(Time last_access_time, Time last_modified_time) { } bool File::GetInfo(Info* info) { - base::ThreadRestrictions::AssertIOAllowed(); + ThreadRestrictions::AssertIOAllowed(); DCHECK(IsValid()); + SCOPED_FILE_TRACE("GetInfo"); + BY_HANDLE_FILE_INFORMATION file_info; if (!GetFileInformationByHandle(file_.Get(), &file_info)) return false; @@ -287,14 +219,17 @@ bool File::GetInfo(Info* info) { info->is_directory = (file_info.dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY) != 0; info->is_symbolic_link = false; // Windows doesn't have symbolic links. - info->last_modified = base::Time::FromFileTime(file_info.ftLastWriteTime); - info->last_accessed = base::Time::FromFileTime(file_info.ftLastAccessTime); - info->creation_time = base::Time::FromFileTime(file_info.ftCreationTime); + info->last_modified = Time::FromFileTime(file_info.ftLastWriteTime); + info->last_accessed = Time::FromFileTime(file_info.ftLastAccessTime); + info->creation_time = Time::FromFileTime(file_info.ftCreationTime); return true; } -File::Error base::File::Lock() { +File::Error File::Lock() { DCHECK(IsValid()); + + SCOPED_FILE_TRACE("Lock"); + BOOL result = LockFile(file_.Get(), 0, 0, MAXDWORD, MAXDWORD); if (!result) return OSErrorToFileError(GetLastError()); @@ -303,12 +238,39 @@ File::Error base::File::Lock() { File::Error File::Unlock() { DCHECK(IsValid()); + + SCOPED_FILE_TRACE("Unlock"); + BOOL result = UnlockFile(file_.Get(), 0, 0, MAXDWORD, MAXDWORD); if (!result) return OSErrorToFileError(GetLastError()); return FILE_OK; } +File File::Duplicate() { + if (!IsValid()) + return File(); + + SCOPED_FILE_TRACE("Duplicate"); + + HANDLE other_handle = nullptr; + + if (!::DuplicateHandle(GetCurrentProcess(), // hSourceProcessHandle + GetPlatformFile(), + GetCurrentProcess(), // hTargetProcessHandle + &other_handle, + 0, // dwDesiredAccess ignored due to SAME_ACCESS + FALSE, // !bInheritHandle + DUPLICATE_SAME_ACCESS)) { + return File(OSErrorToFileError(GetLastError())); + } + + File other(other_handle); + if (async()) + other.async_ = true; + return other.Pass(); +} + // Static. File::Error File::OSErrorToFileError(DWORD last_error) { switch (last_error) { @@ -346,6 +308,96 @@ File::Error File::OSErrorToFileError(DWORD last_error) { } } +void File::DoInitialize(uint32 flags) { + ThreadRestrictions::AssertIOAllowed(); + DCHECK(!IsValid()); + + DWORD disposition = 0; + + if (flags & FLAG_OPEN) + disposition = OPEN_EXISTING; + + if (flags & FLAG_CREATE) { + DCHECK(!disposition); + disposition = CREATE_NEW; + } + + if (flags & FLAG_OPEN_ALWAYS) { + DCHECK(!disposition); + disposition = OPEN_ALWAYS; + } + + if (flags & FLAG_CREATE_ALWAYS) { + DCHECK(!disposition); + DCHECK(flags & FLAG_WRITE); + disposition = CREATE_ALWAYS; + } + + if (flags & FLAG_OPEN_TRUNCATED) { + DCHECK(!disposition); + DCHECK(flags & FLAG_WRITE); + disposition = TRUNCATE_EXISTING; + } + + if (!disposition) { + NOTREACHED(); + return; + } + + DWORD access = 0; + if (flags & FLAG_WRITE) + access = GENERIC_WRITE; + if (flags & FLAG_APPEND) { + DCHECK(!access); + access = FILE_APPEND_DATA; + } + if (flags & FLAG_READ) + access |= GENERIC_READ; + if (flags & FLAG_WRITE_ATTRIBUTES) + access |= FILE_WRITE_ATTRIBUTES; + if (flags & FLAG_EXECUTE) + access |= GENERIC_EXECUTE; + + DWORD sharing = (flags & FLAG_EXCLUSIVE_READ) ? 0 : FILE_SHARE_READ; + if (!(flags & FLAG_EXCLUSIVE_WRITE)) + sharing |= FILE_SHARE_WRITE; + if (flags & FLAG_SHARE_DELETE) + sharing |= FILE_SHARE_DELETE; + + DWORD create_flags = 0; + if (flags & FLAG_ASYNC) + create_flags |= FILE_FLAG_OVERLAPPED; + if (flags & FLAG_TEMPORARY) + create_flags |= FILE_ATTRIBUTE_TEMPORARY; + if (flags & FLAG_HIDDEN) + create_flags |= FILE_ATTRIBUTE_HIDDEN; + if (flags & FLAG_DELETE_ON_CLOSE) + create_flags |= FILE_FLAG_DELETE_ON_CLOSE; + if (flags & FLAG_BACKUP_SEMANTICS) + create_flags |= FILE_FLAG_BACKUP_SEMANTICS; + + file_.Set(CreateFile(path_.value().c_str(), access, sharing, NULL, + disposition, create_flags, NULL)); + + if (file_.IsValid()) { + error_details_ = FILE_OK; + async_ = ((flags & FLAG_ASYNC) == FLAG_ASYNC); + + if (flags & (FLAG_OPEN_ALWAYS)) + created_ = (ERROR_ALREADY_EXISTS != GetLastError()); + else if (flags & (FLAG_CREATE_ALWAYS | FLAG_CREATE)) + created_ = true; + } else { + error_details_ = OSErrorToFileError(GetLastError()); + } +} + +bool File::DoFlush() { + ThreadRestrictions::AssertIOAllowed(); + DCHECK(IsValid()); + return ::FlushFileBuffers(file_.Get()) != FALSE; +} + void File::SetPlatformFile(PlatformFile file) { file_.Set(file); } diff --git a/chromium/base/files/important_file_writer.cc b/chromium/base/files/important_file_writer.cc index d7579abb112..814fc7b9add 100644 --- a/chromium/base/files/important_file_writer.cc +++ b/chromium/base/files/important_file_writer.cc @@ -10,12 +10,14 @@ #include "base/bind.h" #include "base/critical_closure.h" +#include "base/debug/alias.h" #include "base/files/file.h" #include "base/files/file_path.h" #include "base/files/file_util.h" #include "base/logging.h" #include "base/metrics/histogram.h" #include "base/strings/string_number_conversions.h" +#include "base/strings/string_util.h" #include "base/task_runner.h" #include "base/task_runner_util.h" #include "base/threading/thread.h" @@ -27,10 +29,14 @@ namespace { const int kDefaultCommitIntervalMs = 10000; +// This enum is used to define the buckets for an enumerated UMA histogram. +// Hence, +// (a) existing enumerated constants should never be deleted or reordered, and +// (b) new constants should only be appended at the end of the enumeration. enum TempFileFailure { FAILED_CREATING, FAILED_OPENING, - FAILED_CLOSING, + FAILED_CLOSING, // Unused. FAILED_WRITING, FAILED_RENAMING, FAILED_FLUSHING, @@ -45,11 +51,31 @@ void LogFailure(const FilePath& path, TempFileFailure failure_code, << " : " << message; } +// Helper function to call WriteFileAtomically() with a scoped_ptr<std::string>. +bool WriteScopedStringToFileAtomically(const FilePath& path, + scoped_ptr<std::string> data) { + return ImportantFileWriter::WriteFileAtomically(path, *data); +} + } // namespace // static bool ImportantFileWriter::WriteFileAtomically(const FilePath& path, const std::string& data) { +#if defined(OS_CHROMEOS) + // On Chrome OS, chrome gets killed when it cannot finish shutdown quickly, + // and this function seems to be one of the slowest shutdown steps. + // Include some info to the report for investigation. crbug.com/418627 + // TODO(hashimoto): Remove this. + struct { + size_t data_size; + char path[128]; + } file_info; + file_info.data_size = data.size(); + base::strlcpy(file_info.path, path.value().c_str(), + arraysize(file_info.path)); + base::debug::Alias(&file_info); +#endif // Write the data to a temp file then rename to avoid data loss if we crash // while writing the file. Ensure that the temp file is on the same volume // as target file, so it can be moved in one step, and that the temp file @@ -104,7 +130,7 @@ ImportantFileWriter::ImportantFileWriter( commit_interval_(TimeDelta::FromMilliseconds(kDefaultCommitIntervalMs)), weak_factory_(this) { DCHECK(CalledOnValidThread()); - DCHECK(task_runner_.get()); + DCHECK(task_runner_); } ImportantFileWriter::~ImportantFileWriter() { @@ -119,9 +145,9 @@ bool ImportantFileWriter::HasPendingWrite() const { return timer_.IsRunning(); } -void ImportantFileWriter::WriteNow(const std::string& data) { +void ImportantFileWriter::WriteNow(scoped_ptr<std::string> data) { DCHECK(CalledOnValidThread()); - if (data.length() > static_cast<size_t>(kint32max)) { + if (data->length() > static_cast<size_t>(kint32max)) { NOTREACHED(); return; } @@ -129,13 +155,14 @@ void ImportantFileWriter::WriteNow(const std::string& data) { if (HasPendingWrite()) timer_.Stop(); - if (!PostWriteTask(data)) { + auto task = Bind(&WriteScopedStringToFileAtomically, path_, Passed(&data)); + if (!PostWriteTask(task)) { // 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. NOTREACHED(); - WriteFileAtomically(path_, data); + task.Run(); } } @@ -153,9 +180,9 @@ void ImportantFileWriter::ScheduleWrite(DataSerializer* serializer) { void ImportantFileWriter::DoScheduledWrite() { DCHECK(serializer_); - std::string data; - if (serializer_->SerializeData(&data)) { - WriteNow(data); + scoped_ptr<std::string> data(new std::string); + if (serializer_->SerializeData(data.get())) { + WriteNow(data.Pass()); } else { DLOG(WARNING) << "failed to serialize data to be saved in " << path_.value().c_str(); @@ -169,7 +196,7 @@ void ImportantFileWriter::RegisterOnNextSuccessfulWriteCallback( on_next_successful_write_ = on_next_successful_write; } -bool ImportantFileWriter::PostWriteTask(const std::string& data) { +bool ImportantFileWriter::PostWriteTask(const Callback<bool()>& task) { // 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 @@ -179,16 +206,13 @@ bool ImportantFileWriter::PostWriteTask(const std::string& data) { return base::PostTaskAndReplyWithResult( task_runner_.get(), FROM_HERE, - MakeCriticalClosure( - Bind(&ImportantFileWriter::WriteFileAtomically, path_, data)), + MakeCriticalClosure(task), Bind(&ImportantFileWriter::ForwardSuccessfulWrite, weak_factory_.GetWeakPtr())); } return task_runner_->PostTask( FROM_HERE, - MakeCriticalClosure( - Bind(IgnoreResult(&ImportantFileWriter::WriteFileAtomically), - path_, data))); + MakeCriticalClosure(base::Bind(IgnoreResult(task)))); } void ImportantFileWriter::ForwardSuccessfulWrite(bool result) { diff --git a/chromium/base/files/important_file_writer.h b/chromium/base/files/important_file_writer.h index 9ead9c14bf2..99f1a7c6814 100644 --- a/chromium/base/files/important_file_writer.h +++ b/chromium/base/files/important_file_writer.h @@ -78,7 +78,7 @@ class BASE_EXPORT ImportantFileWriter : public NonThreadSafe { // Save |data| to target filename. Does not block. If there is a pending write // scheduled by ScheduleWrite, it is cancelled. - void WriteNow(const std::string& data); + void WriteNow(scoped_ptr<std::string> data); // Schedule a save to target filename. Data will be serialized and saved // to disk after the commit interval. If another ScheduleWrite is issued @@ -106,7 +106,7 @@ class BASE_EXPORT ImportantFileWriter : public NonThreadSafe { private: // Helper method for WriteNow(). - bool PostWriteTask(const std::string& data); + bool PostWriteTask(const Callback<bool()>& task); // If |result| is true and |on_next_successful_write_| is set, invokes // |on_successful_write_| and then resets it; no-ops otherwise. diff --git a/chromium/base/files/important_file_writer_unittest.cc b/chromium/base/files/important_file_writer_unittest.cc index 96d0d059bb8..d376cdc35ab 100644 --- a/chromium/base/files/important_file_writer_unittest.cc +++ b/chromium/base/files/important_file_writer_unittest.cc @@ -9,9 +9,11 @@ #include "base/files/file_path.h" #include "base/files/file_util.h" #include "base/files/scoped_temp_dir.h" +#include "base/location.h" #include "base/logging.h" -#include "base/message_loop/message_loop.h" #include "base/run_loop.h" +#include "base/single_thread_task_runner.h" +#include "base/thread_task_runner_handle.h" #include "base/threading/thread.h" #include "base/time/time.h" #include "testing/gtest/include/gtest/gtest.h" @@ -82,7 +84,7 @@ bool SuccessfulWriteObserver::GetAndResetObservationState() { class ImportantFileWriterTest : public testing::Test { public: ImportantFileWriterTest() { } - virtual void SetUp() { + void SetUp() override { ASSERT_TRUE(temp_dir_.CreateUniqueTempDir()); file_ = temp_dir_.path().AppendASCII("test-file"); } @@ -97,10 +99,10 @@ class ImportantFileWriterTest : public testing::Test { }; TEST_F(ImportantFileWriterTest, Basic) { - ImportantFileWriter writer(file_, MessageLoopProxy::current().get()); + ImportantFileWriter writer(file_, ThreadTaskRunnerHandle::Get()); EXPECT_FALSE(PathExists(writer.path())); EXPECT_FALSE(successful_write_observer_.GetAndResetObservationState()); - writer.WriteNow("foo"); + writer.WriteNow(make_scoped_ptr(new std::string("foo"))); RunLoop().RunUntilIdle(); EXPECT_FALSE(successful_write_observer_.GetAndResetObservationState()); @@ -109,11 +111,11 @@ TEST_F(ImportantFileWriterTest, Basic) { } TEST_F(ImportantFileWriterTest, BasicWithSuccessfulWriteObserver) { - ImportantFileWriter writer(file_, MessageLoopProxy::current().get()); + ImportantFileWriter writer(file_, ThreadTaskRunnerHandle::Get()); EXPECT_FALSE(PathExists(writer.path())); EXPECT_FALSE(successful_write_observer_.GetAndResetObservationState()); successful_write_observer_.ObserveNextSuccessfulWrite(&writer); - writer.WriteNow("foo"); + writer.WriteNow(make_scoped_ptr(new std::string("foo"))); RunLoop().RunUntilIdle(); // Confirm that the observer is invoked. @@ -124,7 +126,7 @@ TEST_F(ImportantFileWriterTest, BasicWithSuccessfulWriteObserver) { // Confirm that re-installing the observer works for another write. EXPECT_FALSE(successful_write_observer_.GetAndResetObservationState()); successful_write_observer_.ObserveNextSuccessfulWrite(&writer); - writer.WriteNow("bar"); + writer.WriteNow(make_scoped_ptr(new std::string("bar"))); RunLoop().RunUntilIdle(); EXPECT_TRUE(successful_write_observer_.GetAndResetObservationState()); @@ -134,7 +136,7 @@ TEST_F(ImportantFileWriterTest, BasicWithSuccessfulWriteObserver) { // Confirm that writing again without re-installing the observer doesn't // result in a notification. EXPECT_FALSE(successful_write_observer_.GetAndResetObservationState()); - writer.WriteNow("baz"); + writer.WriteNow(make_scoped_ptr(new std::string("baz"))); RunLoop().RunUntilIdle(); EXPECT_FALSE(successful_write_observer_.GetAndResetObservationState()); @@ -143,15 +145,14 @@ TEST_F(ImportantFileWriterTest, BasicWithSuccessfulWriteObserver) { } TEST_F(ImportantFileWriterTest, ScheduleWrite) { - ImportantFileWriter writer(file_, MessageLoopProxy::current().get()); + ImportantFileWriter writer(file_, ThreadTaskRunnerHandle::Get()); writer.set_commit_interval(TimeDelta::FromMilliseconds(25)); EXPECT_FALSE(writer.HasPendingWrite()); DataSerializer serializer("foo"); writer.ScheduleWrite(&serializer); EXPECT_TRUE(writer.HasPendingWrite()); - MessageLoop::current()->PostDelayedTask( - FROM_HERE, - MessageLoop::QuitWhenIdleClosure(), + ThreadTaskRunnerHandle::Get()->PostDelayedTask( + FROM_HERE, MessageLoop::QuitWhenIdleClosure(), TimeDelta::FromMilliseconds(100)); MessageLoop::current()->Run(); EXPECT_FALSE(writer.HasPendingWrite()); @@ -160,15 +161,14 @@ TEST_F(ImportantFileWriterTest, ScheduleWrite) { } TEST_F(ImportantFileWriterTest, DoScheduledWrite) { - ImportantFileWriter writer(file_, MessageLoopProxy::current().get()); + ImportantFileWriter writer(file_, ThreadTaskRunnerHandle::Get()); EXPECT_FALSE(writer.HasPendingWrite()); DataSerializer serializer("foo"); writer.ScheduleWrite(&serializer); EXPECT_TRUE(writer.HasPendingWrite()); writer.DoScheduledWrite(); - MessageLoop::current()->PostDelayedTask( - FROM_HERE, - MessageLoop::QuitWhenIdleClosure(), + ThreadTaskRunnerHandle::Get()->PostDelayedTask( + FROM_HERE, MessageLoop::QuitWhenIdleClosure(), TimeDelta::FromMilliseconds(100)); MessageLoop::current()->Run(); EXPECT_FALSE(writer.HasPendingWrite()); @@ -177,15 +177,14 @@ TEST_F(ImportantFileWriterTest, DoScheduledWrite) { } TEST_F(ImportantFileWriterTest, BatchingWrites) { - ImportantFileWriter writer(file_, MessageLoopProxy::current().get()); + ImportantFileWriter writer(file_, ThreadTaskRunnerHandle::Get()); writer.set_commit_interval(TimeDelta::FromMilliseconds(25)); DataSerializer foo("foo"), bar("bar"), baz("baz"); writer.ScheduleWrite(&foo); writer.ScheduleWrite(&bar); writer.ScheduleWrite(&baz); - MessageLoop::current()->PostDelayedTask( - FROM_HERE, - MessageLoop::QuitWhenIdleClosure(), + ThreadTaskRunnerHandle::Get()->PostDelayedTask( + FROM_HERE, MessageLoop::QuitWhenIdleClosure(), TimeDelta::FromMilliseconds(100)); MessageLoop::current()->Run(); ASSERT_TRUE(PathExists(writer.path())); diff --git a/chromium/base/files/memory_mapped_file.cc b/chromium/base/files/memory_mapped_file.cc index 745a5ff1f45..891fcffc4cb 100644 --- a/chromium/base/files/memory_mapped_file.cc +++ b/chromium/base/files/memory_mapped_file.cc @@ -31,6 +31,7 @@ MemoryMappedFile::~MemoryMappedFile() { CloseHandles(); } +#if !defined(OS_NACL) bool MemoryMappedFile::Initialize(const FilePath& file_name) { if (IsValid()) return false; @@ -85,5 +86,6 @@ void MemoryMappedFile::CalculateVMAlignedBoundaries(int64 start, *aligned_start = start & ~mask; *aligned_size = (size + *offset + mask) & ~mask; } +#endif } // namespace base diff --git a/chromium/base/files/memory_mapped_file_posix.cc b/chromium/base/files/memory_mapped_file_posix.cc index ebf38779f03..168da923d46 100644 --- a/chromium/base/files/memory_mapped_file_posix.cc +++ b/chromium/base/files/memory_mapped_file_posix.cc @@ -16,6 +16,7 @@ namespace base { MemoryMappedFile::MemoryMappedFile() : data_(NULL), length_(0) { } +#if !defined(OS_NACL) bool MemoryMappedFile::MapFileRegionToMemory( const MemoryMappedFile::Region& region) { ThreadRestrictions::AssertIOAllowed(); @@ -74,6 +75,7 @@ bool MemoryMappedFile::MapFileRegionToMemory( data_ += data_offset; return true; } +#endif void MemoryMappedFile::CloseHandles() { ThreadRestrictions::AssertIOAllowed(); diff --git a/chromium/base/files/memory_mapped_file_unittest.cc b/chromium/base/files/memory_mapped_file_unittest.cc index 36999bfba92..d0833b5cc9a 100644 --- a/chromium/base/files/memory_mapped_file_unittest.cc +++ b/chromium/base/files/memory_mapped_file_unittest.cc @@ -29,12 +29,12 @@ bool CheckBufferContents(const uint8* data, size_t size, size_t offset) { class MemoryMappedFileTest : public PlatformTest { protected: - virtual void SetUp() override { + void SetUp() override { PlatformTest::SetUp(); CreateTemporaryFile(&temp_file_path_); } - virtual void TearDown() { EXPECT_TRUE(DeleteFile(temp_file_path_, false)); } + void TearDown() override { EXPECT_TRUE(DeleteFile(temp_file_path_, false)); } void CreateTemporaryTestFile(size_t size) { File file(temp_file_path_, |