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);
   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_;
 };
 
 DevToolsFileWatcher::SharedFileWatcher::SharedFileWatcher()
     : last_dispatch_cost_(
           base::TimeDelta::FromMilliseconds(kDefaultThrottleTimeout)) {
   DevToolsFileWatcher::s_shared_watcher_ = this;
 }
@@ skipping 19 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_[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();
   }
 }
 
 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_) {
-    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& it : current_times) {

[dgozman, 2016/10/04 04:41:01]

I usually call it |pair| or |path_time|.

[lushnikov, 2016/10/04 15:56:35]

|path_time| is really nice, thanks

+      auto old_timestamp = old_times.find(it.first);
+      if (old_timestamp == old_times.end()) {
+        old_times[it.first] = it.second;

[dgozman, 2016/10/04 04:41:01]

Don't modify, you do swap below.

[lushnikov, 2016/10/04 15:56:35]

My bad. Done.

+        added_paths.push_back(it.first.AsUTF8Unsafe());
+      } else if (old_timestamp->second != it.second) {
+        old_timestamp->second = it.second;

[dgozman, 2016/10/04 04:41:01]

ditto

[lushnikov, 2016/10/04 15:56:35]

Done.

+        changed_paths.push_back(it.first.AsUTF8Unsafe());
       }
-      file_path = enumerator.Next();
     }
+    for (const auto& it : old_times) {

[dgozman, 2016/10/04 04:41:01]

I'd rather do:

for (const auto& path_time : old_times) {
  base::FilePath& path = path_time.first;
  if (current_times.find(path) == current_times.end())
    removed_paths.push_back(path.AsUTF8Unsafe());
}

[lushnikov, 2016/10/04 15:56:35]

This is much more clear, thanks

+      auto new_timestamp = current_times.find(it.first);
+      if (new_timestamp == current_times.end()) {
+        removed_paths.push_back(it.first.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;
 }
 
 // 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);
 }