Create test fixture for SafeBrowsingService

chrome/browser/safe_browsing/unverified_download_policy_unittest.cc

Index: chrome/browser/safe_browsing/unverified_download_policy_unittest.cc
diff --git a/chrome/browser/safe_browsing/unverified_download_policy_unittest.cc b/chrome/browser/safe_browsing/unverified_download_policy_unittest.cc
index 67dc35c62e7f8c8488d23e8b535b1956b377f6cd..b9f4c890e0ee2ae246569d15fc711ce8c57a2a52 100644
--- a/chrome/browser/safe_browsing/unverified_download_policy_unittest.cc
+++ b/chrome/browser/safe_browsing/unverified_download_policy_unittest.cc
@@ -10,7 +10,7 @@
 #include "base/command_line.h"
 #include "base/files/file_path.h"
 #include "base/run_loop.h"
-#include "chrome/browser/safe_browsing/safe_browsing_service.h"
+#include "chrome/browser/safe_browsing/test_safe_browsing_service.h"
 #include "chrome/common/chrome_switches.h"
 #include "chrome/common/pref_names.h"
 #include "chrome/test/base/testing_browser_process.h"
@@ -37,28 +37,6 @@ class FakeDatabaseManager : public TestSafeBrowsingDatabaseManager {
   ~FakeDatabaseManager() override {}
 };
 
-class TestSafeBrowsingService : public SafeBrowsingService {
- public:
-  SafeBrowsingDatabaseManager* CreateDatabaseManager() override {
-    return new FakeDatabaseManager();
-  }
-
- protected:
-  ~TestSafeBrowsingService() override {}
-
-  SafeBrowsingProtocolManagerDelegate* GetProtocolManagerDelegate() override {
-    // Our FakeDatabaseManager doesn't implement this delegate.
-    return NULL;
-  }
-};
-
-class TestSafeBrowsingServiceFactory : public SafeBrowsingServiceFactory {
- public:
-  SafeBrowsingService* CreateSafeBrowsingService() override {
-    return new TestSafeBrowsingService();
-  }
-};
-
 class CompletionCallback {
  public:
   CompletionCallback() {}
@@ -95,13 +73,15 @@ class CompletionCallback {
 class UnverifiedDownloadPolicyTest : public ::testing::Test {
  public:
   void SetUp() override {
-    SafeBrowsingService::RegisterFactory(&test_safe_browsing_service_factory_);
-    testing_safe_browsing_service_ =
-        SafeBrowsingService::CreateSafeBrowsingService();
+    SafeBrowsingService::RegisterFactory(
+        &testing_safe_browsing_service_factory_);
     TestingBrowserProcess* browser_process = TestingBrowserProcess::GetGlobal();
     browser_process->SetSafeBrowsingService(
-        testing_safe_browsing_service_.get());
+        testing_safe_browsing_service_factory_.CreateSafeBrowsingService());
     browser_process->safe_browsing_service()->Initialize();
+    // Set up FakeDatabaseManager.
+    testing_safe_browsing_service_factory_.test_safe_browsing_service()

[Nathan Parker, 2016/05/07 00:05:32]

Shouldn't this set the FakeDatabaseManager on the factory obj, and then let it
create/initialize it normally?

Also, do you need to register the factory if you're then invoking it directly?

[Jialiu Lin, 2016/05/12 21:53:58]

Ah, this is not a InProcessBrowserTest but rather a unit tests, and browser
process is already created. So we don't have to register the factory, but
calling browser_process->SetSafeBrowsingService() to force instantiate a
TestSafeBrowsingService instance. 
Though it is good idea to make it more consistent with other tests.

Used SetTestSafeBrowsingService() on factory obj instead. 

+        ->SetDatabaseManager(new FakeDatabaseManager());
     base::RunLoop().RunUntilIdle();
 
     testing_profile_manager_.reset(
@@ -116,16 +96,14 @@ class UnverifiedDownloadPolicyTest : public ::testing::Test {
     TestingBrowserProcess* browser_process = TestingBrowserProcess::GetGlobal();
     browser_process->safe_browsing_service()->ShutDown();
     browser_process->SetSafeBrowsingService(nullptr);

[Jialiu Lin, 2016/05/12 21:53:58]

No need this line, since TestSafeBrowsingFactory destructor takes care of it.

-    testing_safe_browsing_service_ = nullptr;
     SafeBrowsingService::RegisterFactory(nullptr);
     base::RunLoop().RunUntilIdle();
   }
 
  protected:
   content::TestBrowserThreadBundle thread_bundle_;
-  TestSafeBrowsingServiceFactory test_safe_browsing_service_factory_;
+  TestSafeBrowsingServiceFactory testing_safe_browsing_service_factory_;
   std::unique_ptr<TestingProfileManager> testing_profile_manager_;
-  scoped_refptr<SafeBrowsingService> testing_safe_browsing_service_;
   TestingProfile* testing_profile_ = nullptr;
 };