Revert "Make AdBlockService into a singleton"

This reverts commit 052c7a3.
brave · Mar 28, 2018 · 1eb5105 · 1eb5105
1 parent 6acd8d3
commit 1eb5105
Show file tree

Hide file tree

Showing 5 changed files with 28 additions and 21 deletions.
diff --git a/browser/brave_browser_process_impl.cc b/browser/brave_browser_process_impl.cc
@@ -23,7 +23,11 @@ BraveBrowserProcessImpl::BraveBrowserProcessImpl(
 
 brave_shields::BaseBraveShieldsService*
 BraveBrowserProcessImpl::ad_block_service() {
-  return brave_shields::AdBlockService::GetInstance();
+  if (ad_block_service_)
+    return ad_block_service_.get();
+
+  ad_block_service_ = brave_shields::AdBlockServiceFactory();
+  return ad_block_service_.get();
 }
 
 brave_shields::BaseBraveShieldsService*

diff --git a/browser/brave_browser_process_impl.h b/browser/brave_browser_process_impl.h
@@ -24,6 +24,7 @@ class BraveBrowserProcessImpl : public BrowserProcessImpl {
       https_everywhere_service();
 
  private:
+  std::unique_ptr<brave_shields::BaseBraveShieldsService> ad_block_service_;
   std::unique_ptr<brave_shields::BaseBraveShieldsService>
       tracking_protection_service_;
   std::unique_ptr<brave_shields::HTTPSEverywhereService>

diff --git a/components/brave_shields/browser/ad_block_service.cc b/components/brave_shields/browser/ad_block_service.cc
@@ -16,7 +16,6 @@
 #include "base/logging.h"
 #include "base/macros.h"
 #include "base/memory/ptr_util.h"
-#include "base/memory/singleton.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/threading/thread_restrictions.h"
 #include "brave/components/brave_shields/browser/dat_file_util.h"
@@ -82,11 +81,17 @@ bool AdBlockService::Init() {
   return true;
 }
 
+// static
+void AdBlockService::SetAdBlockURLForTest(const GURL& url) {
+  g_ad_block_url = url;
+}
+
 ///////////////////////////////////////////////////////////////////////////////
 
-// static
-AdBlockService* AdBlockService::GetInstance() {
-  return base::Singleton<AdBlockService>::get();
+// The brave shields factory. Using the Brave Shields as a singleton
+// is the job of the browser process.
+std::unique_ptr<BaseBraveShieldsService> AdBlockServiceFactory() {
+  return base::MakeUnique<AdBlockService>();
 }
 
 }  // namespace brave_shields
diff --git a/components/brave_shields/browser/ad_block_service.h b/components/brave_shields/browser/ad_block_service.h
@@ -16,11 +16,7 @@
 #include "content/public/common/resource_type.h"
 
 class AdBlockClient;
-
-namespace base {
-template <typename Type>
-struct DefaultSingletonTraits;
-}
+class AdBlockServiceTest;
 
 namespace brave_shields {
 
@@ -30,25 +26,26 @@ class AdBlockService : public BaseBraveShieldsService {
    AdBlockService();
    ~AdBlockService() override;
 
-  static AdBlockService* GetInstance();
-
   bool ShouldStartRequest(const GURL &url,
     content::ResourceType resource_type,
     const std::string& tab_host) override;
 
-  // Useful for tests to customize
-  static GURL g_ad_block_url;
-
  protected:
-  friend struct base::DefaultSingletonTraits<AdBlockService>;
   bool Init() override;
   void Cleanup() override;
 
  private:
+  friend class ::AdBlockServiceTest;
+  static GURL g_ad_block_url;
+  static void SetAdBlockURLForTest(const GURL& url);
+
   std::vector<unsigned char> buffer_;
   std::unique_ptr<AdBlockClient> ad_block_client_;
 };
 
+// Creates the AdBlockService
+std::unique_ptr<BaseBraveShieldsService> AdBlockServiceFactory();
+
 }  // namespace brave_shields
 
 #endif  // BRAVE_COMPONENTS_BRAVE_SHIELDS_BROWSER_AD_BLOCK_SERVICE_H_
diff --git a/components/brave_shields/browser/ad_block_service_browsertest.cc b/components/brave_shields/browser/ad_block_service_browsertest.cc
@@ -4,6 +4,7 @@
 
 #include "base/path_service.h"
 #include "base/test/thread_test_helper.h"
+#include "brave/browser/brave_browser_process_impl.h"
 #include "brave/common/brave_paths.h"
 #include "brave/components/brave_shields/browser/ad_block_service.h"
 #include "chrome/browser/ui/browser.h"
@@ -28,8 +29,7 @@ class AdBlockServiceTest : public InProcessBrowserTest {
   void PreRunTestOnMainThread() override {
     InProcessBrowserTest::PreRunTestOnMainThread();
     WaitForAdBlockServiceThread();
-    ASSERT_TRUE(
-      brave_shields::AdBlockService::GetInstance()->IsInitialized());
+    ASSERT_TRUE(g_brave_browser_process->ad_block_service()->IsInitialized());
   }
 
   void InitEmbeddedTestServer() {
@@ -41,14 +41,14 @@ class AdBlockServiceTest : public InProcessBrowserTest {
   }
 
   void InitAdBlock() {
-    brave_shields::AdBlockService::g_ad_block_url =
-        embedded_test_server()->GetURL("adblock-data/3/ABPFilterParserData.dat");
+    brave_shields::AdBlockService::SetAdBlockURLForTest(
+        embedded_test_server()->GetURL("adblock-data/3/ABPFilterParserData.dat"));
   }
 
   void WaitForAdBlockServiceThread() {
     scoped_refptr<base::ThreadTestHelper> io_helper(
         new base::ThreadTestHelper(
-            brave_shields::AdBlockService::GetInstance()->GetTaskRunner()));
+            g_brave_browser_process->ad_block_service()->GetTaskRunner()));
     ASSERT_TRUE(io_helper->Run());
   }
 };