Revert "Initial implementation of the FileSystemScanner"

This reverts commit 1e9bd7b2.

Reason for revert: Tests crash on MSan. See

https://ci.chromium.org/p/chromium/builders/ci/Linux%20ChromiumOS%20MSan%20Tests/17015

Original change's description:
> Initial implementation of the FileSystemScanner
> 
> This implements the initial version of FileSystemScanner
> (go/arc-fs-scanner).
> 
> Bug: b:145254635
> Test: unit_tests --gtest_filter="Arc*"
> Test: Manually populated a big file system and triggered different file
> Test: system events.
> Change-Id: I14bebb3719532afac54e1913229045fb2740a9ff
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1946177
> Reviewed-by: Hidehiko Abe <hidehiko@chromium.org>
> Reviewed-by: Mattias Nissler <mnissler@chromium.org>
> Commit-Queue: Umut Barış Öztunç <umutoztunc@google.com>
> Auto-Submit: Umut Barış Öztunç <umutoztunc@google.com>
> Cr-Commit-Position: refs/heads/master@{#727555}

TBR=hashimoto@chromium.org,yusukes@chromium.org,fukino@chromium.org,hidehiko@chromium.org,elijahtaylor@chromium.org,mnissler@chromium.org,kerrnel@chromium.org,risan@chromium.org,umutoztunc@google.com

Change-Id: I9b27f3bdd8d74586efdd6e141d0debc8b85b4a70
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: b:145254635
Bug: 1038049
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1981267Reviewed-by: Xiaocheng Hu <xiaochengh@chromium.org>
Commit-Queue: Xiaocheng Hu <xiaochengh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#727579}

chrome/browser/chromeos/BUILD.gn

@@ -531,8 +531,6 @@ source_set("chromeos") {
     "arc/file_system_watcher/arc_file_system_watcher_service.h",
     "arc/file_system_watcher/arc_file_system_watcher_util.cc",
     "arc/file_system_watcher/arc_file_system_watcher_util.h",
-    "arc/file_system_watcher/file_system_scanner.cc",
-    "arc/file_system_watcher/file_system_scanner.h",
     "arc/fileapi/arc_content_file_system_async_file_util.cc",
     "arc/fileapi/arc_content_file_system_async_file_util.h",
     "arc/fileapi/arc_content_file_system_backend_delegate.cc",
@@ -2597,7 +2595,6 @@ source_set("unit_tests") {
     "arc/extensions/arc_support_message_host_unittest.cc",
     "arc/file_system_watcher/arc_file_system_watcher_service_unittest.cc",
     "arc/file_system_watcher/arc_file_system_watcher_util_unittest.cc",
-    "arc/file_system_watcher/file_system_scanner_unittest.cc",
     "arc/fileapi/arc_content_file_system_async_file_util_unittest.cc",
     "arc/fileapi/arc_content_file_system_file_stream_reader_unittest.cc",
     "arc/fileapi/arc_content_file_system_file_stream_writer_unittest.cc",

chrome/browser/chromeos/arc/file_system_watcher/file_system_scanner.h

-// Copyright 2019 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.
-
-#ifndef CHROME_BROWSER_CHROMEOS_ARC_FILE_SYSTEM_WATCHER_FILE_SYSTEM_SCANNER_H_
-#define CHROME_BROWSER_CHROMEOS_ARC_FILE_SYSTEM_WATCHER_FILE_SYSTEM_SCANNER_H_
-
-#include <string>
-#include <vector>
-
-#include "base/callback.h"
-#include "base/files/file_path.h"
-#include "base/memory/ref_counted.h"
-#include "base/memory/weak_ptr.h"
-#include "base/time/time.h"
-#include "base/timer/timer.h"
-
-namespace arc {
-
-class ArcBridgeService;
-
-struct RegularScanResult {
-  RegularScanResult();
-  ~RegularScanResult();
-  RegularScanResult(RegularScanResult&&);
-  RegularScanResult& operator=(RegularScanResult&&);
-
-  std::vector<std::string> modified_files;
-  std::vector<std::string> modified_directories;
-};
-
-// Periodical scanner to detect file system modifications in a directory. It is
-// activated when FileSystemWatcher throws an error, e.g., the file system is
-// too large.
-// TODO(risan): Address all remaining feedbacks from next iterations of
-// https://chromium-review.googlesource.com/c/chromium/src/+/1946177 before
-// enabling this.
-class FileSystemScanner {
- public:
-  using GetLastChangeTimeCallback =
-      base::RepeatingCallback<base::Time(const base::FilePath& path)>;
-
-  // |cros_dir| is the directory that will be scanned by the scanner.
-  // |android_dir| is the respective path of |cros_dir| which is mounted to the
-  // Android container.
-  FileSystemScanner(const base::FilePath& cros_dir,
-                    const base::FilePath& android_dir,
-                    ArcBridgeService* arc_bridge_service);
-
-  // This constructor is only testing.
-  FileSystemScanner(const base::FilePath& cros_dir,
-                    const base::FilePath& android_dir,
-                    ArcBridgeService* arc_bridge_service,
-                    GetLastChangeTimeCallback ctime_callback);
-
-  FileSystemScanner(const FileSystemScanner&) = delete;
-  FileSystemScanner& operator=(const FileSystemScanner&) = delete;
-
-  ~FileSystemScanner();
-
-  // Starts scanning the directory.
-  void Start();
-
- private:
-  // Schedules a full scan that triggers RequestMediaScan for all files and
-  // triggers ReindexDirectory for |android_dir_|. The reason for reindexing is
-  // to ensure that there are no indexes for non-existing files in MediaStore.
-  void ScheduleFullScan();
-
-  // Called after a full scan is finished. It updates |previous_scan_time_|
-  // accordingly. It also  updates the state, so that a regular scan can be
-  // scheduled.
-  void OnFullScanFinished(base::Time current_scan_time,
-                          std::vector<std::string> media_files);
-
-  // Schedules a regular scan if there is no ongoing scan at that time. Regular
-  // scan triggers RequestMediaScan only for the files that are modified after
-  // the previous (full or regular) scan. It also calls RequestFileRemovalScan
-  // for the modified directories to detect the removed files and directories
-  // under them. So that, their entries are removed from MediaStore.
-  void ScheduleRegularScan();
-
-  // Called after a regular scan is finished. It updates |previous_scan_time_|
-  // accordingly. It also updates the state, so that another regular scan can be
-  // done without skipping unless there is a full scan scheduled.
-  void OnRegularScanFinished(base::Time current_scan_time,
-                             RegularScanResult result);
-
-  // Wrapper function that calls ReindexDirectory through mojo interface.
-  void ReindexDirectory(const std::string& directory_path);
-
-  // Wrapper function that calls RequestFileRemovalScan through mojo interface.
-  void RequestFileRemovalScan(const std::vector<std::string>& directory_paths);
-
-  // Wrapper function that calls RequestMediaScan through mojo interface.
-  void RequestMediaScan(const std::vector<std::string>& files);
-
-  // Internal state which is used to skip regular scans when there is an ongoing
-  // regular scan or there is a full scan scheduled and has not finished yet.
-  enum class State {
-    // Neither a regular scan nor full scan is happening. A scan can only be
-    // scheduled in this state.
-    kIdle,
-    // There is a scan which is already PostTask'ed but haven't finished yet.
-    // State will return to |kIdle| in the OnScanFinished function.
-    kWaitingForScanToFinish,
-  };
-  State state_;
-
-  const base::FilePath cros_dir_;
-  const base::FilePath android_dir_;
-  ArcBridgeService* const arc_bridge_service_;
-
-  // The timestamp of the start of the last scan.
-  base::Time previous_scan_time_;
-  // The task runner which runs the scans to avoid blocking the UI thread.
-  scoped_refptr<base::SequencedTaskRunner> scan_runner_;
-  // Calls ScheduleRegularScan every |kFileSystemScannerInterval|.
-  base::RepeatingTimer timer_;
-  GetLastChangeTimeCallback ctime_callback_;
-  base::WeakPtrFactory<FileSystemScanner> weak_ptr_factory_{this};
-
-  FRIEND_TEST_ALL_PREFIXES(ArcFileSystemScannerTest, ScheduleFullScan);
-  FRIEND_TEST_ALL_PREFIXES(ArcFileSystemScannerTest,
-                           ScheduleRegularScanCreateTopLevelFile);
-  FRIEND_TEST_ALL_PREFIXES(ArcFileSystemScannerTest,
-                           ScheduleRegularScanCreateTopLevelDirectory);
-  FRIEND_TEST_ALL_PREFIXES(ArcFileSystemScannerTest,
-                           ScheduleRegularScanModifyTopLevelFile);
-  FRIEND_TEST_ALL_PREFIXES(ArcFileSystemScannerTest,
-                           ScheduleRegularScanModifyTopLevelDirectory);
-  FRIEND_TEST_ALL_PREFIXES(ArcFileSystemScannerTest,
-                           ScheduleRegularScanRenameTopLevelFile);
-  FRIEND_TEST_ALL_PREFIXES(ArcFileSystemScannerTest,
-                           ScheduleRegularScanRenameTopLevelDirectory);
-  FRIEND_TEST_ALL_PREFIXES(ArcFileSystemScannerTest,
-                           ScheduleRegularScanDeleteTopLevelFile);
-  FRIEND_TEST_ALL_PREFIXES(ArcFileSystemScannerTest,
-                           ScheduleRegularScanDeleteTopLevelDirectory);
-  FRIEND_TEST_ALL_PREFIXES(ArcFileSystemScannerTest,
-                           ScheduleRegularScanCreateNestedFile);
-  FRIEND_TEST_ALL_PREFIXES(ArcFileSystemScannerTest,
-                           ScheduleRegularScanCreateNestedDirectory);
-  FRIEND_TEST_ALL_PREFIXES(ArcFileSystemScannerTest,
-                           ScheduleRegularScanModifyNestedFile);
-  FRIEND_TEST_ALL_PREFIXES(ArcFileSystemScannerTest,
-                           ScheduleRegularScanModifyNestedDirectory);
-  FRIEND_TEST_ALL_PREFIXES(ArcFileSystemScannerTest,
-                           ScheduleRegularScanRenameNestedFile);
-  FRIEND_TEST_ALL_PREFIXES(ArcFileSystemScannerTest,
-                           ScheduleRegularScanRenameNestedDirectory);
-  FRIEND_TEST_ALL_PREFIXES(ArcFileSystemScannerTest,
-                           ScheduleRegularScanDeleteNestedFile);
-  FRIEND_TEST_ALL_PREFIXES(ArcFileSystemScannerTest,
-                           ScheduleRegularScanDeleteNestedDirectory);
-  FRIEND_TEST_ALL_PREFIXES(ArcFileSystemScannerTest,
-                           ScheduleRegularScanNoChange);
-};
-
-}  // namespace arc
-
-#endif  // CHROME_BROWSER_CHROMEOS_ARC_FILE_SYSTEM_WATCHER_FILE_SYSTEM_SCANNER_H_

components/arc/mojom/file_system.mojom

@@ -2,7 +2,7 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 //
-// Next MinVersion: 15
+// Next MinVersion: 14
 
 module arc.mojom;
 
@@ -278,7 +278,7 @@ interface FileSystemHost {
       (FileSelectorElements elements);
 };
 
-// Next method ID: 21
+// Next method ID: 19
 interface FileSystemInstance {
   // Notes about Android Documents Provider:
   //
@@ -409,13 +409,6 @@ interface FileSystemInstance {
   // MediaProvider is removed.
   RequestMediaScan@0(array<string> paths);
 
-  // Reloads and refreshes entries in MediaStore under |directory_path|.
-  [MinVersion=14] ReindexDirectory@19(string directory_path);
-
-  // Searches for the removed files/directories under the given
-  // |directory_paths| (non-recursively) in MediaStore and removes them.
-  [MinVersion=14] RequestFileRemovalScan@20(array<string> directory_paths);
-
   // Opens URLs by sending an intent to the specified activity.
   // Since this grants read/write URL permissions to the activity, callers must
   // ensure that the user is correctly aware of the URLs and the activity they

components/arc/test/fake_file_system_instance.cc

@@ -62,8 +62,6 @@ constexpr size_t kMaxBytesToReadFromPipe = 8 * 1024;  // 8KB;
 
 }  // namespace
 
-constexpr base::FilePath::CharType FakeFileSystemInstance::kFakeAndroidPath[];
-
 FakeFileSystemInstance::File::File(const std::string& url,
                                    const std::string& content,
                                    const std::string& mime_type,
@@ -176,23 +174,6 @@ void FakeFileSystemInstance::AddRoot(const Root& root) {
   roots_.push_back(root);
 }
 
-void FakeFileSystemInstance::SetGetLastChangeTimeCallback(
-    GetLastChangeTimeCallback ctime_callback) {
-  DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
-  ctime_callback_ = ctime_callback;
-}
-
-void FakeFileSystemInstance::SetCrosDir(const base::FilePath& cros_dir) {
-  DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
-  cros_dir_ = cros_dir;
-}
-
-void FakeFileSystemInstance::SetMediaStore(
-    const std::map<base::FilePath, base::Time>& media_store) {
-  DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
-  media_store_ = media_store;
-}
-
 void FakeFileSystemInstance::TriggerWatchers(
     const std::string& authority,
     const std::string& document_id,
@@ -574,65 +555,10 @@ void FakeFileSystemInstance::RemoveWatcher(int64_t watcher_id,
       FROM_HERE, base::BindOnce(std::move(callback), true));
 }
 
-// TODO(risan): "Added" directory might not be handled. Please double check
-// this.
 void FakeFileSystemInstance::RequestMediaScan(
     const std::vector<std::string>& paths) {
   DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
-  // TODO(risan): This is to prevent crashing other tests that expect nothing
-  // from RequestMediaScan, e.g., the following:
-  // FilesAppBrowserTest.Test/dirContextMenuDocumentsProvider_DocumentsProvider
-  if (cros_dir_.empty())
-    return;
-  for (const auto& path : paths) {
-    base::FilePath file_path = base::FilePath(path);
-    base::FilePath cros_path = GetCrosPath(file_path);
-    if (PathExists(cros_path)) {
-      // For each existing path, index itself and all parent directories of
-      // it.
-      base::Time ctime;
-      if (!DirectoryExists(cros_path))
-        ctime = ctime_callback_.Run(cros_path);
-      media_store_[file_path] = ctime;
-      file_path = file_path.DirName();
-      while (file_path != base::FilePath(kFakeAndroidPath).DirName()) {
-        media_store_[file_path] = base::Time();
-        file_path = file_path.DirName();
-      }
-    } else {
-      // When a file or directory does not exist, it means it has been
-      // deleted. So we need to erase its index entry in |media_store_|, and
-      // also the entries of all files/directories underneath it if it is a
-      // directory.
-      for (auto it = media_store_.begin(); it != media_store_.end();) {
-        if (it->first == file_path || file_path.IsParent(it->first))
-          media_store_.erase(it++);
-        else
-          ++it;
-      }
-    }
-  }
-}
-
-void FakeFileSystemInstance::RequestFileRemovalScan(
-    const std::vector<std::string>& directory_paths) {
-  DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
-  ReindexDirectory(kFakeAndroidPath);
-}
-
-void FakeFileSystemInstance::ReindexDirectory(
-    const std::string& directory_path) {
-  DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
-  std::vector<std::string> paths = {directory_path};
-  base::FilePath directory_file_path(directory_path);
-  for (const auto& entry : media_store_) {
-    base::FilePath entry_path = entry.first;
-    if (!directory_file_path.IsParent(entry_path)) {
-      continue;
-    }
-    paths.push_back(entry_path.value());
-  }
-  RequestMediaScan(paths);
+  // Do nothing and pretend we scaned them.
 }
 
 void FakeFileSystemInstance::OpenUrlsWithPermission(
@@ -711,12 +637,4 @@ base::ScopedFD FakeFileSystemInstance::CreateStreamFileDescriptorToWrite(
   return fd_write;
 }
 
-base::FilePath FakeFileSystemInstance::GetCrosPath(
-    const base::FilePath& android_path) const {
-  base::FilePath cros_path(cros_dir_);
-  base::FilePath android_dir(kFakeAndroidPath);
-  android_dir.AppendRelativePath(android_path, &cros_path);
-  return cros_path;
-}
-
 }  // namespace arc

components/arc/test/fake_file_system_instance.h

@@ -54,15 +54,6 @@ namespace arc {
 class FakeFileSystemInstance : public mojom::FileSystemInstance {
  public:
   // Specification of a fake file available to content URL based methods.
-  using GetLastChangeTimeCallback =
-      base::RepeatingCallback<base::Time(const base::FilePath& path)>;
-
-  // TODO(risan): Consider if this is really needed, and whether we should just
-  // use the simpler option by using the original directory as is.
-  // The path for the top level directory considered in MediaStore.
-  static constexpr base::FilePath::CharType kFakeAndroidPath[] =
-      FILE_PATH_LITERAL("/android/path");
-
   struct File {
     enum class Seekable {
       NO,
@@ -190,20 +181,6 @@ class FakeFileSystemInstance : public mojom::FileSystemInstance {
   // Adds a root accessible by document provider based methods.
   void AddRoot(const Root& root);
 
-  // Fake the GetLastChangedTime implementation.
-  void SetGetLastChangeTimeCallback(GetLastChangeTimeCallback ctime_callback);
-
-  // Sets the path for the top level cros directory considered in MediaStore.
-  // TODO(risan): Consider to not use this (if we go without different android
-  // path approach). Another possibility to consider is probably to bind mount
-  // the directory in the caller test and do android_path conversion for the
-  // caller? In any case, this we don't want cros to be exposed to this instance
-  // that lives under Android.
-  void SetCrosDir(const base::FilePath& cros_dir);
-
-  // Sets the initial media store content.
-  void SetMediaStore(const std::map<base::FilePath, base::Time>& media_store);
-
   // Triggers watchers installed to a document.
   void TriggerWatchers(const std::string& authority,
                        const std::string& document_id,
@@ -248,11 +225,6 @@ class FakeFileSystemInstance : public mojom::FileSystemInstance {
     return handled_url_requests_;
   }
 
-  // Returns the content of the fake media store index.
-  const std::map<base::FilePath, base::Time>& GetMediaStore() const {
-    return media_store_;
-  }
-
   // mojom::FileSystemInstance:
   void AddWatcher(const std::string& authority,
                   const std::string& document_id,
@@ -301,9 +273,6 @@ class FakeFileSystemInstance : public mojom::FileSystemInstance {
   void RemoveWatcher(int64_t watcher_id,
                      RemoveWatcherCallback callback) override;
   void RequestMediaScan(const std::vector<std::string>& paths) override;
-  void RequestFileRemovalScan(
-      const std::vector<std::string>& directory_paths) override;
-  void ReindexDirectory(const std::string& directory_path) override;
   void OpenUrlsWithPermission(mojom::OpenUrlsRequestPtr request,
                               OpenUrlsWithPermissionCallback callback) override;
 
@@ -333,9 +302,6 @@ class FakeFileSystemInstance : public mojom::FileSystemInstance {
   // read by calling GetFileContent() with the passed |url|.
   base::ScopedFD CreateStreamFileDescriptorToWrite(const std::string& url);
 
-  // Returns the cros file path for |android_path|.
-  base::FilePath GetCrosPath(const base::FilePath& android_path) const;
-
   THREAD_CHECKER(thread_checker_);
 
   base::ScopedTempDir temp_dir_;
@@ -374,15 +340,6 @@ class FakeFileSystemInstance : public mojom::FileSystemInstance {
   // List of roots added by AddRoot().
   std::vector<Root> roots_;
 
-  // Fake MediaStore database index.
-  std::map<base::FilePath, base::Time> media_store_;
-
-  // Fake GetLastChangedTime function callback.
-  GetLastChangeTimeCallback ctime_callback_;
-
-  // The path for the top level cros directory considered in MediaStore.
-  base::FilePath cros_dir_;
-
   int64_t next_watcher_id_ = 1;
   int get_child_documents_count_ = 0;