Make RecentModel::GetRecentFiles accept file-type filter.

As described in the bug, we are going to filter files returned by
RecentModel::GetRecentFiles() based on media types.
As a first step, I'm  adding FileType parameter on the GetRecentFiles().

Collecting files are implemented in RecentDiskSource, RecentDriveSource,
and RecentArcMediaSource. Actual filtering code will be implemented in
them in follow-up patches based on the passed |file_type|.

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

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

@@ -1022,6 +1022,7 @@ FileManagerPrivateInternalGetRecentFilesFunction::Run() {
   model->GetRecentFiles(
       file_system_context.get(),
       Extension::GetBaseURLFromExtensionId(extension_id()),
+      chromeos::RecentModel::FileType::kAll,
       base::BindOnce(
           &FileManagerPrivateInternalGetRecentFilesFunction::OnGetRecentFiles,
           this, params->restriction));

chrome/browser/chromeos/fileapi/recent_arc_media_source.cc

@@ -322,6 +322,7 @@ void RecentArcMediaSource::GetRecentFiles(Params params) {
     root->GetRecentFiles(
         Params(params_.value().file_system_context(), params_.value().origin(),
                params_.value().max_files(), params_.value().cutoff_time(),
+               params_.value().file_type(),
                base::BindOnce(&RecentArcMediaSource::OnGetRecentFilesForRoot,
                               weak_ptr_factory_.GetWeakPtr())));
   }

chrome/browser/chromeos/fileapi/recent_arc_media_source_unittest.cc

@@ -124,6 +124,7 @@ class RecentArcMediaSourceTest : public testing::Test {
     source_->GetRecentFiles(RecentSource::Params(
         nullptr /* file_system_context */, GURL() /* origin */,
         1 /* max_files: ignored */, base::Time() /* cutoff_time: ignored */,
+        RecentSource::FileType::kAll /* file_type: ignored */,
         base::BindOnce(
             [](base::RunLoop* run_loop, std::vector<RecentFile>* out_files,
                std::vector<RecentFile> files) {

chrome/browser/chromeos/fileapi/recent_disk_source_unittest.cc

@@ -77,6 +77,7 @@ class RecentDiskSourceTest : public testing::Test {
 
     source_->GetRecentFiles(RecentSource::Params(
         file_system_context_.get(), origin_, max_files, cutoff_time,
+        RecentSource::FileType::kAll,
         base::BindOnce(
             [](base::RunLoop* run_loop, std::vector<RecentFile>* out_files,
                std::vector<RecentFile> files) {

chrome/browser/chromeos/fileapi/recent_model.cc

@@ -87,6 +87,7 @@ RecentModel::~RecentModel() {
 void RecentModel::GetRecentFiles(
     storage::FileSystemContext* file_system_context,
     const GURL& origin,
+    FileType file_type,
     GetRecentFilesCallback callback) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
 
@@ -122,7 +123,7 @@ void RecentModel::GetRecentFiles(
 
   for (const auto& source : sources_) {
     source->GetRecentFiles(RecentSource::Params(
-        file_system_context, origin, max_files_, cutoff_time,
+        file_system_context, origin, max_files_, cutoff_time, file_type,
         base::BindOnce(&RecentModel::OnGetRecentFiles,
                        weak_ptr_factory_.GetWeakPtr(), max_files_,
                        cutoff_time)));

chrome/browser/chromeos/fileapi/recent_model.h

@@ -42,6 +42,7 @@ class RecentModel : public KeyedService {
  public:
   using GetRecentFilesCallback =
       base::OnceCallback<void(const std::vector<RecentFile>& files)>;
+  using FileType = RecentSource::FileType;
 
   ~RecentModel() override;
 
@@ -57,6 +58,7 @@ class RecentModel : public KeyedService {
   // Results might be internally cached for better performance.
   void GetRecentFiles(storage::FileSystemContext* file_system_context,
                       const GURL& origin,
+                      FileType file_type,
                       GetRecentFilesCallback callback);
 
   // KeyedService overrides:

chrome/browser/chromeos/fileapi/recent_model_unittest.cc

@@ -35,11 +35,11 @@ RecentFile MakeRecentFile(const std::string& name,
 std::vector<std::unique_ptr<RecentSource>> BuildDefaultSources() {
   auto source1 = std::make_unique<FakeRecentSource>();
   source1->AddFile(MakeRecentFile("aaa.jpg", base::Time::FromJavaTime(1000)));
-  source1->AddFile(MakeRecentFile("ccc.jpg", base::Time::FromJavaTime(3000)));
+  source1->AddFile(MakeRecentFile("ccc.mp4", base::Time::FromJavaTime(3000)));
 
   auto source2 = std::make_unique<FakeRecentSource>();
-  source2->AddFile(MakeRecentFile("bbb.jpg", base::Time::FromJavaTime(2000)));
-  source2->AddFile(MakeRecentFile("ddd.jpg", base::Time::FromJavaTime(4000)));
+  source2->AddFile(MakeRecentFile("bbb.png", base::Time::FromJavaTime(2000)));
+  source2->AddFile(MakeRecentFile("ddd.ogg", base::Time::FromJavaTime(4000)));
 
   std::vector<std::unique_ptr<RecentSource>> sources;
   sources.emplace_back(std::move(source1));
@@ -60,7 +60,8 @@ class RecentModelTest : public testing::Test {
   std::vector<RecentFile> BuildModelAndGetRecentFiles(
       RecentSourceListFactory source_list_factory,
       size_t max_files,
-      const base::Time& cutoff_time) {
+      const base::Time& cutoff_time,
+      RecentModel::FileType file_type) {
     RecentModel* model = static_cast<RecentModel*>(
         RecentModelFactory::GetInstance()->SetTestingFactoryAndUse(
             &profile_,
@@ -80,7 +81,7 @@ class RecentModelTest : public testing::Test {
     base::RunLoop run_loop;
 
     model->GetRecentFiles(
-        nullptr /* file_system_context */, GURL() /* origin */,
+        nullptr /* file_system_context */, GURL() /* origin */, file_type,
         base::BindOnce(
             [](base::RunLoop* run_loop, std::vector<RecentFile>* files_out,
                const std::vector<RecentFile>& files) {
@@ -99,42 +100,44 @@ class RecentModelTest : public testing::Test {
 };
 
 TEST_F(RecentModelTest, GetRecentFiles) {
-  std::vector<RecentFile> files = BuildModelAndGetRecentFiles(
-      base::BindRepeating(&BuildDefaultSources), 10, base::Time());
+  std::vector<RecentFile> files =
+      BuildModelAndGetRecentFiles(base::BindRepeating(&BuildDefaultSources), 10,
+                                  base::Time(), RecentModel::FileType::kAll);
 
   ASSERT_EQ(4u, files.size());
-  EXPECT_EQ("ddd.jpg", files[0].url().path().value());
+  EXPECT_EQ("ddd.ogg", files[0].url().path().value());
   EXPECT_EQ(base::Time::FromJavaTime(4000), files[0].last_modified());
-  EXPECT_EQ("ccc.jpg", files[1].url().path().value());
+  EXPECT_EQ("ccc.mp4", files[1].url().path().value());
   EXPECT_EQ(base::Time::FromJavaTime(3000), files[1].last_modified());
-  EXPECT_EQ("bbb.jpg", files[2].url().path().value());
+  EXPECT_EQ("bbb.png", files[2].url().path().value());
   EXPECT_EQ(base::Time::FromJavaTime(2000), files[2].last_modified());
   EXPECT_EQ("aaa.jpg", files[3].url().path().value());
   EXPECT_EQ(base::Time::FromJavaTime(1000), files[3].last_modified());
 }
 
 TEST_F(RecentModelTest, GetRecentFiles_MaxFiles) {
-  std::vector<RecentFile> files = BuildModelAndGetRecentFiles(
-      base::BindRepeating(&BuildDefaultSources), 3, base::Time());
+  std::vector<RecentFile> files =
+      BuildModelAndGetRecentFiles(base::BindRepeating(&BuildDefaultSources), 3,
+                                  base::Time(), RecentModel::FileType::kAll);
 
   ASSERT_EQ(3u, files.size());
-  EXPECT_EQ("ddd.jpg", files[0].url().path().value());
+  EXPECT_EQ("ddd.ogg", files[0].url().path().value());
   EXPECT_EQ(base::Time::FromJavaTime(4000), files[0].last_modified());
-  EXPECT_EQ("ccc.jpg", files[1].url().path().value());
+  EXPECT_EQ("ccc.mp4", files[1].url().path().value());
   EXPECT_EQ(base::Time::FromJavaTime(3000), files[1].last_modified());
-  EXPECT_EQ("bbb.jpg", files[2].url().path().value());
+  EXPECT_EQ("bbb.png", files[2].url().path().value());
   EXPECT_EQ(base::Time::FromJavaTime(2000), files[2].last_modified());
 }
 
 TEST_F(RecentModelTest, GetRecentFiles_CutoffTime) {
-  std::vector<RecentFile> files =
-      BuildModelAndGetRecentFiles(base::BindRepeating(&BuildDefaultSources), 10,
-                                  base::Time::FromJavaTime(2500));
+  std::vector<RecentFile> files = BuildModelAndGetRecentFiles(
+      base::BindRepeating(&BuildDefaultSources), 10,
+      base::Time::FromJavaTime(2500), RecentModel::FileType::kAll);
 
   ASSERT_EQ(2u, files.size());
-  EXPECT_EQ("ddd.jpg", files[0].url().path().value());
+  EXPECT_EQ("ddd.ogg", files[0].url().path().value());
   EXPECT_EQ(base::Time::FromJavaTime(4000), files[0].last_modified());
-  EXPECT_EQ("ccc.jpg", files[1].url().path().value());
+  EXPECT_EQ("ccc.mp4", files[1].url().path().value());
   EXPECT_EQ(base::Time::FromJavaTime(3000), files[1].last_modified());
 }
 
@@ -143,9 +146,41 @@ TEST_F(RecentModelTest, GetRecentFiles_UmaStats) {
 
   BuildModelAndGetRecentFiles(
       base::BindRepeating([]() { return RecentSourceList(); }), 10,
-      base::Time());
+      base::Time(), RecentModel::FileType::kAll);
 
   histogram_tester.ExpectTotalCount(RecentModel::kLoadHistogramName, 1);
 }
 
+TEST_F(RecentModelTest, GetRecentFiles_Audio) {
+  std::vector<RecentFile> files =
+      BuildModelAndGetRecentFiles(base::BindRepeating(&BuildDefaultSources), 10,
+                                  base::Time(), RecentModel::FileType::kAudio);
+
+  ASSERT_EQ(1u, files.size());
+  EXPECT_EQ("ddd.ogg", files[0].url().path().value());
+  EXPECT_EQ(base::Time::FromJavaTime(4000), files[0].last_modified());
+}
+
+TEST_F(RecentModelTest, GetRecentFiles_Image) {
+  std::vector<RecentFile> files =
+      BuildModelAndGetRecentFiles(base::BindRepeating(&BuildDefaultSources), 10,
+                                  base::Time(), RecentModel::FileType::kImage);
+
+  ASSERT_EQ(2u, files.size());
+  EXPECT_EQ("bbb.png", files[0].url().path().value());
+  EXPECT_EQ(base::Time::FromJavaTime(2000), files[0].last_modified());
+  EXPECT_EQ("aaa.jpg", files[1].url().path().value());
+  EXPECT_EQ(base::Time::FromJavaTime(1000), files[1].last_modified());
+}
+
+TEST_F(RecentModelTest, GetRecentFiles_Video) {
+  std::vector<RecentFile> files =
+      BuildModelAndGetRecentFiles(base::BindRepeating(&BuildDefaultSources), 10,
+                                  base::Time(), RecentModel::FileType::kVideo);
+
+  ASSERT_EQ(1u, files.size());
+  EXPECT_EQ("ccc.mp4", files[0].url().path().value());
+  EXPECT_EQ(base::Time::FromJavaTime(3000), files[0].last_modified());
+}
+
 }  // namespace chromeos

chrome/browser/chromeos/fileapi/recent_source.cc

@@ -14,11 +14,13 @@ RecentSource::Params::Params(storage::FileSystemContext* file_system_context,
                              const GURL& origin,
                              size_t max_files,
                              const base::Time& cutoff_time,
+                             FileType file_type,
                              GetRecentFilesCallback callback)
     : file_system_context_(file_system_context),
       origin_(origin),
       max_files_(max_files),
       cutoff_time_(cutoff_time),
+      file_type_(file_type),
       callback_(std::move(callback)) {
   DCHECK(!callback_.is_null());
 }

chrome/browser/chromeos/fileapi/recent_source.h

@@ -35,6 +35,14 @@ class RecentSource {
   using GetRecentFilesCallback =
       base::OnceCallback<void(std::vector<RecentFile> files)>;
 
+  // File types to filter the results of GetRecentFiles().
+  enum class FileType {
+    kAll,
+    kAudio,
+    kImage,
+    kVideo,
+  };
+
   // Parameters passed to GetRecentFiles().
   class Params {
    public:
@@ -42,6 +50,7 @@ class RecentSource {
            const GURL& origin,
            size_t max_files,
            const base::Time& cutoff_time,
+           FileType file_type,
            GetRecentFilesCallback callback);
 
     Params(const Params& other) = delete;
@@ -68,6 +77,10 @@ class RecentSource {
     // requested here, but they will be filtered out by RecentModel.
     const base::Time& cutoff_time() const { return cutoff_time_; }
 
+    // File type to filter the results from RecentSource. RecentSource is
+    // expected to return files which matches the specified file type.
+    FileType file_type() const { return file_type_; }
+
     // Callback to be called for the result of GetRecentFiles().
     GetRecentFilesCallback& callback() { return callback_; }
 
@@ -76,6 +89,7 @@ class RecentSource {
     GURL origin_;
     size_t max_files_;
     base::Time cutoff_time_;
+    FileType file_type_;
     GetRecentFilesCallback callback_;
   };
 

chrome/browser/chromeos/fileapi/test/fake_recent_source.cc

@@ -4,9 +4,11 @@
 
 #include "chrome/browser/chromeos/fileapi/test/fake_recent_source.h"
 
+#include <string>
 #include <utility>
 
 #include "chrome/browser/chromeos/fileapi/recent_file.h"
+#include "net/base/mime_util.h"
 
 namespace chromeos {
 
@@ -19,7 +21,33 @@ void FakeRecentSource::AddFile(const RecentFile& file) {
 }
 
 void FakeRecentSource::GetRecentFiles(Params params) {
-  std::move(params.callback()).Run(canned_files_);
+  std::vector<RecentFile> result;
+  for (const auto& file : canned_files_) {
+    if (MatchesFileType(file, params.file_type()))
+      result.push_back(file);
+  }
+  std::move(params.callback()).Run(std::move(result));
+}
+
+bool FakeRecentSource::MatchesFileType(const RecentFile& file,
+                                       RecentSource::FileType file_type) const {
+  if (file_type == FileType::kAll)
+    return true;
+
+  std::string mime_type;
+  if (!net::GetMimeTypeFromFile(file.url().path(), &mime_type))
+    return false;
+
+  switch (file_type) {
+    case FileType::kAudio:
+      return net::MatchesMimeType("audio/*", mime_type);
+    case FileType::kImage:
+      return net::MatchesMimeType("image/*", mime_type);
+    case FileType::kVideo:
+      return net::MatchesMimeType("video/*", mime_type);
+    default:
+      return false;
+  }
 }
 
 }  // namespace chromeos

chrome/browser/chromeos/fileapi/test/fake_recent_source.h

@@ -29,6 +29,10 @@ class FakeRecentSource : public RecentSource {
   void GetRecentFiles(Params params) override;
 
  private:
+  // Returns true if the file matches the given file type.
+  bool MatchesFileType(const RecentFile& file,
+                       RecentSource::FileType file_type) const;
+
   std::vector<RecentFile> canned_files_;
 
   DISALLOW_COPY_AND_ASSIGN(FakeRecentSource);