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

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 <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"
 #include "base/memory/weak_ptr.h"
 #include "base/scoped_observer.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"
@@ skipping 19 matching lines @@
   virtual void OnMostVisitedURLsAvailable(
       const std::vector<base::string16>& titles,
       const std::vector<std::string>& urls,
       const std::vector<std::string>& whitelist_icon_paths) = 0;
   virtual void OnPopularURLsAvailable(
       const std::vector<std::string>& urls,
       const std::vector<std::string>& favicon_urls,
       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 */)>;
   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;
 
-  // Registers JNI methods.
-  static bool Register(JNIEnv* env);
-
   static void RegisterProfilePrefs(user_prefs::PrefRegistrySyncable* registry);
 
  private:
   friend class MostVisitedSitesTest;
 
   // The source of the Most Visited sites.
   enum MostVisitedSource { TOP_SITES, SUGGESTIONS_SERVICE, POPULAR, WHITELIST };
 
   struct Suggestion {
     base::string16 title;
@@ skipping 11 matching lines @@
 
     // Get the Histogram name associated with the source.
     std::string GetSourceHistogramName() const;
 
    private:
     DISALLOW_COPY_AND_ASSIGN(Suggestion);
   };
 
   using SuggestionsVector = std::vector<std::unique_ptr<Suggestion>>;
 
-  ~MostVisitedSites() override;
   void QueryMostVisitedURLs();
 
   // Initialize the query to Top Sites. Called if the SuggestionsService is not
   // enabled, or if it returns no data.
   void InitiateTopSitesQuery();
 
   // If there's a whitelist entry point for the URL, return the large icon path.
   base::FilePath GetWhitelistLargeIconPath(const GURL& url);
 
   // Callback for when data is available from TopSites.
@@ skipping 81 matching lines @@
   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_;
 
-  std::unique_ptr<MostVisitedSitesObserver> observer_;
+  MostVisitedSitesObserver* 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_;
 
   // Whether we have received the set of popular sites. Immediately set to true
   // if popular sites are disabled.
@@ skipping 15 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_