Convert indirect uses of TestingFactoryFunction

TestingFactoryFunction is a simple function pointer. It is
deprecated in favor of TestingFactory which is a Callback<>.
Convert indirect uses by using base::BindRepeating() in all
invocation of SetTestingFactory/AddTestingFactory.

Remove the RecentModelFactory::SetForProfileAndUseForTest
method as it pass a std::unique_ptr<> object to return from
the factory which is difficult to adapt to the new API that
requires a base::RepeatingCallback<>.

Instead inline the implementation in the sole unit test that
called this method and modify it to pass a factory returning
a std::vector<std::unique_ptr<RecentModel>>.

This converts uses in src//chrome/browser/chromeos/fileapi.

This CL was uploaded by git cl split.

R=slangley@chromium.org

Bug: 809610
Change-Id: Idb8bae335e3defca5b2c6736d302586faf97d929
Reviewed-on: https://chromium-review.googlesource.com/c/1259014Reviewed-by: Stuart Langley <slangley@chromium.org>
Commit-Queue: Sylvain Defresne <sdefresne@chromium.org>
Cr-Commit-Position: refs/heads/master@{#601644}

chrome/browser/chromeos/fileapi/recent_arc_media_source_unittest.cc

@@ -4,6 +4,7 @@
 
 #include <utility>
 
+#include "base/bind.h"
 #include "base/run_loop.h"
 #include "base/test/metrics/histogram_tester.h"
 #include "base/time/time.h"
@@ -66,7 +67,9 @@ class RecentArcMediaSourceTest : public testing::Test {
     runner_ = static_cast<arc::ArcFileSystemOperationRunner*>(
         arc::ArcFileSystemOperationRunner::GetFactory()
             ->SetTestingFactoryAndUse(
-                profile_.get(), &CreateFileSystemOperationRunnerForTesting));
+                profile_.get(),
+                base::BindRepeating(
+                    &CreateFileSystemOperationRunnerForTesting)));
 
     // Mount ARC file systems.
     arc::ArcFileSystemMounter::GetForBrowserContext(profile_.get());

chrome/browser/chromeos/fileapi/recent_model_factory.cc

