Index: base/files/file_path_watcher_unittest.cc |
diff --git a/base/files/file_path_watcher_unittest.cc b/base/files/file_path_watcher_unittest.cc |
index d2ec37bbec1ba32acfb31407a8141ea1838ae36d..fc6f5a85fc919d4a080feac6d1b05283fdf92471 100644 |
--- a/base/files/file_path_watcher_unittest.cc |
+++ b/base/files/file_path_watcher_unittest.cc |
@@ -28,6 +28,7 @@ |
#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" |
@@ -35,10 +36,6 @@ |
#if defined(OS_ANDROID) |
#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 { |
@@ -134,19 +131,30 @@ |
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() |
-#if defined(OS_POSIX) |
- : file_descriptor_watcher_(&loop_) |
-#endif |
- { |
- } |
+ : file_thread_("FilePathWatcherTest") {} |
~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 |
@@ -163,6 +171,10 @@ |
void TearDown() override { RunLoop().RunUntilIdle(); } |
+ void DeleteDelegateOnFileThread(TestDelegate* delegate) { |
+ file_thread_.task_runner()->DeleteSoon(FROM_HERE, delegate); |
+ } |
+ |
FilePath test_file() { |
return temp_dir_.GetPath().AppendASCII("FilePathWatcherTest"); |
} |
@@ -184,23 +196,18 @@ |
bool WaitForEvents() WARN_UNUSED_RESULT { |
collector_->Reset(); |
- |
- RunLoop run_loop; |
// Make sure we timeout if we don't get notified. |
- ThreadTaskRunnerHandle::Get()->PostDelayedTask( |
- FROM_HERE, run_loop.QuitWhenIdleClosure(), |
- TestTimeouts::action_timeout()); |
- run_loop.Run(); |
+ loop_.task_runner()->PostDelayedTask(FROM_HERE, |
+ MessageLoop::QuitWhenIdleClosure(), |
+ TestTimeouts::action_timeout()); |
+ RunLoop().Run(); |
return collector_->Success(); |
} |
NotificationCollector* collector() { return collector_.get(); } |
- MessageLoopForIO loop_; |
-#if defined(OS_POSIX) |
- FileDescriptorWatcher file_descriptor_watcher_; |
-#endif |
- |
+ MessageLoop loop_; |
+ base::Thread file_thread_; |
ScopedTempDir temp_dir_; |
scoped_refptr<NotificationCollector> collector_; |
@@ -212,9 +219,14 @@ |
FilePathWatcher* watcher, |
TestDelegateBase* delegate, |
bool recursive_watch) { |
- return watcher->Watch( |
- target, recursive_watch, |
- base::Bind(&TestDelegateBase::OnFileChanged, delegate->AsWeakPtr())); |
+ 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; |
} |
// Basic test: Create the file and verify that we notice. |
@@ -225,6 +237,7 @@ |
ASSERT_TRUE(WriteFile(test_file(), "content")); |
ASSERT_TRUE(WaitForEvents()); |
+ DeleteDelegateOnFileThread(delegate.release()); |
} |
// Verify that modifying the file is caught. |
@@ -238,6 +251,7 @@ |
// 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. |
@@ -252,6 +266,7 @@ |
// 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) { |
@@ -264,6 +279,7 @@ |
// 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. |
@@ -311,9 +327,11 @@ |
// 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; |
- ASSERT_TRUE(SetupWatch(test_file(), &watcher, delegate.get(), false)); |
- ASSERT_TRUE(WriteFile(test_file(), "content")); |
+ FilePathWatcher* watcher = new FilePathWatcher; |
+ 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) { |
@@ -325,6 +343,8 @@ |
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 |
@@ -350,6 +370,7 @@ |
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 |
@@ -382,6 +403,7 @@ |
ASSERT_TRUE(WriteFile(file, "content v2")); |
VLOG(1) << "Waiting for file modification"; |
ASSERT_TRUE(WaitForEvents()); |
+ DeleteDelegateOnFileThread(delegate.release()); |
} |
#if defined(OS_MACOSX) |
@@ -399,6 +421,7 @@ |
ASSERT_TRUE(base::DeleteFile(dir, true)); |
ASSERT_TRUE(WaitForEvents()); |
+ DeleteDelegateOnFileThread(delegate.release()); |
} |
// Tests that a file that is deleted and reappears is tracked correctly. |
@@ -415,6 +438,7 @@ |
ASSERT_TRUE(WriteFile(test_file(), "content")); |
VLOG(1) << "Waiting for file creation"; |
ASSERT_TRUE(WaitForEvents()); |
+ DeleteDelegateOnFileThread(delegate.release()); |
} |
TEST_F(FilePathWatcherTest, WatchDirectory) { |
@@ -447,6 +471,7 @@ |
ASSERT_TRUE(WriteFile(file2, "content")); |
VLOG(1) << "Waiting for file2 creation"; |
ASSERT_TRUE(WaitForEvents()); |
+ DeleteDelegateOnFileThread(delegate.release()); |
} |
TEST_F(FilePathWatcherTest, MoveParent) { |
@@ -472,6 +497,8 @@ |
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) { |
@@ -481,6 +508,7 @@ |
bool setup_result = SetupWatch(dir, &watcher, delegate.get(), true); |
if (!FilePathWatcher::RecursiveWatchAvailable()) { |
ASSERT_FALSE(setup_result); |
+ DeleteDelegateOnFileThread(delegate.release()); |
return; |
} |
ASSERT_TRUE(setup_result); |
@@ -536,6 +564,7 @@ |
// 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) |
@@ -583,6 +612,8 @@ |
FilePath target2_file(target2.AppendASCII("file")); |
ASSERT_TRUE(WriteFile(target2_file, "content")); |
ASSERT_TRUE(WaitForEvents()); |
+ |
+ DeleteDelegateOnFileThread(delegate.release()); |
} |
#endif // OS_POSIX |
@@ -609,6 +640,8 @@ |
// 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 |
@@ -629,6 +662,7 @@ |
// 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) |
@@ -644,6 +678,7 @@ |
// 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. |
@@ -659,6 +694,7 @@ |
// 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 |
@@ -674,6 +710,7 @@ |
// 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 |
@@ -688,6 +725,7 @@ |
// 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 |
@@ -703,6 +741,7 @@ |
// 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 |
@@ -731,6 +770,7 @@ |
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 |
@@ -760,6 +800,7 @@ |
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 |
@@ -787,6 +828,7 @@ |
ASSERT_TRUE(base::DeleteFile(file, false)); |
VLOG(1) << "Waiting for file deletion"; |
ASSERT_TRUE(WaitForEvents()); |
+ DeleteDelegateOnFileThread(delegate.release()); |
} |
#endif // OS_LINUX |
@@ -864,6 +906,7 @@ |
ASSERT_TRUE(ChangeFilePermissions(test_dir1, Execute, false)); |
ASSERT_TRUE(WaitForEvents()); |
ASSERT_TRUE(ChangeFilePermissions(test_dir1, Execute, true)); |
+ DeleteDelegateOnFileThread(delegate.release()); |
} |
#endif // OS_MACOSX |