Invalidate cache for recent files when requested file type changes

Results from RecentModel are cached for 10 seconds.
However, if the requested file type is different from previous one, the
cache should be invalidated and not reused.

Bug: 1040049
Test: Run browser_tests
Change-Id: I0e9a90d6faeb612531ebdd64ff5b152bd9989755
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2035509
Auto-Submit: Naoki Fukino <fukino@chromium.org>
Commit-Queue: Naoki Fukino <fukino@chromium.org>
Reviewed-by: Noel Gordon <noel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#738477}

chrome/browser/chromeos/extensions/file_manager/file_manager_private_apitest.cc

@@ -445,12 +445,18 @@ IN_PROC_BROWSER_TEST_F(FileManagerPrivateApiTest, Recent) {
   ASSERT_TRUE(file_manager::VolumeManager::Get(browser()->profile())
                   ->RegisterDownloadsDirectoryForTesting(downloads_dir));
 
-  // Create an empty file.
+  // Create test files.
   {
     base::ScopedAllowBlockingForTesting allow_io;
-    base::File file(downloads_dir.Append("all-justice.jpg"),
-                    base::File::FLAG_CREATE | base::File::FLAG_WRITE);
-    ASSERT_TRUE(file.IsValid());
+    base::File image_file(downloads_dir.Append("all-justice.jpg"),
+                          base::File::FLAG_CREATE | base::File::FLAG_WRITE);
+    ASSERT_TRUE(image_file.IsValid());
+    base::File audio_file(downloads_dir.Append("all-justice.mp3"),
+                          base::File::FLAG_CREATE | base::File::FLAG_WRITE);
+    ASSERT_TRUE(audio_file.IsValid());
+    base::File video_file(downloads_dir.Append("all-justice.mp4"),
+                          base::File::FLAG_CREATE | base::File::FLAG_WRITE);
+    ASSERT_TRUE(video_file.IsValid());
   }
 
   ASSERT_TRUE(RunComponentExtensionTest("file_browser/recent_test"));

chrome/browser/chromeos/fileapi/recent_model.cc

@@ -93,8 +93,11 @@ void RecentModel::GetRecentFiles(
 
   // Use cache if available.
   if (cached_files_.has_value()) {
-    std::move(callback).Run(cached_files_.value());
-    return;
+    if (cached_files_type_ == file_type) {
+      std::move(callback).Run(cached_files_.value());
+      return;
+    }
+    cached_files_.reset();
   }
 
   bool builder_already_running = !pending_callbacks_.empty();
@@ -113,7 +116,7 @@ void RecentModel::GetRecentFiles(
 
   num_inflight_sources_ = sources_.size();
   if (sources_.empty()) {
-    OnGetRecentFilesCompleted();
+    OnGetRecentFilesCompleted(file_type);
     return;
   }
 
@@ -125,8 +128,8 @@ void RecentModel::GetRecentFiles(
     source->GetRecentFiles(RecentSource::Params(
         file_system_context, origin, max_files_, cutoff_time, file_type,
         base::BindOnce(&RecentModel::OnGetRecentFiles,
-                       weak_ptr_factory_.GetWeakPtr(), max_files_,
-                       cutoff_time)));
+                       weak_ptr_factory_.GetWeakPtr(), max_files_, cutoff_time,
+                       file_type)));
   }
 }
 