@@ -17,39 +17,12 @@
 
 namespace chromeos {
 
-// static
-RecentModel* RecentModelFactory::model_for_test_ = nullptr;
-
 // static
 RecentModel* RecentModelFactory::GetForProfile(Profile* profile) {
   return static_cast<RecentModel*>(
       GetInstance()->GetServiceForBrowserContext(profile, true));
 }
 
-// static
-RecentModel* RecentModelFactory::SetForProfileAndUseForTest(
-    Profile* profile,
-    std::unique_ptr<RecentModel> model) {
-  DCHECK(model);
-  DCHECK(!model_for_test_);
-
-  RecentModel* saved_model = model.get();
-  model_for_test_ = model.release();
-
-  KeyedService* used_model = GetInstance()->SetTestingFactoryAndUse(
-      profile,
-      [](content::BrowserContext* context) -> std::unique_ptr<KeyedService> {
-        std::unique_ptr<KeyedService> model(model_for_test_);
-        model_for_test_ = nullptr;
-        return model;
-      });
-
-  DCHECK_EQ(used_model, saved_model);
-  DCHECK(!model_for_test_);
-
-  return saved_model;
-}
-
 RecentModelFactory::RecentModelFactory()
     : BrowserContextKeyedServiceFactory(
           "RecentModel",

chrome/browser/chromeos/fileapi/recent_model_factory.h

@@ -22,11 +22,6 @@ class RecentModelFactory : public BrowserContextKeyedServiceFactory {
   // Returns the RecentModel for |profile|, creating it if not created yet.
   static RecentModel* GetForProfile(Profile* profile);
 
-  // Sets the RecentModel for |profile| for testing.
-  static RecentModel* SetForProfileAndUseForTest(
-      Profile* profile,
-      std::unique_ptr<RecentModel> model);
-
   // Returns the singleton RecentModelFactory instance.
   static RecentModelFactory* GetInstance();
 
@@ -42,8 +37,6 @@ class RecentModelFactory : public BrowserContextKeyedServiceFactory {
   KeyedService* BuildServiceInstanceFor(
       content::BrowserContext* context) const override;
 
-  static RecentModel* model_for_test_;
-
   DISALLOW_COPY_AND_ASSIGN(RecentModelFactory);
 };
 

chrome/browser/chromeos/fileapi/recent_model_unittest.cc

@@ -6,6 +6,7 @@
 #include <string>
 #include <utility>
 
+#include "base/bind.h"
 #include "base/files/file_path.h"
 #include "base/run_loop.h"
 #include "base/test/metrics/histogram_tester.h"
@@ -31,6 +32,21 @@ RecentFile MakeRecentFile(const std::string& name,
   return RecentFile(url, last_modified);
 }
 
+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)));
+
+  auto source2 = std::make_unique<FakeRecentSource>();
+  source2->AddFile(MakeRecentFile("bbb.jpg", base::Time::FromJavaTime(2000)));
+  source2->AddFile(MakeRecentFile("ddd.jpg", base::Time::FromJavaTime(4000)));
+
+  std::vector<std::unique_ptr<RecentSource>> sources;
+  sources.emplace_back(std::move(source1));
+  sources.emplace_back(std::move(source2));
+  return sources;
+}
+
 }  // namespace
 
 class RecentModelTest : public testing::Test {
@@ -38,27 +54,23 @@ class RecentModelTest : public testing::Test {
   RecentModelTest() = default;
 
  protected:
-  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)));
-
-    auto source2 = std::make_unique<FakeRecentSource>();
-    source2->AddFile(MakeRecentFile("bbb.jpg", base::Time::FromJavaTime(2000)));
-    source2->AddFile(MakeRecentFile("ddd.jpg", base::Time::FromJavaTime(4000)));
-
-    std::vector<std::unique_ptr<RecentSource>> sources;
-    sources.emplace_back(std::move(source1));
-    sources.emplace_back(std::move(source2));
-    return sources;
-  }
+  using RecentSourceList = std::vector<std::unique_ptr<RecentSource>>;
+  using RecentSourceListFactory = base::RepeatingCallback<RecentSourceList()>;
 
   std::vector<RecentFile> BuildModelAndGetRecentFiles(
-      std::vector<std::unique_ptr<RecentSource>> sources,
+      RecentSourceListFactory source_list_factory,
       size_t max_files,
       const base::Time& cutoff_time) {
-    RecentModel* model = RecentModelFactory::SetForProfileAndUseForTest(
-        &profile_, RecentModel::CreateForTest(std::move(sources)));
+    RecentModel* model = static_cast<RecentModel*>(
+        RecentModelFactory::GetInstance()->SetTestingFactoryAndUse(
+            &profile_,
+            base::BindRepeating(
+                [](const RecentSourceListFactory& source_list_factory,
+                   content::BrowserContext* context)
+                    -> std::unique_ptr<KeyedService> {
+                  return RecentModel::CreateForTest(source_list_factory.Run());
+                },
+                std::move(source_list_factory))));
 
     model->SetMaxFilesForTest(max_files);
     model->SetForcedCutoffTimeForTest(cutoff_time);
@@ -87,8 +99,8 @@ class RecentModelTest : public testing::Test {
 };
 
 TEST_F(RecentModelTest, GetRecentFiles) {
-  std::vector<RecentFile> files =
-      BuildModelAndGetRecentFiles(BuildDefaultSources(), 10, base::Time());
+  std::vector<RecentFile> files = BuildModelAndGetRecentFiles(
+      base::BindRepeating(&BuildDefaultSources), 10, base::Time());
 
   ASSERT_EQ(4u, files.size());
   EXPECT_EQ("ddd.jpg", files[0].url().path().value());
@@ -102,8 +114,8 @@ TEST_F(RecentModelTest, GetRecentFiles) {
 }
 
 TEST_F(RecentModelTest, GetRecentFiles_MaxFiles) {
-  std::vector<RecentFile> files =
-      BuildModelAndGetRecentFiles(BuildDefaultSources(), 3, base::Time());
+  std::vector<RecentFile> files = BuildModelAndGetRecentFiles(
+      base::BindRepeating(&BuildDefaultSources), 3, base::Time());
 
   ASSERT_EQ(3u, files.size());
   EXPECT_EQ("ddd.jpg", files[0].url().path().value());
@@ -115,8 +127,9 @@ TEST_F(RecentModelTest, GetRecentFiles_MaxFiles) {
 }
 
 TEST_F(RecentModelTest, GetRecentFiles_CutoffTime) {
-  std::vector<RecentFile> files = BuildModelAndGetRecentFiles(
-      BuildDefaultSources(), 10, base::Time::FromJavaTime(2500));
+  std::vector<RecentFile> files =
+      BuildModelAndGetRecentFiles(base::BindRepeating(&BuildDefaultSources), 10,
+                                  base::Time::FromJavaTime(2500));
 
   ASSERT_EQ(2u, files.size());
   EXPECT_EQ("ddd.jpg", files[0].url().path().value());
@@ -128,7 +141,9 @@ TEST_F(RecentModelTest, GetRecentFiles_CutoffTime) {
 TEST_F(RecentModelTest, GetRecentFiles_UmaStats) {
   base::HistogramTester histogram_tester;
 
-  BuildModelAndGetRecentFiles({}, 10, base::Time());
+  BuildModelAndGetRecentFiles(
+      base::BindRepeating([]() { return RecentSourceList(); }), 10,
+      base::Time());
 
   histogram_tester.ExpectTotalCount(RecentModel::kLoadHistogramName, 1);
 }