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

Currently, DevTools file watcher reports both added and changed paths
as "changed" and doesn't report removed paths.

This patch improves DevTools file watcher so that it starts reporting
changed, added and removed paths.

BUG=none
R=dgozman, pfeldman

Review-Url: https://codereview.chromium.org/2384343002
Cr-Commit-Position: refs/heads/master@{#422838}

chrome/browser/devtools/devtools_file_helper.cc

@@ -456,6 +456,8 @@ void DevToolsFileHelper::FileSystemPathsSettingChanged() {
 }
 
 void DevToolsFileHelper::FilePathsChanged(
-    const std::vector<std::string>& paths) {
-  delegate_->FilePathsChanged(paths);
+    const std::vector<std::string>& changed_paths,
+    const std::vector<std::string>& added_paths,
+    const std::vector<std::string>& removed_paths) {
+  delegate_->FilePathsChanged(changed_paths, added_paths, removed_paths);
 }

chrome/browser/devtools/devtools_file_helper.h

@@ -47,7 +47,10 @@ class DevToolsFileHelper {
     virtual ~Delegate() {}
     virtual void FileSystemAdded(const FileSystem& file_system) = 0;
     virtual void FileSystemRemoved(const std::string& file_system_path) = 0;
-    virtual void FilePathsChanged(const std::vector<std::string>& paths) = 0;
+    virtual void FilePathsChanged(
+        const std::vector<std::string>& changed_paths,
+        const std::vector<std::string>& added_paths,
+        const std::vector<std::string>& removed_paths) = 0;
   };
 
   DevToolsFileHelper(content::WebContents* web_contents, Profile* profile,
@@ -132,7 +135,9 @@ class DevToolsFileHelper {
       const base::FilePath& path,
       bool allowed);
   void FileSystemPathsSettingChanged();
-  void FilePathsChanged(const std::vector<std::string>& paths);
+  void FilePathsChanged(const std::vector<std::string>& changed_paths,
+                        const std::vector<std::string>& added_paths,
+                        const std::vector<std::string>& removed_paths);
 
   content::WebContents* web_contents_;
   Profile* profile_;

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,13 +39,15 @@ class DevToolsFileWatcher::SharedFileWatcher :
       DevToolsFileWatcher::SharedFileWatcher>;
   ~SharedFileWatcher();
 
+  using FilePathTimesMap = std::map<base::FilePath, base::Time>;
+  void GetModificationTimes(const base::FilePath& path,
+                            FilePathTimesMap* file_path_times);
   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::map<base::FilePath, FilePathTimesMap> file_path_times_;
   std::set<base::FilePath> pending_paths_;
   base::Time last_event_time_;
   base::TimeDelta last_dispatch_cost_;
@@ -84,11 +87,17 @@ void DevToolsFileWatcher::SharedFileWatcher::AddWatch(
   if (!success)
     return;
 
+  GetModificationTimes(path, &file_path_times_[path]);
+}
+
+void DevToolsFileWatcher::SharedFileWatcher::GetModificationTimes(
+    const base::FilePath& path,
+    FilePathTimesMap* times_map) {
   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();
+    (*times_map)[file_path] = file_info.GetLastModifiedTime();
     file_path = enumerator.Next();
   }
 }
@@ -121,26 +130,38 @@ 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_) {
-    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());
+
+  for (const auto& path : pending_paths_) {
+    FilePathTimesMap& old_times = file_path_times_[path];
+    FilePathTimesMap current_times;
+    GetModificationTimes(path, &current_times);
+    for (const auto& path_time : current_times) {
+      const base::FilePath& path = path_time.first;
+      auto old_timestamp = old_times.find(path);
+      if (old_timestamp == old_times.end())
+        added_paths.push_back(path.AsUTF8Unsafe());
+      else if (old_timestamp->second != path_time.second)
+        changed_paths.push_back(path.AsUTF8Unsafe());
     }
-      file_path = enumerator.Next();
+    for (const auto& path_time : old_times) {
+      const base::FilePath& path = path_time.first;
+      if (current_times.find(path) == current_times.end())
+        removed_paths.push_back(path.AsUTF8Unsafe());
     }
+    old_times.swap(current_times);
   }
   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;
 }

chrome/browser/devtools/devtools_file_watcher.h

@@ -17,7 +17,9 @@ class FilePathWatcher;
 
 class DevToolsFileWatcher {
  public:
-  using WatchCallback = base::Callback<void(const std::vector<std::string>&)>;
+  using WatchCallback = base::Callback<void(const std::vector<std::string>&,
+                                            const std::vector<std::string>&,
+                                            const std::vector<std::string>&)>;
   explicit DevToolsFileWatcher(const WatchCallback& callback);
   ~DevToolsFileWatcher();
 

chrome/browser/devtools/devtools_ui_bindings.cc

@@ -977,10 +977,17 @@ void DevToolsUIBindings::FileSystemRemoved(
 }
 
 void DevToolsUIBindings::FilePathsChanged(
-    const std::vector<std::string>& file_paths) {
+    const std::vector<std::string>& changed_paths,
+    const std::vector<std::string>& added_paths,
+    const std::vector<std::string>& removed_paths) {
   base::ListValue list;
-  for (auto path : file_paths)
+  for (auto path : changed_paths)
+    list.AppendString(path);
+  for (auto path : added_paths)
     list.AppendString(path);
+  for (auto path : removed_paths)
+    list.AppendString(path);
+
   CallClientFunction("DevToolsAPI.fileSystemFilesChanged",
                      &list, NULL, NULL);
 }

chrome/browser/devtools/devtools_ui_bindings.h

@@ -177,7 +177,9 @@ class DevToolsUIBindings : public DevToolsEmbedderMessageDispatcher::Delegate,
   void FileSystemAdded(
       const DevToolsFileHelper::FileSystem& file_system) override;
   void FileSystemRemoved(const std::string& file_system_path) override;
-  void FilePathsChanged(const std::vector<std::string>& file_paths) override;
+  void FilePathsChanged(const std::vector<std::string>& changed_paths,
+                        const std::vector<std::string>& added_paths,
+                        const std::vector<std::string>& removed_paths) override;
 
   // DevToolsFileHelper callbacks.
   void FileSavedAs(const std::string& url);

third_party/WebKit/Source/devtools/front_end/devtools.js

@@ -194,9 +194,12 @@ DevToolsAPIImpl.prototype = {
         this._dispatchOnInspectorFrontendAPI("fileSystemAdded", ["", fileSystem]);
     },
 
-    fileSystemFilesChanged: function(path)
+    /**
+     * @param {!Array<string>} changedPaths
+     */
+    fileSystemFilesChanged: function(changedPaths)
     {
-        this._dispatchOnInspectorFrontendAPI("fileSystemFilesChanged", [path]);
+        this._dispatchOnInspectorFrontendAPI("fileSystemFilesChanged", [changedPaths]);
     },
 
     /**

third_party/WebKit/Source/devtools/front_end/workspace/IsolatedFileSystemManager.js

@@ -34,6 +34,8 @@
  */
 WebInspector.IsolatedFileSystemManager = function()
 {
+    WebInspector.Object.call(this);
+
     /** @type {!Map<string, !WebInspector.IsolatedFileSystem>} */
     this._fileSystems = new Map();
     /** @type {!Map<number, function(!Array.<string>)>} */