[Popular Sites] Split PopularSites interface and PopularSitesImpl

components/ntp_tiles/popular_sites.h

 // Copyright 2015 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_NTP_TILES_POPULAR_SITES_H_
 #define COMPONENTS_NTP_TILES_POPULAR_SITES_H_
 
 #include <memory>
 #include <string>
 #include <vector>
@@ skipping 25 matching lines @@
 class PrefService;
 class TemplateURLService;
 
 namespace ntp_tiles {
 
 using ParseJSONCallback = base::Callback<void(
     const std::string& unsafe_json,
     const base::Callback<void(std::unique_ptr<base::Value>)>& success_callback,
     const base::Callback<void(const std::string&)>& error_callback)>;
 
-// Downloads and provides a list of suggested popular sites, for display on
-// the NTP when there are not enough personalized tiles. Caches the downloaded
-// file on disk to avoid re-downloading on every startup.
-class PopularSites : public net::URLFetcherDelegate {
+// Interface to provide a list of suggested popular sites, for display on the
+// NTP when there are not enough personalized tiles.
+class PopularSites {
  public:
   struct Site {
     Site(const base::string16& title,
          const GURL& url,
          const GURL& favicon_url,
          const GURL& large_icon_url,
          const GURL& thumbnail_url);
     Site(const Site& other);
     ~Site();
 
     base::string16 title;
     GURL url;
     GURL favicon_url;
     GURL large_icon_url;
     GURL thumbnail_url;
   };
 
+  using SitesVector = std::vector<Site>;
   using FinishedCallback = base::Callback<void(bool /* success */)>;
 
-  PopularSites(const scoped_refptr<base::SequencedWorkerPool>& blocking_pool,
-               PrefService* prefs,
-               const TemplateURLService* template_url_service,
-               variations::VariationsService* variations_service,
-               net::URLRequestContextGetter* download_context,
-               const base::FilePath& directory,
-               ParseJSONCallback parse_json);
+  virtual ~PopularSites() = default;

[sfiera, 2016/12/13 10:53:57]

I think some bots are going to complain about this being in the header—if so,
you'll need to put the = default in a .cc file.

[mastiz, 2016/12/13 12:14:56]

Acknowledged, no complaints so far.

 
   // Starts the process of retrieving popular sites. When they are available,
   // invokes |callback| with the result, on the same thread as the caller. Never
   // invokes |callback| before returning control to the caller, even if the
   // result is immediately known.
   //
   // Set |force_download| to enforce re-downloading the popular sites file, even
   // if it already exists on disk.
   //
   // Must be called at most once on a given PopularSites object.
-  void StartFetch(bool force_download, const FinishedCallback& callback);
+  // TODO(mastiz): Remove this restriction?
+  virtual void StartFetch(bool force_download,
+                          const FinishedCallback& callback) = 0;
 
-  ~PopularSites() override;
+  // Returns the list of available sites.
+  virtual const SitesVector& sites() const = 0;
+};
 
-  const std::vector<Site>& sites() const { return sites_; }
+// Actual (non-test) implementation of the PopularSites interface. Caches the
+// downloaded file on disk to avoid re-downloading on every startup.
+class PopularSitesImpl : public PopularSites, public net::URLFetcherDelegate {
+ public:
+  PopularSitesImpl(
+      const scoped_refptr<base::SequencedWorkerPool>& blocking_pool,
+      PrefService* prefs,
+      const TemplateURLService* template_url_service,
+      variations::VariationsService* variations_service,
+      net::URLRequestContextGetter* download_context,
+      const base::FilePath& directory,
+      ParseJSONCallback parse_json);
+
+  ~PopularSitesImpl() override;
+
+  // PopularSites implementation.
+  void StartFetch(bool force_download,
+                  const FinishedCallback& callback) override;
+  const SitesVector& sites() const override;
 
   // The URL of the file that was last downloaded.
   GURL LastURL() const;
 
   const base::FilePath& local_path() const { return local_path_; }
 
   // Register preferences used by this class.
   static void RegisterProfilePrefs(
       user_prefs::PrefRegistrySyncable* user_prefs);
 
@@ skipping 20 matching lines @@
   variations::VariationsService* const variations_;
   net::URLRequestContextGetter* const download_context_;
   base::FilePath const local_path_;
   ParseJSONCallback parse_json_;
 
   // Set by StartFetch() and called after fetch completes.
   FinishedCallback callback_;
 
   std::unique_ptr<net::URLFetcher> fetcher_;
   bool is_fallback_;
-  std::vector<Site> sites_;
+  SitesVector sites_;
   GURL pending_url_;
 
-  base::WeakPtrFactory<PopularSites> weak_ptr_factory_;
+  base::WeakPtrFactory<PopularSitesImpl> weak_ptr_factory_;
 
-  DISALLOW_COPY_AND_ASSIGN(PopularSites);
+  DISALLOW_COPY_AND_ASSIGN(PopularSitesImpl);
 };
 
 }  // namespace ntp_tiles
 
 #endif  // COMPONENTS_NTP_TILES_POPULAR_SITES_H_