summaryrefslogtreecommitdiff
path: root/base/files
diff options
context:
space:
mode:
authorHidehiko Abe <hidehiko@google.com>2017-12-13 18:59:30 +0900
committerHidehiko Abe <hidehiko@google.com>2017-12-13 23:41:17 +0900
commit36040ed30c39d2106a2cd5ec033e98b71302a744 (patch)
treedc486be512fe418b1cd79a66c11556174de03c4f /base/files
parent1216c7fc80727dea7cb9c5f946688f4b98ccd23c (diff)
downloadlibchrome-36040ed30c39d2106a2cd5ec033e98b71302a744.tar.gz
libchrome: Uprev the library to r456626 from Chromium
Pulled the latest and greatest version of libchrome from Chromium. The merge was done against r456626 which corresponds to git commit 08266b3fca707804065a2cfd60331722ade41969 of Mar 14, 2017 Notable changes are: - FOR_EACH_OBSERVER macro removed (replaced by use of C++ 11 range-base for loop) - base::Values no more FundamentalValue - stl_util moved to base namespace - some scoped pointers removed in crypto/ in favor of BoringSSL UniquePtr. - path() accessor renamed to GetPath() in ScopedTempDir (and other classes) - introduction of base::CallbackOnce Test: All unit-tests should still pass. Change-Id: I1e65efb167fa708e35ed7c6492f1cb66a6a46104 Merged-In: I180f9defc7607f462389fae17701fff553c4a2d0
Diffstat (limited to 'base/files')
-rw-r--r--base/files/dir_reader_posix_unittest.cc2
-rw-r--r--base/files/file.cc8
-rw-r--r--base/files/file.h71
-rw-r--r--base/files/file_descriptor_watcher_posix.cc210
-rw-r--r--base/files/file_descriptor_watcher_posix.h99
-rw-r--r--base/files/file_descriptor_watcher_posix_unittest.cc318
-rw-r--r--base/files/file_path.cc3
-rw-r--r--base/files/file_path.h7
-rw-r--r--base/files/file_path_unittest.cc1
-rw-r--r--base/files/file_path_watcher.cc9
-rw-r--r--base/files/file_path_watcher.h47
-rw-r--r--base/files/file_path_watcher_fsevents.cc94
-rw-r--r--base/files/file_path_watcher_fsevents.h9
-rw-r--r--base/files/file_path_watcher_kqueue.cc163
-rw-r--r--base/files/file_path_watcher_kqueue.h30
-rw-r--r--base/files/file_path_watcher_linux.cc160
-rw-r--r--base/files/file_path_watcher_mac.cc26
-rw-r--r--base/files/file_path_watcher_unittest.cc140
-rw-r--r--base/files/file_posix.cc9
-rw-r--r--base/files/file_tracing.cc37
-rw-r--r--base/files/file_tracing.h12
-rw-r--r--base/files/file_unittest.cc186
-rw-r--r--base/files/file_util.h19
-rw-r--r--base/files/file_util_mac.mm12
-rw-r--r--base/files/file_util_posix.cc76
-rw-r--r--base/files/important_file_writer.cc61
-rw-r--r--base/files/important_file_writer.h59
-rw-r--r--base/files/important_file_writer_unittest.cc149
-rw-r--r--base/files/memory_mapped_file_posix.cc9
-rw-r--r--base/files/scoped_file.cc8
-rw-r--r--base/files/scoped_temp_dir.cc5
-rw-r--r--base/files/scoped_temp_dir.h4
-rw-r--r--base/files/scoped_temp_dir_unittest.cc8
33 files changed, 1472 insertions, 579 deletions
diff --git a/base/files/dir_reader_posix_unittest.cc b/base/files/dir_reader_posix_unittest.cc
index a75858feeb..5d7fd8b139 100644
--- a/base/files/dir_reader_posix_unittest.cc
+++ b/base/files/dir_reader_posix_unittest.cc
@@ -29,7 +29,7 @@ TEST(DirReaderPosixUnittest, Read) {
base::ScopedTempDir temp_dir;
ASSERT_TRUE(temp_dir.CreateUniqueTempDir());
- const char* dir = temp_dir.path().value().c_str();
+ const char* dir = temp_dir.GetPath().value().c_str();
ASSERT_TRUE(dir);
const int prev_wd = open(".", O_RDONLY | O_DIRECTORY);
diff --git a/base/files/file.cc b/base/files/file.cc
index ab05630062..1b2224e323 100644
--- a/base/files/file.cc
+++ b/base/files/file.cc
@@ -138,12 +138,4 @@ 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/base/files/file.h b/base/files/file.h
index ae2bd1b61b..94a9d5cf49 100644
--- a/base/files/file.h
+++ b/base/files/file.h
@@ -63,28 +63,31 @@ class BASE_EXPORT File {
// FLAG_EXCLUSIVE_(READ|WRITE) only grant exclusive access to the file on
// creation on POSIX; for existing files, consider using Lock().
enum Flags {
- FLAG_OPEN = 1 << 0, // Opens a file, only if it exists.
- FLAG_CREATE = 1 << 1, // Creates a new file, only if it does not
- // already exist.
- FLAG_OPEN_ALWAYS = 1 << 2, // May create a new file.
- FLAG_CREATE_ALWAYS = 1 << 3, // May overwrite an old file.
- FLAG_OPEN_TRUNCATED = 1 << 4, // Opens a file and truncates it, only if it
- // exists.
+ FLAG_OPEN = 1 << 0, // Opens a file, only if it exists.
+ FLAG_CREATE = 1 << 1, // Creates a new file, only if it does not
+ // already exist.
+ FLAG_OPEN_ALWAYS = 1 << 2, // May create a new file.
+ FLAG_CREATE_ALWAYS = 1 << 3, // May overwrite an old file.
+ FLAG_OPEN_TRUNCATED = 1 << 4, // Opens a file and truncates it, only if it
+ // exists.
FLAG_READ = 1 << 5,
FLAG_WRITE = 1 << 6,
FLAG_APPEND = 1 << 7,
- FLAG_EXCLUSIVE_READ = 1 << 8, // EXCLUSIVE is opposite of Windows SHARE.
+ FLAG_EXCLUSIVE_READ = 1 << 8, // EXCLUSIVE is opposite of Windows SHARE.
FLAG_EXCLUSIVE_WRITE = 1 << 9,
FLAG_ASYNC = 1 << 10,
- FLAG_TEMPORARY = 1 << 11, // Used on Windows only.
- FLAG_HIDDEN = 1 << 12, // Used on Windows only.
+ FLAG_TEMPORARY = 1 << 11, // Used on Windows only.
+ FLAG_HIDDEN = 1 << 12, // Used on Windows only.
FLAG_DELETE_ON_CLOSE = 1 << 13,
- FLAG_WRITE_ATTRIBUTES = 1 << 14, // Used on Windows only.
- FLAG_SHARE_DELETE = 1 << 15, // Used on Windows only.
- FLAG_TERMINAL_DEVICE = 1 << 16, // Serial port flags.
- FLAG_BACKUP_SEMANTICS = 1 << 17, // Used on Windows only.
- FLAG_EXECUTE = 1 << 18, // Used on Windows only.
- FLAG_SEQUENTIAL_SCAN = 1 << 19, // Used on Windows only.
+ FLAG_WRITE_ATTRIBUTES = 1 << 14, // Used on Windows only.
+ FLAG_SHARE_DELETE = 1 << 15, // Used on Windows only.
+ FLAG_TERMINAL_DEVICE = 1 << 16, // Serial port flags.
+ FLAG_BACKUP_SEMANTICS = 1 << 17, // Used on Windows only.
+ FLAG_EXECUTE = 1 << 18, // Used on Windows only.
+ FLAG_SEQUENTIAL_SCAN = 1 << 19, // Used on Windows only.
+ FLAG_CAN_DELETE_ON_CLOSE = 1 << 20, // Requests permission to delete a file
+ // via DeleteOnClose() (Windows only).
+ // See DeleteOnClose() for details.
};
// This enum has been recorded in multiple histograms. If the order of the
@@ -290,11 +293,41 @@ class BASE_EXPORT 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();
+ File Duplicate() const;
bool async() const { return async_; }
#if defined(OS_WIN)
+ // Sets or clears the DeleteFile disposition on the handle. Returns true if
+ // the disposition was set or cleared, as indicated by |delete_on_close|.
+ //
+ // Microsoft Windows deletes a file only when the last handle to the
+ // underlying kernel object is closed when the DeleteFile disposition has been
+ // set by any handle holder. This disposition is be set by:
+ // - Calling the Win32 DeleteFile function with the path to a file.
+ // - Opening/creating a file with FLAG_DELETE_ON_CLOSE.
+ // - Opening/creating a file with FLAG_CAN_DELETE_ON_CLOSE and subsequently
+ // calling DeleteOnClose(true).
+ //
+ // In all cases, all pre-existing handles to the file must have been opened
+ // with FLAG_SHARE_DELETE.
+ //
+ // So:
+ // - Use FLAG_SHARE_DELETE when creating/opening a file to allow another
+ // entity on the system to cause it to be deleted when it is closed. (Note:
+ // another entity can delete the file the moment after it is closed, so not
+ // using this permission doesn't provide any protections.)
+ // - Use FLAG_DELETE_ON_CLOSE for any file that is to be deleted after use.
+ // The OS will ensure it is deleted even in the face of process termination.
+ // - Use FLAG_CAN_DELETE_ON_CLOSE in conjunction with DeleteOnClose() to alter
+ // the DeleteFile disposition on an open handle. This fine-grained control
+ // allows for marking a file for deletion during processing so that it is
+ // deleted in the event of untimely process termination, and then clearing
+ // this state once the file is suitable for persistence.
+ bool DeleteOnClose(bool delete_on_close);
+#endif
+
+#if defined(OS_WIN)
static Error OSErrorToFileError(DWORD last_error);
#elif defined(OS_POSIX)
static Error OSErrorToFileError(int saved_errno);
@@ -310,10 +343,6 @@ class BASE_EXPORT File {
// traversal ('..') components.
void DoInitialize(const FilePath& path, uint32_t 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)
diff --git a/base/files/file_descriptor_watcher_posix.cc b/base/files/file_descriptor_watcher_posix.cc
new file mode 100644
index 0000000000..9746e35ea7
--- /dev/null
+++ b/base/files/file_descriptor_watcher_posix.cc
@@ -0,0 +1,210 @@
+// Copyright 2016 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_descriptor_watcher_posix.h"
+
+#include "base/bind.h"
+#include "base/lazy_instance.h"
+#include "base/logging.h"
+#include "base/memory/ptr_util.h"
+#include "base/sequenced_task_runner.h"
+#include "base/single_thread_task_runner.h"
+#include "base/threading/sequenced_task_runner_handle.h"
+#include "base/threading/thread_checker.h"
+#include "base/threading/thread_local.h"
+
+namespace base {
+
+namespace {
+
+// MessageLoopForIO used to watch file descriptors for which callbacks are
+// registered from a given thread.
+LazyInstance<ThreadLocalPointer<MessageLoopForIO>>::Leaky
+ tls_message_loop_for_io = LAZY_INSTANCE_INITIALIZER;
+
+} // namespace
+
+FileDescriptorWatcher::Controller::~Controller() {
+ DCHECK(sequence_checker_.CalledOnValidSequence());
+
+ // Delete |watcher_| on the MessageLoopForIO.
+ //
+ // If the MessageLoopForIO is deleted before Watcher::StartWatching() runs,
+ // |watcher_| is leaked. If the MessageLoopForIO is deleted after
+ // Watcher::StartWatching() runs but before the DeleteSoon task runs,
+ // |watcher_| is deleted from Watcher::WillDestroyCurrentMessageLoop().
+ message_loop_for_io_task_runner_->DeleteSoon(FROM_HERE, watcher_.release());
+
+ // Since WeakPtrs are invalidated by the destructor, RunCallback() won't be
+ // invoked after this returns.
+}
+
+class FileDescriptorWatcher::Controller::Watcher
+ : public MessageLoopForIO::Watcher,
+ public MessageLoop::DestructionObserver {
+ public:
+ Watcher(WeakPtr<Controller> controller, MessageLoopForIO::Mode mode, int fd);
+ ~Watcher() override;
+
+ void StartWatching();
+
+ private:
+ friend class FileDescriptorWatcher;
+
+ // MessageLoopForIO::Watcher:
+ void OnFileCanReadWithoutBlocking(int fd) override;
+ void OnFileCanWriteWithoutBlocking(int fd) override;
+
+ // MessageLoop::DestructionObserver:
+ void WillDestroyCurrentMessageLoop() override;
+
+ // Used to instruct the MessageLoopForIO to stop watching the file descriptor.
+ MessageLoopForIO::FileDescriptorWatcher file_descriptor_watcher_;
+
+ // Runs tasks on the sequence on which this was instantiated (i.e. the
+ // sequence on which the callback must run).
+ const scoped_refptr<SequencedTaskRunner> callback_task_runner_ =
+ SequencedTaskRunnerHandle::Get();
+
+ // The Controller that created this Watcher.
+ WeakPtr<Controller> controller_;
+
+ // Whether this Watcher is notified when |fd_| becomes readable or writable
+ // without blocking.
+ const MessageLoopForIO::Mode mode_;
+
+ // The watched file descriptor.
+ const int fd_;
+
+ // Except for the constructor, every method of this class must run on the same
+ // MessageLoopForIO thread.
+ ThreadChecker thread_checker_;
+
+ // Whether this Watcher was registered as a DestructionObserver on the
+ // MessageLoopForIO thread.
+ bool registered_as_destruction_observer_ = false;
+
+ DISALLOW_COPY_AND_ASSIGN(Watcher);
+};
+
+FileDescriptorWatcher::Controller::Watcher::Watcher(
+ WeakPtr<Controller> controller,
+ MessageLoopForIO::Mode mode,
+ int fd)
+ : file_descriptor_watcher_(FROM_HERE),
+ controller_(controller),
+ mode_(mode),
+ fd_(fd) {
+ DCHECK(callback_task_runner_);
+ thread_checker_.DetachFromThread();
+}
+
+FileDescriptorWatcher::Controller::Watcher::~Watcher() {
+ DCHECK(thread_checker_.CalledOnValidThread());
+ MessageLoopForIO::current()->RemoveDestructionObserver(this);
+}
+
+void FileDescriptorWatcher::Controller::Watcher::StartWatching() {
+ DCHECK(thread_checker_.CalledOnValidThread());
+
+ MessageLoopForIO::current()->WatchFileDescriptor(
+ fd_, false, mode_, &file_descriptor_watcher_, this);
+
+ if (!registered_as_destruction_observer_) {
+ MessageLoopForIO::current()->AddDestructionObserver(this);
+ registered_as_destruction_observer_ = true;
+ }
+}
+
+void FileDescriptorWatcher::Controller::Watcher::OnFileCanReadWithoutBlocking(
+ int fd) {
+ DCHECK_EQ(fd_, fd);
+ DCHECK_EQ(MessageLoopForIO::WATCH_READ, mode_);
+ DCHECK(thread_checker_.CalledOnValidThread());
+
+ // Run the callback on the sequence on which the watch was initiated.
+ callback_task_runner_->PostTask(FROM_HERE,
+ Bind(&Controller::RunCallback, controller_));
+}
+
+void FileDescriptorWatcher::Controller::Watcher::OnFileCanWriteWithoutBlocking(
+ int fd) {
+ DCHECK_EQ(fd_, fd);
+ DCHECK_EQ(MessageLoopForIO::WATCH_WRITE, mode_);
+ DCHECK(thread_checker_.CalledOnValidThread());
+
+ // Run the callback on the sequence on which the watch was initiated.
+ callback_task_runner_->PostTask(FROM_HERE,
+ Bind(&Controller::RunCallback, controller_));
+}
+
+void FileDescriptorWatcher::Controller::Watcher::
+ WillDestroyCurrentMessageLoop() {
+ DCHECK(thread_checker_.CalledOnValidThread());
+
+ // A Watcher is owned by a Controller. When the Controller is deleted, it
+ // transfers ownership of the Watcher to a delete task posted to the
+ // MessageLoopForIO. If the MessageLoopForIO is deleted before the delete task
+ // runs, the following line takes care of deleting the Watcher.
+ delete this;
+}
+
+FileDescriptorWatcher::Controller::Controller(MessageLoopForIO::Mode mode,
+ int fd,
+ const Closure& callback)
+ : callback_(callback),
+ message_loop_for_io_task_runner_(
+ tls_message_loop_for_io.Get().Get()->task_runner()),
+ weak_factory_(this) {
+ DCHECK(!callback_.is_null());
+ DCHECK(message_loop_for_io_task_runner_);
+ watcher_ = MakeUnique<Watcher>(weak_factory_.GetWeakPtr(), mode, fd);
+ StartWatching();
+}
+
+void FileDescriptorWatcher::Controller::StartWatching() {
+ DCHECK(sequence_checker_.CalledOnValidSequence());
+ // It is safe to use Unretained() below because |watcher_| can only be deleted
+ // by a delete task posted to |message_loop_for_io_task_runner_| by this
+ // Controller's destructor. Since this delete task hasn't been posted yet, it
+ // can't run before the task posted below.
+ message_loop_for_io_task_runner_->PostTask(
+ FROM_HERE, Bind(&Watcher::StartWatching, Unretained(watcher_.get())));
+}
+
+void FileDescriptorWatcher::Controller::RunCallback() {
+ DCHECK(sequence_checker_.CalledOnValidSequence());
+
+ WeakPtr<Controller> weak_this = weak_factory_.GetWeakPtr();
+
+ callback_.Run();
+
+ // If |this| wasn't deleted, re-enable the watch.
+ if (weak_this)
+ StartWatching();
+}
+
+FileDescriptorWatcher::FileDescriptorWatcher(
+ MessageLoopForIO* message_loop_for_io) {
+ DCHECK(message_loop_for_io);
+ DCHECK(!tls_message_loop_for_io.Get().Get());
+ tls_message_loop_for_io.Get().Set(message_loop_for_io);
+}
+
+FileDescriptorWatcher::~FileDescriptorWatcher() {
+ tls_message_loop_for_io.Get().Set(nullptr);
+}
+
+std::unique_ptr<FileDescriptorWatcher::Controller>
+FileDescriptorWatcher::WatchReadable(int fd, const Closure& callback) {
+ return WrapUnique(new Controller(MessageLoopForIO::WATCH_READ, fd, callback));
+}
+
+std::unique_ptr<FileDescriptorWatcher::Controller>
+FileDescriptorWatcher::WatchWritable(int fd, const Closure& callback) {
+ return WrapUnique(
+ new Controller(MessageLoopForIO::WATCH_WRITE, fd, callback));
+}
+
+} // namespace base
diff --git a/base/files/file_descriptor_watcher_posix.h b/base/files/file_descriptor_watcher_posix.h
new file mode 100644
index 0000000000..6cc011bb3e
--- /dev/null
+++ b/base/files/file_descriptor_watcher_posix.h
@@ -0,0 +1,99 @@
+// Copyright 2016 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_DESCRIPTOR_WATCHER_POSIX_H_
+#define BASE_FILES_FILE_DESCRIPTOR_WATCHER_POSIX_H_
+
+#include <memory>
+
+#include "base/base_export.h"
+#include "base/callback.h"
+#include "base/macros.h"
+#include "base/memory/ref_counted.h"
+#include "base/memory/weak_ptr.h"
+#include "base/message_loop/message_loop.h"
+#include "base/sequence_checker.h"
+
+namespace base {
+
+class SingleThreadTaskRunner;
+
+// The FileDescriptorWatcher API allows callbacks to be invoked when file
+// descriptors are readable or writable without blocking.
+class BASE_EXPORT FileDescriptorWatcher {
+ public:
+ // Instantiated and returned by WatchReadable() or WatchWritable(). The
+ // constructor registers a callback to be invoked when a file descriptor is
+ // readable or writable without blocking and the destructor unregisters it.
+ class Controller {
+ public:
+ // Unregisters the callback registered by the constructor.
+ ~Controller();
+
+ private:
+ friend class FileDescriptorWatcher;
+ class Watcher;
+
+ // Registers |callback| to be invoked when |fd| is readable or writable
+ // without blocking (depending on |mode|).
+ Controller(MessageLoopForIO::Mode mode, int fd, const Closure& callback);
+
+ // Starts watching the file descriptor.
+ void StartWatching();
+
+ // Runs |callback_|.
+ void RunCallback();
+
+ // The callback to run when the watched file descriptor is readable or
+ // writable without blocking.
+ Closure callback_;
+
+ // TaskRunner associated with the MessageLoopForIO that watches the file
+ // descriptor.
+ const scoped_refptr<SingleThreadTaskRunner>
+ message_loop_for_io_task_runner_;
+
+ // Notified by the MessageLoopForIO associated with
+ // |message_loop_for_io_task_runner_| when the watched file descriptor is
+ // readable or writable without blocking. Posts a task to run RunCallback()
+ // on the sequence on which the Controller was instantiated. When the
+ // Controller is deleted, ownership of |watcher_| is transfered to a delete
+ // task posted to the MessageLoopForIO. This ensures that |watcher_| isn't
+ // deleted while it is being used by the MessageLoopForIO.
+ std::unique_ptr<Watcher> watcher_;
+
+ // Validates that the Controller is used on the sequence on which it was
+ // instantiated.
+ SequenceChecker sequence_checker_;
+
+ WeakPtrFactory<Controller> weak_factory_;
+
+ DISALLOW_COPY_AND_ASSIGN(Controller);
+ };
+
+ // Registers |message_loop_for_io| to watch file descriptors for which
+ // callbacks are registered from the current thread via WatchReadable() or
+ // WatchWritable(). |message_loop_for_io| may run on another thread. The
+ // constructed FileDescriptorWatcher must not outlive |message_loop_for_io|.
+ FileDescriptorWatcher(MessageLoopForIO* message_loop_for_io);
+ ~FileDescriptorWatcher();
+
+ // Registers |callback| to be invoked on the current sequence when |fd| is
+ // readable or writable without blocking. |callback| is unregistered when the
+ // returned Controller is deleted (deletion must happen on the current
+ // sequence). To call these methods, a FileDescriptorWatcher must have been
+ // instantiated on the current thread and SequencedTaskRunnerHandle::IsSet()
+ // must return true.
+ static std::unique_ptr<Controller> WatchReadable(int fd,
+ const Closure& callback);
+ static std::unique_ptr<Controller> WatchWritable(int fd,
+ const Closure& callback);
+
+ private:
+ DISALLOW_COPY_AND_ASSIGN(FileDescriptorWatcher);
+};
+
+} // namespace base
+
+#endif // BASE_FILES_FILE_DESCRIPTOR_WATCHER_POSIX_H_
diff --git a/base/files/file_descriptor_watcher_posix_unittest.cc b/base/files/file_descriptor_watcher_posix_unittest.cc
new file mode 100644
index 0000000000..7ff40c5fee
--- /dev/null
+++ b/base/files/file_descriptor_watcher_posix_unittest.cc
@@ -0,0 +1,318 @@
+// Copyright 2016 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_descriptor_watcher_posix.h"
+
+#include <unistd.h>
+
+#include <memory>
+
+#include "base/bind.h"
+#include "base/files/file_util.h"
+#include "base/macros.h"
+#include "base/memory/ptr_util.h"
+#include "base/message_loop/message_loop.h"
+#include "base/posix/eintr_wrapper.h"
+#include "base/run_loop.h"
+#include "base/test/test_timeouts.h"
+#include "base/threading/platform_thread.h"
+#include "base/threading/thread.h"
+#include "base/threading/thread_checker_impl.h"
+#include "build/build_config.h"
+#include "testing/gmock/include/gmock/gmock.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace base {
+
+namespace {
+
+class Mock {
+ public:
+ Mock() = default;
+
+ MOCK_METHOD0(ReadableCallback, void());
+ MOCK_METHOD0(WritableCallback, void());
+
+ private:
+ DISALLOW_COPY_AND_ASSIGN(Mock);
+};
+
+enum class FileDescriptorWatcherTestType {
+ MESSAGE_LOOP_FOR_IO_ON_MAIN_THREAD,
+ MESSAGE_LOOP_FOR_IO_ON_OTHER_THREAD,
+};
+
+class FileDescriptorWatcherTest
+ : public testing::TestWithParam<FileDescriptorWatcherTestType> {
+ public:
+ FileDescriptorWatcherTest()
+ : message_loop_(GetParam() == FileDescriptorWatcherTestType::
+ MESSAGE_LOOP_FOR_IO_ON_MAIN_THREAD
+ ? new MessageLoopForIO
+ : new MessageLoop),
+ other_thread_("FileDescriptorWatcherTest_OtherThread") {}
+ ~FileDescriptorWatcherTest() override = default;
+
+ void SetUp() override {
+ ASSERT_EQ(0, pipe(pipe_fds_));
+
+ MessageLoop* message_loop_for_io;
+ if (GetParam() ==
+ FileDescriptorWatcherTestType::MESSAGE_LOOP_FOR_IO_ON_OTHER_THREAD) {
+ Thread::Options options;
+ options.message_loop_type = MessageLoop::TYPE_IO;
+ ASSERT_TRUE(other_thread_.StartWithOptions(options));
+ message_loop_for_io = other_thread_.message_loop();
+ } else {
+ message_loop_for_io = message_loop_.get();
+ }
+
+ ASSERT_TRUE(message_loop_for_io->IsType(MessageLoop::TYPE_IO));
+ file_descriptor_watcher_ = MakeUnique<FileDescriptorWatcher>(
+ static_cast<MessageLoopForIO*>(message_loop_for_io));
+ }
+
+ void TearDown() override {
+ if (GetParam() ==
+ FileDescriptorWatcherTestType::MESSAGE_LOOP_FOR_IO_ON_MAIN_THREAD &&
+ message_loop_) {
+ // Allow the delete task posted by the Controller's destructor to run.
+ base::RunLoop().RunUntilIdle();
+ }
+
+ EXPECT_EQ(0, IGNORE_EINTR(close(pipe_fds_[0])));
+ EXPECT_EQ(0, IGNORE_EINTR(close(pipe_fds_[1])));
+ }
+
+ protected:
+ int read_file_descriptor() const { return pipe_fds_[0]; }
+ int write_file_descriptor() const { return pipe_fds_[1]; }
+
+ // Waits for a short delay and run pending tasks.
+ void WaitAndRunPendingTasks() {
+ PlatformThread::Sleep(TestTimeouts::tiny_timeout());
+ RunLoop().RunUntilIdle();
+ }
+
+ // Registers ReadableCallback() to be called on |mock_| when
+ // read_file_descriptor() is readable without blocking.
+ std::unique_ptr<FileDescriptorWatcher::Controller> WatchReadable() {
+ std::unique_ptr<FileDescriptorWatcher::Controller> controller =
+ FileDescriptorWatcher::WatchReadable(
+ read_file_descriptor(),
+ Bind(&Mock::ReadableCallback, Unretained(&mock_)));
+ EXPECT_TRUE(controller);
+
+ // Unless read_file_descriptor() was readable before the callback was
+ // registered, this shouldn't do anything.
+ WaitAndRunPendingTasks();
+
+ return controller;
+ }
+
+ // Registers WritableCallback() to be called on |mock_| when
+ // write_file_descriptor() is writable without blocking.
+ std::unique_ptr<FileDescriptorWatcher::Controller> WatchWritable() {
+ std::unique_ptr<FileDescriptorWatcher::Controller> controller =
+ FileDescriptorWatcher::WatchWritable(
+ read_file_descriptor(),
+ Bind(&Mock::WritableCallback, Unretained(&mock_)));
+ EXPECT_TRUE(controller);
+ return controller;
+ }
+
+ void WriteByte() {
+ constexpr char kByte = '!';
+ ASSERT_TRUE(
+ WriteFileDescriptor(write_file_descriptor(), &kByte, sizeof(kByte)));
+ }
+
+ void ReadByte() {
+ // This is always called as part of the WatchReadable() callback, which
+ // should run on the main thread.
+ EXPECT_TRUE(thread_checker_.CalledOnValidThread());
+
+ char buffer;
+ ASSERT_TRUE(ReadFromFD(read_file_descriptor(), &buffer, sizeof(buffer)));
+ }
+
+ // Mock on wich callbacks are invoked.
+ testing::StrictMock<Mock> mock_;
+
+ // MessageLoop bound to the main thread.
+ std::unique_ptr<MessageLoop> message_loop_;
+
+ // Thread running a MessageLoopForIO. Used when the test type is
+ // MESSAGE_LOOP_FOR_IO_ON_OTHER_THREAD.
+ Thread other_thread_;
+
+ private:
+ // Determines which MessageLoopForIO is used to watch file descriptors for
+ // which callbacks are registered on the main thread.
+ std::unique_ptr<FileDescriptorWatcher> file_descriptor_watcher_;
+
+ // Watched file descriptors.
+ int pipe_fds_[2];
+
+ // Used to verify that callbacks run on the thread on which they are
+ // registered.
+ ThreadCheckerImpl thread_checker_;
+
+ DISALLOW_COPY_AND_ASSIGN(FileDescriptorWatcherTest);
+};
+
+} // namespace
+
+TEST_P(FileDescriptorWatcherTest, WatchWritable) {
+ auto controller = WatchWritable();
+
+// On Mac and iOS, the write end of a newly created pipe is writable without
+// blocking.
+#if defined(OS_MACOSX)
+ RunLoop run_loop;
+ EXPECT_CALL(mock_, WritableCallback())
+ .WillOnce(testing::Invoke(&run_loop, &RunLoop::Quit));
+ run_loop.Run();
+#endif // defined(OS_MACOSX)
+}
+
+TEST_P(FileDescriptorWatcherTest, WatchReadableOneByte) {
+ auto controller = WatchReadable();
+
+ // Write 1 byte to the pipe, making it readable without blocking. Expect one
+ // call to ReadableCallback() which will read 1 byte from the pipe.
+ WriteByte();
+ RunLoop run_loop;
+ EXPECT_CALL(mock_, ReadableCallback())
+ .WillOnce(testing::Invoke([this, &run_loop]() {
+ ReadByte();
+ run_loop.Quit();
+ }));
+ run_loop.Run();
+ testing::Mock::VerifyAndClear(&mock_);
+
+ // No more call to ReadableCallback() is expected.
+ WaitAndRunPendingTasks();
+}
+
+TEST_P(FileDescriptorWatcherTest, WatchReadableTwoBytes) {
+ auto controller = WatchReadable();
+
+ // Write 2 bytes to the pipe. Expect two calls to ReadableCallback() which
+ // will each read 1 byte from the pipe.
+ WriteByte();
+ WriteByte();
+ RunLoop run_loop;
+ EXPECT_CALL(mock_, ReadableCallback())
+ .WillOnce(testing::Invoke([this]() { ReadByte(); }))
+ .WillOnce(testing::Invoke([this, &run_loop]() {
+ ReadByte();
+ run_loop.Quit();
+ }));
+ run_loop.Run();
+ testing::Mock::VerifyAndClear(&mock_);
+
+ // No more call to ReadableCallback() is expected.
+ WaitAndRunPendingTasks();
+}
+
+TEST_P(FileDescriptorWatcherTest, WatchReadableByteWrittenFromCallback) {
+ auto controller = WatchReadable();
+
+ // Write 1 byte to the pipe. Expect one call to ReadableCallback() from which
+ // 1 byte is read and 1 byte is written to the pipe. Then, expect another call
+ // to ReadableCallback() from which the remaining byte is read from the pipe.
+ WriteByte();
+ RunLoop run_loop;
+ EXPECT_CALL(mock_, ReadableCallback())
+ .WillOnce(testing::Invoke([this]() {
+ ReadByte();
+ WriteByte();
+ }))
+ .WillOnce(testing::Invoke([this, &run_loop]() {
+ ReadByte();
+ run_loop.Quit();
+ }));
+ run_loop.Run();
+ testing::Mock::VerifyAndClear(&mock_);
+
+ // No more call to ReadableCallback() is expected.
+ WaitAndRunPendingTasks();
+}
+
+TEST_P(FileDescriptorWatcherTest, DeleteControllerFromCallback) {
+ auto controller = WatchReadable();
+
+ // Write 1 byte to the pipe. Expect one call to ReadableCallback() from which
+ // |controller| is deleted.
+ WriteByte();
+ RunLoop run_loop;
+ EXPECT_CALL(mock_, ReadableCallback())
+ .WillOnce(testing::Invoke([&run_loop, &controller]() {
+ controller = nullptr;
+ run_loop.Quit();
+ }));
+ run_loop.Run();
+ testing::Mock::VerifyAndClear(&mock_);
+
+ // Since |controller| has been deleted, no call to ReadableCallback() is
+ // expected even though the pipe is still readable without blocking.
+ WaitAndRunPendingTasks();
+}
+
+TEST_P(FileDescriptorWatcherTest,
+ DeleteControllerBeforeFileDescriptorReadable) {
+ auto controller = WatchReadable();
+
+ // Cancel the watch.
+ controller = nullptr;
+
+ // Write 1 byte to the pipe to make it readable without blocking.
+ WriteByte();
+
+ // No call to ReadableCallback() is expected.
+ WaitAndRunPendingTasks();
+}
+
+TEST_P(FileDescriptorWatcherTest, DeleteControllerAfterFileDescriptorReadable) {
+ auto controller = WatchReadable();
+
+ // Write 1 byte to the pipe to make it readable without blocking.
+ WriteByte();
+
+ // Cancel the watch.
+ controller = nullptr;
+
+ // No call to ReadableCallback() is expected.
+ WaitAndRunPendingTasks();
+}
+
+TEST_P(FileDescriptorWatcherTest, DeleteControllerAfterDeleteMessageLoopForIO) {
+ auto controller = WatchReadable();
+
+ // Delete the MessageLoopForIO.
+ if (GetParam() ==
+ FileDescriptorWatcherTestType::MESSAGE_LOOP_FOR_IO_ON_MAIN_THREAD) {
+ message_loop_ = nullptr;
+ } else {
+ other_thread_.Stop();
+ }
+
+ // Deleting |controller| shouldn't crash even though that causes a task to be
+ // posted to the MessageLoopForIO thread.
+ controller = nullptr;
+}
+
+INSTANTIATE_TEST_CASE_P(
+ MessageLoopForIOOnMainThread,
+ FileDescriptorWatcherTest,
+ ::testing::Values(
+ FileDescriptorWatcherTestType::MESSAGE_LOOP_FOR_IO_ON_MAIN_THREAD));
+INSTANTIATE_TEST_CASE_P(
+ MessageLoopForIOOnOtherThread,
+ FileDescriptorWatcherTest,
+ ::testing::Values(
+ FileDescriptorWatcherTestType::MESSAGE_LOOP_FOR_IO_ON_OTHER_THREAD));
+
+} // namespace base
diff --git a/base/files/file_path.cc b/base/files/file_path.cc
index 29f12a80aa..9f67f9bc49 100644
--- a/base/files/file_path.cc
+++ b/base/files/file_path.cc
@@ -176,6 +176,7 @@ FilePath::FilePath() {
FilePath::FilePath(const FilePath& that) : path_(that.path_) {
}
+FilePath::FilePath(FilePath&& that) = default;
FilePath::FilePath(StringPieceType path) {
path.CopyToString(&path_);
@@ -192,6 +193,8 @@ FilePath& FilePath::operator=(const FilePath& that) {
return *this;
}
+FilePath& FilePath::operator=(FilePath&& that) = default;
+
bool FilePath::operator==(const FilePath& that) const {
#if defined(FILE_PATH_USES_DRIVE_LETTERS)
return EqualDriveLetterCaseInsensitive(this->path_, that.path_);
diff --git a/base/files/file_path.h b/base/files/file_path.h
index 3234df7bfb..02846f6892 100644
--- a/base/files/file_path.h
+++ b/base/files/file_path.h
@@ -182,6 +182,13 @@ class BASE_EXPORT FilePath {
~FilePath();
FilePath& operator=(const FilePath& that);
+ // Constructs FilePath with the contents of |that|, which is left in valid but
+ // unspecified state.
+ FilePath(FilePath&& that);
+ // Replaces the contents with those of |that|, which is left in valid but
+ // unspecified state.
+ FilePath& operator=(FilePath&& that);
+
bool operator==(const FilePath& that) const;
bool operator!=(const FilePath& that) const;
diff --git a/base/files/file_path_unittest.cc b/base/files/file_path_unittest.cc
index d8c5969513..a091e62dd1 100644
--- a/base/files/file_path_unittest.cc
+++ b/base/files/file_path_unittest.cc
@@ -9,7 +9,6 @@
#include "base/files/file_path.h"
#include "base/macros.h"
#include "base/strings/utf_string_conversions.h"
-#include "base/test/scoped_locale.h"
#include "build/build_config.h"
#include "testing/gtest/include/gtest/gtest.h"
#include "testing/platform_test.h"
diff --git a/base/files/file_path_watcher.cc b/base/files/file_path_watcher.cc
index a4624ab609..245bd8efe2 100644
--- a/base/files/file_path_watcher.cc
+++ b/base/files/file_path_watcher.cc
@@ -8,22 +8,16 @@
#include "base/files/file_path_watcher.h"
#include "base/logging.h"
-#include "base/message_loop/message_loop.h"
#include "build/build_config.h"
namespace base {
FilePathWatcher::~FilePathWatcher() {
+ DCHECK(sequence_checker_.CalledOnValidSequence());
impl_->Cancel();
}
// static
-void FilePathWatcher::CancelWatch(
- const scoped_refptr<PlatformDelegate>& delegate) {
- delegate->CancelOnMessageLoopThread();
-}
-
-// static
bool FilePathWatcher::RecursiveWatchAvailable() {
#if (defined(OS_MACOSX) && !defined(OS_IOS)) || defined(OS_WIN) || \
defined(OS_LINUX) || defined(OS_ANDROID)
@@ -44,6 +38,7 @@ FilePathWatcher::PlatformDelegate::~PlatformDelegate() {
bool FilePathWatcher::Watch(const FilePath& path,
bool recursive,
const Callback& callback) {
+ DCHECK(sequence_checker_.CalledOnValidSequence());
DCHECK(path.IsAbsolute());
return impl_->Watch(path, recursive, callback);
}
diff --git a/base/files/file_path_watcher.h b/base/files/file_path_watcher.h
index d5c6db1acf..9e29d0a9d5 100644
--- a/base/files/file_path_watcher.h
+++ b/base/files/file_path_watcher.h
@@ -7,12 +7,15 @@
#ifndef BASE_FILES_FILE_PATH_WATCHER_H_
#define BASE_FILES_FILE_PATH_WATCHER_H_
+#include <memory>
+
#include "base/base_export.h"
#include "base/callback.h"
#include "base/files/file_path.h"
#include "base/macros.h"
#include "base/memory/ref_counted.h"
-#include "base/single_thread_task_runner.h"
+#include "base/sequence_checker.h"
+#include "base/sequenced_task_runner.h"
namespace base {
@@ -25,6 +28,8 @@ namespace base {
// detect the creation and deletion of files in a watched directory, but will
// not detect modifications to those files. See file_path_watcher_kqueue.cc for
// details.
+//
+// Must be destroyed on the sequence that invokes Watch().
class BASE_EXPORT FilePathWatcher {
public:
// Callback type for Watch(). |path| points to the file that was updated,
@@ -33,9 +38,10 @@ class BASE_EXPORT FilePathWatcher {
typedef base::Callback<void(const FilePath& path, bool error)> Callback;
// Used internally to encapsulate different members on different platforms.
- class PlatformDelegate : public base::RefCountedThreadSafe<PlatformDelegate> {
+ class PlatformDelegate {
public:
PlatformDelegate();
+ virtual ~PlatformDelegate();
// Start watching for the given |path| and notify |delegate| about changes.
virtual bool Watch(const FilePath& path,
@@ -44,25 +50,16 @@ class BASE_EXPORT FilePathWatcher {
// Stop watching. This is called from FilePathWatcher's dtor in order to
// allow to shut down properly while the object is still alive.
- // It can be called from any thread.
virtual void Cancel() = 0;
protected:
- friend class base::RefCountedThreadSafe<PlatformDelegate>;
friend class FilePathWatcher;
- virtual ~PlatformDelegate();
-
- // Stop watching. This is only called on the thread of the appropriate
- // message loop. Since it can also be called more than once, it should
- // check |is_cancelled()| to avoid duplicate work.
- virtual void CancelOnMessageLoopThread() = 0;
-
- scoped_refptr<base::SingleThreadTaskRunner> task_runner() const {
+ scoped_refptr<SequencedTaskRunner> task_runner() const {
return task_runner_;
}
- void set_task_runner(scoped_refptr<base::SingleThreadTaskRunner> runner) {
+ void set_task_runner(scoped_refptr<SequencedTaskRunner> runner) {
task_runner_ = std::move(runner);
}
@@ -76,32 +73,34 @@ class BASE_EXPORT FilePathWatcher {
}
private:
- scoped_refptr<base::SingleThreadTaskRunner> task_runner_;
+ scoped_refptr<SequencedTaskRunner> task_runner_;
bool cancelled_;
+
+ DISALLOW_COPY_AND_ASSIGN(PlatformDelegate);
};
FilePathWatcher();
- virtual ~FilePathWatcher();
-
- // A callback that always cleans up the PlatformDelegate, either when executed
- // or when deleted without having been executed at all, as can happen during
- // shutdown.
- static void CancelWatch(const scoped_refptr<PlatformDelegate>& delegate);
+ ~FilePathWatcher();
// Returns true if the platform and OS version support recursive watches.
static bool RecursiveWatchAvailable();
// Invokes |callback| whenever updates to |path| are detected. This should be
- // called at most once, and from a MessageLoop of TYPE_IO. Set |recursive| to
- // true, to watch |path| and its children. The callback will be invoked on
- // the same loop. Returns true on success.
+ // called at most once. Set |recursive| to true to watch |path| and its
+ // children. The callback will be invoked on the same sequence. Returns true
+ // on success.
+ //
+ // On POSIX, this must be called from a thread that supports
+ // FileDescriptorWatcher.
//
// Recursive watch is not supported on all platforms and file systems.
// Watch() will return false in the case of failure.
bool Watch(const FilePath& path, bool recursive, const Callback& callback);
private:
- scoped_refptr<PlatformDelegate> impl_;
+ std::unique_ptr<PlatformDelegate> impl_;
+
+ SequenceChecker sequence_checker_;
DISALLOW_COPY_AND_ASSIGN(FilePathWatcher);
};
diff --git a/base/files/file_path_watcher_fsevents.cc b/base/files/file_path_watcher_fsevents.cc
index e9d25080e7..e9a87b0e05 100644
--- a/base/files/file_path_watcher_fsevents.cc
+++ b/base/files/file_path_watcher_fsevents.cc
@@ -13,10 +13,8 @@
#include "base/lazy_instance.h"
#include "base/logging.h"
#include "base/mac/scoped_cftyperef.h"
-#include "base/macros.h"
-#include "base/message_loop/message_loop.h"
#include "base/strings/stringprintf.h"
-#include "base/threading/thread_task_runner_handle.h"
+#include "base/threading/sequenced_task_runner_handle.h"
namespace base {
@@ -70,16 +68,21 @@ FilePath ResolvePath(const FilePath& path) {
FilePathWatcherFSEvents::FilePathWatcherFSEvents()
: queue_(dispatch_queue_create(
- base::StringPrintf(
- "org.chromium.base.FilePathWatcher.%p", this).c_str(),
+ base::StringPrintf("org.chromium.base.FilePathWatcher.%p", this)
+ .c_str(),
DISPATCH_QUEUE_SERIAL)),
- fsevent_stream_(nullptr) {
+ fsevent_stream_(nullptr),
+ weak_factory_(this) {}
+
+FilePathWatcherFSEvents::~FilePathWatcherFSEvents() {
+ DCHECK(!task_runner() || task_runner()->RunsTasksOnCurrentThread());
+ DCHECK(callback_.is_null())
+ << "Cancel() must be called before FilePathWatcher is destroyed.";
}
bool FilePathWatcherFSEvents::Watch(const FilePath& path,
bool recursive,
const FilePathWatcher::Callback& callback) {
- DCHECK(MessageLoopForIO::current());
DCHECK(!callback.is_null());
DCHECK(callback_.is_null());
@@ -88,7 +91,7 @@ bool FilePathWatcherFSEvents::Watch(const FilePath& path,
if (!recursive)
return false;
- set_task_runner(ThreadTaskRunnerHandle::Get());
+ set_task_runner(SequencedTaskRunnerHandle::Get());
callback_ = callback;
FSEventStreamEventId start_event = FSEventsGetCurrentEventId();
@@ -107,11 +110,15 @@ void FilePathWatcherFSEvents::Cancel() {
set_cancelled();
callback_.Reset();
- // Switch to the dispatch queue to tear down the event stream. As the queue
- // is owned by this object, and this method is called from the destructor,
- // execute the block synchronously.
+ // Switch to the dispatch queue to tear down the event stream. As the queue is
+ // owned by |this|, and this method is called from the destructor, execute the
+ // block synchronously.
dispatch_sync(queue_, ^{
- CancelOnMessageLoopThread();
+ if (fsevent_stream_) {
+ DestroyEventStream();
+ target_.clear();
+ resolved_target_.clear();
+ }
});
}
@@ -142,31 +149,40 @@ void FilePathWatcherFSEvents::FSEventsCallback(
// the directory to be watched gets created.
if (root_changed) {
// Resetting the event stream from within the callback fails (FSEvents spews
- // bad file descriptor errors), so post a task to do the reset.
- dispatch_async(watcher->queue_, ^{
- watcher->UpdateEventStream(root_change_at);
- });
+ // bad file descriptor errors), so do the reset asynchronously.
+ //
+ // We can't dispatch_async a call to UpdateEventStream() directly because
+ // there would be no guarantee that |watcher| still exists when it runs.
+ //
+ // Instead, bounce on task_runner() and use a WeakPtr to verify that
+ // |watcher| still exists. If it does, dispatch_async a call to
+ // UpdateEventStream(). Because the destructor of |watcher| runs on
+ // task_runner() and calls dispatch_sync, it is guaranteed that |watcher|
+ // still exists when UpdateEventStream() runs.
+ watcher->task_runner()->PostTask(
+ FROM_HERE, Bind(
+ [](WeakPtr<FilePathWatcherFSEvents> weak_watcher,
+ FSEventStreamEventId root_change_at) {
+ if (!weak_watcher)
+ return;
+ FilePathWatcherFSEvents* watcher = weak_watcher.get();
+ dispatch_async(watcher->queue_, ^{
+ watcher->UpdateEventStream(root_change_at);
+ });
+ },
+ watcher->weak_factory_.GetWeakPtr(), root_change_at));
}
watcher->OnFilePathsChanged(paths);
}
-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) {
DCHECK(!resolved_target_.empty());
task_runner()->PostTask(
- FROM_HERE, Bind(&FilePathWatcherFSEvents::DispatchEvents, this, paths,
- target_, resolved_target_));
+ FROM_HERE,
+ Bind(&FilePathWatcherFSEvents::DispatchEvents, weak_factory_.GetWeakPtr(),
+ paths, target_, resolved_target_));
}
void FilePathWatcherFSEvents::DispatchEvents(const std::vector<FilePath>& paths,
@@ -187,18 +203,6 @@ void FilePathWatcherFSEvents::DispatchEvents(const std::vector<FilePath>& paths,
}
}
-void FilePathWatcherFSEvents::CancelOnMessageLoopThread() {
- // For all other implementations, the "message loop thread" is the IO thread,
- // as returned by task_runner(). This implementation, however, needs to
- // cancel pending work on the Dispatch Queue thread.
-
- if (fsevent_stream_) {
- DestroyEventStream();
- target_.clear();
- resolved_target_.clear();
- }
-}
-
void FilePathWatcherFSEvents::UpdateEventStream(
FSEventStreamEventId start_event) {
// It can happen that the watcher gets canceled while tasks that call this
@@ -234,8 +238,9 @@ void FilePathWatcherFSEvents::UpdateEventStream(
FSEventStreamSetDispatchQueue(fsevent_stream_, queue_);
if (!FSEventStreamStart(fsevent_stream_)) {
- task_runner()->PostTask(
- FROM_HERE, Bind(&FilePathWatcherFSEvents::ReportError, this, target_));
+ task_runner()->PostTask(FROM_HERE,
+ Bind(&FilePathWatcherFSEvents::ReportError,
+ weak_factory_.GetWeakPtr(), target_));
}
}
@@ -244,8 +249,9 @@ bool FilePathWatcherFSEvents::ResolveTargetPath() {
bool changed = resolved != resolved_target_;
resolved_target_ = resolved;
if (resolved_target_.empty()) {
- task_runner()->PostTask(
- FROM_HERE, Bind(&FilePathWatcherFSEvents::ReportError, this, target_));
+ task_runner()->PostTask(FROM_HERE,
+ Bind(&FilePathWatcherFSEvents::ReportError,
+ weak_factory_.GetWeakPtr(), target_));
}
return changed;
}
diff --git a/base/files/file_path_watcher_fsevents.h b/base/files/file_path_watcher_fsevents.h
index cfbe020b51..dcdf2fbf9d 100644
--- a/base/files/file_path_watcher_fsevents.h
+++ b/base/files/file_path_watcher_fsevents.h
@@ -14,6 +14,7 @@
#include "base/files/file_path_watcher.h"
#include "base/mac/scoped_dispatch_object.h"
#include "base/macros.h"
+#include "base/memory/weak_ptr.h"
namespace base {
@@ -26,6 +27,7 @@ namespace base {
class FilePathWatcherFSEvents : public FilePathWatcher::PlatformDelegate {
public:
FilePathWatcherFSEvents();
+ ~FilePathWatcherFSEvents() override;
// FilePathWatcher::PlatformDelegate overrides.
bool Watch(const FilePath& path,
@@ -41,8 +43,6 @@ class FilePathWatcherFSEvents : public FilePathWatcher::PlatformDelegate {
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);
@@ -53,9 +53,6 @@ class FilePathWatcherFSEvents : public FilePathWatcher::PlatformDelegate {
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);
@@ -92,6 +89,8 @@ class FilePathWatcherFSEvents : public FilePathWatcher::PlatformDelegate {
// (Only accessed from the libdispatch queue.)
FSEventStreamRef fsevent_stream_;
+ WeakPtrFactory<FilePathWatcherFSEvents> weak_factory_;
+
DISALLOW_COPY_AND_ASSIGN(FilePathWatcherFSEvents);
};
diff --git a/base/files/file_path_watcher_kqueue.cc b/base/files/file_path_watcher_kqueue.cc
index 6d034cd9a2..a28726acb0 100644
--- a/base/files/file_path_watcher_kqueue.cc
+++ b/base/files/file_path_watcher_kqueue.cc
@@ -12,7 +12,7 @@
#include "base/files/file_util.h"
#include "base/logging.h"
#include "base/strings/stringprintf.h"
-#include "base/threading/thread_task_runner_handle.h"
+#include "base/threading/sequenced_task_runner_handle.h"
// On some platforms these are not defined.
#if !defined(EV_RECEIPT)
@@ -26,7 +26,9 @@ namespace base {
FilePathWatcherKQueue::FilePathWatcherKQueue() : kqueue_(-1) {}
-FilePathWatcherKQueue::~FilePathWatcherKQueue() {}
+FilePathWatcherKQueue::~FilePathWatcherKQueue() {
+ DCHECK(!task_runner() || task_runner()->RunsTasksOnCurrentThread());
+}
void FilePathWatcherKQueue::ReleaseEvent(struct kevent& event) {
CloseFileDescriptor(&event.ident);
@@ -36,7 +38,6 @@ void FilePathWatcherKQueue::ReleaseEvent(struct kevent& event) {
}
int FilePathWatcherKQueue::EventsForPath(FilePath path, EventVector* events) {
- DCHECK(MessageLoopForIO::current());
// Make sure that we are working with a clean slate.
DCHECK(events->empty());
@@ -230,9 +231,74 @@ bool FilePathWatcherKQueue::UpdateWatches(bool* target_file_affected) {
return true;
}
-void FilePathWatcherKQueue::OnFileCanReadWithoutBlocking(int fd) {
- DCHECK(MessageLoopForIO::current());
- DCHECK_EQ(fd, kqueue_);
+bool FilePathWatcherKQueue::Watch(const FilePath& path,
+ bool recursive,
+ const FilePathWatcher::Callback& callback) {
+ DCHECK(target_.value().empty()); // Can only watch one path.
+ DCHECK(!callback.is_null());
+ DCHECK_EQ(kqueue_, -1);
+ // Recursive watch is not supported using kqueue.
+ DCHECK(!recursive);
+
+ callback_ = callback;
+ target_ = path;
+
+ set_task_runner(SequencedTaskRunnerHandle::Get());
+
+ kqueue_ = kqueue();
+ if (kqueue_ == -1) {
+ DPLOG(ERROR) << "kqueue";
+ return false;
+ }
+
+ int last_entry = EventsForPath(target_, &events_);
+ DCHECK_NE(last_entry, 0);
+
+ EventVector responses(last_entry);
+
+ int count = HANDLE_EINTR(kevent(kqueue_, &events_[0], last_entry,
+ &responses[0], last_entry, NULL));
+ if (!AreKeventValuesValid(&responses[0], count)) {
+ // Calling Cancel() here to close any file descriptors that were opened.
+ // This would happen in the destructor anyways, but FilePathWatchers tend to
+ // be long lived, and if an error has occurred, there is no reason to waste
+ // the file descriptors.
+ Cancel();
+ return false;
+ }
+
+ // It's safe to use Unretained() because the watch is cancelled and the
+ // callback cannot be invoked after |kqueue_watch_controller_| (which is a
+ // member of |this|) has been deleted.
+ kqueue_watch_controller_ = FileDescriptorWatcher::WatchReadable(
+ kqueue_,
+ Bind(&FilePathWatcherKQueue::OnKQueueReadable, Unretained(this)));
+
+ return true;
+}
+
+void FilePathWatcherKQueue::Cancel() {
+ if (!task_runner()) {
+ set_cancelled();
+ return;
+ }
+
+ DCHECK(task_runner()->RunsTasksOnCurrentThread());
+ if (!is_cancelled()) {
+ set_cancelled();
+ kqueue_watch_controller_.reset();
+ if (IGNORE_EINTR(close(kqueue_)) != 0) {
+ DPLOG(ERROR) << "close kqueue";
+ }
+ kqueue_ = -1;
+ std::for_each(events_.begin(), events_.end(), ReleaseEvent);
+ events_.clear();
+ callback_.Reset();
+ }
+}
+
+void FilePathWatcherKQueue::OnKQueueReadable() {
+ DCHECK(task_runner()->RunsTasksOnCurrentThread());
DCHECK(events_.size());
// Request the file system update notifications that have occurred and return
@@ -303,89 +369,4 @@ void FilePathWatcherKQueue::OnFileCanReadWithoutBlocking(int fd) {
}
}
-void FilePathWatcherKQueue::OnFileCanWriteWithoutBlocking(int /* fd */) {
- NOTREACHED();
-}
-
-void FilePathWatcherKQueue::WillDestroyCurrentMessageLoop() {
- CancelOnMessageLoopThread();
-}
-
-bool FilePathWatcherKQueue::Watch(const FilePath& path,
- bool recursive,
- const FilePathWatcher::Callback& callback) {
- DCHECK(MessageLoopForIO::current());
- DCHECK(target_.value().empty()); // Can only watch one path.
- DCHECK(!callback.is_null());
- DCHECK_EQ(kqueue_, -1);
-
- if (recursive) {
- // Recursive watch is not supported using kqueue.
- NOTIMPLEMENTED();
- return false;
- }
-
- callback_ = callback;
- target_ = path;
-
- MessageLoop::current()->AddDestructionObserver(this);
- io_task_runner_ = ThreadTaskRunnerHandle::Get();
-
- kqueue_ = kqueue();
- if (kqueue_ == -1) {
- DPLOG(ERROR) << "kqueue";
- return false;
- }
-
- int last_entry = EventsForPath(target_, &events_);
- DCHECK_NE(last_entry, 0);
-
- EventVector responses(last_entry);
-
- int count = HANDLE_EINTR(kevent(kqueue_, &events_[0], last_entry,
- &responses[0], last_entry, NULL));
- if (!AreKeventValuesValid(&responses[0], count)) {
- // Calling Cancel() here to close any file descriptors that were opened.
- // This would happen in the destructor anyways, but FilePathWatchers tend to
- // be long lived, and if an error has occurred, there is no reason to waste
- // the file descriptors.
- Cancel();
- return false;
- }
-
- return MessageLoopForIO::current()->WatchFileDescriptor(
- kqueue_, true, MessageLoopForIO::WATCH_READ, &kqueue_watcher_, this);
-}
-
-void FilePathWatcherKQueue::Cancel() {
- SingleThreadTaskRunner* task_runner = io_task_runner_.get();
- if (!task_runner) {
- set_cancelled();
- return;
- }
- if (!task_runner->BelongsToCurrentThread()) {
- task_runner->PostTask(FROM_HERE,
- base::Bind(&FilePathWatcherKQueue::Cancel, this));
- return;
- }
- CancelOnMessageLoopThread();
-}
-
-void FilePathWatcherKQueue::CancelOnMessageLoopThread() {
- DCHECK(MessageLoopForIO::current());
- if (!is_cancelled()) {
- set_cancelled();
- kqueue_watcher_.StopWatchingFileDescriptor();
- if (IGNORE_EINTR(close(kqueue_)) != 0) {
- DPLOG(ERROR) << "close kqueue";
- }
- kqueue_ = -1;
- std::for_each(events_.begin(), events_.end(), ReleaseEvent);
- events_.clear();
- io_task_runner_ = NULL;
- MessageLoop::current()->RemoveDestructionObserver(this);
- callback_.Reset();
- }
-}
-
} // namespace base
diff --git a/base/files/file_path_watcher_kqueue.h b/base/files/file_path_watcher_kqueue.h
index d9db8c2587..ef79be5596 100644
--- a/base/files/file_path_watcher_kqueue.h
+++ b/base/files/file_path_watcher_kqueue.h
@@ -6,13 +6,14 @@
#define BASE_FILES_FILE_PATH_WATCHER_KQUEUE_H_
#include <sys/event.h>
+
+#include <memory>
#include <vector>
+#include "base/files/file_descriptor_watcher_posix.h"
#include "base/files/file_path.h"
#include "base/files/file_path_watcher.h"
#include "base/macros.h"
-#include "base/message_loop/message_loop.h"
-#include "base/single_thread_task_runner.h"
namespace base {
@@ -27,18 +28,10 @@ namespace base {
// detect the creation and deletion of files, just not the modification of
// files. It does however detect the attribute changes that the FSEvents impl
// would miss.
-class FilePathWatcherKQueue : public FilePathWatcher::PlatformDelegate,
- public MessageLoopForIO::Watcher,
- public MessageLoop::DestructionObserver {
+class FilePathWatcherKQueue : public FilePathWatcher::PlatformDelegate {
public:
FilePathWatcherKQueue();
-
- // MessageLoopForIO::Watcher overrides.
- void OnFileCanReadWithoutBlocking(int fd) override;
- void OnFileCanWriteWithoutBlocking(int fd) override;
-
- // MessageLoop::DestructionObserver overrides.
- void WillDestroyCurrentMessageLoop() override;
+ ~FilePathWatcherKQueue() override;
// FilePathWatcher::PlatformDelegate overrides.
bool Watch(const FilePath& path,
@@ -46,9 +39,6 @@ class FilePathWatcherKQueue : public FilePathWatcher::PlatformDelegate,
const FilePathWatcher::Callback& callback) override;
void Cancel() override;
- protected:
- ~FilePathWatcherKQueue() override;
-
private:
class EventData {
public:
@@ -60,8 +50,8 @@ class FilePathWatcherKQueue : public FilePathWatcher::PlatformDelegate,
typedef std::vector<struct kevent> EventVector;
- // Can only be called on |io_task_runner_|'s thread.
- void CancelOnMessageLoopThread() override;
+ // Called when data is available in |kqueue_|.
+ void OnKQueueReadable();
// Returns true if the kevent values are error free.
bool AreKeventValuesValid(struct kevent* kevents, int count);
@@ -119,12 +109,14 @@ class FilePathWatcherKQueue : public FilePathWatcher::PlatformDelegate,
}
EventVector events_;
- scoped_refptr<base::SingleThreadTaskRunner> io_task_runner_;
- MessageLoopForIO::FileDescriptorWatcher kqueue_watcher_;
FilePathWatcher::Callback callback_;
FilePath target_;
int kqueue_;
+ // Throughout the lifetime of this, OnKQueueReadable() will be called when
+ // data is available in |kqueue_|.
+ std::unique_ptr<FileDescriptorWatcher::Controller> kqueue_watch_controller_;
+
DISALLOW_COPY_AND_ASSIGN(FilePathWatcherKQueue);
};
diff --git a/base/files/file_path_watcher_linux.cc b/base/files/file_path_watcher_linux.cc
index 87bddd3dea..1dc833dc88 100644
--- a/base/files/file_path_watcher_linux.cc
+++ b/base/files/file_path_watcher_linux.cc
@@ -28,12 +28,14 @@
#include "base/location.h"
#include "base/logging.h"
#include "base/macros.h"
+#include "base/memory/ptr_util.h"
+#include "base/memory/weak_ptr.h"
#include "base/posix/eintr_wrapper.h"
#include "base/single_thread_task_runner.h"
#include "base/stl_util.h"
#include "base/synchronization/lock.h"
+#include "base/threading/sequenced_task_runner_handle.h"
#include "base/threading/thread.h"
-#include "base/threading/thread_task_runner_handle.h"
#include "base/trace_event/trace_event.h"
namespace base {
@@ -61,12 +63,14 @@ class InotifyReader {
void OnInotifyEvent(const inotify_event* event);
private:
- friend struct DefaultLazyInstanceTraits<InotifyReader>;
+ friend struct LazyInstanceTraitsBase<InotifyReader>;
typedef std::set<FilePathWatcherImpl*> WatcherSet;
InotifyReader();
- ~InotifyReader();
+ // There is no destructor because |g_inotify_reader| is a
+ // base::LazyInstace::Leaky object. Having a destructor causes build
+ // issues with GCC 6 (http://crbug.com/636346).
// We keep track of which delegates want to be notified on which watches.
hash_map<Watch, WatcherSet> watchers_;
@@ -80,19 +84,16 @@ class InotifyReader {
// File descriptor returned by inotify_init.
const int inotify_fd_;
- // Use self-pipe trick to unblock select during shutdown.
- int shutdown_pipe_[2];
-
// Flag set to true when startup was successful.
bool valid_;
DISALLOW_COPY_AND_ASSIGN(InotifyReader);
};
-class FilePathWatcherImpl : public FilePathWatcher::PlatformDelegate,
- public MessageLoop::DestructionObserver {
+class FilePathWatcherImpl : public FilePathWatcher::PlatformDelegate {
public:
FilePathWatcherImpl();
+ ~FilePathWatcherImpl() override;
// Called for each event coming from the watch. |fired_watch| identifies the
// watch that fired, |child| indicates what has changed, and is relative to
@@ -107,10 +108,13 @@ class FilePathWatcherImpl : public FilePathWatcher::PlatformDelegate,
bool deleted,
bool is_dir);
- protected:
- ~FilePathWatcherImpl() override {}
-
private:
+ void OnFilePathChangedOnOriginSequence(InotifyReader::Watch fired_watch,
+ const FilePath::StringType& child,
+ bool created,
+ bool deleted,
+ bool is_dir);
+
// Start watching |path| for changes and notify |delegate| on each change.
// Returns true if watch for |path| has been added successfully.
bool Watch(const FilePath& path,
@@ -120,14 +124,6 @@ class FilePathWatcherImpl : public FilePathWatcher::PlatformDelegate,
// Cancel the watch. This unregisters the instance with InotifyReader.
void Cancel() override;
- // Cleans up and stops observing the message_loop() thread.
- void CancelOnMessageLoopThread() override;
-
- // Deletion of the FilePathWatcher will call Cancel() to dispose of this
- // object in the right thread. This also observes destruction of the required
- // cleanup thread, in case it quits before Cancel() is called.
- void WillDestroyCurrentMessageLoop() override;
-
// Inotify watches are installed for all directory components of |target_|.
// A WatchEntry instance holds:
// - |watch|: the watch descriptor for a component.
@@ -191,16 +187,15 @@ class FilePathWatcherImpl : public FilePathWatcher::PlatformDelegate,
hash_map<InotifyReader::Watch, FilePath> recursive_paths_by_watch_;
std::map<FilePath, InotifyReader::Watch> recursive_watches_by_path_;
+ WeakPtrFactory<FilePathWatcherImpl> weak_factory_;
+
DISALLOW_COPY_AND_ASSIGN(FilePathWatcherImpl);
};
-void InotifyReaderCallback(InotifyReader* reader, int inotify_fd,
- int shutdown_fd) {
+void InotifyReaderCallback(InotifyReader* reader, int inotify_fd) {
// Make sure the file descriptors are good for use with select().
CHECK_LE(0, inotify_fd);
CHECK_GT(FD_SETSIZE, inotify_fd);
- CHECK_LE(0, shutdown_fd);
- CHECK_GT(FD_SETSIZE, shutdown_fd);
trace_event::TraceLog::GetInstance()->SetCurrentThreadBlocksMessageLoop();
@@ -208,20 +203,15 @@ void InotifyReaderCallback(InotifyReader* reader, int inotify_fd,
fd_set rfds;
FD_ZERO(&rfds);
FD_SET(inotify_fd, &rfds);
- FD_SET(shutdown_fd, &rfds);
// Wait until some inotify events are available.
int select_result =
- HANDLE_EINTR(select(std::max(inotify_fd, shutdown_fd) + 1,
- &rfds, NULL, NULL, NULL));
+ HANDLE_EINTR(select(inotify_fd + 1, &rfds, NULL, NULL, NULL));
if (select_result < 0) {
DPLOG(WARNING) << "select failed";
return;
}
- if (FD_ISSET(shutdown_fd, &rfds))
- return;
-
// Adjust buffer size to current event queue size.
int buffer_size;
int ioctl_result = HANDLE_EINTR(ioctl(inotify_fd, FIONREAD,
@@ -263,33 +253,14 @@ InotifyReader::InotifyReader()
if (inotify_fd_ < 0)
PLOG(ERROR) << "inotify_init() failed";
- shutdown_pipe_[0] = -1;
- shutdown_pipe_[1] = -1;
- if (inotify_fd_ >= 0 && pipe(shutdown_pipe_) == 0 && thread_.Start()) {
+ if (inotify_fd_ >= 0 && thread_.Start()) {
thread_.task_runner()->PostTask(
FROM_HERE,
- Bind(&InotifyReaderCallback, this, inotify_fd_, shutdown_pipe_[0]));
+ Bind(&InotifyReaderCallback, this, inotify_fd_));
valid_ = true;
}
}
-InotifyReader::~InotifyReader() {
- if (valid_) {
- // Write to the self-pipe so that the select call in InotifyReaderTask
- // returns.
- ssize_t ret = HANDLE_EINTR(write(shutdown_pipe_[1], "", 1));
- DPCHECK(ret > 0);
- DCHECK_EQ(ret, 1);
- thread_.Stop();
- }
- if (inotify_fd_ >= 0)
- close(inotify_fd_);
- if (shutdown_pipe_[0] >= 0)
- close(shutdown_pipe_[0]);
- if (shutdown_pipe_[1] >= 0)
- close(shutdown_pipe_[1]);
-}
-
InotifyReader::Watch InotifyReader::AddWatch(
const FilePath& path, FilePathWatcherImpl* watcher) {
if (!valid_)
@@ -343,7 +314,10 @@ void InotifyReader::OnInotifyEvent(const inotify_event* event) {
}
FilePathWatcherImpl::FilePathWatcherImpl()
- : recursive_(false) {
+ : recursive_(false), weak_factory_(this) {}
+
+FilePathWatcherImpl::~FilePathWatcherImpl() {
+ DCHECK(!task_runner() || task_runner()->RunsTasksOnCurrentThread());
}
void FilePathWatcherImpl::OnFilePathChanged(InotifyReader::Watch fired_watch,
@@ -351,22 +325,25 @@ void FilePathWatcherImpl::OnFilePathChanged(InotifyReader::Watch fired_watch,
bool created,
bool deleted,
bool 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;
- }
-
- // Check to see if CancelOnMessageLoopThread() has already been called.
- // May happen when code flow reaches here from the PostTask() above.
- if (watches_.empty()) {
- DCHECK(target_.empty());
- return;
- }
+ DCHECK(!task_runner()->RunsTasksOnCurrentThread());
+
+ // This method is invoked on the Inotify thread. Switch to task_runner() to
+ // access |watches_| safely. Use a WeakPtr to prevent the callback from
+ // running after |this| is destroyed (i.e. after the watch is cancelled).
+ task_runner()->PostTask(
+ FROM_HERE, Bind(&FilePathWatcherImpl::OnFilePathChangedOnOriginSequence,
+ weak_factory_.GetWeakPtr(), fired_watch, child, created,
+ deleted, is_dir));
+}
- DCHECK(MessageLoopForIO::current());
+void FilePathWatcherImpl::OnFilePathChangedOnOriginSequence(
+ InotifyReader::Watch fired_watch,
+ const FilePath::StringType& child,
+ bool created,
+ bool deleted,
+ bool is_dir) {
+ DCHECK(task_runner()->RunsTasksOnCurrentThread());
+ DCHECK(!watches_.empty());
DCHECK(HasValidWatchVector());
// Used below to avoid multiple recursive updates.
@@ -451,13 +428,11 @@ bool FilePathWatcherImpl::Watch(const FilePath& path,
bool recursive,
const FilePathWatcher::Callback& callback) {
DCHECK(target_.empty());
- DCHECK(MessageLoopForIO::current());
- set_task_runner(ThreadTaskRunnerHandle::Get());
+ set_task_runner(SequencedTaskRunnerHandle::Get());
callback_ = callback;
target_ = path;
recursive_ = recursive;
- MessageLoop::current()->AddDestructionObserver(this);
std::vector<FilePath::StringType> comps;
target_.GetComponents(&comps);
@@ -470,47 +445,29 @@ bool FilePathWatcherImpl::Watch(const FilePath& path,
}
void FilePathWatcherImpl::Cancel() {
- if (callback_.is_null()) {
- // Watch was never called, or the message_loop() thread is already gone.
+ if (!callback_) {
+ // Watch() was never called.
set_cancelled();
return;
}
- // Switch to the message_loop() if necessary so we can access |watches_|.
- if (!task_runner()->BelongsToCurrentThread()) {
- task_runner()->PostTask(FROM_HERE, Bind(&FilePathWatcher::CancelWatch,
- make_scoped_refptr(this)));
- } else {
- CancelOnMessageLoopThread();
- }
-}
+ DCHECK(task_runner()->RunsTasksOnCurrentThread());
+ DCHECK(!is_cancelled());
-void FilePathWatcherImpl::CancelOnMessageLoopThread() {
- DCHECK(task_runner()->BelongsToCurrentThread());
set_cancelled();
-
- if (!callback_.is_null()) {
- MessageLoop::current()->RemoveDestructionObserver(this);
- callback_.Reset();
- }
+ callback_.Reset();
for (size_t i = 0; i < watches_.size(); ++i)
g_inotify_reader.Get().RemoveWatch(watches_[i].watch, this);
watches_.clear();
target_.clear();
-
- if (recursive_)
- RemoveRecursiveWatches();
-}
-
-void FilePathWatcherImpl::WillDestroyCurrentMessageLoop() {
- CancelOnMessageLoopThread();
+ RemoveRecursiveWatches();
}
void FilePathWatcherImpl::UpdateWatches() {
- // Ensure this runs on the message_loop() exclusively in order to avoid
+ // Ensure this runs on the task_runner() exclusively in order to avoid
// concurrency issues.
- DCHECK(task_runner()->BelongsToCurrentThread());
+ DCHECK(task_runner()->RunsTasksOnCurrentThread());
DCHECK(HasValidWatchVector());
// Walk the list of watches and update them as we go.
@@ -541,6 +498,8 @@ void FilePathWatcherImpl::UpdateWatches() {
void FilePathWatcherImpl::UpdateRecursiveWatches(
InotifyReader::Watch fired_watch,
bool is_dir) {
+ DCHECK(HasValidWatchVector());
+
if (!recursive_)
return;
@@ -551,7 +510,8 @@ void FilePathWatcherImpl::UpdateRecursiveWatches(
// Check to see if this is a forced update or if some component of |target_|
// has changed. For these cases, redo the watches for |target_| and below.
- if (!ContainsKey(recursive_paths_by_watch_, fired_watch)) {
+ if (!ContainsKey(recursive_paths_by_watch_, fired_watch) &&
+ fired_watch != watches_.back().watch) {
UpdateRecursiveWatchesForPath(target_);
return;
}
@@ -560,7 +520,10 @@ void FilePathWatcherImpl::UpdateRecursiveWatches(
if (!is_dir)
return;
- const FilePath& changed_dir = recursive_paths_by_watch_[fired_watch];
+ const FilePath& changed_dir =
+ ContainsKey(recursive_paths_by_watch_, fired_watch) ?
+ recursive_paths_by_watch_[fired_watch] :
+ target_;
std::map<FilePath, InotifyReader::Watch>::iterator start_it =
recursive_watches_by_path_.lower_bound(changed_dir);
@@ -683,7 +646,8 @@ bool FilePathWatcherImpl::HasValidWatchVector() const {
} // namespace
FilePathWatcher::FilePathWatcher() {
- impl_ = new FilePathWatcherImpl();
+ sequence_checker_.DetachFromSequence();
+ impl_ = MakeUnique<FilePathWatcherImpl>();
}
} // namespace base
diff --git a/base/files/file_path_watcher_mac.cc b/base/files/file_path_watcher_mac.cc
index 7338eafa44..2520b9288a 100644
--- a/base/files/file_path_watcher_mac.cc
+++ b/base/files/file_path_watcher_mac.cc
@@ -2,8 +2,12 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
+#include <memory>
+
#include "base/files/file_path_watcher.h"
#include "base/files/file_path_watcher_kqueue.h"
+#include "base/macros.h"
+#include "base/memory/ptr_util.h"
#include "build/build_config.h"
#if !defined(OS_IOS)
@@ -16,6 +20,9 @@ namespace {
class FilePathWatcherImpl : public FilePathWatcher::PlatformDelegate {
public:
+ FilePathWatcherImpl() = default;
+ ~FilePathWatcherImpl() override = default;
+
bool Watch(const FilePath& path,
bool recursive,
const FilePathWatcher::Callback& callback) override {
@@ -25,10 +32,10 @@ class FilePathWatcherImpl : public FilePathWatcher::PlatformDelegate {
if (!FilePathWatcher::RecursiveWatchAvailable())
return false;
#if !defined(OS_IOS)
- impl_ = new FilePathWatcherFSEvents();
+ impl_ = MakeUnique<FilePathWatcherFSEvents>();
#endif // OS_IOS
} else {
- impl_ = new FilePathWatcherKQueue();
+ impl_ = MakeUnique<FilePathWatcherKQueue>();
}
DCHECK(impl_.get());
return impl_->Watch(path, recursive, callback);
@@ -40,22 +47,17 @@ class FilePathWatcherImpl : public FilePathWatcher::PlatformDelegate {
set_cancelled();
}
- void CancelOnMessageLoopThread() override {
- if (impl_.get())
- impl_->Cancel();
- set_cancelled();
- }
-
- protected:
- ~FilePathWatcherImpl() override {}
+ private:
+ std::unique_ptr<PlatformDelegate> impl_;
- scoped_refptr<PlatformDelegate> impl_;
+ DISALLOW_COPY_AND_ASSIGN(FilePathWatcherImpl);
};
} // namespace
FilePathWatcher::FilePathWatcher() {
- impl_ = new FilePathWatcherImpl();
+ sequence_checker_.DetachFromSequence();
+ impl_ = MakeUnique<FilePathWatcherImpl>();
}
} // namespace base
diff --git a/base/files/file_path_watcher_unittest.cc b/base/files/file_path_watcher_unittest.cc
index a40e4858b4..d2ec37bbec 100644
--- a/base/files/file_path_watcher_unittest.cc
+++ b/base/files/file_path_watcher_unittest.cc
@@ -28,7 +28,6 @@
#include "base/synchronization/waitable_event.h"
#include "base/test/test_file_util.h"
#include "base/test/test_timeouts.h"
-#include "base/threading/thread.h"
#include "base/threading/thread_task_runner_handle.h"
#include "build/build_config.h"
#include "testing/gtest/include/gtest/gtest.h"
@@ -37,6 +36,10 @@
#include "base/android/path_utils.h"
#endif // defined(OS_ANDROID)
+#if defined(OS_POSIX)
+#include "base/files/file_descriptor_watcher_posix.h"
+#endif // defined(OS_POSIX)
+
namespace base {
namespace {
@@ -131,30 +134,19 @@ class TestDelegate : public TestDelegateBase {
DISALLOW_COPY_AND_ASSIGN(TestDelegate);
};
-void SetupWatchCallback(const FilePath& target,
- FilePathWatcher* watcher,
- TestDelegateBase* delegate,
- bool recursive_watch,
- bool* result,
- base::WaitableEvent* completion) {
- *result = watcher->Watch(target, recursive_watch,
- base::Bind(&TestDelegateBase::OnFileChanged,
- delegate->AsWeakPtr()));
- completion->Signal();
-}
-
class FilePathWatcherTest : public testing::Test {
public:
FilePathWatcherTest()
- : file_thread_("FilePathWatcherTest") {}
+#if defined(OS_POSIX)
+ : file_descriptor_watcher_(&loop_)
+#endif
+ {
+ }
~FilePathWatcherTest() override {}
protected:
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
@@ -171,16 +163,12 @@ class FilePathWatcherTest : public testing::Test {
void TearDown() override { RunLoop().RunUntilIdle(); }
- void DeleteDelegateOnFileThread(TestDelegate* delegate) {
- file_thread_.task_runner()->DeleteSoon(FROM_HERE, delegate);
- }
-
FilePath test_file() {
- return temp_dir_.path().AppendASCII("FilePathWatcherTest");
+ return temp_dir_.GetPath().AppendASCII("FilePathWatcherTest");
}
FilePath test_link() {
- return temp_dir_.path().AppendASCII("FilePathWatcherTest.lnk");
+ return temp_dir_.GetPath().AppendASCII("FilePathWatcherTest.lnk");
}
// Write |content| to |file|. Returns true on success.
@@ -196,18 +184,23 @@ class FilePathWatcherTest : public testing::Test {
bool WaitForEvents() WARN_UNUSED_RESULT {
collector_->Reset();
+
+ RunLoop run_loop;
// Make sure we timeout if we don't get notified.
- loop_.PostDelayedTask(FROM_HERE,
- MessageLoop::QuitWhenIdleClosure(),
- TestTimeouts::action_timeout());
- RunLoop().Run();
+ ThreadTaskRunnerHandle::Get()->PostDelayedTask(
+ FROM_HERE, run_loop.QuitWhenIdleClosure(),
+ TestTimeouts::action_timeout());
+ run_loop.Run();
return collector_->Success();
}
NotificationCollector* collector() { return collector_.get(); }
- MessageLoop loop_;
- base::Thread file_thread_;
+ MessageLoopForIO loop_;
+#if defined(OS_POSIX)
+ FileDescriptorWatcher file_descriptor_watcher_;
+#endif
+
ScopedTempDir temp_dir_;
scoped_refptr<NotificationCollector> collector_;
@@ -219,14 +212,9 @@ bool FilePathWatcherTest::SetupWatch(const FilePath& target,
FilePathWatcher* watcher,
TestDelegateBase* delegate,
bool recursive_watch) {
- base::WaitableEvent completion(WaitableEvent::ResetPolicy::AUTOMATIC,
- WaitableEvent::InitialState::NOT_SIGNALED);
- bool result;
- file_thread_.task_runner()->PostTask(
- FROM_HERE, base::Bind(SetupWatchCallback, target, watcher, delegate,
- recursive_watch, &result, &completion));
- completion.Wait();
- return result;
+ return watcher->Watch(
+ target, recursive_watch,
+ base::Bind(&TestDelegateBase::OnFileChanged, delegate->AsWeakPtr()));
}
// Basic test: Create the file and verify that we notice.
@@ -237,7 +225,6 @@ TEST_F(FilePathWatcherTest, NewFile) {
ASSERT_TRUE(WriteFile(test_file(), "content"));
ASSERT_TRUE(WaitForEvents());
- DeleteDelegateOnFileThread(delegate.release());
}
// Verify that modifying the file is caught.
@@ -251,12 +238,11 @@ TEST_F(FilePathWatcherTest, ModifiedFile) {
// Now make sure we get notified if the file is modified.
ASSERT_TRUE(WriteFile(test_file(), "new content"));
ASSERT_TRUE(WaitForEvents());
- DeleteDelegateOnFileThread(delegate.release());
}
// Verify that moving the file into place is caught.
TEST_F(FilePathWatcherTest, MovedFile) {
- FilePath source_file(temp_dir_.path().AppendASCII("source"));
+ FilePath source_file(temp_dir_.GetPath().AppendASCII("source"));
ASSERT_TRUE(WriteFile(source_file, "content"));
FilePathWatcher watcher;
@@ -266,7 +252,6 @@ TEST_F(FilePathWatcherTest, MovedFile) {
// Now make sure we get notified if the file is modified.
ASSERT_TRUE(base::Move(source_file, test_file()));
ASSERT_TRUE(WaitForEvents());
- DeleteDelegateOnFileThread(delegate.release());
}
TEST_F(FilePathWatcherTest, DeletedFile) {
@@ -279,7 +264,6 @@ TEST_F(FilePathWatcherTest, DeletedFile) {
// Now make sure we get notified if the file is deleted.
base::DeleteFile(test_file(), false);
ASSERT_TRUE(WaitForEvents());
- DeleteDelegateOnFileThread(delegate.release());
}
// Used by the DeleteDuringNotify test below.
@@ -327,11 +311,9 @@ TEST_F(FilePathWatcherTest, DeleteDuringNotify) {
// Flaky on MacOS (and ARM linux): http://crbug.com/85930
TEST_F(FilePathWatcherTest, DISABLED_DestroyWithPendingNotification) {
std::unique_ptr<TestDelegate> delegate(new TestDelegate(collector()));
- FilePathWatcher* watcher = new FilePathWatcher;
- ASSERT_TRUE(SetupWatch(test_file(), watcher, delegate.get(), false));
+ FilePathWatcher watcher;
+ ASSERT_TRUE(SetupWatch(test_file(), &watcher, delegate.get(), false));
ASSERT_TRUE(WriteFile(test_file(), "content"));
- file_thread_.task_runner()->DeleteSoon(FROM_HERE, watcher);
- DeleteDelegateOnFileThread(delegate.release());
}
TEST_F(FilePathWatcherTest, MultipleWatchersSingleFile) {
@@ -343,15 +325,13 @@ TEST_F(FilePathWatcherTest, MultipleWatchersSingleFile) {
ASSERT_TRUE(WriteFile(test_file(), "content"));
ASSERT_TRUE(WaitForEvents());
- DeleteDelegateOnFileThread(delegate1.release());
- DeleteDelegateOnFileThread(delegate2.release());
}
// Verify that watching a file whose parent directory doesn't exist yet works if
// the directory and file are created eventually.
TEST_F(FilePathWatcherTest, NonExistentDirectory) {
FilePathWatcher watcher;
- FilePath dir(temp_dir_.path().AppendASCII("dir"));
+ FilePath dir(temp_dir_.GetPath().AppendASCII("dir"));
FilePath file(dir.AppendASCII("file"));
std::unique_ptr<TestDelegate> delegate(new TestDelegate(collector()));
ASSERT_TRUE(SetupWatch(file, &watcher, delegate.get(), false));
@@ -370,13 +350,12 @@ TEST_F(FilePathWatcherTest, NonExistentDirectory) {
ASSERT_TRUE(base::DeleteFile(file, false));
VLOG(1) << "Waiting for file deletion";
ASSERT_TRUE(WaitForEvents());
- DeleteDelegateOnFileThread(delegate.release());
}
// Exercises watch reconfiguration for the case that directories on the path
// are rapidly created.
TEST_F(FilePathWatcherTest, DirectoryChain) {
- FilePath path(temp_dir_.path());
+ FilePath path(temp_dir_.GetPath());
std::vector<std::string> dir_names;
for (int i = 0; i < 20; i++) {
std::string dir(base::StringPrintf("d%d", i));
@@ -389,7 +368,7 @@ TEST_F(FilePathWatcherTest, DirectoryChain) {
std::unique_ptr<TestDelegate> delegate(new TestDelegate(collector()));
ASSERT_TRUE(SetupWatch(file, &watcher, delegate.get(), false));
- FilePath sub_path(temp_dir_.path());
+ FilePath sub_path(temp_dir_.GetPath());
for (std::vector<std::string>::const_iterator d(dir_names.begin());
d != dir_names.end(); ++d) {
sub_path = sub_path.AppendASCII(*d);
@@ -403,7 +382,6 @@ TEST_F(FilePathWatcherTest, DirectoryChain) {
ASSERT_TRUE(WriteFile(file, "content v2"));
VLOG(1) << "Waiting for file modification";
ASSERT_TRUE(WaitForEvents());
- DeleteDelegateOnFileThread(delegate.release());
}
#if defined(OS_MACOSX)
@@ -412,7 +390,7 @@ TEST_F(FilePathWatcherTest, DirectoryChain) {
#endif
TEST_F(FilePathWatcherTest, DisappearingDirectory) {
FilePathWatcher watcher;
- FilePath dir(temp_dir_.path().AppendASCII("dir"));
+ FilePath dir(temp_dir_.GetPath().AppendASCII("dir"));
FilePath file(dir.AppendASCII("file"));
ASSERT_TRUE(base::CreateDirectory(dir));
ASSERT_TRUE(WriteFile(file, "content"));
@@ -421,7 +399,6 @@ TEST_F(FilePathWatcherTest, DisappearingDirectory) {
ASSERT_TRUE(base::DeleteFile(dir, true));
ASSERT_TRUE(WaitForEvents());
- DeleteDelegateOnFileThread(delegate.release());
}
// Tests that a file that is deleted and reappears is tracked correctly.
@@ -438,12 +415,11 @@ TEST_F(FilePathWatcherTest, DeleteAndRecreate) {
ASSERT_TRUE(WriteFile(test_file(), "content"));
VLOG(1) << "Waiting for file creation";
ASSERT_TRUE(WaitForEvents());
- DeleteDelegateOnFileThread(delegate.release());
}
TEST_F(FilePathWatcherTest, WatchDirectory) {
FilePathWatcher watcher;
- FilePath dir(temp_dir_.path().AppendASCII("dir"));
+ FilePath dir(temp_dir_.GetPath().AppendASCII("dir"));
FilePath file1(dir.AppendASCII("file1"));
FilePath file2(dir.AppendASCII("file2"));
std::unique_ptr<TestDelegate> delegate(new TestDelegate(collector()));
@@ -471,14 +447,13 @@ TEST_F(FilePathWatcherTest, WatchDirectory) {
ASSERT_TRUE(WriteFile(file2, "content"));
VLOG(1) << "Waiting for file2 creation";
ASSERT_TRUE(WaitForEvents());
- DeleteDelegateOnFileThread(delegate.release());
}
TEST_F(FilePathWatcherTest, MoveParent) {
FilePathWatcher file_watcher;
FilePathWatcher subdir_watcher;
- FilePath dir(temp_dir_.path().AppendASCII("dir"));
- FilePath dest(temp_dir_.path().AppendASCII("dest"));
+ FilePath dir(temp_dir_.GetPath().AppendASCII("dir"));
+ FilePath dest(temp_dir_.GetPath().AppendASCII("dest"));
FilePath subdir(dir.AppendASCII("subdir"));
FilePath file(subdir.AppendASCII("file"));
std::unique_ptr<TestDelegate> file_delegate(new TestDelegate(collector()));
@@ -497,18 +472,15 @@ TEST_F(FilePathWatcherTest, MoveParent) {
base::Move(dir, dest);
VLOG(1) << "Waiting for directory move";
ASSERT_TRUE(WaitForEvents());
- DeleteDelegateOnFileThread(file_delegate.release());
- DeleteDelegateOnFileThread(subdir_delegate.release());
}
TEST_F(FilePathWatcherTest, RecursiveWatch) {
FilePathWatcher watcher;
- FilePath dir(temp_dir_.path().AppendASCII("dir"));
+ FilePath dir(temp_dir_.GetPath().AppendASCII("dir"));
std::unique_ptr<TestDelegate> delegate(new TestDelegate(collector()));
bool setup_result = SetupWatch(dir, &watcher, delegate.get(), true);
if (!FilePathWatcher::RecursiveWatchAvailable()) {
ASSERT_FALSE(setup_result);
- DeleteDelegateOnFileThread(delegate.release());
return;
}
ASSERT_TRUE(setup_result);
@@ -564,7 +536,6 @@ TEST_F(FilePathWatcherTest, RecursiveWatch) {
// Delete "$dir/subdir/subdir_child_dir/child_dir_file1".
ASSERT_TRUE(base::DeleteFile(child_dir_file1, false));
ASSERT_TRUE(WaitForEvents());
- DeleteDelegateOnFileThread(delegate.release());
}
#if defined(OS_POSIX)
@@ -581,14 +552,14 @@ TEST_F(FilePathWatcherTest, RecursiveWithSymLink) {
return;
FilePathWatcher watcher;
- FilePath test_dir(temp_dir_.path().AppendASCII("test_dir"));
+ FilePath test_dir(temp_dir_.GetPath().AppendASCII("test_dir"));
ASSERT_TRUE(base::CreateDirectory(test_dir));
FilePath symlink(test_dir.AppendASCII("symlink"));
std::unique_ptr<TestDelegate> delegate(new TestDelegate(collector()));
ASSERT_TRUE(SetupWatch(symlink, &watcher, delegate.get(), true));
// Link creation.
- FilePath target1(temp_dir_.path().AppendASCII("target1"));
+ FilePath target1(temp_dir_.GetPath().AppendASCII("target1"));
ASSERT_TRUE(base::CreateSymbolicLink(target1, symlink));
ASSERT_TRUE(WaitForEvents());
@@ -602,7 +573,7 @@ TEST_F(FilePathWatcherTest, RecursiveWithSymLink) {
ASSERT_TRUE(WaitForEvents());
// Link change.
- FilePath target2(temp_dir_.path().AppendASCII("target2"));
+ FilePath target2(temp_dir_.GetPath().AppendASCII("target2"));
ASSERT_TRUE(base::CreateDirectory(target2));
ASSERT_TRUE(base::DeleteFile(symlink, false));
ASSERT_TRUE(base::CreateSymbolicLink(target2, symlink));
@@ -612,18 +583,16 @@ TEST_F(FilePathWatcherTest, RecursiveWithSymLink) {
FilePath target2_file(target2.AppendASCII("file"));
ASSERT_TRUE(WriteFile(target2_file, "content"));
ASSERT_TRUE(WaitForEvents());
-
- DeleteDelegateOnFileThread(delegate.release());
}
#endif // OS_POSIX
TEST_F(FilePathWatcherTest, MoveChild) {
FilePathWatcher file_watcher;
FilePathWatcher subdir_watcher;
- FilePath source_dir(temp_dir_.path().AppendASCII("source"));
+ FilePath source_dir(temp_dir_.GetPath().AppendASCII("source"));
FilePath source_subdir(source_dir.AppendASCII("subdir"));
FilePath source_file(source_subdir.AppendASCII("file"));
- FilePath dest_dir(temp_dir_.path().AppendASCII("dest"));
+ FilePath dest_dir(temp_dir_.GetPath().AppendASCII("dest"));
FilePath dest_subdir(dest_dir.AppendASCII("subdir"));
FilePath dest_file(dest_subdir.AppendASCII("file"));
@@ -640,8 +609,6 @@ TEST_F(FilePathWatcherTest, MoveChild) {
// Move the directory into place, s.t. the watched file appears.
ASSERT_TRUE(base::Move(source_dir, dest_dir));
ASSERT_TRUE(WaitForEvents());
- DeleteDelegateOnFileThread(file_delegate.release());
- DeleteDelegateOnFileThread(subdir_delegate.release());
}
// Verify that changing attributes on a file is caught
@@ -662,7 +629,6 @@ TEST_F(FilePathWatcherTest, FileAttributesChanged) {
// Now make sure we get notified if the file is modified.
ASSERT_TRUE(base::MakeFileUnreadable(test_file()));
ASSERT_TRUE(WaitForEvents());
- DeleteDelegateOnFileThread(delegate.release());
}
#if defined(OS_LINUX)
@@ -678,7 +644,6 @@ TEST_F(FilePathWatcherTest, CreateLink) {
// Note that test_file() doesn't have to exist.
ASSERT_TRUE(CreateSymbolicLink(test_file(), test_link()));
ASSERT_TRUE(WaitForEvents());
- DeleteDelegateOnFileThread(delegate.release());
}
// Verify that deleting a symlink is caught.
@@ -694,7 +659,6 @@ TEST_F(FilePathWatcherTest, DeleteLink) {
// Now make sure we get notified if the link is deleted.
ASSERT_TRUE(base::DeleteFile(test_link(), false));
ASSERT_TRUE(WaitForEvents());
- DeleteDelegateOnFileThread(delegate.release());
}
// Verify that modifying a target file that a link is pointing to
@@ -710,7 +674,6 @@ TEST_F(FilePathWatcherTest, ModifiedLinkedFile) {
// Now make sure we get notified if the file is modified.
ASSERT_TRUE(WriteFile(test_file(), "new content"));
ASSERT_TRUE(WaitForEvents());
- DeleteDelegateOnFileThread(delegate.release());
}
// Verify that creating a target file that a link is pointing to
@@ -725,7 +688,6 @@ TEST_F(FilePathWatcherTest, CreateTargetLinkedFile) {
// Now make sure we get notified if the target file is created.
ASSERT_TRUE(WriteFile(test_file(), "content"));
ASSERT_TRUE(WaitForEvents());
- DeleteDelegateOnFileThread(delegate.release());
}
// Verify that deleting a target file that a link is pointing to
@@ -741,15 +703,14 @@ TEST_F(FilePathWatcherTest, DeleteTargetLinkedFile) {
// Now make sure we get notified if the target file is deleted.
ASSERT_TRUE(base::DeleteFile(test_file(), false));
ASSERT_TRUE(WaitForEvents());
- DeleteDelegateOnFileThread(delegate.release());
}
// Verify that watching a file whose parent directory is a link that
// doesn't exist yet works if the symlink is created eventually.
TEST_F(FilePathWatcherTest, LinkedDirectoryPart1) {
FilePathWatcher watcher;
- FilePath dir(temp_dir_.path().AppendASCII("dir"));
- FilePath link_dir(temp_dir_.path().AppendASCII("dir.lnk"));
+ FilePath dir(temp_dir_.GetPath().AppendASCII("dir"));
+ FilePath link_dir(temp_dir_.GetPath().AppendASCII("dir.lnk"));
FilePath file(dir.AppendASCII("file"));
FilePath linkfile(link_dir.AppendASCII("file"));
std::unique_ptr<TestDelegate> delegate(new TestDelegate(collector()));
@@ -770,15 +731,14 @@ TEST_F(FilePathWatcherTest, LinkedDirectoryPart1) {
ASSERT_TRUE(base::DeleteFile(file, false));
VLOG(1) << "Waiting for file deletion";
ASSERT_TRUE(WaitForEvents());
- DeleteDelegateOnFileThread(delegate.release());
}
// Verify that watching a file whose parent directory is a
// dangling symlink works if the directory is created eventually.
TEST_F(FilePathWatcherTest, LinkedDirectoryPart2) {
FilePathWatcher watcher;
- FilePath dir(temp_dir_.path().AppendASCII("dir"));
- FilePath link_dir(temp_dir_.path().AppendASCII("dir.lnk"));
+ FilePath dir(temp_dir_.GetPath().AppendASCII("dir"));
+ FilePath link_dir(temp_dir_.GetPath().AppendASCII("dir.lnk"));
FilePath file(dir.AppendASCII("file"));
FilePath linkfile(link_dir.AppendASCII("file"));
std::unique_ptr<TestDelegate> delegate(new TestDelegate(collector()));
@@ -800,15 +760,14 @@ TEST_F(FilePathWatcherTest, LinkedDirectoryPart2) {
ASSERT_TRUE(base::DeleteFile(file, false));
VLOG(1) << "Waiting for file deletion";
ASSERT_TRUE(WaitForEvents());
- DeleteDelegateOnFileThread(delegate.release());
}
// Verify that watching a file with a symlink on the path
// to the file works.
TEST_F(FilePathWatcherTest, LinkedDirectoryPart3) {
FilePathWatcher watcher;
- FilePath dir(temp_dir_.path().AppendASCII("dir"));
- FilePath link_dir(temp_dir_.path().AppendASCII("dir.lnk"));
+ FilePath dir(temp_dir_.GetPath().AppendASCII("dir"));
+ FilePath link_dir(temp_dir_.GetPath().AppendASCII("dir.lnk"));
FilePath file(dir.AppendASCII("file"));
FilePath linkfile(link_dir.AppendASCII("file"));
std::unique_ptr<TestDelegate> delegate(new TestDelegate(collector()));
@@ -828,7 +787,6 @@ TEST_F(FilePathWatcherTest, LinkedDirectoryPart3) {
ASSERT_TRUE(base::DeleteFile(file, false));
VLOG(1) << "Waiting for file deletion";
ASSERT_TRUE(WaitForEvents());
- DeleteDelegateOnFileThread(delegate.release());
}
#endif // OS_LINUX
@@ -879,7 +837,8 @@ bool ChangeFilePermissions(const FilePath& path, Permission perm, bool allow) {
// Verify that changing attributes on a directory works.
TEST_F(FilePathWatcherTest, DirAttributesChanged) {
- FilePath test_dir1(temp_dir_.path().AppendASCII("DirAttributesChangedDir1"));
+ FilePath test_dir1(
+ temp_dir_.GetPath().AppendASCII("DirAttributesChangedDir1"));
FilePath test_dir2(test_dir1.AppendASCII("DirAttributesChangedDir2"));
FilePath test_file(test_dir2.AppendASCII("DirAttributesChangedFile"));
// Setup a directory hierarchy.
@@ -905,7 +864,6 @@ TEST_F(FilePathWatcherTest, DirAttributesChanged) {
ASSERT_TRUE(ChangeFilePermissions(test_dir1, Execute, false));
ASSERT_TRUE(WaitForEvents());
ASSERT_TRUE(ChangeFilePermissions(test_dir1, Execute, true));
- DeleteDelegateOnFileThread(delegate.release());
}
#endif // OS_MACOSX
diff --git a/base/files/file_posix.cc b/base/files/file_posix.cc
index 12f80c4f8f..2738d6c45c 100644
--- a/base/files/file_posix.cc
+++ b/base/files/file_posix.cc
@@ -11,7 +11,7 @@
#include <unistd.h>
#include "base/logging.h"
-#include "base/metrics/sparse_histogram.h"
+#include "base/metrics/histogram_macros.h"
#include "base/posix/eintr_wrapper.h"
#include "base/strings/utf_string_conversions.h"
#include "base/threading/thread_restrictions.h"
@@ -323,7 +323,7 @@ int64_t File::GetLength() {
stat_wrapper_t file_info;
if (CallFstat(file_.get(), &file_info))
- return false;
+ return -1;
return file_info.st_size;
}
@@ -372,7 +372,7 @@ File::Error File::Unlock() {
return CallFcntlFlock(file_.get(), false);
}
-File File::Duplicate() {
+File File::Duplicate() const {
if (!IsValid())
return File();
@@ -513,9 +513,10 @@ void File::DoInitialize(const FilePath& path, uint32_t flags) {
}
#endif // !defined(OS_NACL)
-bool File::DoFlush() {
+bool File::Flush() {
ThreadRestrictions::AssertIOAllowed();
DCHECK(IsValid());
+ SCOPED_FILE_TRACE("Flush");
#if defined(OS_NACL)
NOTIMPLEMENTED(); // NaCl doesn't implement fsync.
diff --git a/base/files/file_tracing.cc b/base/files/file_tracing.cc
index 6d11cbc746..48f57412f9 100644
--- a/base/files/file_tracing.cc
+++ b/base/files/file_tracing.cc
@@ -4,47 +4,62 @@
#include "base/files/file_tracing.h"
+#include "base/atomicops.h"
#include "base/files/file.h"
+using base::subtle::AtomicWord;
+
namespace base {
namespace {
-FileTracing::Provider* g_provider = nullptr;
+AtomicWord g_provider;
+}
+
+FileTracing::Provider* GetProvider() {
+ AtomicWord provider = base::subtle::Acquire_Load(&g_provider);
+ return reinterpret_cast<FileTracing::Provider*>(provider);
}
// static
bool FileTracing::IsCategoryEnabled() {
- return g_provider && g_provider->FileTracingCategoryIsEnabled();
+ FileTracing::Provider* provider = GetProvider();
+ return provider && provider->FileTracingCategoryIsEnabled();
}
// static
void FileTracing::SetProvider(FileTracing::Provider* provider) {
- g_provider = provider;
+ base::subtle::Release_Store(&g_provider,
+ reinterpret_cast<AtomicWord>(provider));
}
FileTracing::ScopedEnabler::ScopedEnabler() {
- if (g_provider)
- g_provider->FileTracingEnable(this);
+ FileTracing::Provider* provider = GetProvider();
+ if (provider)
+ provider->FileTracingEnable(this);
}
FileTracing::ScopedEnabler::~ScopedEnabler() {
- if (g_provider)
- g_provider->FileTracingDisable(this);
+ FileTracing::Provider* provider = GetProvider();
+ if (provider)
+ provider->FileTracingDisable(this);
}
FileTracing::ScopedTrace::ScopedTrace() : id_(nullptr) {}
FileTracing::ScopedTrace::~ScopedTrace() {
- if (id_ && g_provider)
- g_provider->FileTracingEventEnd(name_, id_);
+ if (id_) {
+ FileTracing::Provider* provider = GetProvider();
+ if (provider)
+ provider->FileTracingEventEnd(name_, id_);
+ }
}
void FileTracing::ScopedTrace::Initialize(const char* name,
- File* file,
+ const File* file,
int64_t size) {
id_ = &file->trace_enabler_;
name_ = name;
- g_provider->FileTracingEventBegin(name_, id_, file->tracing_path_, size);
+ GetProvider()->FileTracingEventBegin(name_, id_, file->tracing_path_, size);
}
} // namespace base
diff --git a/base/files/file_tracing.h b/base/files/file_tracing.h
index bedd7be64b..1fbfcd4498 100644
--- a/base/files/file_tracing.h
+++ b/base/files/file_tracing.h
@@ -37,21 +37,21 @@ class BASE_EXPORT FileTracing {
virtual bool FileTracingCategoryIsEnabled() const = 0;
// Enables file tracing for |id|. Must be called before recording events.
- virtual void FileTracingEnable(void* id) = 0;
+ virtual void FileTracingEnable(const void* id) = 0;
// Disables file tracing for |id|.
- virtual void FileTracingDisable(void* id) = 0;
+ virtual void FileTracingDisable(const 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 the number
// of bytes involved in the event.
virtual void FileTracingEventBegin(const char* name,
- void* id,
+ const void* id,
const FilePath& path,
int64_t size) = 0;
// Ends an event for |id| with |name|.
- virtual void FileTracingEventEnd(const char* name, void* id) = 0;
+ virtual void FileTracingEventEnd(const char* name, const void* id) = 0;
};
// Sets a global file tracing provider to query categories and record events.
@@ -73,12 +73,12 @@ class BASE_EXPORT FileTracing {
// event to trace (e.g. "Read", "Write") and must have an application
// lifetime (e.g. static or literal). |file| is the file being traced; must
// outlive this class. |size| is the size (in bytes) of this event.
- void Initialize(const char* name, File* file, int64_t size);
+ void Initialize(const char* name, const File* file, int64_t size);
private:
// The ID of this trace. Based on the |file| passed to |Initialize()|. Must
// outlive this class.
- void* id_;
+ const void* id_;
// The name of the event to trace (e.g. "Read", "Write"). Prefixed with
// "File".
diff --git a/base/files/file_unittest.cc b/base/files/file_unittest.cc
index 2445f7e128..66c312b60d 100644
--- a/base/files/file_unittest.cc
+++ b/base/files/file_unittest.cc
@@ -9,6 +9,7 @@
#include <utility>
#include "base/files/file_util.h"
+#include "base/files/memory_mapped_file.h"
#include "base/files/scoped_temp_dir.h"
#include "base/time/time.h"
#include "build/build_config.h"
@@ -20,7 +21,7 @@ using base::FilePath;
TEST(FileTest, Create) {
base::ScopedTempDir temp_dir;
ASSERT_TRUE(temp_dir.CreateUniqueTempDir());
- FilePath file_path = temp_dir.path().AppendASCII("create_file_1");
+ FilePath file_path = temp_dir.GetPath().AppendASCII("create_file_1");
{
// Don't create a File at all.
@@ -92,7 +93,7 @@ TEST(FileTest, Create) {
{
// Create a delete-on-close file.
- file_path = temp_dir.path().AppendASCII("create_file_2");
+ file_path = temp_dir.GetPath().AppendASCII("create_file_2");
File file(file_path,
base::File::FLAG_OPEN_ALWAYS | base::File::FLAG_READ |
base::File::FLAG_DELETE_ON_CLOSE);
@@ -107,7 +108,7 @@ TEST(FileTest, Create) {
TEST(FileTest, Async) {
base::ScopedTempDir temp_dir;
ASSERT_TRUE(temp_dir.CreateUniqueTempDir());
- FilePath file_path = temp_dir.path().AppendASCII("create_file");
+ FilePath file_path = temp_dir.GetPath().AppendASCII("create_file");
{
File file(file_path, base::File::FLAG_OPEN_ALWAYS | base::File::FLAG_ASYNC);
@@ -125,7 +126,7 @@ TEST(FileTest, Async) {
TEST(FileTest, DeleteOpenFile) {
base::ScopedTempDir temp_dir;
ASSERT_TRUE(temp_dir.CreateUniqueTempDir());
- FilePath file_path = temp_dir.path().AppendASCII("create_file_1");
+ FilePath file_path = temp_dir.GetPath().AppendASCII("create_file_1");
// Create a file.
File file(file_path,
@@ -152,7 +153,7 @@ TEST(FileTest, DeleteOpenFile) {
TEST(FileTest, ReadWrite) {
base::ScopedTempDir temp_dir;
ASSERT_TRUE(temp_dir.CreateUniqueTempDir());
- FilePath file_path = temp_dir.path().AppendASCII("read_write_file");
+ FilePath file_path = temp_dir.GetPath().AppendASCII("read_write_file");
File file(file_path,
base::File::FLAG_CREATE | base::File::FLAG_READ |
base::File::FLAG_WRITE);
@@ -224,7 +225,7 @@ TEST(FileTest, ReadWrite) {
TEST(FileTest, Append) {
base::ScopedTempDir temp_dir;
ASSERT_TRUE(temp_dir.CreateUniqueTempDir());
- FilePath file_path = temp_dir.path().AppendASCII("append_file");
+ FilePath file_path = temp_dir.GetPath().AppendASCII("append_file");
File file(file_path, base::File::FLAG_CREATE | base::File::FLAG_APPEND);
ASSERT_TRUE(file.IsValid());
@@ -272,7 +273,7 @@ TEST(FileTest, Append) {
TEST(FileTest, Length) {
base::ScopedTempDir temp_dir;
ASSERT_TRUE(temp_dir.CreateUniqueTempDir());
- FilePath file_path = temp_dir.path().AppendASCII("truncate_file");
+ FilePath file_path = temp_dir.GetPath().AppendASCII("truncate_file");
File file(file_path,
base::File::FLAG_CREATE | base::File::FLAG_READ |
base::File::FLAG_WRITE);
@@ -324,7 +325,7 @@ TEST(FileTest, DISABLED_TouchGetInfo) {
#endif
base::ScopedTempDir temp_dir;
ASSERT_TRUE(temp_dir.CreateUniqueTempDir());
- File file(temp_dir.path().AppendASCII("touch_get_info_file"),
+ File file(temp_dir.GetPath().AppendASCII("touch_get_info_file"),
base::File::FLAG_CREATE | base::File::FLAG_WRITE |
base::File::FLAG_WRITE_ATTRIBUTES);
ASSERT_TRUE(file.IsValid());
@@ -387,7 +388,8 @@ TEST(FileTest, DISABLED_TouchGetInfo) {
TEST(FileTest, ReadAtCurrentPosition) {
base::ScopedTempDir temp_dir;
ASSERT_TRUE(temp_dir.CreateUniqueTempDir());
- FilePath file_path = temp_dir.path().AppendASCII("read_at_current_position");
+ FilePath file_path =
+ temp_dir.GetPath().AppendASCII("read_at_current_position");
File file(file_path,
base::File::FLAG_CREATE | base::File::FLAG_READ |
base::File::FLAG_WRITE);
@@ -411,7 +413,8 @@ TEST(FileTest, ReadAtCurrentPosition) {
TEST(FileTest, WriteAtCurrentPosition) {
base::ScopedTempDir temp_dir;
ASSERT_TRUE(temp_dir.CreateUniqueTempDir());
- FilePath file_path = temp_dir.path().AppendASCII("write_at_current_position");
+ FilePath file_path =
+ temp_dir.GetPath().AppendASCII("write_at_current_position");
File file(file_path,
base::File::FLAG_CREATE | base::File::FLAG_READ |
base::File::FLAG_WRITE);
@@ -434,7 +437,7 @@ TEST(FileTest, WriteAtCurrentPosition) {
TEST(FileTest, Seek) {
base::ScopedTempDir temp_dir;
ASSERT_TRUE(temp_dir.CreateUniqueTempDir());
- FilePath file_path = temp_dir.path().AppendASCII("seek_file");
+ FilePath file_path = temp_dir.GetPath().AppendASCII("seek_file");
File file(file_path,
base::File::FLAG_CREATE | base::File::FLAG_READ |
base::File::FLAG_WRITE);
@@ -451,7 +454,7 @@ TEST(FileTest, Seek) {
TEST(FileTest, Duplicate) {
base::ScopedTempDir temp_dir;
ASSERT_TRUE(temp_dir.CreateUniqueTempDir());
- FilePath file_path = temp_dir.path().AppendASCII("file");
+ FilePath file_path = temp_dir.GetPath().AppendASCII("file");
File file(file_path,(base::File::FLAG_CREATE |
base::File::FLAG_READ |
base::File::FLAG_WRITE));
@@ -478,7 +481,7 @@ TEST(FileTest, Duplicate) {
TEST(FileTest, DuplicateDeleteOnClose) {
base::ScopedTempDir temp_dir;
ASSERT_TRUE(temp_dir.CreateUniqueTempDir());
- FilePath file_path = temp_dir.path().AppendASCII("file");
+ FilePath file_path = temp_dir.GetPath().AppendASCII("file");
File file(file_path,(base::File::FLAG_CREATE |
base::File::FLAG_READ |
base::File::FLAG_WRITE |
@@ -495,7 +498,8 @@ TEST(FileTest, DuplicateDeleteOnClose) {
TEST(FileTest, GetInfoForDirectory) {
base::ScopedTempDir temp_dir;
ASSERT_TRUE(temp_dir.CreateUniqueTempDir());
- FilePath empty_dir = temp_dir.path().Append(FILE_PATH_LITERAL("gpfi_test"));
+ FilePath empty_dir =
+ temp_dir.GetPath().Append(FILE_PATH_LITERAL("gpfi_test"));
ASSERT_TRUE(CreateDirectory(empty_dir));
base::File dir(
@@ -514,4 +518,158 @@ TEST(FileTest, GetInfoForDirectory) {
EXPECT_FALSE(info.is_symbolic_link);
EXPECT_EQ(0, info.size);
}
+
+TEST(FileTest, DeleteNoop) {
+ base::ScopedTempDir temp_dir;
+ ASSERT_TRUE(temp_dir.CreateUniqueTempDir());
+ FilePath file_path = temp_dir.GetPath().AppendASCII("file");
+
+ // Creating and closing a file with DELETE perms should do nothing special.
+ File file(file_path,
+ (base::File::FLAG_CREATE | base::File::FLAG_READ |
+ base::File::FLAG_WRITE | base::File::FLAG_CAN_DELETE_ON_CLOSE));
+ ASSERT_TRUE(file.IsValid());
+ file.Close();
+ ASSERT_TRUE(base::PathExists(file_path));
+}
+
+TEST(FileTest, Delete) {
+ base::ScopedTempDir temp_dir;
+ ASSERT_TRUE(temp_dir.CreateUniqueTempDir());
+ FilePath file_path = temp_dir.GetPath().AppendASCII("file");
+
+ // Creating a file with DELETE and then marking for delete on close should
+ // delete it.
+ File file(file_path,
+ (base::File::FLAG_CREATE | base::File::FLAG_READ |
+ base::File::FLAG_WRITE | base::File::FLAG_CAN_DELETE_ON_CLOSE));
+ ASSERT_TRUE(file.IsValid());
+ ASSERT_TRUE(file.DeleteOnClose(true));
+ file.Close();
+ ASSERT_FALSE(base::PathExists(file_path));
+}
+
+TEST(FileTest, DeleteThenRevoke) {
+ base::ScopedTempDir temp_dir;
+ ASSERT_TRUE(temp_dir.CreateUniqueTempDir());
+ FilePath file_path = temp_dir.GetPath().AppendASCII("file");
+
+ // Creating a file with DELETE, marking it for delete, then clearing delete on
+ // close should not delete it.
+ File file(file_path,
+ (base::File::FLAG_CREATE | base::File::FLAG_READ |
+ base::File::FLAG_WRITE | base::File::FLAG_CAN_DELETE_ON_CLOSE));
+ ASSERT_TRUE(file.IsValid());
+ ASSERT_TRUE(file.DeleteOnClose(true));
+ ASSERT_TRUE(file.DeleteOnClose(false));
+ file.Close();
+ ASSERT_TRUE(base::PathExists(file_path));
+}
+
+TEST(FileTest, IrrevokableDeleteOnClose) {
+ base::ScopedTempDir temp_dir;
+ ASSERT_TRUE(temp_dir.CreateUniqueTempDir());
+ FilePath file_path = temp_dir.GetPath().AppendASCII("file");
+
+ // DELETE_ON_CLOSE cannot be revoked by this opener.
+ File file(
+ file_path,
+ (base::File::FLAG_CREATE | base::File::FLAG_READ |
+ base::File::FLAG_WRITE | base::File::FLAG_DELETE_ON_CLOSE |
+ base::File::FLAG_SHARE_DELETE | base::File::FLAG_CAN_DELETE_ON_CLOSE));
+ ASSERT_TRUE(file.IsValid());
+ // https://msdn.microsoft.com/library/windows/desktop/aa364221.aspx says that
+ // setting the dispositon has no effect if the handle was opened with
+ // FLAG_DELETE_ON_CLOSE. Do not make the test's success dependent on whether
+ // or not SetFileInformationByHandle indicates success or failure. (It happens
+ // to indicate success on Windows 10.)
+ file.DeleteOnClose(false);
+ file.Close();
+ ASSERT_FALSE(base::PathExists(file_path));
+}
+
+TEST(FileTest, IrrevokableDeleteOnCloseOther) {
+ base::ScopedTempDir temp_dir;
+ ASSERT_TRUE(temp_dir.CreateUniqueTempDir());
+ FilePath file_path = temp_dir.GetPath().AppendASCII("file");
+
+ // DELETE_ON_CLOSE cannot be revoked by another opener.
+ File file(
+ file_path,
+ (base::File::FLAG_CREATE | base::File::FLAG_READ |
+ base::File::FLAG_WRITE | base::File::FLAG_DELETE_ON_CLOSE |
+ base::File::FLAG_SHARE_DELETE | base::File::FLAG_CAN_DELETE_ON_CLOSE));
+ ASSERT_TRUE(file.IsValid());
+
+ File file2(
+ file_path,
+ (base::File::FLAG_OPEN | base::File::FLAG_READ | base::File::FLAG_WRITE |
+ base::File::FLAG_SHARE_DELETE | base::File::FLAG_CAN_DELETE_ON_CLOSE));
+ ASSERT_TRUE(file2.IsValid());
+
+ file2.DeleteOnClose(false);
+ file2.Close();
+ ASSERT_TRUE(base::PathExists(file_path));
+ file.Close();
+ ASSERT_FALSE(base::PathExists(file_path));
+}
+
+TEST(FileTest, DeleteWithoutPermission) {
+ base::ScopedTempDir temp_dir;
+ ASSERT_TRUE(temp_dir.CreateUniqueTempDir());
+ FilePath file_path = temp_dir.GetPath().AppendASCII("file");
+
+ // It should not be possible to mark a file for deletion when it was not
+ // created/opened with DELETE.
+ File file(file_path, (base::File::FLAG_CREATE | base::File::FLAG_READ |
+ base::File::FLAG_WRITE));
+ ASSERT_TRUE(file.IsValid());
+ ASSERT_FALSE(file.DeleteOnClose(true));
+ file.Close();
+ ASSERT_TRUE(base::PathExists(file_path));
+}
+
+TEST(FileTest, UnsharedDeleteOnClose) {
+ base::ScopedTempDir temp_dir;
+ ASSERT_TRUE(temp_dir.CreateUniqueTempDir());
+ FilePath file_path = temp_dir.GetPath().AppendASCII("file");
+
+ // Opening with DELETE_ON_CLOSE when a previous opener hasn't enabled sharing
+ // will fail.
+ File file(file_path, (base::File::FLAG_CREATE | base::File::FLAG_READ |
+ base::File::FLAG_WRITE));
+ ASSERT_TRUE(file.IsValid());
+ File file2(
+ file_path,
+ (base::File::FLAG_OPEN | base::File::FLAG_READ | base::File::FLAG_WRITE |
+ base::File::FLAG_DELETE_ON_CLOSE | base::File::FLAG_SHARE_DELETE));
+ ASSERT_FALSE(file2.IsValid());
+
+ file.Close();
+ ASSERT_TRUE(base::PathExists(file_path));
+}
+
+TEST(FileTest, NoDeleteOnCloseWithMappedFile) {
+ base::ScopedTempDir temp_dir;
+ ASSERT_TRUE(temp_dir.CreateUniqueTempDir());
+ FilePath file_path = temp_dir.GetPath().AppendASCII("file");
+
+ // Mapping a file into memory blocks DeleteOnClose.
+ File file(file_path,
+ (base::File::FLAG_CREATE | base::File::FLAG_READ |
+ base::File::FLAG_WRITE | base::File::FLAG_CAN_DELETE_ON_CLOSE));
+ ASSERT_TRUE(file.IsValid());
+ ASSERT_EQ(5, file.WriteAtCurrentPos("12345", 5));
+
+ {
+ base::MemoryMappedFile mapping;
+ ASSERT_TRUE(mapping.Initialize(file.Duplicate()));
+ ASSERT_EQ(5U, mapping.length());
+
+ EXPECT_FALSE(file.DeleteOnClose(true));
+ }
+
+ file.Close();
+ ASSERT_TRUE(base::PathExists(file_path));
+}
#endif // defined(OS_WIN)
diff --git a/base/files/file_util.h b/base/files/file_util.h
index 420dcaee61..5ada35f9a4 100644
--- a/base/files/file_util.h
+++ b/base/files/file_util.h
@@ -294,10 +294,6 @@ BASE_EXPORT bool DevicePathToDriveLetterPath(const FilePath& device_path,
// be resolved with this function.
BASE_EXPORT bool NormalizeToNativeFilePath(const FilePath& path,
FilePath* nt_path);
-
-// Given an existing file in |path|, returns whether this file is on a network
-// drive or not. If |path| does not exist, this function returns false.
-BASE_EXPORT bool IsOnNetworkDrive(const base::FilePath& path);
#endif
// This function will return if the given file is a symlink or not.
@@ -311,7 +307,9 @@ BASE_EXPORT bool TouchFile(const FilePath& path,
const Time& last_accessed,
const Time& last_modified);
-// Wrapper for fopen-like calls. Returns non-NULL FILE* on success.
+// Wrapper for fopen-like calls. Returns non-NULL FILE* on success. The
+// underlying file descriptor (POSIX) or handle (Windows) is unconditionally
+// configured to not be propagated to child processes.
BASE_EXPORT FILE* OpenFile(const FilePath& filename, const char* mode);
// Closes file opened by OpenFile. Returns true on success.
@@ -365,6 +363,17 @@ BASE_EXPORT int GetUniquePathNumber(const FilePath& path,
BASE_EXPORT bool SetNonBlocking(int fd);
#if defined(OS_POSIX)
+// Creates a non-blocking, close-on-exec pipe.
+// This creates a non-blocking pipe that is not intended to be shared with any
+// child process. This will be done atomically if the operating system supports
+// it. Returns true if it was able to create the pipe, otherwise false.
+BASE_EXPORT bool CreateLocalNonBlockingPipe(int fds[2]);
+
+// Sets the given |fd| to close-on-exec mode.
+// Returns true if it was able to set it in the close-on-exec mode, otherwise
+// false.
+BASE_EXPORT bool SetCloseOnExec(int fd);
+
// Test that |path| can only be changed by a given user and members of
// a given set of groups.
// Specifically, test that all parts of |path| under (and including) |base|:
diff --git a/base/files/file_util_mac.mm b/base/files/file_util_mac.mm
index e9c6c65159..5a99aa0e81 100644
--- a/base/files/file_util_mac.mm
+++ b/base/files/file_util_mac.mm
@@ -4,8 +4,9 @@
#include "base/files/file_util.h"
-#include <copyfile.h>
#import <Foundation/Foundation.h>
+#include <copyfile.h>
+#include <stdlib.h>
#include "base/files/file_path.h"
#include "base/mac/foundation_util.h"
@@ -23,6 +24,15 @@ bool CopyFile(const FilePath& from_path, const FilePath& to_path) {
}
bool GetTempDir(base::FilePath* path) {
+ // In order to facilitate hermetic runs on macOS, first check $TMPDIR.
+ // NOTE: $TMPDIR is ALMOST ALWAYS set on macOS (unless the user un-set it).
+ const char* env_tmpdir = getenv("TMPDIR");
+ if (env_tmpdir) {
+ *path = base::FilePath(env_tmpdir);
+ return true;
+ }
+
+ // If we didn't find it, fall back to the native function.
NSString* tmp = NSTemporaryDirectory();
if (tmp == nil)
return false;
diff --git a/base/files/file_util_posix.cc b/base/files/file_util_posix.cc
index 85a1b41d46..a03ca8d8d8 100644
--- a/base/files/file_util_posix.cc
+++ b/base/files/file_util_posix.cc
@@ -185,6 +185,19 @@ bool DetermineDevShmExecutable() {
#endif // defined(OS_LINUX)
#endif // !defined(OS_NACL_NONSFI)
+#if !defined(OS_MACOSX)
+// Appends |mode_char| to |mode| before the optional character set encoding; see
+// https://www.gnu.org/software/libc/manual/html_node/Opening-Streams.html for
+// details.
+std::string AppendModeCharacter(StringPiece mode, char mode_char) {
+ std::string result(mode.as_string());
+ size_t comma_pos = result.find(',');
+ result.insert(comma_pos == std::string::npos ? result.length() : comma_pos, 1,
+ mode_char);
+ return result;
+}
+#endif
+
} // namespace
#if !defined(OS_NACL_NONSFI)
@@ -276,11 +289,8 @@ bool CopyDirectory(const FilePath& from_path,
FilePath real_from_path = MakeAbsoluteFilePath(from_path);
if (real_from_path.empty())
return false;
- if (real_to_path.value().size() >= real_from_path.value().size() &&
- real_to_path.value().compare(0, real_from_path.value().size(),
- real_from_path.value()) == 0) {
+ if (real_to_path == real_from_path || real_from_path.IsParent(real_to_path))
return false;
- }
int traverse_type = FileEnumerator::FILES | FileEnumerator::SHOW_SYM_LINKS;
if (recursive)
@@ -351,6 +361,29 @@ bool CopyDirectory(const FilePath& from_path,
}
#endif // !defined(OS_NACL_NONSFI)
+bool CreateLocalNonBlockingPipe(int fds[2]) {
+#if defined(OS_LINUX)
+ return pipe2(fds, O_CLOEXEC | O_NONBLOCK) == 0;
+#else
+ int raw_fds[2];
+ if (pipe(raw_fds) != 0)
+ return false;
+ ScopedFD fd_out(raw_fds[0]);
+ ScopedFD fd_in(raw_fds[1]);
+ if (!SetCloseOnExec(fd_out.get()))
+ return false;
+ if (!SetCloseOnExec(fd_in.get()))
+ return false;
+ if (!SetNonBlocking(fd_out.get()))
+ return false;
+ if (!SetNonBlocking(fd_in.get()))
+ return false;
+ fds[0] = fd_out.release();
+ fds[1] = fd_in.release();
+ return true;
+#endif
+}
+
bool SetNonBlocking(int fd) {
const int flags = fcntl(fd, F_GETFL);
if (flags == -1)
@@ -362,6 +395,21 @@ bool SetNonBlocking(int fd) {
return true;
}
+bool SetCloseOnExec(int fd) {
+#if defined(OS_NACL_NONSFI)
+ const int flags = 0;
+#else
+ const int flags = fcntl(fd, F_GETFD);
+ if (flags == -1)
+ return false;
+ if (flags & FD_CLOEXEC)
+ return true;
+#endif // defined(OS_NACL_NONSFI)
+ if (HANDLE_EINTR(fcntl(fd, F_SETFD, flags | FD_CLOEXEC)) == -1)
+ return false;
+ return true;
+}
+
bool PathExists(const FilePath& path) {
ThreadRestrictions::AssertIOAllowed();
#if defined(OS_ANDROID)
@@ -677,11 +725,29 @@ bool GetFileInfo(const FilePath& file_path, File::Info* results) {
#endif // !defined(OS_NACL_NONSFI)
FILE* OpenFile(const FilePath& filename, const char* mode) {
+ // 'e' is unconditionally added below, so be sure there is not one already
+ // present before a comma in |mode|.
+ DCHECK(
+ strchr(mode, 'e') == nullptr ||
+ (strchr(mode, ',') != nullptr && strchr(mode, 'e') > strchr(mode, ',')));
ThreadRestrictions::AssertIOAllowed();
FILE* result = NULL;
+#if defined(OS_MACOSX)
+ // macOS does not provide a mode character to set O_CLOEXEC; see
+ // https://developer.apple.com/legacy/library/documentation/Darwin/Reference/ManPages/man3/fopen.3.html.
+ const char* the_mode = mode;
+#else
+ std::string mode_with_e(AppendModeCharacter(mode, 'e'));
+ const char* the_mode = mode_with_e.c_str();
+#endif
do {
- result = fopen(filename.value().c_str(), mode);
+ result = fopen(filename.value().c_str(), the_mode);
} while (!result && errno == EINTR);
+#if defined(OS_MACOSX)
+ // Mark the descriptor as close-on-exec.
+ if (result)
+ SetCloseOnExec(fileno(result));
+#endif
return result;
}
diff --git a/base/files/important_file_writer.cc b/base/files/important_file_writer.cc
index 28550ad52f..b46846277b 100644
--- a/base/files/important_file_writer.cc
+++ b/base/files/important_file_writer.cc
@@ -18,7 +18,7 @@
#include "base/files/file_util.h"
#include "base/logging.h"
#include "base/macros.h"
-#include "base/metrics/histogram.h"
+#include "base/metrics/histogram_macros.h"
#include "base/numerics/safe_conversions.h"
#include "base/strings/string_number_conversions.h"
#include "base/strings/string_util.h"
@@ -57,9 +57,18 @@ void LogFailure(const FilePath& path, TempFileFailure failure_code,
// Helper function to call WriteFileAtomically() with a
// std::unique_ptr<std::string>.
-bool WriteScopedStringToFileAtomically(const FilePath& path,
- std::unique_ptr<std::string> data) {
- return ImportantFileWriter::WriteFileAtomically(path, *data);
+void WriteScopedStringToFileAtomically(
+ const FilePath& path,
+ std::unique_ptr<std::string> data,
+ Closure before_write_callback,
+ Callback<void(bool success)> after_write_callback) {
+ if (!before_write_callback.is_null())
+ before_write_callback.Run();
+
+ bool result = ImportantFileWriter::WriteFileAtomically(path, *data);
+
+ if (!after_write_callback.is_null())
+ after_write_callback.Run(result);
}
} // namespace
@@ -94,6 +103,7 @@ bool ImportantFileWriter::WriteFileAtomically(const FilePath& path,
File tmp_file(tmp_file_path, File::FLAG_OPEN | File::FLAG_WRITE);
if (!tmp_file.IsValid()) {
LogFailure(path, FAILED_OPENING, "could not open temporary file");
+ DeleteFile(tmp_file_path, false);
return false;
}
@@ -168,8 +178,11 @@ void ImportantFileWriter::WriteNow(std::unique_ptr<std::string> data) {
if (HasPendingWrite())
timer_.Stop();
- auto task = Bind(&WriteScopedStringToFileAtomically, path_, Passed(&data));
- if (!PostWriteTask(task)) {
+ Closure task = Bind(&WriteScopedStringToFileAtomically, path_, Passed(&data),
+ Passed(&before_next_write_callback_),
+ Passed(&after_next_write_callback_));
+
+ if (!task_runner_->PostTask(FROM_HERE, MakeCriticalClosure(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.
@@ -203,37 +216,11 @@ void ImportantFileWriter::DoScheduledWrite() {
serializer_ = nullptr;
}
-void ImportantFileWriter::RegisterOnNextSuccessfulWriteCallback(
- const Closure& on_next_successful_write) {
- DCHECK(on_next_successful_write_.is_null());
- on_next_successful_write_ = on_next_successful_write;
-}
-
-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
- // suppressing all of those is unrealistic hence we avoid most of them by
- // using PostTask() in the typical scenario below.
- if (!on_next_successful_write_.is_null()) {
- return PostTaskAndReplyWithResult(
- task_runner_.get(),
- FROM_HERE,
- MakeCriticalClosure(task),
- Bind(&ImportantFileWriter::ForwardSuccessfulWrite,
- weak_factory_.GetWeakPtr()));
- }
- return task_runner_->PostTask(
- FROM_HERE,
- MakeCriticalClosure(Bind(IgnoreResult(task))));
-}
-
-void ImportantFileWriter::ForwardSuccessfulWrite(bool result) {
- DCHECK(CalledOnValidThread());
- if (result && !on_next_successful_write_.is_null()) {
- on_next_successful_write_.Run();
- on_next_successful_write_.Reset();
- }
+void ImportantFileWriter::RegisterOnNextWriteCallbacks(
+ const Closure& before_next_write_callback,
+ const Callback<void(bool success)>& after_next_write_callback) {
+ before_next_write_callback_ = before_next_write_callback;
+ after_next_write_callback_ = after_next_write_callback;
}
} // namespace base
diff --git a/base/files/important_file_writer.h b/base/files/important_file_writer.h
index 0bd8a7fd35..f154b043b2 100644
--- a/base/files/important_file_writer.h
+++ b/base/files/important_file_writer.h
@@ -20,24 +20,21 @@
namespace base {
class SequencedTaskRunner;
-class Thread;
-// Helper to ensure that a file won't be corrupted by the write (for example on
-// application crash). Consider a naive way to save an important file F:
+// Helper for atomically writing a file to ensure that it won't be corrupted by
+// *application* crash during write (implemented as create, flush, rename).
//
-// 1. Open F for writing, truncating it.
-// 2. Write new data to F.
+// As an added benefit, ImportantFileWriter makes it less likely that the file
+// is corrupted by *system* crash, though even if the ImportantFileWriter call
+// has already returned at the time of the crash it is not specified which
+// version of the file (old or new) is preserved. And depending on system
+// configuration (hardware and software) a significant likelihood of file
+// corruption may remain, thus using ImportantFileWriter is not a valid
+// substitute for file integrity checks and recovery codepaths for malformed
+// files.
//
-// It's good when it works, but it gets very bad if step 2. doesn't complete.
-// It can be caused by a crash, a computer hang, or a weird I/O error. And you
-// end up with a broken file.
-//
-// To be safe, we don't start with writing directly to F. Instead, we write to
-// to a temporary file. Only after that write is successful, we rename the
-// temporary file to target filename.
-//
-// If you want to know more about this approach and ext3/ext4 fsync issues, see
-// http://blog.valerieaurora.org/2009/04/16/dont-panic-fsync-ext34-and-your-data/
+// Also note that ImportantFileWriter can be *really* slow (cf. File::Flush()
+// for details) and thus please don't block shutdown on ImportantFileWriter.
class BASE_EXPORT ImportantFileWriter : public NonThreadSafe {
public:
// Used by ScheduleSave to lazily provide the data to be saved. Allows us
@@ -53,8 +50,9 @@ class BASE_EXPORT ImportantFileWriter : public NonThreadSafe {
virtual ~DataSerializer() {}
};
- // Save |data| to |path| in an atomic manner (see the class comment above).
- // Blocks and writes data on the current thread.
+ // Save |data| to |path| in an atomic manner. Blocks and writes data on the
+ // current thread. Does not guarantee file integrity across system crash (see
+ // the class comment above).
static bool WriteFileAtomically(const FilePath& path, StringPiece data);
// Initialize the writer.
@@ -95,25 +93,26 @@ class BASE_EXPORT ImportantFileWriter : public NonThreadSafe {
// Serialize data pending to be saved and execute write on backend thread.
void DoScheduledWrite();
- // Registers |on_next_successful_write| to be called once, on the next
- // successful write event. Only one callback can be set at once.
- void RegisterOnNextSuccessfulWriteCallback(
- const Closure& on_next_successful_write);
+ // Registers |before_next_write_callback| and |after_next_write_callback| to
+ // be synchronously invoked from WriteFileAtomically() before its next write
+ // and after its next write, respectively. The boolean passed to
+ // |after_next_write_callback| indicates whether the write was successful.
+ // Both callbacks must be thread safe as they will be called on |task_runner_|
+ // and may be called during Chrome shutdown.
+ // If called more than once before a write is scheduled on |task_runner|, the
+ // latest callbacks clobber the others.
+ void RegisterOnNextWriteCallbacks(
+ const Closure& before_next_write_callback,
+ const Callback<void(bool success)>& after_next_write_callback);
TimeDelta commit_interval() const {
return commit_interval_;
}
private:
- // Helper method for WriteNow().
- 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.
- void ForwardSuccessfulWrite(bool result);
-
- // Invoked once and then reset on the next successful write event.
- Closure on_next_successful_write_;
+ // Invoked synchronously on the next write event.
+ Closure before_next_write_callback_;
+ Callback<void(bool success)> after_next_write_callback_;
// Path being written to.
const FilePath path_;
diff --git a/base/files/important_file_writer_unittest.cc b/base/files/important_file_writer_unittest.cc
index 43e051ebcf..9b8dcfd4e3 100644
--- a/base/files/important_file_writer_unittest.cc
+++ b/base/files/important_file_writer_unittest.cc
@@ -46,39 +46,61 @@ class DataSerializer : public ImportantFileWriter::DataSerializer {
const std::string data_;
};
-class SuccessfulWriteObserver {
+enum WriteCallbackObservationState {
+ NOT_CALLED,
+ CALLED_WITH_ERROR,
+ CALLED_WITH_SUCCESS,
+};
+
+class WriteCallbacksObserver {
public:
- SuccessfulWriteObserver() : successful_write_observed_(false) {}
+ WriteCallbacksObserver() = default;
- // Register on_successful_write() to be called on the next successful write
+ // Register OnBeforeWrite() and OnAfterWrite() to be called on the next write
// of |writer|.
- void ObserveNextSuccessfulWrite(ImportantFileWriter* writer);
+ void ObserveNextWriteCallbacks(ImportantFileWriter* writer);
- // Returns true if a successful write was observed via on_successful_write()
- // and resets the observation state to false regardless.
- bool GetAndResetObservationState();
+ // Returns the |WriteCallbackObservationState| which was observed, then resets
+ // it to |NOT_CALLED|.
+ WriteCallbackObservationState GetAndResetObservationState();
private:
- void on_successful_write() {
- EXPECT_FALSE(successful_write_observed_);
- successful_write_observed_ = true;
+ void OnBeforeWrite() {
+ EXPECT_FALSE(before_write_called_);
+ before_write_called_ = true;
+ }
+
+ void OnAfterWrite(bool success) {
+ EXPECT_EQ(NOT_CALLED, after_write_observation_state_);
+ after_write_observation_state_ =
+ success ? CALLED_WITH_SUCCESS : CALLED_WITH_ERROR;
}
- bool successful_write_observed_;
+ bool before_write_called_ = false;
+ WriteCallbackObservationState after_write_observation_state_ = NOT_CALLED;
- DISALLOW_COPY_AND_ASSIGN(SuccessfulWriteObserver);
+ DISALLOW_COPY_AND_ASSIGN(WriteCallbacksObserver);
};
-void SuccessfulWriteObserver::ObserveNextSuccessfulWrite(
+void WriteCallbacksObserver::ObserveNextWriteCallbacks(
ImportantFileWriter* writer) {
- writer->RegisterOnNextSuccessfulWriteCallback(base::Bind(
- &SuccessfulWriteObserver::on_successful_write, base::Unretained(this)));
+ writer->RegisterOnNextWriteCallbacks(
+ base::Bind(&WriteCallbacksObserver::OnBeforeWrite,
+ base::Unretained(this)),
+ base::Bind(&WriteCallbacksObserver::OnAfterWrite,
+ base::Unretained(this)));
}
-bool SuccessfulWriteObserver::GetAndResetObservationState() {
- bool was_successful_write_observed = successful_write_observed_;
- successful_write_observed_ = false;
- return was_successful_write_observed;
+WriteCallbackObservationState
+WriteCallbacksObserver::GetAndResetObservationState() {
+ EXPECT_EQ(after_write_observation_state_ != NOT_CALLED, before_write_called_)
+ << "The before-write callback should always be called before the "
+ "after-write callback";
+
+ WriteCallbackObservationState state = after_write_observation_state_;
+ before_write_called_ = false;
+ after_write_observation_state_ = NOT_CALLED;
+ return state;
}
} // namespace
@@ -88,11 +110,11 @@ class ImportantFileWriterTest : public testing::Test {
ImportantFileWriterTest() { }
void SetUp() override {
ASSERT_TRUE(temp_dir_.CreateUniqueTempDir());
- file_ = temp_dir_.path().AppendASCII("test-file");
+ file_ = temp_dir_.GetPath().AppendASCII("test-file");
}
protected:
- SuccessfulWriteObserver successful_write_observer_;
+ WriteCallbacksObserver write_callback_observer_;
FilePath file_;
MessageLoop loop_;
@@ -103,49 +125,102 @@ class ImportantFileWriterTest : public testing::Test {
TEST_F(ImportantFileWriterTest, Basic) {
ImportantFileWriter writer(file_, ThreadTaskRunnerHandle::Get());
EXPECT_FALSE(PathExists(writer.path()));
- EXPECT_FALSE(successful_write_observer_.GetAndResetObservationState());
- writer.WriteNow(WrapUnique(new std::string("foo")));
+ EXPECT_EQ(NOT_CALLED, write_callback_observer_.GetAndResetObservationState());
+ writer.WriteNow(MakeUnique<std::string>("foo"));
RunLoop().RunUntilIdle();
- EXPECT_FALSE(successful_write_observer_.GetAndResetObservationState());
+ EXPECT_EQ(NOT_CALLED, write_callback_observer_.GetAndResetObservationState());
ASSERT_TRUE(PathExists(writer.path()));
EXPECT_EQ("foo", GetFileContent(writer.path()));
}
-TEST_F(ImportantFileWriterTest, BasicWithSuccessfulWriteObserver) {
+TEST_F(ImportantFileWriterTest, WriteWithObserver) {
ImportantFileWriter writer(file_, ThreadTaskRunnerHandle::Get());
EXPECT_FALSE(PathExists(writer.path()));
- EXPECT_FALSE(successful_write_observer_.GetAndResetObservationState());
- successful_write_observer_.ObserveNextSuccessfulWrite(&writer);
- writer.WriteNow(WrapUnique(new std::string("foo")));
- RunLoop().RunUntilIdle();
+ EXPECT_EQ(NOT_CALLED, write_callback_observer_.GetAndResetObservationState());
// Confirm that the observer is invoked.
- EXPECT_TRUE(successful_write_observer_.GetAndResetObservationState());
+ write_callback_observer_.ObserveNextWriteCallbacks(&writer);
+ writer.WriteNow(MakeUnique<std::string>("foo"));
+ RunLoop().RunUntilIdle();
+
+ EXPECT_EQ(CALLED_WITH_SUCCESS,
+ write_callback_observer_.GetAndResetObservationState());
ASSERT_TRUE(PathExists(writer.path()));
EXPECT_EQ("foo", GetFileContent(writer.path()));
// Confirm that re-installing the observer works for another write.
- EXPECT_FALSE(successful_write_observer_.GetAndResetObservationState());
- successful_write_observer_.ObserveNextSuccessfulWrite(&writer);
- writer.WriteNow(WrapUnique(new std::string("bar")));
+ EXPECT_EQ(NOT_CALLED, write_callback_observer_.GetAndResetObservationState());
+ write_callback_observer_.ObserveNextWriteCallbacks(&writer);
+ writer.WriteNow(MakeUnique<std::string>("bar"));
RunLoop().RunUntilIdle();
- EXPECT_TRUE(successful_write_observer_.GetAndResetObservationState());
+ EXPECT_EQ(CALLED_WITH_SUCCESS,
+ write_callback_observer_.GetAndResetObservationState());
ASSERT_TRUE(PathExists(writer.path()));
EXPECT_EQ("bar", GetFileContent(writer.path()));
// Confirm that writing again without re-installing the observer doesn't
// result in a notification.
- EXPECT_FALSE(successful_write_observer_.GetAndResetObservationState());
- writer.WriteNow(WrapUnique(new std::string("baz")));
+ EXPECT_EQ(NOT_CALLED, write_callback_observer_.GetAndResetObservationState());
+ writer.WriteNow(MakeUnique<std::string>("baz"));
RunLoop().RunUntilIdle();
- EXPECT_FALSE(successful_write_observer_.GetAndResetObservationState());
+ EXPECT_EQ(NOT_CALLED, write_callback_observer_.GetAndResetObservationState());
ASSERT_TRUE(PathExists(writer.path()));
EXPECT_EQ("baz", GetFileContent(writer.path()));
}
+TEST_F(ImportantFileWriterTest, FailedWriteWithObserver) {
+ // Use an invalid file path (relative paths are invalid) to get a
+ // FILE_ERROR_ACCESS_DENIED error when trying to write the file.
+ ImportantFileWriter writer(FilePath().AppendASCII("bad/../path"),
+ ThreadTaskRunnerHandle::Get());
+ EXPECT_FALSE(PathExists(writer.path()));
+ EXPECT_EQ(NOT_CALLED, write_callback_observer_.GetAndResetObservationState());
+ write_callback_observer_.ObserveNextWriteCallbacks(&writer);
+ writer.WriteNow(MakeUnique<std::string>("foo"));
+ RunLoop().RunUntilIdle();
+
+ // Confirm that the write observer was invoked with its boolean parameter set
+ // to false.
+ EXPECT_EQ(CALLED_WITH_ERROR,
+ write_callback_observer_.GetAndResetObservationState());
+ EXPECT_FALSE(PathExists(writer.path()));
+}
+
+TEST_F(ImportantFileWriterTest, CallbackRunsOnWriterThread) {
+ base::Thread file_writer_thread("ImportantFileWriter test thread");
+ file_writer_thread.Start();
+ ImportantFileWriter writer(file_, file_writer_thread.task_runner());
+ EXPECT_EQ(NOT_CALLED, write_callback_observer_.GetAndResetObservationState());
+
+ // Block execution on |file_writer_thread| to verify that callbacks are
+ // executed on it.
+ base::WaitableEvent wait_helper(
+ base::WaitableEvent::ResetPolicy::MANUAL,
+ base::WaitableEvent::InitialState::NOT_SIGNALED);
+ file_writer_thread.task_runner()->PostTask(
+ FROM_HERE,
+ base::Bind(&base::WaitableEvent::Wait, base::Unretained(&wait_helper)));
+
+ write_callback_observer_.ObserveNextWriteCallbacks(&writer);
+ writer.WriteNow(MakeUnique<std::string>("foo"));
+ RunLoop().RunUntilIdle();
+
+ // Expect the callback to not have been executed before the
+ // |file_writer_thread| is unblocked.
+ EXPECT_EQ(NOT_CALLED, write_callback_observer_.GetAndResetObservationState());
+
+ wait_helper.Signal();
+ file_writer_thread.FlushForTesting();
+
+ EXPECT_EQ(CALLED_WITH_SUCCESS,
+ write_callback_observer_.GetAndResetObservationState());
+ ASSERT_TRUE(PathExists(writer.path()));
+ EXPECT_EQ("foo", GetFileContent(writer.path()));
+}
+
TEST_F(ImportantFileWriterTest, ScheduleWrite) {
ImportantFileWriter writer(file_,
ThreadTaskRunnerHandle::Get(),
diff --git a/base/files/memory_mapped_file_posix.cc b/base/files/memory_mapped_file_posix.cc
index 4899cf0cda..90ba6f49c1 100644
--- a/base/files/memory_mapped_file_posix.cc
+++ b/base/files/memory_mapped_file_posix.cc
@@ -31,7 +31,7 @@ bool MemoryMappedFile::MapFileRegionToMemory(
if (region == MemoryMappedFile::Region::kWholeFile) {
int64_t file_len = file_.GetLength();
- if (file_len == -1) {
+ if (file_len < 0) {
DPLOG(ERROR) << "fstat " << file_.GetPlatformFile();
return false;
}
@@ -78,7 +78,12 @@ bool MemoryMappedFile::MapFileRegionToMemory(
// POSIX won't auto-extend the file when it is written so it must first
// be explicitly extended to the maximum size. Zeros will fill the new
// space.
- file_.SetLength(std::max(file_.GetLength(), region.offset + region.size));
+ auto file_len = file_.GetLength();
+ if (file_len < 0) {
+ DPLOG(ERROR) << "fstat " << file_.GetPlatformFile();
+ return false;
+ }
+ file_.SetLength(std::max(file_len, region.offset + region.size));
flags |= PROT_READ | PROT_WRITE;
break;
}
diff --git a/base/files/scoped_file.cc b/base/files/scoped_file.cc
index 8ce45b8ba3..78d4ca5263 100644
--- a/base/files/scoped_file.cc
+++ b/base/files/scoped_file.cc
@@ -37,6 +37,14 @@ void ScopedFDCloseTraits::Free(int fd) {
int close_errno = errno;
base::debug::Alias(&close_errno);
+#if defined(OS_LINUX)
+ // NB: Some file descriptors can return errors from close() e.g. network
+ // filesystems such as NFS and Linux input devices. On Linux, errors from
+ // close other than EBADF do not indicate failure to actually close the fd.
+ if (ret != 0 && errno != EBADF)
+ ret = 0;
+#endif
+
PCHECK(0 == ret);
}
diff --git a/base/files/scoped_temp_dir.cc b/base/files/scoped_temp_dir.cc
index 27b758ed90..26815217c6 100644
--- a/base/files/scoped_temp_dir.cc
+++ b/base/files/scoped_temp_dir.cc
@@ -76,6 +76,11 @@ FilePath ScopedTempDir::Take() {
return ret;
}
+const FilePath& ScopedTempDir::GetPath() const {
+ DCHECK(!path_.empty()) << "Did you call CreateUniqueTempDir* before?";
+ return path_;
+}
+
bool ScopedTempDir::IsValid() const {
return !path_.empty() && DirectoryExists(path_);
}
diff --git a/base/files/scoped_temp_dir.h b/base/files/scoped_temp_dir.h
index b1f2f5b874..a5aaf84362 100644
--- a/base/files/scoped_temp_dir.h
+++ b/base/files/scoped_temp_dir.h
@@ -47,7 +47,9 @@ class BASE_EXPORT ScopedTempDir {
// when this object goes out of scope.
FilePath Take();
- const FilePath& path() const { return path_; }
+ // Returns the path to the created directory. Call one of the
+ // CreateUniqueTempDir* methods before getting the path.
+ const FilePath& GetPath() const;
// Returns true if path_ is non-empty and exists.
bool IsValid() const;
diff --git a/base/files/scoped_temp_dir_unittest.cc b/base/files/scoped_temp_dir_unittest.cc
index 3b2f28e50e..024b438aa0 100644
--- a/base/files/scoped_temp_dir_unittest.cc
+++ b/base/files/scoped_temp_dir_unittest.cc
@@ -53,7 +53,7 @@ TEST(ScopedTempDir, TempDir) {
{
ScopedTempDir dir;
EXPECT_TRUE(dir.CreateUniqueTempDir());
- test_path = dir.path();
+ test_path = dir.GetPath();
EXPECT_TRUE(DirectoryExists(test_path));
FilePath tmp_dir;
EXPECT_TRUE(base::GetTempDir(&tmp_dir));
@@ -72,7 +72,7 @@ TEST(ScopedTempDir, UniqueTempDirUnderPath) {
{
ScopedTempDir dir;
EXPECT_TRUE(dir.CreateUniqueTempDirUnderPath(base_path));
- test_path = dir.path();
+ test_path = dir.GetPath();
EXPECT_TRUE(DirectoryExists(test_path));
EXPECT_TRUE(base_path.IsParent(test_path));
EXPECT_TRUE(test_path.value().find(base_path.value()) != std::string::npos);
@@ -99,12 +99,12 @@ TEST(ScopedTempDir, MultipleInvocations) {
TEST(ScopedTempDir, LockedTempDir) {
ScopedTempDir dir;
EXPECT_TRUE(dir.CreateUniqueTempDir());
- base::File file(dir.path().Append(FILE_PATH_LITERAL("temp")),
+ base::File file(dir.GetPath().Append(FILE_PATH_LITERAL("temp")),
base::File::FLAG_CREATE_ALWAYS | base::File::FLAG_WRITE);
EXPECT_TRUE(file.IsValid());
EXPECT_EQ(base::File::FILE_OK, file.error_details());
EXPECT_FALSE(dir.Delete()); // We should not be able to delete.
- EXPECT_FALSE(dir.path().empty()); // We should still have a valid path.
+ EXPECT_FALSE(dir.GetPath().empty()); // We should still have a valid path.
file.Close();
// Now, we should be able to delete.
EXPECT_TRUE(dir.Delete());