Don't block FILE thread for media scanning

chrome/browser/chromeos/arc/arc_downloads_watcher_service.cc

 // Copyright 2016 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/chromeos/arc/arc_downloads_watcher_service.h"
 
 #include <map>
 #include <memory>
 #include <utility>
 
@@ skipping 68 matching lines @@
   using Callback = base::Callback<void(mojo::Array<mojo::String> paths)>;
 
   explicit DownloadsWatcher(const Callback& callback);
   ~DownloadsWatcher();
 
   // Starts watching Downloads directory.
   void Start();
 
  private:
   // Called by base::FilePathWatcher to notify file changes.
+  // Wrapper around HandleFilePathChanged to pass the notification timestamp.
   void OnFilePathChanged(const base::FilePath& path, bool error);
 
+  // If there has not been a full scan of updated files since notify_time,
+  // update the last_timestamp_map_ and send any new/changed files to the
+  // android media scanner.
+  void HandleFilePathChanged(base::Time notify_time);
+
   // Scans files under |downloads_dir_| recursively and builds a map from file
   // paths (in Android filesystem) to last modified timestamps.
   TimestampMap BuildTimestampMap() const;
 
   Callback callback_;
   base::FilePath downloads_dir_;
   std::unique_ptr<base::FilePathWatcher> watcher_;
   TimestampMap last_timestamp_map_;
+  base::Time last_update_time_;
 
   // Note: This should remain the last member so it'll be destroyed and
   // invalidate the weak pointers before any other members are destroyed.
   base::WeakPtrFactory<DownloadsWatcher> weak_ptr_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(DownloadsWatcher);
 };
 
 ArcDownloadsWatcherService::DownloadsWatcher::DownloadsWatcher(
     const Callback& callback)
-    : callback_(callback), weak_ptr_factory_(this) {
+    : callback_(callback),
+      last_update_time_(base::Time()),
+      weak_ptr_factory_(this) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
 
   downloads_dir_ = DownloadPrefs(ProfileManager::GetActiveUserProfile())
                        .GetDefaultDownloadDirectoryForProfile()
                        .StripTrailingSeparators();
 }
 
 ArcDownloadsWatcherService::DownloadsWatcher::~DownloadsWatcher() {
   DCHECK_CURRENTLY_ON(BrowserThread::FILE);
 }
@@ skipping 11 matching lines @@
                   base::Bind(&DownloadsWatcher::OnFilePathChanged,
                              weak_ptr_factory_.GetWeakPtr()));
 }
 
 void ArcDownloadsWatcherService::DownloadsWatcher::OnFilePathChanged(
     const base::FilePath& path,
     bool error) {
   // On Linux, |error| is always false. Also, |path| is always the same path
   // as one given to FilePathWatcher::Watch().
   DCHECK_CURRENTLY_ON(BrowserThread::FILE);
+  BrowserThread::PostTask(
+      FROM_HERE, base::Bind(&DownloadsWatcher::HandleFilePathChanged,
+                            weak_ptr_factory_.GetWeakPtr(), base::Time::Now()));
+}
+
+void ArcDownloadsWatcherService::DownloadsWatcher::HandleFilePathChanged(
+    base::Time notify_time) {
+  // Because this logic handles all updates and not just the update that
+  // triggered the callback we can return early if we have already processed
+  // updates to the directory after this callback was created.
+  if (notify_time < last_update_time_) {
+    return;
+  }
+  last_update_time_ = base::Time::Now();
 
   TimestampMap current_timestamp_map = BuildTimestampMap();

[hidehiko, 2016/09/07 01:44:32]

For the record.

As we chatted offline, because the heavy operation still blocks FILE thread, can
we move the heaviest part of this function into WorkerPool, with keeping
timestamp check in FILE thread? Something like...

OnFilePathChanged() {
  if (is_build_timestamp_map_running) {
    // Suspend the task.
    last_on_file_path_changed_time = Time::Now();
    return;
  }
  PostTaskAndReply(...,
                   Bind(HandleFilePathChanged, ...),
                   Bind(OnBuildTimestampMap, GetWeakPtr());
}

HandleFilePathChanged(...) {
  Time update_time = Time::Now();
  ... heavy operation ...
  return pair(update_time, timestamp_map);
}

OnBuildTimestampMap(update_time, timestamp_map) {
  last_timestamp_map_ = timestamp_map;
  if (last_on_file_path_changed_ > update_time) {
    // During the map building, another change had come. Re-run.
    PostTask(...);
  }
}

 
   std::vector<base::FilePath> changed_paths =
       CollectChangedPaths(last_timestamp_map_, current_timestamp_map);
 
   last_timestamp_map_ = std::move(current_timestamp_map);
 
   mojo::Array<mojo::String> mojo_paths(changed_paths.size());
   for (size_t i = 0; i < changed_paths.size(); ++i) {
     mojo_paths[i] = changed_paths[i].value();
   }
@@ skipping 72 matching lines @@
     mojo::Array<mojo::String> paths) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
 
   auto* instance = arc_bridge_service()->file_system()->instance();
   if (!instance)
     return;
   instance->RequestMediaScan(std::move(paths));
 }
 
 }  // namespace arc