diff options
11 files changed, 279 insertions, 587 deletions
diff --git a/chrome/browser/policy/file_based_policy_loader.cc b/chrome/browser/policy/file_based_policy_loader.cc index 62114fa..c12bbcf 100644 --- a/chrome/browser/policy/file_based_policy_loader.cc +++ b/chrome/browser/policy/file_based_policy_loader.cc @@ -97,7 +97,8 @@ void FileBasedPolicyLoader::InitOnFileThread() { if (!config_file_path().empty() && !watcher_->Watch( config_file_path(), - new FileBasedPolicyWatcherDelegate(this))) { + new FileBasedPolicyWatcherDelegate(this), + BrowserThread::GetMessageLoopProxyForThread(BrowserThread::UI))) { OnError(); } diff --git a/chrome/browser/user_style_sheet_watcher.cc b/chrome/browser/user_style_sheet_watcher.cc index 516501d..9a314b3 100644 --- a/chrome/browser/user_style_sheet_watcher.cc +++ b/chrome/browser/user_style_sheet_watcher.cc @@ -150,7 +150,8 @@ void UserStyleSheetWatcher::Init() { .AppendASCII(kUserStyleSheetFile); if (!file_watcher_->Watch( style_sheet_file, - loader_.get())) { + loader_.get(), + BrowserThread::GetMessageLoopProxyForThread(BrowserThread::UI))) { LOG(ERROR) << "Failed to setup watch for " << style_sheet_file.value(); } loader_->LoadStyleSheet(style_sheet_file); diff --git a/chrome/common/service_process_util_mac.mm b/chrome/common/service_process_util_mac.mm index 62a0371..d06a93e 100644 --- a/chrome/common/service_process_util_mac.mm +++ b/chrome/common/service_process_util_mac.mm @@ -179,6 +179,7 @@ bool ServiceProcessState::Initialize() { return false; } state_->launchd_conf_.reset(dict); + state_->ui_message_loop_= base::MessageLoopProxy::CreateForCurrentThread(); return true; } @@ -322,7 +323,9 @@ bool ServiceProcessState::StateData::WatchExecutable() { LOG(ERROR) << "executable_watcher_.Init " << executable_path.value(); return false; } - if (!executable_watcher_.Watch(executable_path, delegate.release())) { + if (!executable_watcher_.Watch(executable_path, + delegate.release(), + ui_message_loop_)) { LOG(ERROR) << "executable_watcher_.watch " << executable_path.value(); return false; } diff --git a/chrome/common/service_process_util_posix.h b/chrome/common/service_process_util_posix.h index b0a908b..9928917 100644 --- a/chrome/common/service_process_util_posix.h +++ b/chrome/common/service_process_util_posix.h @@ -21,6 +21,7 @@ MultiProcessLock* TakeServiceRunningLock(bool waiting); #if defined(OS_MACOSX) #include "base/mac/scoped_cftyperef.h" +#include "base/message_loop_proxy.h" #include "content/common/file_path_watcher/file_path_watcher.h" class CommandLine; @@ -67,6 +68,7 @@ struct ServiceProcessState::StateData base::mac::ScopedCFTypeRef<CFDictionaryRef> launchd_conf_; FilePathWatcher executable_watcher_; + scoped_refptr<base::MessageLoopProxy> ui_message_loop_; #endif // OS_MACOSX #if defined(OS_LINUX) scoped_ptr<MultiProcessLock> initializing_lock_; diff --git a/content/browser/plugin_service.cc b/content/browser/plugin_service.cc index 860836c..46230c1 100644 --- a/content/browser/plugin_service.cc +++ b/content/browser/plugin_service.cc @@ -549,7 +549,8 @@ void PluginService::RegisterFilePathWatcher( FilePathWatcher *watcher, const FilePath& path, FilePathWatcher::Delegate* delegate) { - bool result = watcher->Watch(path, delegate); + bool result = watcher->Watch( + path, delegate, base::MessageLoopProxy::CreateForCurrentThread()); DCHECK(result); } #endif diff --git a/content/common/file_path_watcher/file_path_watcher.cc b/content/common/file_path_watcher/file_path_watcher.cc index 41f3e2b..f40c8c8 100644 --- a/content/common/file_path_watcher/file_path_watcher.cc +++ b/content/common/file_path_watcher/file_path_watcher.cc @@ -14,9 +14,11 @@ FilePathWatcher::~FilePathWatcher() { impl_->Cancel(); } -bool FilePathWatcher::Watch(const FilePath& path, Delegate* delegate) { +bool FilePathWatcher::Watch(const FilePath& path, + Delegate* delegate, + base::MessageLoopProxy* loop) { DCHECK(path.IsAbsolute()); - return impl_->Watch(path, delegate); + return impl_->Watch(path, delegate, loop); } FilePathWatcher::PlatformDelegate::PlatformDelegate(): cancelled_(false) { diff --git a/content/common/file_path_watcher/file_path_watcher.h b/content/common/file_path_watcher/file_path_watcher.h index 20ccfcc..2ec7af5 100644 --- a/content/common/file_path_watcher/file_path_watcher.h +++ b/content/common/file_path_watcher/file_path_watcher.h @@ -16,12 +16,8 @@ // This class lets you register interest in changes on a FilePath. // The delegate will get called whenever the file or directory referenced by the // FilePath is changed, including created or deleted. Due to limitations in the -// underlying OS APIs, FilePathWatcher has slightly different semantics on OS X -// than on Windows or Linux. FilePathWatcher on Linux and Windows will detect -// modifications to files in a watched directory. FilePathWatcher on Mac will -// detect the creation and deletion of files in a watched directory, but will -// not detect modifications to those files. See file_path_watcher_mac.cc for -// details. +// underlying OS APIs, spurious notifications might occur that don't relate to +// an actual change to the watch target. class FilePathWatcher { public: // Declares the callback client code implements to receive notifications. Note @@ -40,10 +36,15 @@ class FilePathWatcher { ~FilePathWatcher(); // Register interest in any changes on |path|. OnPathChanged will be called - // back for each change. Returns true on success. + // back for each change. Returns true on success. |loop| is only used + // by the Mac implementation right now, and must be backed by a CFRunLoop + // based MessagePump. This is usually going to be a MessageLoop of type + // TYPE_UI. // OnFilePathChanged() will be called on the same thread as Watch() is called, // which should have a MessageLoop of TYPE_IO. - bool Watch(const FilePath& path, Delegate* delegate) WARN_UNUSED_RESULT; + bool Watch(const FilePath& path, + Delegate* delegate, + base::MessageLoopProxy* loop) WARN_UNUSED_RESULT; class PlatformDelegate; @@ -72,8 +73,13 @@ class FilePathWatcher { PlatformDelegate(); // Start watching for the given |path| and notify |delegate| about changes. - virtual bool Watch(const FilePath& path, - Delegate* delegate) WARN_UNUSED_RESULT = 0; + // |loop| is only used by the Mac implementation right now, and must be + // backed by a CFRunLoop based MessagePump. This is usually going to be a + // MessageLoop of type TYPE_UI. + virtual bool Watch( + const FilePath& path, + Delegate* delegate, + base::MessageLoopProxy* loop) WARN_UNUSED_RESULT = 0; // Stop watching. This is called from FilePathWatcher's dtor in order to // allow to shut down properly while the object is still alive. diff --git a/content/common/file_path_watcher/file_path_watcher_browsertest.cc b/content/common/file_path_watcher/file_path_watcher_browsertest.cc index dda9fc5..c91e690 100644 --- a/content/common/file_path_watcher/file_path_watcher_browsertest.cc +++ b/content/common/file_path_watcher/file_path_watcher_browsertest.cc @@ -6,13 +6,6 @@ #include <set> -#if defined(OS_WIN) -#include <windows.h> -#include <aclapi.h> -#elif defined(OS_POSIX) -#include <sys/stat.h> -#endif - #include "base/basictypes.h" #include "base/compiler_specific.h" #include "base/file_path.h" @@ -28,6 +21,14 @@ #include "base/threading/thread.h" #include "testing/gtest/include/gtest/gtest.h" +#if defined(OS_MACOSX) +// TODO(mnissler): There are flakes on Mac (http://crbug.com/54822) at least for +// FilePathWatcherTest.MultipleWatchersSingleFile. +#define MAYBE(name) FLAKY_ ## name +#else +#define MAYBE(name) name +#endif + namespace { class TestDelegate; @@ -112,15 +113,17 @@ class SetupWatchTask : public Task { FilePathWatcher* watcher, FilePathWatcher::Delegate* delegate, bool* result, - base::WaitableEvent* completion) + base::WaitableEvent* completion, + base::MessageLoopProxy* mac_run_loop) : target_(target), watcher_(watcher), delegate_(delegate), result_(result), - completion_(completion) {} + completion_(completion), + mac_run_loop_(mac_run_loop) {} void Run() { - *result_ = watcher_->Watch(target_, delegate_); + *result_ = watcher_->Watch(target_, delegate_, mac_run_loop_); completion_->Signal(); } @@ -130,6 +133,7 @@ class SetupWatchTask : public Task { FilePathWatcher::Delegate* delegate_; bool* result_; base::WaitableEvent* completion_; + scoped_refptr<base::MessageLoopProxy> mac_run_loop_; DISALLOW_COPY_AND_ASSIGN(SetupWatchTask); }; @@ -137,7 +141,8 @@ class SetupWatchTask : public Task { class FilePathWatcherTest : public testing::Test { public: FilePathWatcherTest() - : file_thread_("FilePathWatcherTest") { } + : loop_(MessageLoop::TYPE_UI), + file_thread_("FilePathWatcherTest") { } virtual ~FilePathWatcherTest() { } @@ -175,7 +180,8 @@ class FilePathWatcherTest : public testing::Test { watcher, delegate, &result, - &completion)); + &completion, + base::MessageLoopProxy::CreateForCurrentThread())); completion.Wait(); return result; } @@ -195,7 +201,7 @@ class FilePathWatcherTest : public testing::Test { }; // Basic test: Create the file and verify that we notice. -TEST_F(FilePathWatcherTest, NewFile) { +TEST_F(FilePathWatcherTest, MAYBE(NewFile)) { FilePathWatcher watcher; scoped_refptr<TestDelegate> delegate(new TestDelegate(collector())); ASSERT_TRUE(SetupWatch(test_file(), &watcher, delegate.get())); @@ -205,7 +211,7 @@ TEST_F(FilePathWatcherTest, NewFile) { } // Verify that modifying the file is caught. -TEST_F(FilePathWatcherTest, ModifiedFile) { +TEST_F(FilePathWatcherTest, MAYBE(ModifiedFile)) { ASSERT_TRUE(WriteFile(test_file(), "content")); FilePathWatcher watcher; @@ -218,7 +224,7 @@ TEST_F(FilePathWatcherTest, ModifiedFile) { } // Verify that moving the file into place is caught. -TEST_F(FilePathWatcherTest, MovedFile) { +TEST_F(FilePathWatcherTest, MAYBE(MovedFile)) { FilePath source_file(temp_dir_.path().AppendASCII("source")); ASSERT_TRUE(WriteFile(source_file, "content")); @@ -231,7 +237,7 @@ TEST_F(FilePathWatcherTest, MovedFile) { ASSERT_TRUE(WaitForEvents()); } -TEST_F(FilePathWatcherTest, DeletedFile) { +TEST_F(FilePathWatcherTest, MAYBE(DeletedFile)) { ASSERT_TRUE(WriteFile(test_file(), "content")); FilePathWatcher watcher; @@ -286,7 +292,7 @@ TEST_F(FilePathWatcherTest, DestroyWithPendingNotification) { file_thread_.message_loop_proxy()->DeleteSoon(FROM_HERE, watcher); } -TEST_F(FilePathWatcherTest, MultipleWatchersSingleFile) { +TEST_F(FilePathWatcherTest, MAYBE(MultipleWatchersSingleFile)) { FilePathWatcher watcher1, watcher2; scoped_refptr<TestDelegate> delegate1(new TestDelegate(collector())); scoped_refptr<TestDelegate> delegate2(new TestDelegate(collector())); @@ -399,12 +405,9 @@ TEST_F(FilePathWatcherTest, WatchDirectory) { VLOG(1) << "Waiting for file1 creation"; ASSERT_TRUE(WaitForEvents()); -#if !defined(OS_MACOSX) - // Mac implementation does not detect files modified in a directory. ASSERT_TRUE(WriteFile(file1, "content v2")); VLOG(1) << "Waiting for file1 modification"; ASSERT_TRUE(WaitForEvents()); -#endif // !OS_MACOSX ASSERT_TRUE(file_util::Delete(file1, false)); VLOG(1) << "Waiting for file1 deletion"; @@ -463,150 +466,4 @@ TEST_F(FilePathWatcherTest, MoveChild) { ASSERT_TRUE(WaitForEvents()); } -#if !defined(OS_LINUX) -// Linux implementation of FilePathWatcher doesn't catch attribute changes. -// http://crbug.com/78043 - -// Verify that changing attributes on a file is caught -TEST_F(FilePathWatcherTest, FileAttributesChanged) { - ASSERT_TRUE(WriteFile(test_file(), "content")); - FilePathWatcher watcher; - scoped_refptr<TestDelegate> delegate(new TestDelegate(collector())); - ASSERT_TRUE(SetupWatch(test_file(), &watcher, delegate.get())); - - // Now make sure we get notified if the file is modified. - ASSERT_TRUE(file_util::MakeFileUnreadable(test_file())); - ASSERT_TRUE(WaitForEvents()); -} - -#endif // !OS_LINUX - -enum Permission { - Read, - Write, - Execute -}; - -bool ChangeFilePermissions(const FilePath& path, Permission perm, bool allow) { -#if defined(OS_POSIX) - struct stat stat_buf; - - if (stat(path.value().c_str(), &stat_buf) != 0) - return false; - - mode_t mode = 0; - switch (perm) { - case Read: - mode = S_IRUSR | S_IRGRP | S_IROTH; - break; - case Write: - mode = S_IWUSR | S_IWGRP | S_IWOTH; - break; - case Execute: - mode = S_IXUSR | S_IXGRP | S_IXOTH; - break; - default: - ADD_FAILURE() << "unknown perm " << perm; - return false; - } - if (allow) { - stat_buf.st_mode |= mode; - } else { - stat_buf.st_mode &= ~mode; - } - return chmod(path.value().c_str(), stat_buf.st_mode) == 0; - -#elif defined(OS_WIN) - PACL old_dacl; - PSECURITY_DESCRIPTOR security_descriptor; - if (GetNamedSecurityInfo(const_cast<wchar_t*>(path.value().c_str()), - SE_FILE_OBJECT, - DACL_SECURITY_INFORMATION, NULL, NULL, &old_dacl, - NULL, &security_descriptor) != ERROR_SUCCESS) - return false; - - DWORD mode = 0; - switch (perm) { - case Read: - mode = GENERIC_READ; - break; - case Write: - mode = GENERIC_WRITE; - break; - case Execute: - mode = GENERIC_EXECUTE; - break; - default: - ADD_FAILURE() << "unknown perm " << perm; - return false; - } - - // Deny Read access for the current user. - EXPLICIT_ACCESS change; - change.grfAccessPermissions = mode; - change.grfAccessMode = allow ? GRANT_ACCESS : DENY_ACCESS; - change.grfInheritance = 0; - change.Trustee.pMultipleTrustee = NULL; - change.Trustee.MultipleTrusteeOperation = NO_MULTIPLE_TRUSTEE; - change.Trustee.TrusteeForm = TRUSTEE_IS_NAME; - change.Trustee.TrusteeType = TRUSTEE_IS_USER; - change.Trustee.ptstrName = L"CURRENT_USER"; - - PACL new_dacl; - if (SetEntriesInAcl(1, &change, old_dacl, &new_dacl) != ERROR_SUCCESS) { - LocalFree(security_descriptor); - return false; - } - - DWORD rc = SetNamedSecurityInfo(const_cast<wchar_t*>(path.value().c_str()), - SE_FILE_OBJECT, DACL_SECURITY_INFORMATION, - NULL, NULL, new_dacl, NULL); - LocalFree(security_descriptor); - LocalFree(new_dacl); - - return rc == ERROR_SUCCESS; -#else - NOTIMPLEMENTED(); - return false; -#endif -} - -#if defined(OS_MACOSX) -// Linux implementation of FilePathWatcher doesn't catch attribute changes. -// http://crbug.com/78043 -// Windows implementation of FilePathWatcher catches attribute changes that -// don't affect the path being watched. -// http://crbug.com/78045 - -// Verify that changing attributes on a directory works. -TEST_F(FilePathWatcherTest, DirAttributesChanged) { - FilePath test_dir1(temp_dir_.path().AppendASCII("DirAttributesChangedDir1")); - FilePath test_dir2(test_dir1.AppendASCII("DirAttributesChangedDir2")); - FilePath test_file(test_dir2.AppendASCII("DirAttributesChangedFile")); - // Setup a directory hierarchy. - ASSERT_TRUE(file_util::CreateDirectory(test_dir1)); - ASSERT_TRUE(file_util::CreateDirectory(test_dir2)); - ASSERT_TRUE(WriteFile(test_file, "content")); - - FilePathWatcher watcher; - scoped_refptr<TestDelegate> delegate(new TestDelegate(collector())); - ASSERT_TRUE(SetupWatch(test_file, &watcher, delegate.get())); - - // We should not get notified in this case as it hasn't affected our ability - // to access the file. - ASSERT_TRUE(ChangeFilePermissions(test_dir1, Read, false)); - loop_.PostDelayedTask(FROM_HERE, - new MessageLoop::QuitTask, - TestTimeouts::tiny_timeout_ms()); - ASSERT_FALSE(WaitForEvents()); - ASSERT_TRUE(ChangeFilePermissions(test_dir1, Read, true)); - - // We should get notified in this case because filepathwatcher can no - // longer access the file - ASSERT_TRUE(ChangeFilePermissions(test_dir1, Execute, false)); - ASSERT_TRUE(WaitForEvents()); - ASSERT_TRUE(ChangeFilePermissions(test_dir1, Execute, true)); -} - -#endif // OS_MACOSX } // namespace diff --git a/content/common/file_path_watcher/file_path_watcher_inotify.cc b/content/common/file_path_watcher/file_path_watcher_inotify.cc index 4bc7d8c..50d7342 100644 --- a/content/common/file_path_watcher/file_path_watcher_inotify.cc +++ b/content/common/file_path_watcher/file_path_watcher_inotify.cc @@ -98,7 +98,8 @@ class FilePathWatcherImpl : public FilePathWatcher::PlatformDelegate, // Start watching |path| for changes and notify |delegate| on each change. // Returns true if watch for |path| has been added successfully. virtual bool Watch(const FilePath& path, - FilePathWatcher::Delegate* delegate) OVERRIDE; + FilePathWatcher::Delegate* delegate, + base::MessageLoopProxy* loop) OVERRIDE; // Cancel the watch. This unregisters the instance with InotifyReader. virtual void Cancel() OVERRIDE; @@ -363,7 +364,8 @@ void FilePathWatcherImpl::OnFilePathChanged( } bool FilePathWatcherImpl::Watch(const FilePath& path, - FilePathWatcher::Delegate* delegate) { + FilePathWatcher::Delegate* delegate, + base::MessageLoopProxy*) { DCHECK(target_.empty()); DCHECK(MessageLoopForIO::current()); diff --git a/content/common/file_path_watcher/file_path_watcher_mac.cc b/content/common/file_path_watcher/file_path_watcher_mac.cc index de01cc0..ba0c1c3 100644 --- a/content/common/file_path_watcher/file_path_watcher_mac.cc +++ b/content/common/file_path_watcher/file_path_watcher_mac.cc @@ -4,480 +4,295 @@ #include "content/common/file_path_watcher/file_path_watcher.h" -#include <fcntl.h> -#include <sys/event.h> -#include <sys/param.h> - -#include <vector> +#include <CoreServices/CoreServices.h> +#include <set> +#include "base/file_path.h" #include "base/file_util.h" +#include "base/logging.h" +#include "base/mac/scoped_cftyperef.h" +#include "base/memory/singleton.h" #include "base/message_loop.h" -#include "base/message_loop_proxy.h" -#include "base/stringprintf.h" +#include "base/time.h" + +// Note to future well meaning engineers. Unless kqueue semantics have changed +// considerably, do NOT try to reimplement this class using kqueue. The main +// problem is that this class requires the ability to watch a directory +// and notice changes to any files within it. A kqueue on a directory can watch +// for creation and deletion of files, but not for modifications to files within +// the directory. To do this with the current kqueue semantics would require +// kqueueing every file in the directory, and file descriptors are a limited +// resource. If you have a good idea on how to get around this, the source for a +// reasonable implementation of this class using kqueues is attached here: +// http://code.google.com/p/chromium/issues/detail?id=54822#c13 namespace { -// Mac-specific file watcher implementation based on kqueue. -// Originally it was based on FSEvents so that the semantics were equivalent -// on Linux, OSX and Windows where it was able to detect: -// - file creation/deletion/modification in a watched directory -// - file creation/deletion/modification for a watched file -// - modifications to the paths to a watched object that would affect the -// object such as renaming/attibute changes etc. -// The FSEvents version did all of the above except handling attribute changes -// to path components. Unfortunately FSEvents appears to have an issue where the -// current implementation (Mac OS X 10.6.7) sometimes drops events and doesn't -// send notifications. See -// http://code.google.com/p/chromium/issues/detail?id=54822#c31 for source that -// will reproduce the problem. FSEvents also required having a CFRunLoop -// backing the thread that it was running on, that caused added complexity -// in the interfaces. -// The kqueue implementation will handle all of the items in the list above -// except for detecting modifications to files in a watched directory. It will -// detect the creation and deletion of files, just not the modification of -// files. It does however detect the attribute changes that the FSEvents impl -// would miss. +// The latency parameter passed to FSEventsStreamCreate(). +const CFAbsoluteTime kEventLatencySeconds = 0.3; + +// Mac-specific file watcher implementation based on the FSEvents API. class FilePathWatcherImpl : public FilePathWatcher::PlatformDelegate, - public MessageLoopForIO::Watcher, public MessageLoop::DestructionObserver { public: - FilePathWatcherImpl() : kqueue_(-1) {} - virtual ~FilePathWatcherImpl() {} + FilePathWatcherImpl(); - // MessageLoopForIO::Watcher overrides. - virtual void OnFileCanReadWithoutBlocking(int fd) OVERRIDE; - virtual void OnFileCanWriteWithoutBlocking(int fd) OVERRIDE; + // Called from the FSEvents callback whenever there is a change to the paths + void OnFilePathChanged(); - // MessageLoop::DestructionObserver overrides. - virtual void WillDestroyCurrentMessageLoop() OVERRIDE; + // (Re-)Initialize the event stream to start reporting events from + // |start_event|. + void UpdateEventStream(FSEventStreamEventId start_event); // FilePathWatcher::PlatformDelegate overrides. virtual bool Watch(const FilePath& path, - FilePathWatcher::Delegate* delegate) OVERRIDE; + FilePathWatcher::Delegate* delegate, + base::MessageLoopProxy* loop) OVERRIDE; virtual void Cancel() OVERRIDE; - private: - class EventData { - public: - EventData(const FilePath& path, const FilePath::StringType& subdir) - : path_(path), subdir_(subdir) { } - FilePath path_; // Full path to this item. - FilePath::StringType subdir_; // Path to any sub item. - }; - typedef std::vector<struct kevent> EventVector; - - // Can only be called on |io_message_loop_|'s thread. - virtual void CancelOnMessageLoopThread() OVERRIDE; - - // Returns true if the kevent values are error free. - bool AreKeventValuesValid(struct kevent* kevents, int count); - - // Respond to a change of attributes of the path component represented by - // |event|. Sets |target_file_affected| to true if |target_| is affected. - // Sets |update_watches| to true if |events_| need to be updated. - void HandleAttributesChange(const EventVector::iterator& event, - bool* target_file_affected, - bool* update_watches); - - // Respond to a move of deletion of the path component represented by - // |event|. Sets |target_file_affected| to true if |target_| is affected. - // Sets |update_watches| to true if |events_| need to be updated. - void HandleDeleteOrMoveChange(const EventVector::iterator& event, - bool* target_file_affected, - bool* update_watches); - - // Respond to a creation of an item in the path component represented by - // |event|. Sets |target_file_affected| to true if |target_| is affected. - // Sets |update_watches| to true if |events_| need to be updated. - void HandleCreateItemChange(const EventVector::iterator& event, - bool* target_file_affected, - bool* update_watches); - - // Update |events_| with the current status of the system. - // Sets |target_file_affected| to true if |target_| is affected. - // Returns false if an error occurs. - bool UpdateWatches(bool* target_file_affected); - - // Fills |events| with one kevent per component in |path|. - // Returns the number of valid events created where a valid event is - // defined as one that has a ident (file descriptor) field != -1. - static int EventsForPath(FilePath path, EventVector *events); - - // Release a kevent generated by EventsForPath. - static void ReleaseEvent(struct kevent& event); - - // Returns a file descriptor that will not block the system from deleting - // the file it references. - static int FileDescriptorForPath(const FilePath& path); - - // Closes |*fd| and sets |*fd| to -1. - static void CloseFileDescriptor(int* fd); - - // Returns true if kevent has open file descriptor. - static bool IsKeventFileDescriptorOpen(const struct kevent& event) { - return event.ident != static_cast<uintptr_t>(-1); - } + // Deletion of the FilePathWatcher will call Cancel() to dispose of this + // object in the right thread. This also observes destruction of the required + // cleanup thread, in case it quits before Cancel() is called. + virtual void WillDestroyCurrentMessageLoop() OVERRIDE; - static EventData* EventDataForKevent(const struct kevent& event) { - return reinterpret_cast<EventData*>(event.udata); + scoped_refptr<base::MessageLoopProxy> run_loop_message_loop() { + return run_loop_message_loop_; } - EventVector events_; - scoped_refptr<base::MessageLoopProxy> io_message_loop_; - MessageLoopForIO::FileDescriptorWatcher kqueue_watcher_; - scoped_refptr<FilePathWatcher::Delegate> delegate_; - FilePath target_; - int kqueue_; + private: + virtual ~FilePathWatcherImpl() {} - DISALLOW_COPY_AND_ASSIGN(FilePathWatcherImpl); -}; + // Destroy the event stream. + void DestroyEventStream(); -void FilePathWatcherImpl::ReleaseEvent(struct kevent& event) { - CloseFileDescriptor(reinterpret_cast<int*>(&event.ident)); - EventData* entry = EventDataForKevent(event); - delete entry; - event.udata = NULL; -} + // Start observing the destruction of the |run_loop_message_loop_| thread, + // and watching the FSEventStream. + void StartObserverAndEventStream(FSEventStreamEventId start_event); -int FilePathWatcherImpl::EventsForPath(FilePath path, EventVector* events) { - DCHECK(MessageLoopForIO::current()); - // Make sure that we are working with a clean slate. - DCHECK(events->empty()); + // Cleans up and stops observing the |run_loop_message_loop_| thread. + void CancelOnMessageLoopThread() OVERRIDE; - std::vector<FilePath::StringType> components; - path.GetComponents(&components); + // Delegate to notify upon changes. + scoped_refptr<FilePathWatcher::Delegate> delegate_; - if (components.size() < 1) { - return -1; - } + // Target path to watch (passed to delegate). + FilePath target_; - int last_existing_entry = 0; - FilePath built_path; - bool path_still_exists = true; - for(std::vector<FilePath::StringType>::iterator i = components.begin(); - i != components.end(); ++i) { - if (i == components.begin()) { - built_path = FilePath(*i); - } else { - built_path = built_path.Append(*i); - } - int fd = -1; - if (path_still_exists) { - fd = FileDescriptorForPath(built_path); - if (fd == -1) { - path_still_exists = false; - } else { - ++last_existing_entry; - } - } - FilePath::StringType subdir = (i != (components.end() - 1)) ? *(i + 1) : ""; - EventData* data = new EventData(built_path, subdir); - struct kevent event; - EV_SET(&event, fd, EVFILT_VNODE, (EV_ADD | EV_CLEAR | EV_RECEIPT), - (NOTE_DELETE | NOTE_WRITE | NOTE_ATTRIB | - NOTE_RENAME | NOTE_REVOKE | NOTE_EXTEND), 0, data); - events->push_back(event); - } - return last_existing_entry; -} + // Keep track of the last modified time of the file. We use nulltime + // to represent the file not existing. + base::Time last_modified_; -int FilePathWatcherImpl::FileDescriptorForPath(const FilePath& path) { - return HANDLE_EINTR(open(path.value().c_str(), O_EVTONLY | O_NONBLOCK)); -} + // The time at which we processed the first notification with the + // |last_modified_| time stamp. + base::Time first_notification_; -void FilePathWatcherImpl::CloseFileDescriptor(int *fd) { - if (*fd == -1) { - return; - } + // Backend stream we receive event callbacks from (strong reference). + FSEventStreamRef fsevent_stream_; - if (HANDLE_EINTR(close(*fd)) != 0) { - PLOG(ERROR) << "close"; - } - *fd = -1; -} + // Run loop for FSEventStream to run on. + scoped_refptr<base::MessageLoopProxy> run_loop_message_loop_; -bool FilePathWatcherImpl::AreKeventValuesValid(struct kevent* kevents, - int count) { - if (count < 0) { - PLOG(ERROR) << "kevent"; - return false; - } - bool valid = true; - for (int i = 0; i < count; ++i) { - if (kevents[i].flags & EV_ERROR && kevents[i].data) { - // Find the kevent in |events_| that matches the kevent with the error. - EventVector::iterator event = events_.begin(); - for (; event != events_.end(); ++event) { - if (event->ident == kevents[i].ident) { - break; - } - } - std::string path_name; - if (event != events_.end()) { - EventData* event_data = EventDataForKevent(*event); - if (event_data != NULL) { - path_name = event_data->path_.value(); - } - } - if (path_name.empty()) { - path_name = base::StringPrintf( - "fd %d", *reinterpret_cast<int*>(&kevents[i].ident)); - } - LOG(ERROR) << "Error: " << kevents[i].data << " for " << path_name; - valid = false; - } - } - return valid; -} + DISALLOW_COPY_AND_ASSIGN(FilePathWatcherImpl); +}; -void FilePathWatcherImpl::HandleAttributesChange( - const EventVector::iterator& event, - bool* target_file_affected, - bool* update_watches) { - EventVector::iterator next_event = event + 1; - EventData* next_event_data = EventDataForKevent(*next_event); - // Check to see if the next item in path is still accessible. - int have_access = FileDescriptorForPath(next_event_data->path_); - if (have_access == -1) { - *target_file_affected = true; - *update_watches = true; - EventVector::iterator local_event(event); - for (; local_event != events_.end(); ++local_event) { - // Close all nodes from the event down. This has the side effect of - // potentially rendering other events in |updates| invalid. - // There is no need to remove the events from |kqueue_| because this - // happens as a side effect of closing the file descriptor. - CloseFileDescriptor(reinterpret_cast<int*>(&local_event->ident)); - } - } else { - CloseFileDescriptor(&have_access); +// The callback passed to FSEventStreamCreate(). +void FSEventsCallback(ConstFSEventStreamRef stream, + void* event_watcher, size_t num_events, + void* event_paths, const FSEventStreamEventFlags flags[], + const FSEventStreamEventId event_ids[]) { + FilePathWatcherImpl* watcher = + reinterpret_cast<FilePathWatcherImpl*>(event_watcher); + DCHECK(watcher->run_loop_message_loop()->BelongsToCurrentThread()); + + bool root_changed = false; + FSEventStreamEventId root_change_at = FSEventStreamGetLatestEventId(stream); + for (size_t i = 0; i < num_events; i++) { + if (flags[i] & kFSEventStreamEventFlagRootChanged) + root_changed = true; + if (event_ids[i]) + root_change_at = std::min(root_change_at, event_ids[i]); } -} -void FilePathWatcherImpl::HandleDeleteOrMoveChange( - const EventVector::iterator& event, - bool* target_file_affected, - bool* update_watches) { - *target_file_affected = true; - *update_watches = true; - EventVector::iterator local_event(event); - for (; local_event != events_.end(); ++local_event) { - // Close all nodes from the event down. This has the side effect of - // potentially rendering other events in |updates| invalid. - // There is no need to remove the events from |kqueue_| because this - // happens as a side effect of closing the file descriptor. - CloseFileDescriptor(reinterpret_cast<int*>(&local_event->ident)); + // Reinitialize the event stream if we find changes to the root. This is + // necessary since FSEvents doesn't report any events for the subtree after + // the directory to be watched gets created. + if (root_changed) { + // Resetting the event stream from within the callback fails (FSEvents spews + // bad file descriptor errors), so post a task to do the reset. + watcher->run_loop_message_loop()->PostTask(FROM_HERE, + NewRunnableMethod(watcher, &FilePathWatcherImpl::UpdateEventStream, + root_change_at)); } -} -void FilePathWatcherImpl::HandleCreateItemChange( - const EventVector::iterator& event, - bool* target_file_affected, - bool* update_watches) { - // Get the next item in the path. - EventVector::iterator next_event = event + 1; - EventData* next_event_data = EventDataForKevent(*next_event); - - // Check to see if it already has a valid file descriptor. - if (!IsKeventFileDescriptorOpen(*next_event)) { - // If not, attempt to open a file descriptor for it. - next_event->ident = FileDescriptorForPath(next_event_data->path_); - if (IsKeventFileDescriptorOpen(*next_event)) { - *update_watches = true; - if (next_event_data->subdir_.empty()) { - *target_file_affected = true; - } - } - } + watcher->OnFilePathChanged(); } -bool FilePathWatcherImpl::UpdateWatches(bool* target_file_affected) { - // Iterate over events adding kevents for items that exist to the kqueue. - // Then check to see if new components in the path have been created. - // Repeat until no new components in the path are detected. - // This is to get around races in directory creation in a watched path. - bool update_watches = true; - while (update_watches) { - size_t valid; - for (valid = 0; valid < events_.size(); ++valid) { - if (!IsKeventFileDescriptorOpen(events_[valid])) { - break; - } - } - if (valid == 0) { - // The root of the file path is inaccessible? - return false; - } +// FilePathWatcherImpl implementation: - EventVector updates(valid); - int count = HANDLE_EINTR(kevent(kqueue_, &events_[0], valid, &updates[0], - valid, NULL)); - if (!AreKeventValuesValid(&updates[0], count)) { - return false; - } - update_watches = false; - for (; valid < events_.size(); ++valid) { - EventData* event_data = EventDataForKevent(events_[valid]); - events_[valid].ident = FileDescriptorForPath(event_data->path_); - if (IsKeventFileDescriptorOpen(events_[valid])) { - update_watches = true; - if (event_data->subdir_.empty()) { - *target_file_affected = true; - } - } else { - break; - } - } - } - return true; +FilePathWatcherImpl::FilePathWatcherImpl() + : fsevent_stream_(NULL) { } -void FilePathWatcherImpl::OnFileCanReadWithoutBlocking(int fd) { - DCHECK(MessageLoopForIO::current()); - CHECK_EQ(fd, kqueue_); - CHECK(events_.size()); - - // Request the file system update notifications that have occurred and return - // them in |updates|. |count| will contain the number of updates that have - // occurred. - EventVector updates(events_.size()); - struct timespec timeout = {0, 0}; - int count = HANDLE_EINTR(kevent(kqueue_, NULL, 0, &updates[0], updates.size(), - &timeout)); - - // Error values are stored within updates, so check to make sure that no - // errors occurred. - if (!AreKeventValuesValid(&updates[0], count)) { - delegate_->OnError(); - Cancel(); +void FilePathWatcherImpl::OnFilePathChanged() { + // Switch to the CFRunLoop based thread if necessary, so we can tear down + // the event stream. + if (!message_loop()->BelongsToCurrentThread()) { + message_loop()->PostTask( + FROM_HERE, + NewRunnableMethod(this, &FilePathWatcherImpl::OnFilePathChanged)); return; } - bool update_watches = false; - bool send_notification = false; - - // Iterate through each of the updates and react to them. - for (int i = 0; i < count; ++i) { - // Find our kevent record that matches the update notification. - EventVector::iterator event = events_.begin(); - for (; event != events_.end(); ++event) { - if (!IsKeventFileDescriptorOpen(*event) || - event->ident == updates[i].ident) { - break; - } - } - if (!IsKeventFileDescriptorOpen(*event) || event == events_.end()) { - // The event may no longer exist in |events_| because another event - // modified |events_| in such a way to make it invalid. For example if - // the path is /foo/bar/bam and foo is deleted, NOTE_DELETE events for - // foo, bar and bam will be sent. If foo is processed first, then - // the file descriptors for bar and bam will already be closed and set - // to -1 before they get a chance to be processed. - continue; - } + DCHECK(message_loop()->BelongsToCurrentThread()); + DCHECK(!target_.empty()); - EventData* event_data = EventDataForKevent(*event); - - // If the subdir is empty, this is the last item on the path and is the - // target file. - bool target_file_affected = event_data->subdir_.empty(); - if ((updates[i].fflags & NOTE_ATTRIB) && !target_file_affected) { - HandleAttributesChange(event, &target_file_affected, &update_watches); - } else if (updates[i].fflags & (NOTE_DELETE | NOTE_REVOKE | NOTE_RENAME)) { - HandleDeleteOrMoveChange(event, &target_file_affected, &update_watches); - } else if (updates[i].fflags & NOTE_WRITE && !target_file_affected) { - HandleCreateItemChange(event, &target_file_affected, &update_watches); - } - send_notification |= target_file_affected; - } - - if (update_watches) { - if (!UpdateWatches(&send_notification)) { - delegate_->OnError(); - Cancel(); + base::PlatformFileInfo file_info; + bool file_exists = file_util::GetFileInfo(target_, &file_info); + if (file_exists && (last_modified_.is_null() || + last_modified_ != file_info.last_modified)) { + last_modified_ = file_info.last_modified; + first_notification_ = base::Time::Now(); + delegate_->OnFilePathChanged(target_); + } else if (file_exists && !first_notification_.is_null()) { + // The target's last modification time is equal to what's on record. This + // means that either an unrelated event occurred, or the target changed + // again (file modification times only have a resolution of 1s). Comparing + // file modification times against the wall clock is not reliable to find + // out whether the change is recent, since this code might just run too + // late. Moreover, there's no guarantee that file modification time and wall + // clock times come from the same source. + // + // Instead, the time at which the first notification carrying the current + // |last_notified_| time stamp is recorded. Later notifications that find + // the same file modification time only need to be forwarded until wall + // clock has advanced one second from the initial notification. After that + // interval, client code is guaranteed to having seen the current revision + // of the file. + if (base::Time::Now() - first_notification_ > + base::TimeDelta::FromSeconds(1)) { + // Stop further notifications for this |last_modification_| time stamp. + first_notification_ = base::Time(); } - } - - if (send_notification) { + delegate_->OnFilePathChanged(target_); + } else if (!file_exists && !last_modified_.is_null()) { + last_modified_ = base::Time(); delegate_->OnFilePathChanged(target_); } } -void FilePathWatcherImpl::OnFileCanWriteWithoutBlocking(int fd) { - NOTREACHED(); -} - -void FilePathWatcherImpl::WillDestroyCurrentMessageLoop() { - CancelOnMessageLoopThread(); -} - bool FilePathWatcherImpl::Watch(const FilePath& path, - FilePathWatcher::Delegate* delegate) { + FilePathWatcher::Delegate* delegate, + base::MessageLoopProxy* loop) { + DCHECK(target_.value().empty()); DCHECK(MessageLoopForIO::current()); - DCHECK(target_.value().empty()); // Can only watch one path. - DCHECK(delegate); - DCHECK_EQ(kqueue_, -1); - delegate_ = delegate; + set_message_loop(base::MessageLoopProxy::CreateForCurrentThread()); + run_loop_message_loop_ = loop; target_ = path; + delegate_ = delegate; - MessageLoop::current()->AddDestructionObserver(this); - io_message_loop_ = base::MessageLoopProxy::CreateForCurrentThread(); + FSEventStreamEventId start_event = FSEventsGetCurrentEventId(); - kqueue_ = kqueue(); - if (kqueue_ == -1) { - PLOG(ERROR) << "kqueue"; - return false; + base::PlatformFileInfo file_info; + if (file_util::GetFileInfo(target_, &file_info)) { + last_modified_ = file_info.last_modified; + first_notification_ = base::Time::Now(); } - int last_entry = EventsForPath(target_, &events_); - CHECK_NE(last_entry, 0); + run_loop_message_loop()->PostTask(FROM_HERE, + NewRunnableMethod(this, &FilePathWatcherImpl::StartObserverAndEventStream, + start_event)); - 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 true; +} - return MessageLoopForIO::current()->WatchFileDescriptor( - kqueue_, true, MessageLoopForIO::WATCH_READ, &kqueue_watcher_, this); +void FilePathWatcherImpl::StartObserverAndEventStream( + FSEventStreamEventId start_event) { + DCHECK(run_loop_message_loop()->BelongsToCurrentThread()); + MessageLoop::current()->AddDestructionObserver(this); + UpdateEventStream(start_event); } void FilePathWatcherImpl::Cancel() { - base::MessageLoopProxy* proxy = io_message_loop_.get(); - if (!proxy) { + if (!run_loop_message_loop().get()) { + // Watch was never called, so exit. set_cancelled(); return; } - if (!proxy->BelongsToCurrentThread()) { - proxy->PostTask(FROM_HERE, - NewRunnableMethod(this, &FilePathWatcherImpl::Cancel)); - return; + + // Switch to the CFRunLoop based thread if necessary, so we can tear down + // the event stream. + if (!run_loop_message_loop()->BelongsToCurrentThread()) { + run_loop_message_loop()->PostTask(FROM_HERE, + new FilePathWatcher::CancelTask(this)); + } else { + CancelOnMessageLoopThread(); } - CancelOnMessageLoopThread(); } void FilePathWatcherImpl::CancelOnMessageLoopThread() { - DCHECK(MessageLoopForIO::current()); - if (!is_cancelled()) { - set_cancelled(); - kqueue_watcher_.StopWatchingFileDescriptor(); - CloseFileDescriptor(&kqueue_); - std::for_each(events_.begin(), events_.end(), ReleaseEvent); - events_.clear(); - io_message_loop_.release(); + set_cancelled(); + if (fsevent_stream_) { + DestroyEventStream(); MessageLoop::current()->RemoveDestructionObserver(this); delegate_ = NULL; } } +void FilePathWatcherImpl::WillDestroyCurrentMessageLoop() { + CancelOnMessageLoopThread(); +} + +void FilePathWatcherImpl::UpdateEventStream(FSEventStreamEventId start_event) { + DCHECK(run_loop_message_loop()->BelongsToCurrentThread()); + DCHECK(MessageLoopForUI::current()); + + // It can happen that the watcher gets canceled while tasks that call this + // function are still in flight, so abort if this situation is detected. + if (is_cancelled()) + return; + + if (fsevent_stream_) + DestroyEventStream(); + + base::mac::ScopedCFTypeRef<CFStringRef> cf_path(CFStringCreateWithCString( + NULL, target_.value().c_str(), kCFStringEncodingMacHFS)); + base::mac::ScopedCFTypeRef<CFStringRef> cf_dir_path(CFStringCreateWithCString( + NULL, target_.DirName().value().c_str(), kCFStringEncodingMacHFS)); + CFStringRef paths_array[] = { cf_path.get(), cf_dir_path.get() }; + base::mac::ScopedCFTypeRef<CFArrayRef> watched_paths(CFArrayCreate( + NULL, reinterpret_cast<const void**>(paths_array), arraysize(paths_array), + &kCFTypeArrayCallBacks)); + + FSEventStreamContext context; + context.version = 0; + context.info = this; + context.retain = NULL; + context.release = NULL; + context.copyDescription = NULL; + + fsevent_stream_ = FSEventStreamCreate(NULL, &FSEventsCallback, &context, + watched_paths, + start_event, + kEventLatencySeconds, + kFSEventStreamCreateFlagWatchRoot); + FSEventStreamScheduleWithRunLoop(fsevent_stream_, CFRunLoopGetCurrent(), + kCFRunLoopDefaultMode); + if (!FSEventStreamStart(fsevent_stream_)) { + message_loop()->PostTask(FROM_HERE, + NewRunnableMethod(delegate_.get(), + &FilePathWatcher::Delegate::OnError)); + } +} + +void FilePathWatcherImpl::DestroyEventStream() { + FSEventStreamStop(fsevent_stream_); + FSEventStreamUnscheduleFromRunLoop(fsevent_stream_, CFRunLoopGetCurrent(), + kCFRunLoopDefaultMode); + FSEventStreamRelease(fsevent_stream_); + fsevent_stream_ = NULL; +} + } // namespace FilePathWatcher::FilePathWatcher() { diff --git a/content/common/file_path_watcher/file_path_watcher_win.cc b/content/common/file_path_watcher/file_path_watcher_win.cc index d1bb0bf..448e857 100644 --- a/content/common/file_path_watcher/file_path_watcher_win.cc +++ b/content/common/file_path_watcher/file_path_watcher_win.cc @@ -22,7 +22,8 @@ class FilePathWatcherImpl : public FilePathWatcher::PlatformDelegate, // FilePathWatcher::PlatformDelegate overrides. virtual bool Watch(const FilePath& path, - FilePathWatcher::Delegate* delegate) OVERRIDE; + FilePathWatcher::Delegate* delegate, + base::MessageLoopProxy* loop) OVERRIDE; virtual void Cancel() OVERRIDE; // Deletion of the FilePathWatcher will call Cancel() to dispose of this @@ -75,7 +76,8 @@ class FilePathWatcherImpl : public FilePathWatcher::PlatformDelegate, }; bool FilePathWatcherImpl::Watch(const FilePath& path, - FilePathWatcher::Delegate* delegate) { + FilePathWatcher::Delegate* delegate, + base::MessageLoopProxy*) { DCHECK(target_.value().empty()); // Can only watch one path. set_message_loop(base::MessageLoopProxy::CreateForCurrentThread()); |