NTP tiles: Split C++ and Java MostVisitedSites classes.

chrome/browser/android/ntp/most_visited_sites.h

Index: chrome/browser/android/ntp/most_visited_sites.h
diff --git a/chrome/browser/android/ntp/most_visited_sites.h b/chrome/browser/android/ntp/most_visited_sites.h
index f16b5ead7c6b89e5eb6e7ec773da10020d7a2004..5fa7f934726179df8992c5a6cd6a9033f805e34e 100644
--- a/chrome/browser/android/ntp/most_visited_sites.h
+++ b/chrome/browser/android/ntp/most_visited_sites.h
@@ -5,14 +5,12 @@
 #ifndef CHROME_BROWSER_ANDROID_NTP_MOST_VISITED_SITES_H_
 #define CHROME_BROWSER_ANDROID_NTP_MOST_VISITED_SITES_H_
 
-#include <jni.h>
 #include <stddef.h>
 
 #include <memory>
 #include <string>
 #include <vector>
 
-#include "base/android/scoped_java_ref.h"
 #include "base/compiler_specific.h"
 #include "base/files/file_path.h"
 #include "base/macros.h"
@@ -52,45 +50,16 @@ class MostVisitedSitesObserver {
       const std::vector<std::string>& large_icon_urls) = 0;
 };
 
-// Provides the list of most visited sites and their thumbnails to Java.
+// Tracks the list of most visited sites and their thumbnails.
 class MostVisitedSites : public history::TopSitesObserver,
                          public SupervisedUserServiceObserver {
  public:
   explicit MostVisitedSites(Profile* profile);
-  void Destroy(JNIEnv* env, const base::android::JavaParamRef<jobject>& obj);
 
-  // Java methods
+  ~MostVisitedSites() override;
 
   void SetMostVisitedURLsObserver(
-      JNIEnv* env,
-      const base::android::JavaParamRef<jobject>& obj,
-      const base::android::JavaParamRef<jobject>& j_observer,
-      jint num_sites);
-
-  void GetURLThumbnail(
-      JNIEnv* env,
-      const base::android::JavaParamRef<jobject>& obj,
-      const base::android::JavaParamRef<jstring>& url,
-      const base::android::JavaParamRef<jobject>& j_callback);
-  void AddOrRemoveBlacklistedUrl(
-      JNIEnv* env,
-      const base::android::JavaParamRef<jobject>& obj,
-      const base::android::JavaParamRef<jstring>& j_url,
-      jboolean add_url);
-  void RecordTileTypeMetrics(
-      JNIEnv* env,
-      const base::android::JavaParamRef<jobject>& obj,
-      const base::android::JavaParamRef<jintArray>& jtile_types);
-  void RecordOpenedMostVisitedItem(
-      JNIEnv* env,
-      const base::android::JavaParamRef<jobject>& obj,
-      jint index,
-      jint tile_type);
-
-  // C++ methods
-
-  void SetMostVisitedURLsObserver(
-      std::unique_ptr<MostVisitedSitesObserver> observer, int num_sites);
+      MostVisitedSitesObserver* observer, int num_sites);

[Bernhard Bauer, 2016/04/20 13:35:11]

Add a comment about ownership of |observer|?

Actually, is there a reason why the observer is not owned anymore? That would
make things a bit easier, I think.

[Marc Treib, 2016/04/20 13:41:19]

That's kinda my fault - IMO the observed class shouldn't own the observer,
that's the wrong way around.
This is also a bit of an intermediate state, eventually we want to support
multiple observers, and having this class own them just doesn't make sense.

[Bernhard Bauer, 2016/04/20 14:22:51]

OTOH, this does require being more careful about ownership, and if we want to
support multiple observers, presumably we won't have MostVisitedSitesBridge
directly contain MostVisitedSites anymore either, so we're going to have to
change the whole ownership model there anyway. Given that, I would slightly lean
towards ownership here (if it helps, you could think of this more as a delegate
than an observer), and change back to raw pointers when we need it. I don't
reeally feel strongly about it though.

 
   using ThumbnailCallback = base::Callback<
       void(bool /* is_local_thumbnail */, const SkBitmap* /* bitmap */)>;
@@ -102,9 +71,6 @@ class MostVisitedSites : public history::TopSitesObserver,
   // SupervisedUserServiceObserver implementation.
   void OnURLFilterChanged() override;
 
-  // Registers JNI methods.
-  static bool Register(JNIEnv* env);
-
   static void RegisterProfilePrefs(user_prefs::PrefRegistrySyncable* registry);
 
  private:
@@ -136,7 +102,6 @@ class MostVisitedSites : public history::TopSitesObserver,
 
   using SuggestionsVector = std::vector<std::unique_ptr<Suggestion>>;
 
-  ~MostVisitedSites() override;
   void QueryMostVisitedURLs();
 
   // Initialize the query to Top Sites. Called if the SuggestionsService is not
@@ -238,7 +203,7 @@ class MostVisitedSites : public history::TopSitesObserver,
   // The profile whose most visited sites will be queried.
   Profile* profile_;
 
-  std::unique_ptr<MostVisitedSitesObserver> observer_;
+  MostVisitedSitesObserver* observer_;
 
   // The maximum number of most visited sites to return.
   int num_sites_;