DevTools: improve DevTools file watcher to send added and removed paths

chrome/browser/devtools/devtools_file_watcher.cc

Index: chrome/browser/devtools/devtools_file_watcher.cc
diff --git a/chrome/browser/devtools/devtools_file_watcher.cc b/chrome/browser/devtools/devtools_file_watcher.cc
index e5af5a5f198df8f249f0c90fead55bb3a152d088..d367cfb5c6f3ca667258ae243baeba5d68fd6085 100644
--- a/chrome/browser/devtools/devtools_file_watcher.cc
+++ b/chrome/browser/devtools/devtools_file_watcher.cc
@@ -13,6 +13,7 @@
 #include "base/files/file_enumerator.h"
 #include "base/files/file_path.h"
 #include "base/files/file_path_watcher.h"
+#include "base/files/file_util.h"
 #include "base/memory/ref_counted.h"
 #include "content/public/browser/browser_thread.h"
 
@@ -38,12 +39,14 @@ class DevToolsFileWatcher::SharedFileWatcher :
       DevToolsFileWatcher::SharedFileWatcher>;
   ~SharedFileWatcher();
 
+  using FilePathTimesMap = std::map<base::FilePath, base::Time>;
+  void GetModificationTimes(const base::FilePath& path,
+                            FilePathTimesMap& file_path_times);

[dgozman, 2016/10/04 00:17:02]

Out parameters must be passed by pointer.

[lushnikov, 2016/10/04 01:04:18]

Done.

   void DirectoryChanged(const base::FilePath& path, bool error);
   void DispatchNotifications();
 
   std::vector<DevToolsFileWatcher*> listeners_;
   std::map<base::FilePath, std::unique_ptr<base::FilePathWatcher>> watchers_;
-  using FilePathTimesMap = std::map<base::FilePath, base::Time>;
   FilePathTimesMap file_path_times_;
   std::set<base::FilePath> pending_paths_;
   base::Time last_event_time_;
@@ -84,11 +87,17 @@ void DevToolsFileWatcher::SharedFileWatcher::AddWatch(
   if (!success)
     return;
 
+  GetModificationTimes(path, file_path_times_);
+}
+
+void DevToolsFileWatcher::SharedFileWatcher::GetModificationTimes(
+    const base::FilePath& path,
+    FilePathTimesMap& file_path_times) {
   base::FileEnumerator enumerator(path, true, base::FileEnumerator::FILES);
   base::FilePath file_path = enumerator.Next();
   while (!file_path.empty()) {
     base::FileEnumerator::FileInfo file_info = enumerator.GetInfo();
-    file_path_times_[file_path] = file_info.GetLastModifiedTime();
+    file_path_times[file_path] = file_info.GetLastModifiedTime();
     file_path = enumerator.Next();
   }
 }
@@ -121,26 +130,45 @@ void DevToolsFileWatcher::SharedFileWatcher::DirectoryChanged(
 }
 
 void DevToolsFileWatcher::SharedFileWatcher::DispatchNotifications() {
+  if (!pending_paths_.size())
+    return;
   base::Time start = base::Time::Now();
+  std::vector<std::string> added_paths;
+  std::vector<std::string> removed_paths;
   std::vector<std::string> changed_paths;
-  for (auto path : pending_paths_) {
+
+  for (auto iterator = file_path_times_.begin();
+       iterator != file_path_times_.end();) {

[dgozman, 2016/10/04 00:17:02]

Looks to be expensive to check all files on any single update.

[lushnikov, 2016/10/04 01:04:18]

This method is actually throttled, so should be fine (see
DevToolsFileWatcher::SharedFileWatcher::DirectoryChanged)

+    if (!base::PathExists(iterator->first)) {
+      removed_paths.push_back(iterator->first.AsUTF8Unsafe());
+      file_path_times_.erase(iterator++);
+    } else {
+      ++iterator;
+    }
+  }
+
+  for (const auto& path : pending_paths_) {
     base::FileEnumerator enumerator(path, true, base::FileEnumerator::FILES);
     base::FilePath file_path = enumerator.Next();
-    while (!file_path.empty()) {
-      base::FileEnumerator::FileInfo file_info = enumerator.GetInfo();
-      base::Time new_time = file_info.GetLastModifiedTime();
-      if (file_path_times_[file_path] != new_time) {
-        file_path_times_[file_path] = new_time;
-        changed_paths.push_back(file_path.AsUTF8Unsafe());
+    FilePathTimesMap new_file_times;
+    GetModificationTimes(path, new_file_times);
+    for (auto it = new_file_times.begin(); it != new_file_times.end(); ++it) {
+      auto old_timestamp = file_path_times_.find(it->first);
+      if (old_timestamp == file_path_times_.end()) {
+        file_path_times_[it->first] = it->second;
+        added_paths.push_back(it->first.AsUTF8Unsafe());
+      } else if (old_timestamp->second != it->second) {
+        old_timestamp->second = it->second;
+        changed_paths.push_back(it->first.AsUTF8Unsafe());
       }
-      file_path = enumerator.Next();
     }
   }
   pending_paths_.clear();
 
   for (auto* watcher : listeners_) {
     BrowserThread::PostTask(BrowserThread::UI, FROM_HERE,
-                            base::Bind(watcher->callback_, changed_paths));
+                            base::Bind(watcher->callback_, changed_paths,
+                                       added_paths, removed_paths));
   }
   last_dispatch_cost_ = base::Time::Now() - start;
 }