[Popular Sites] Split PopularSites interface and PopularSitesImpl

chrome/browser/android/ntp/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 CHROME_BROWSER_ANDROID_NTP_POPULAR_SITES_H_
 #define CHROME_BROWSER_ANDROID_NTP_POPULAR_SITES_H_
 
 #include <memory>
 
 #include "base/files/file_path.h"
 #include "base/macros.h"
 
 class Profile;
 
 namespace ntp_tiles {
-class PopularSites;
+class PopularSitesImpl;
 }  // namespace ntp_tiles
 
 // TODO(sfiera): move to chrome_popular_sites.h
 class ChromePopularSites {
  public:
-  static std::unique_ptr<ntp_tiles::PopularSites> NewForProfile(
+  static std::unique_ptr<ntp_tiles::PopularSitesImpl> NewForProfile(

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

Should the factory be promising that it creates a particular implementation?

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

The alternative would be to move the two remaining functions (local_path,
LastURL) to the interface. I didn't like this idea before, but I'm not convinced
anymore. WDYT?

[sfiera, 2016/12/13 12:30:27]

Hmm. They do feel like implementation details, but I think they are only used
for diagnostics in the webui, right? I think I'm OK with them being in the
interface for that purpose.

In the future, maybe the interface can have just base::Value DebugInfo().

[mastiz, 2016/12/13 12:31:53]

Went ahead and exported a new patchset following this suggestion. I actually
think it's cleaner, and it shouldn't bother future tests much.

       Profile* profile);
 
  private:
   DISALLOW_IMPLICIT_CONSTRUCTORS(ChromePopularSites);
 };
 
 #endif  // CHROME_BROWSER_ANDROID_NTP_POPULAR_SITES_H_