[SuggestionsService] Split SuggestionsService interface from impl

components/suggestions/suggestions_service.h

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #ifndef COMPONENTS_SUGGESTIONS_SUGGESTIONS_SERVICE_H_
 #define COMPONENTS_SUGGESTIONS_SUGGESTIONS_SERVICE_H_
 
 #include <stdint.h>
 
 #include <memory>
 #include <string>
 
 #include "base/callback.h"
 #include "base/callback_list.h"
 #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"
 #include "components/keyed_service/core/keyed_service.h"
 #include "components/suggestions/proto/suggestions.pb.h"
 #include "components/sync/driver/sync_service_observer.h"
 #include "net/url_request/url_fetcher_delegate.h"
 #include "url/gurl.h"
 
 class OAuth2TokenService;
@@ skipping 15 matching lines @@
 class PrefRegistrySyncable;
 }  // namespace user_prefs
 
 namespace suggestions {
 
 class BlacklistStore;
 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&)>;
 
   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.
   static bool GetBlacklistedUrl(const net::URLFetcher& request, GURL* url);
 
   // Register SuggestionsService related prefs in the Profile prefs.
   static void RegisterProfilePrefs(user_prefs::PrefRegistrySyncable* registry);
 
  private:
   friend class SuggestionsServiceTest;
@@ skipping 100 matching lines @@
   // if cancelled.
   std::unique_ptr<net::URLFetcher> pending_request_;
 
   // The start time of the previous suggestions request. This is used to measure
   // the latency of requests. Initially zero.
   base::TimeTicks last_request_started_time_;
 
   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
 
 #endif  // COMPONENTS_SUGGESTIONS_SUGGESTIONS_SERVICE_H_