chromeos: Flush only files directly in profile dir

Subdirs are added to profile dir over time. Subdirs such as
"Storage" could contains many files and are not critical to
bring up chrome. It could make profile flushing take a longer
time and stuck during shutdown. This CL changes to only
flush files directly under profile dir.

Bug: 998677
Change-Id: I5cf3e858c4f60ee6c9b4983431533bc9a07854ef
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1776782Reviewed-by: Achuith Bhandarkar <achuith@chromium.org>
Commit-Queue: Xiyuan Xia <xiyuan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#691809}

chrome/browser/chromeos/base/file_flusher.cc

@@ -18,20 +18,6 @@
 
 namespace chromeos {
 
-namespace {
-
-std::set<base::FilePath> MakeAbsoutePathSet(
-    const base::FilePath& base,
-    const std::vector<base::FilePath>& paths) {
-  std::set<base::FilePath> result;
-  for (const auto& path : paths) {
-    result.insert(path.IsAbsolute() ? path : base.Append(path));
-  }
-  return result;
-}
-
-}  // namespace
-
 ////////////////////////////////////////////////////////////////////////////////
 // FileFlusher::Job
 
@@ -39,7 +25,7 @@ class FileFlusher::Job {
  public:
   Job(const base::WeakPtr<FileFlusher>& master,
       const base::FilePath& path,
-      const std::vector<base::FilePath>& excludes,
+      bool recursive,
       const FileFlusher::OnFlushCallback& on_flush_callback,
       const base::Closure& callback);
   ~Job() = default;
@@ -54,9 +40,6 @@ class FileFlusher::Job {
   // Flush files on a blocking pool thread.
   void FlushAsync();
 
-  // Whether to exclude the |path| from flushing.
-  bool ShouldExclude(const base::FilePath& path) const;
-
   // Schedule a FinishOnUIThread task to run on the UI thread.
   void ScheduleFinish();
 
@@ -65,7 +48,7 @@ class FileFlusher::Job {
 
   base::WeakPtr<FileFlusher> master_;
   const base::FilePath path_;
-  const std::set<base::FilePath> excludes_;
+  const bool recursive_;
   const FileFlusher::OnFlushCallback on_flush_callback_;
   const base::Closure callback_;
 
@@ -78,12 +61,12 @@ class FileFlusher::Job {
 
 FileFlusher::Job::Job(const base::WeakPtr<FileFlusher>& master,
                       const base::FilePath& path,
-                      const std::vector<base::FilePath>& excludes,
+                      bool recursive,
                       const FileFlusher::OnFlushCallback& on_flush_callback,
                       const base::Closure& callback)
     : master_(master),
       path_(path),
-      excludes_(MakeAbsoutePathSet(path, excludes)),
+      recursive_(recursive),
       on_flush_callback_(on_flush_callback),
       callback_(callback) {}
 
@@ -119,13 +102,10 @@ void FileFlusher::Job::Cancel() {
 void FileFlusher::Job::FlushAsync() {
   VLOG(1) << "Flushing files under " << path_.value();
 
-  base::FileEnumerator traversal(path_, true /* recursive */,
+  base::FileEnumerator traversal(path_, recursive_,
                                  base::FileEnumerator::FILES);
   for (base::FilePath current = traversal.Next();
        !current.empty() && !cancel_flag_.IsSet(); current = traversal.Next()) {
-    if (ShouldExclude(current))
-      continue;
-
     base::File currentFile(current,
                            base::File::FLAG_OPEN | base::File::FLAG_WRITE);
     if (!currentFile.IsValid()) {
@@ -141,10 +121,6 @@ void FileFlusher::Job::FlushAsync() {
   }
 }
 
-bool FileFlusher::Job::ShouldExclude(const base::FilePath& path) const {
-  return excludes_.find(path) != excludes_.end();
-}
-
 void FileFlusher::Job::ScheduleFinish() {
   if (finish_scheduled_)
     return;
@@ -178,14 +154,14 @@ FileFlusher::~FileFlusher() {
 }
 
 void FileFlusher::RequestFlush(const base::FilePath& path,
-                               const std::vector<base::FilePath>& excludes,
+                               bool recursive,
                                const base::Closure& callback) {
   for (auto* job : jobs_) {
     if (path == job->path() || path.IsParent(job->path()))
       job->Cancel();
   }
 
-  jobs_.push_back(new Job(weak_factory_.GetWeakPtr(), path, excludes,
+  jobs_.push_back(new Job(weak_factory_.GetWeakPtr(), path, recursive,
                           on_flush_callback_for_test_, callback));
   ScheduleJob();
 }

chrome/browser/chromeos/base/file_flusher.h

@@ -22,12 +22,11 @@ class FileFlusher {
   FileFlusher();
   ~FileFlusher();
 
-  // Flush everything under |path| except the directories/files in |excludes|.
-  // |excluedes| can hold both absolute or relative paths, where absolute paths
-  // are used as-is and relative paths are appended to |path| to make it
-  // absolute. |callback| is invoked when the request is finished or canceled.
+  // Flush files under |path|.
+  // |recursive| whether to go down and flush sub trees under |path|.
+  // |callback| is invoked when the request is finished or canceled.
   void RequestFlush(const base::FilePath& path,
-                    const std::vector<base::FilePath>& excludes,
+                    bool recursive,
                     const base::Closure& callback);
 
   // Set a callback for every file flush for test. Note the callback is

chrome/browser/chromeos/base/file_flusher_unittest.cc

@@ -89,9 +89,9 @@ class FileFlusherTest : public testing::Test {
 TEST_F(FileFlusherTest, Flush) {
   std::unique_ptr<FileFlusher> flusher(CreateFileFlusher());
   base::RunLoop run_loop;
-  flusher->RequestFlush(GetTestFilePath("dir1"), std::vector<base::FilePath>(),
+  flusher->RequestFlush(GetTestFilePath("dir1"), /*recursive=*/false,
                         base::Closure());
-  flusher->RequestFlush(GetTestFilePath("dir2"), std::vector<base::FilePath>(),
+  flusher->RequestFlush(GetTestFilePath("dir2"), /*recursive=*/false,
                         run_loop.QuitClosure());
   run_loop.Run();
 
@@ -104,36 +104,13 @@ TEST_F(FileFlusherTest, Flush) {
   EXPECT_EQ(1, GetFlushCount("dir2/file3"));
 }
 
-TEST_F(FileFlusherTest, Exclude) {
-  std::unique_ptr<FileFlusher> flusher(CreateFileFlusher());
-
-  std::vector<base::FilePath> excludes;
-  // Relative exclude
-  excludes.push_back(base::FilePath::FromUTF8Unsafe("file1"));
-  // Absolute exclude
-  excludes.push_back(GetTestFilePath("dir1/file2"));
-  // Invalid exclude will be ignored.
-  excludes.push_back(base::FilePath::FromUTF8Unsafe("Bad file"));
-
-  base::RunLoop run_loop;
-  flusher->RequestFlush(GetTestFilePath("dir1"), excludes,
-                        run_loop.QuitClosure());
-  run_loop.Run();
-
-  EXPECT_EQ(0, GetFlushCount("dir1/file1"));
-  EXPECT_EQ(0, GetFlushCount("dir1/file2"));
-  EXPECT_EQ(1, GetFlushCount("dir1/file3"));
-
-  EXPECT_EQ(0, GetFlushCount("dir1/Bad file"));
-}
-
 TEST_F(FileFlusherTest, DuplicateRequests) {
   std::unique_ptr<FileFlusher> flusher(CreateFileFlusher());
   base::RunLoop run_loop;
   flusher->PauseForTest();
-  flusher->RequestFlush(GetTestFilePath("dir1"), std::vector<base::FilePath>(),
+  flusher->RequestFlush(GetTestFilePath("dir1"), /*recursive=*/false,
                         base::Closure());
-  flusher->RequestFlush(GetTestFilePath("dir1"), std::vector<base::FilePath>(),
+  flusher->RequestFlush(GetTestFilePath("dir1"), /*recursive=*/false,
                         run_loop.QuitClosure());
   flusher->ResumeForTest();
   run_loop.Run();

chrome/browser/chromeos/profiles/profile_helper.cc

@@ -17,14 +17,12 @@
 #include "chrome/browser/chromeos/login/signin/oauth2_login_manager_factory.h"
 #include "chrome/browser/chromeos/login/signin_partition_manager.h"
 #include "chrome/browser/chromeos/login/users/chrome_user_manager.h"
-#include "chrome/browser/download/download_prefs.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/profiles/profile_manager.h"
 #include "chrome/browser/profiles/profiles_state.h"
 #include "chrome/browser/ui/browser_list.h"
 #include "chrome/common/chrome_constants.h"
 #include "chrome/common/chrome_switches.h"
-#include "chromeos/constants/chromeos_constants.h"
 #include "chromeos/constants/chromeos_switches.h"
 #include "components/account_id/account_id.h"
 #include "components/prefs/pref_service.h"
@@ -33,7 +31,6 @@
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/browsing_data_remover.h"
 #include "extensions/browser/pref_names.h"
-#include "extensions/common/constants.h"
 
 namespace chromeos {
 
@@ -568,23 +565,10 @@ void ProfileHelper::FlushProfile(Profile* profile) {
   if (!profile_flusher_)
     profile_flusher_.reset(new FileFlusher);
 
-  // Files/directories that do not need to be flushed.
-  std::vector<base::FilePath> excludes;
-
-  // Preferences file is handled by ImportantFileWriter.
-  excludes.push_back(base::FilePath(chrome::kPreferencesFilename));
-  // Do not flush cache files.
-  excludes.push_back(base::FilePath(chrome::kCacheDirname));
-  excludes.push_back(base::FilePath(FILE_PATH_LITERAL("GPUCache")));
-  // Do not flush user Downloads.
-  excludes.push_back(
-      DownloadPrefs::FromBrowserContext(profile)->DownloadPath());
-  // Let extension system handle extension files.
-  excludes.push_back(base::FilePath(extensions::kInstallDirectoryName));
-  // Do not flush Drive cache.
-  excludes.push_back(base::FilePath(chromeos::kDriveCacheDirname));
-
-  profile_flusher_->RequestFlush(profile->GetPath(), excludes, base::Closure());
+  // Flushes files directly under profile path since these are the critical
+  // ones.
+  profile_flusher_->RequestFlush(profile->GetPath(), /*recursive=*/false,
+                                 base::Closure());
 }
 
 }  // namespace chromeos