[SuggestionsService] Split SuggestionsService interface from impl

components/suggestions/suggestions_service.h

Index: components/suggestions/suggestions_service.h
diff --git a/components/suggestions/suggestions_service.h b/components/suggestions/suggestions_service.h
index 5b118e11d2a62c80503d63e46b964bf57d0ecd97..1ab55ab7d8a58a2a0692849845924de0b970c01c 100644
--- a/components/suggestions/suggestions_service.h
+++ b/components/suggestions/suggestions_service.h
@@ -15,6 +15,7 @@
 #include "base/gtest_prod_util.h"
 #include "base/macros.h"
 #include "base/memory/weak_ptr.h"
+#include "base/optional.h"
 #include "base/scoped_observer.h"
 #include "base/threading/thread_checker.h"
 #include "base/time/time.h"
@@ -50,9 +51,7 @@ class ImageManager;
 class SuggestionsStore;
 
 // An interface to fetch server suggestions asynchronously.
-class SuggestionsService : public KeyedService,
-                           public net::URLFetcherDelegate,
-                           public syncer::SyncServiceObserver {
+class SuggestionsService : public KeyedService {
  public:
   using ResponseCallback = base::Callback<void(const SuggestionsProfile&)>;
   using BitmapCallback = base::Callback<void(const GURL&, const gfx::Image&)>;
@@ -60,48 +59,71 @@ class SuggestionsService : public KeyedService,
   using ResponseCallbackList =
       base::CallbackList<void(const SuggestionsProfile&)>;
 
-  SuggestionsService(const SigninManagerBase* signin_manager,
-                     OAuth2TokenService* token_service,
-                     syncer::SyncService* sync_service,
-                     net::URLRequestContextGetter* url_request_context,
-                     std::unique_ptr<SuggestionsStore> suggestions_store,
-                     std::unique_ptr<ImageManager> thumbnail_manager,
-                     std::unique_ptr<BlacklistStore> blacklist_store);
-  ~SuggestionsService() override;
-
   // Initiates a network request for suggestions if sync state allows and there
   // is no pending request. Returns true iff sync state allowed for a request,
   // whether a new request was actually sent or not.
-  bool FetchSuggestionsData();
+  virtual bool FetchSuggestionsData() = 0;
 
   // Returns the current set of suggestions from the cache.
-  SuggestionsProfile GetSuggestionsDataFromCache() const;
+  virtual base::Optional<SuggestionsProfile> GetSuggestionsDataFromCache()
+      const = 0;
 
   // Adds a callback that is called when the suggestions are updated.
-  std::unique_ptr<ResponseCallbackList::Subscription> AddCallback(
-      const ResponseCallback& callback) WARN_UNUSED_RESULT;
+  virtual std::unique_ptr<ResponseCallbackList::Subscription> AddCallback(
+      const ResponseCallback& callback) WARN_UNUSED_RESULT = 0;
 
   // Retrieves stored thumbnail for website |url| asynchronously. Calls
   // |callback| with Bitmap pointer if found, and NULL otherwise.
-  void GetPageThumbnail(const GURL& url, const BitmapCallback& callback);
+  virtual void GetPageThumbnail(const GURL& url,
+                                const BitmapCallback& callback) = 0;
 
   // A version of |GetPageThumbnail| that explicitly supplies the download URL
   // for the thumbnail. Replaces any pre-existing thumbnail URL with the
   // supplied one.
-  void GetPageThumbnailWithURL(const GURL& url,
-                               const GURL& thumbnail_url,
-                               const BitmapCallback& callback);
+  virtual void GetPageThumbnailWithURL(const GURL& url,
+                                       const GURL& thumbnail_url,
+                                       const BitmapCallback& callback) = 0;
 
   // Adds a URL to the blacklist cache, returning true on success or false on
   // failure. The URL will eventually be uploaded to the server.
-  bool BlacklistURL(const GURL& candidate_url);
+  virtual bool BlacklistURL(const GURL& candidate_url) = 0;
 
   // Removes a URL from the local blacklist, returning true on success or false
   // on failure.
-  bool UndoBlacklistURL(const GURL& url);
+  virtual bool UndoBlacklistURL(const GURL& url) = 0;
 
   // Removes all URLs from the blacklist.
-  void ClearBlacklist();
+  virtual void ClearBlacklist() = 0;
+};
+
+// Actual (non-test) implementation of the SuggestionsService interface.
+class SuggestionsServiceImpl : public SuggestionsService,

[Marc Treib, 2016/12/13 15:12:07]

Hm, I think nobody expect the factory (and tests) need to know about the Impl.
Would it make sense to move it to a separate file?

[mastiz, 2016/12/13 16:18:49]

Done, moved impl to dedicated file, ptal. git cl format mangled a few lines, I
can revert those if you prefer.

[Marc Treib, 2016/12/13 16:30:27]

Nah that's fine, IMO it's generally a Good Thing to have everything git cl
formatted.

+                               public net::URLFetcherDelegate,
+                               public syncer::SyncServiceObserver {
+ public:
+  SuggestionsServiceImpl(const SigninManagerBase* signin_manager,
+                         OAuth2TokenService* token_service,
+                         syncer::SyncService* sync_service,
+                         net::URLRequestContextGetter* url_request_context,
+                         std::unique_ptr<SuggestionsStore> suggestions_store,
+                         std::unique_ptr<ImageManager> thumbnail_manager,
+                         std::unique_ptr<BlacklistStore> blacklist_store);
+  ~SuggestionsServiceImpl() override;
+
+  // SuggestionsService implementation.
+  bool FetchSuggestionsData() override;
+  base::Optional<SuggestionsProfile> GetSuggestionsDataFromCache()
+      const override;
+  std::unique_ptr<ResponseCallbackList::Subscription> AddCallback(
+      const ResponseCallback& callback) override WARN_UNUSED_RESULT;
+  void GetPageThumbnail(const GURL& url,
+                        const BitmapCallback& callback) override;
+  void GetPageThumbnailWithURL(const GURL& url,
+                               const GURL& thumbnail_url,
+                               const BitmapCallback& callback) override;
+  bool BlacklistURL(const GURL& candidate_url) override;
+  bool UndoBlacklistURL(const GURL& url) override;
+  void ClearBlacklist() override;
 
   // Determines which URL a blacklist request was for, irrespective of the
   // request's status. Returns false if |request| is not a blacklist request.
@@ -222,9 +244,9 @@ class SuggestionsService : public KeyedService,
   ResponseCallbackList callback_list_;
 
   // For callbacks may be run after destruction.
-  base::WeakPtrFactory<SuggestionsService> weak_ptr_factory_;
+  base::WeakPtrFactory<SuggestionsServiceImpl> weak_ptr_factory_;
 
-  DISALLOW_COPY_AND_ASSIGN(SuggestionsService);
+  DISALLOW_COPY_AND_ASSIGN(SuggestionsServiceImpl);
 };
 
 }  // namespace suggestions