@@ -140,6 +143,7 @@ void RecentModel::Shutdown() {
 
 void RecentModel::OnGetRecentFiles(size_t max_files,
                                    const base::Time& cutoff_time,
+                                   FileType file_type,
                                    std::vector<RecentFile> files) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
 
@@ -155,10 +159,10 @@ void RecentModel::OnGetRecentFiles(size_t max_files,
 
   --num_inflight_sources_;
   if (num_inflight_sources_ == 0)
-    OnGetRecentFilesCompleted();
+    OnGetRecentFilesCompleted(file_type);
 }
 
-void RecentModel::OnGetRecentFilesCompleted() {
+void RecentModel::OnGetRecentFilesCompleted(FileType file_type) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
 
   DCHECK_EQ(0, num_inflight_sources_);
@@ -172,6 +176,7 @@ void RecentModel::OnGetRecentFilesCompleted() {
   }
   std::reverse(files.begin(), files.end());
   cached_files_ = std::move(files);
+  cached_files_type_ = file_type;
 
   DCHECK(cached_files_.has_value());
   DCHECK(intermediate_files_.empty());

chrome/browser/chromeos/fileapi/recent_model.h

@@ -76,8 +76,9 @@ class RecentModel : public KeyedService {
 
   void OnGetRecentFiles(size_t max_files,
                         const base::Time& cutoff_time,
+                        FileType file_type,
                         std::vector<RecentFile> files);
-  void OnGetRecentFilesCompleted();
+  void OnGetRecentFilesCompleted(FileType file_type);
   void ClearCache();
 
   void SetMaxFilesForTest(size_t max_files);
@@ -96,6 +97,9 @@ class RecentModel : public KeyedService {
   // Cached GetRecentFiles() response.
   base::Optional<std::vector<RecentFile>> cached_files_ = base::nullopt;
 
+  // File type of the cached GetRecentFiles() response.
+  FileType cached_files_type_ = FileType::kAll;
+
   // Timer to clear the cache.
   base::OneShotTimer cache_clear_timer_;
 

chrome/test/data/extensions/api_test/file_browser/recent_test/test.js

@@ -35,17 +35,69 @@ function requestAllFileSystems() {
 
 // Run the tests.
 requestAllFileSystems().then(function() {
+  function exists(entries, fileName) {
+    for (let i = 0; i < entries.length; ++i) {
+      if (entries[i].name === fileName) {
+        return true;
+      }
+    }
+    return false;
+  }
   chrome.test.runTests([
     function testGetRecentFiles() {
       chrome.fileManagerPrivate.getRecentFiles(
-          'native_source', 'all', chrome.test.callbackPass(function(entries) {
-            var found = false;
-            for (var i = 0; i < entries.length; ++i) {
-              if (entries[i].name === 'all-justice.jpg') {
-                found = true;
-              }
-            }
-            chrome.test.assertTrue(found, 'all-justice.jpg not found');
+          'native_source', 'all', chrome.test.callbackPass(entries => {
+            chrome.test.assertTrue(
+                exists(entries, 'all-justice.jpg'),
+                'all-justice.jpg not found');
+            chrome.test.assertTrue(
+                exists(entries, 'all-justice.mp3'),
+                'all-justice.mp3 not found');
+            chrome.test.assertTrue(
+                exists(entries, 'all-justice.mp4'),
+                'all-justice.mp4 not found');
+          }));
+    },
+    function testGetRecentAudioFiles() {
+      chrome.fileManagerPrivate.getRecentFiles(
+          'native_source', 'audio', chrome.test.callbackPass(entries => {
+            chrome.test.assertFalse(
+                exists(entries, 'all-justice.jpg'),
+                'all-justice.jpg unexpectedly found');
+            chrome.test.assertTrue(
+                exists(entries, 'all-justice.mp3'),
+                'all-justice.mp3 not found');
+            chrome.test.assertFalse(
+                exists(entries, 'all-justice.mp4'),
+                'all-justice.mp4 unexpectedly found');
+          }));
+    },
+    function testGetRecentImageFiles() {
+      chrome.fileManagerPrivate.getRecentFiles(
+          'native_source', 'image', chrome.test.callbackPass(entries => {
+            chrome.test.assertTrue(
+                exists(entries, 'all-justice.jpg'),
+                'all-justice.jpg not found');
+            chrome.test.assertFalse(
+                exists(entries, 'all-justice.mp3'),
+                'all-justice.mp3 unexpectedly found');
+            chrome.test.assertFalse(
+                exists(entries, 'all-justice.mp4'),
+                'all-justice.mp4 unexpectedly found');
+          }));
+    },
+    function testGetRecentVideoFiles() {
+      chrome.fileManagerPrivate.getRecentFiles(
+          'native_source', 'video', chrome.test.callbackPass(entries => {
+            chrome.test.assertFalse(
+                exists(entries, 'all-justice.jpg'),
+                'all-justice.jpg unexpectedly found');
+            chrome.test.assertFalse(
+                exists(entries, 'all-justice.mp3'),
+                'all-justice.mp3 unexpectedly found');
+            chrome.test.assertTrue(
+                exists(entries, 'all-justice.mp4'),
+                'all-justice.mp4 not found');
           }));
     }
   ]);