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

chrome/browser/devtools/devtools_file_watcher.cc

 // Copyright (c) 2015 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 "chrome/browser/devtools/devtools_file_watcher.h"
 
 #include <algorithm>
 #include <map>
 #include <memory>
 #include <set>
 
 #include "base/bind.h"
 #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"
 
 using content::BrowserThread;
 
 static int kFirstThrottleTimeout = 10;
 static int kDefaultThrottleTimeout = 200;
 
 // DevToolsFileWatcher::SharedFileWatcher --------------------------------------
 
 class DevToolsFileWatcher::SharedFileWatcher :
     public base::RefCounted<SharedFileWatcher> {
  public:
   SharedFileWatcher();
 
   void AddListener(DevToolsFileWatcher* watcher);
   void RemoveListener(DevToolsFileWatcher* watcher);
   void AddWatch(const base::FilePath& path);
   void RemoveWatch(const base::FilePath& path);
 
  private:
   friend class base::RefCounted<
       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_;
   base::TimeDelta last_dispatch_cost_;
 };
 
 DevToolsFileWatcher::SharedFileWatcher::SharedFileWatcher()
     : last_dispatch_cost_(
           base::TimeDelta::FromMilliseconds(kDefaultThrottleTimeout)) {
   DevToolsFileWatcher::s_shared_watcher_ = this;
@@ skipping 20 matching lines @@
     return;
   if (!base::FilePathWatcher::RecursiveWatchAvailable())
     return;
   watchers_[path].reset(new base::FilePathWatcher());
   bool success = watchers_[path]->Watch(
       path, true,
       base::Bind(&SharedFileWatcher::DirectoryChanged, base::Unretained(this)));
   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();
   }
 }
 
 void DevToolsFileWatcher::SharedFileWatcher::RemoveWatch(
     const base::FilePath& path) {
   watchers_.erase(path);
 }
 
 void DevToolsFileWatcher::SharedFileWatcher::DirectoryChanged(
@@ skipping 12 matching lines @@
           last_dispatch_cost_ * 2;
 
   BrowserThread::PostDelayedTask(
       BrowserThread::FILE, FROM_HERE,
       base::Bind(&DevToolsFileWatcher::SharedFileWatcher::DispatchNotifications,
                  this), shedule_for);
   last_event_time_ = now;
 }
 
 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;
 }
 
 // static
 DevToolsFileWatcher::SharedFileWatcher*
 DevToolsFileWatcher::s_shared_watcher_ = nullptr;
 
 // DevToolsFileWatcher ---------------------------------------------------------
 
@@ skipping 19 matching lines @@
 
 void DevToolsFileWatcher::AddWatch(const base::FilePath& path) {
   DCHECK_CURRENTLY_ON(BrowserThread::FILE);
   shared_watcher_->AddWatch(path);
 }
 
 void DevToolsFileWatcher::RemoveWatch(const base::FilePath& path) {
   DCHECK_CURRENTLY_ON(BrowserThread::FILE);
   shared_watcher_->RemoveWatch(path);
 }