Remove MostVisitedSites => SupervisedUserService dep.

chrome/browser/android/ntp/most_visited_sites.h

 // Copyright 2013 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_MOST_VISITED_SITES_H_
 #define CHROME_BROWSER_ANDROID_NTP_MOST_VISITED_SITES_H_
 
 #include <stddef.h>
 
 #include <memory>
 #include <string>
 #include <vector>
 
 #include "base/compiler_specific.h"
 #include "base/files/file_path.h"
 #include "base/macros.h"
 #include "base/memory/weak_ptr.h"
 #include "base/scoped_observer.h"
 #include "chrome/browser/android/ntp/popular_sites.h"
-#include "chrome/browser/supervised_user/supervised_user_service.h"
-#include "chrome/browser/supervised_user/supervised_user_service_observer.h"
 #include "components/history/core/browser/history_types.h"
 #include "components/history/core/browser/top_sites_observer.h"
 #include "components/suggestions/proto/suggestions.pb.h"
 #include "components/suggestions/suggestions_service.h"
 #include "url/gurl.h"
 
 namespace history {
 class TopSites;
 }
 
 namespace suggestions {
 class SuggestionsService;
 }
 
 namespace user_prefs {
 class PrefRegistrySyncable;
 }
 
 namespace variations {
 class VariationsService;
 }
 
+// Shim interface for SupervisedUserService.
+class MostVisitedSitesSupervisor {
+ public:
+  struct Whitelist {
+    base::string16 title;
+    GURL entry_point;
+    base::FilePath large_icon_path;
+  };
+
+  class Observer {
+   public:
+    virtual void OnBlockedSitesChanged() = 0;
+
+   protected:
+    ~Observer() {}
+  };
+
+  // Pass non-null to set observer, or null to remove observer.
+  // If setting observer, there must not yet be an observer set.
+  // If removing observer, there must already be one to remove.
+  // Does not take ownership. Observer must outlive this object.
+  virtual void SetObserver(Observer* new_observer) = 0;

[Marc Treib, 2016/05/09 09:55:13]

nit: I'd call this just "observer" - any reason for the "new_" prefix?

[sfiera, 2016/05/09 10:26:17]

I found the naming to help with reading the implementation of this method.

+
+  // If true, |url| should not be shown on the NTP.
+  virtual bool IsBlocked(const GURL& url) = 0;
+
+  // Explicit suggestions for sites to show on NTP.
+  virtual std::vector<Whitelist> whitelists() = 0;
+
+  // If true, be conservative about suggesting sites from outside sources.
+  virtual bool IsChildProfile() = 0;
+
+ protected:
+  virtual ~MostVisitedSitesSupervisor() {}
+};
+
 // Tracks the list of most visited sites and their thumbnails.
 //
 // Do not use, except from MostVisitedSitesBridge. The interface is in flux
 // while we are extracting the functionality of the Java class to make available
 // in C++.
 //
 // TODO(sfiera): finalize interface.
 class MostVisitedSites : public history::TopSitesObserver,
-                         public SupervisedUserServiceObserver {
+                         public MostVisitedSitesSupervisor::Observer {
  public:
   struct Suggestion;
   using SuggestionsVector = std::vector<Suggestion>;
   using PopularSitesVector = std::vector<PopularSites::Site>;
 
   // The source of the Most Visited sites.
   enum MostVisitedSource { TOP_SITES, SUGGESTIONS_SERVICE, POPULAR, WHITELIST };
 
   // The observer to be notified when the list of most visited sites changes.
   class Observer {
@@ skipping 26 matching lines @@
    private:
     DISALLOW_COPY_AND_ASSIGN(Suggestion);
   };
 
   MostVisitedSites(PrefService* prefs,
                    const TemplateURLService* template_url_service,
                    variations::VariationsService* variations_service,
                    net::URLRequestContextGetter* download_context,
                    scoped_refptr<history::TopSites> top_sites,
                    suggestions::SuggestionsService* suggestions,
-                   bool is_child_profile,
-                   Profile* profile);
+                   MostVisitedSitesSupervisor* supervisor);
 
   ~MostVisitedSites() override;
 
   // Does not take ownership of |observer|, which must outlive this object and
   // must not be null.
   void SetMostVisitedURLsObserver(Observer* observer, int num_sites);
 
   using ThumbnailCallback = base::Callback<
       void(bool /* is_local_thumbnail */, const SkBitmap* /* bitmap */)>;
   void GetURLThumbnail(const GURL& url, const ThumbnailCallback& callback);
   void AddOrRemoveBlacklistedUrl(const GURL& url, bool add_url);
   void RecordTileTypeMetrics(const std::vector<int>& tile_types);
   void RecordOpenedMostVisitedItem(int index, int tile_type);
 
-  // SupervisedUserServiceObserver implementation.
-  void OnURLFilterChanged() override;
+  // MostVisitedSitesSupervisor::Observer implementation.
+  void OnBlockedSitesChanged() override;

[Marc Treib, 2016/05/09 09:55:12]

Not your doing, but this can be private.

 
   static void RegisterProfilePrefs(user_prefs::PrefRegistrySyncable* registry);
 
  private:
   friend class MostVisitedSitesTest;
 
   // TODO(treib): use SuggestionsVector in internal functions. crbug.com/601734
   using SuggestionsPtrVector = std::vector<std::unique_ptr<Suggestion>>;
 
   void BuildCurrentSuggestions();
@@ skipping 87 matching lines @@
   void RecordThumbnailUMAMetrics();
 
   // Records UMA histogram metrics related to the number of impressions.
   void RecordImpressionUMAMetrics();
 
   // history::TopSitesObserver implementation.
   void TopSitesLoaded(history::TopSites* top_sites) override;
   void TopSitesChanged(history::TopSites* top_sites,
                        ChangeReason change_reason) override;
 
-  // The profile whose most visited sites will be queried.
-  Profile* profile_;
-
   PrefService* prefs_;
   const TemplateURLService* template_url_service_;
   variations::VariationsService* variations_service_;
   net::URLRequestContextGetter* download_context_;
   scoped_refptr<history::TopSites> top_sites_;
   suggestions::SuggestionsService* suggestions_service_;
-
-  // Children will not be shown popular sites as suggestions.
-  // TODO(sfiera): enable/disable suggestions if the profile is marked or
-  // unmarked as a child profile during the lifetime of the object. For now, it
-  // doesn't matter, because a MostVisitedSites is instantiated for each NTP,
-  // but a longer-lived object would need to update.
-  bool is_child_profile_;
+  MostVisitedSitesSupervisor* supervisor_;
 
   Observer* observer_;
 
   // The maximum number of most visited sites to return.
   int num_sites_;
 
   // Whether we have received an initial set of most visited sites (from either
   // TopSites or the SuggestionsService).
   bool received_most_visited_sites_;
 
@@ skipping 17 matching lines @@
 
   SuggestionsVector current_suggestions_;
 
   // For callbacks may be run after destruction.
   base::WeakPtrFactory<MostVisitedSites> weak_ptr_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(MostVisitedSites);
 };
 
 #endif  // CHROME_BROWSER_ANDROID_NTP_MOST_VISITED_SITES_H_