[NTP::SectionOrder] Replace CategoryFactory with a category ranker.

chrome/browser/android/ntp/ntp_snippets_bridge.cc

 // Copyright 2016 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.
 
 #include "chrome/browser/android/ntp/ntp_snippets_bridge.h"
 
 #include <jni.h>
 #include <utility>
 #include <vector>
 
@@ skipping 153 matching lines @@
     return;
   }
 
   service->RescheduleFetching(/*force=*/true);
 }
 
 static void OnSuggestionTargetVisited(JNIEnv* env,
                                       const JavaParamRef<jclass>& caller,
                                       jint j_category_id,
                                       jlong visit_time_ms) {
-  Profile* profile = ProfileManager::GetLastUsedProfile();
-  ntp_snippets::ContentSuggestionsService* content_suggestions_service =
-      ContentSuggestionsServiceFactory::GetForProfile(profile);
   ntp_snippets::metrics::OnSuggestionTargetVisited(
-      content_suggestions_service->category_factory()->FromIDValue(
-          j_category_id),
+      Category::FromIDValue(j_category_id),
       base::TimeDelta::FromMilliseconds(visit_time_ms));
 }
 
 NTPSnippetsBridge::NTPSnippetsBridge(JNIEnv* env,
                                      const JavaParamRef<jobject>& j_profile)
     : content_suggestions_service_observer_(this), weak_ptr_factory_(this) {
   Profile* profile = ProfileAndroid::FromProfileAndroid(j_profile);
   content_suggestions_service_ =
       ContentSuggestionsServiceFactory::GetForProfile(profile);
   history_service_ =
@@ skipping 19 matching lines @@
   for (Category category : content_suggestions_service_->GetCategories()) {
     category_ids.push_back(category.id());
   }
   return ToJavaIntArray(env, category_ids);
 }
 
 int NTPSnippetsBridge::GetCategoryStatus(JNIEnv* env,
                                          const JavaParamRef<jobject>& obj,
                                          jint j_category_id) {
   return static_cast<int>(content_suggestions_service_->GetCategoryStatus(
-      CategoryFromIDValue(j_category_id)));
+      Category::FromIDValue(j_category_id)));
 }
 
 base::android::ScopedJavaLocalRef<jobject> NTPSnippetsBridge::GetCategoryInfo(
     JNIEnv* env,
     const base::android::JavaParamRef<jobject>& obj,
     jint j_category_id) {
   base::Optional<CategoryInfo> info =
       content_suggestions_service_->GetCategoryInfo(
-          CategoryFromIDValue(j_category_id));
+          Category::FromIDValue(j_category_id));
   if (!info) {
     return base::android::ScopedJavaLocalRef<jobject>(env, nullptr);
   }
   return Java_SnippetsBridge_createSuggestionsCategoryInfo(
       env, j_category_id, ConvertUTF16ToJavaString(env, info->title()),
       static_cast<int>(info->card_layout()), info->has_more_action(),
       info->has_reload_action(), info->has_view_all_action(),
       info->show_if_empty(),
       ConvertUTF16ToJavaString(env, info->no_suggestions_message()));
 }
 
 ScopedJavaLocalRef<jobject> NTPSnippetsBridge::GetSuggestionsForCategory(
     JNIEnv* env,
     const base::android::JavaParamRef<jobject>& obj,
     jint j_category_id) {
-  Category category = CategoryFromIDValue(j_category_id);
+  Category category = Category::FromIDValue(j_category_id);
   return ToJavaSuggestionList(
       env, category,
       content_suggestions_service_->GetSuggestionsForCategory(category));
 }
 
 void NTPSnippetsBridge::FetchSuggestionImage(
     JNIEnv* env,
     const JavaParamRef<jobject>& obj,
     jint j_category_id,
     const JavaParamRef<jstring>& id_within_category,
     const JavaParamRef<jobject>& j_callback) {
   base::android::ScopedJavaGlobalRef<jobject> callback(j_callback);
   content_suggestions_service_->FetchSuggestionImage(
-      ContentSuggestion::ID(CategoryFromIDValue(j_category_id),
+      ContentSuggestion::ID(Category::FromIDValue(j_category_id),
                             ConvertJavaStringToUTF8(env, id_within_category)),
       base::Bind(&NTPSnippetsBridge::OnImageFetched,
                  weak_ptr_factory_.GetWeakPtr(), callback));
 }
 
 void NTPSnippetsBridge::Fetch(
     JNIEnv* env,
     const JavaParamRef<jobject>& obj,
     jint j_category_id,
     const JavaParamRef<jobjectArray>& j_displayed_suggestions) {
   std::vector<std::string> known_suggestion_ids;
   AppendJavaStringArrayToStringVector(env, j_displayed_suggestions,
                                       &known_suggestion_ids);
 
-  Category category = CategoryFromIDValue(j_category_id);
+  Category category = Category::FromIDValue(j_category_id);
   content_suggestions_service_->Fetch(
       category, std::set<std::string>(known_suggestion_ids.begin(),
                                       known_suggestion_ids.end()),
       base::Bind(&NTPSnippetsBridge::OnSuggestionsFetched,
                  weak_ptr_factory_.GetWeakPtr(), category));
 }
 
 void NTPSnippetsBridge::DismissSuggestion(
     JNIEnv* env,
     const JavaParamRef<jobject>& obj,
     const JavaParamRef<jstring>& jurl,
     jint global_position,
     jint j_category_id,
     jint category_position,
     const JavaParamRef<jstring>& id_within_category) {
-  Category category = CategoryFromIDValue(j_category_id);
+  Category category = Category::FromIDValue(j_category_id);
 
   content_suggestions_service_->DismissSuggestion(ContentSuggestion::ID(
       category, ConvertJavaStringToUTF8(env, id_within_category)));
 
   history_service_->QueryURL(
       GURL(ConvertJavaStringToUTF8(env, jurl)), /*want_visits=*/false,
       base::Bind(
           [](int global_position, Category category, int category_position,
              bool success, const history::URLRow& row,
              const history::VisitVector& visit_vector) {
             bool visited = success && row.visit_count() != 0;
             ntp_snippets::metrics::OnSuggestionDismissed(
                 global_position, category, category_position, visited);
           },
           global_position, category, category_position),
       &tracker_);
 }
 
 void NTPSnippetsBridge::DismissCategory(JNIEnv* env,
                                         const JavaParamRef<jobject>& obj,
                                         jint j_category_id) {
-  Category category = CategoryFromIDValue(j_category_id);
+  Category category = Category::FromIDValue(j_category_id);
 
   content_suggestions_service_->DismissCategory(category);
 
   ntp_snippets::metrics::OnCategoryDismissed(category);
 }
 
 void NTPSnippetsBridge::RestoreDismissedCategories(
     JNIEnv* env,
     const JavaParamRef<jobject>& obj) {
   content_suggestions_service_->RestoreDismissedCategories();
 }
 
 void NTPSnippetsBridge::OnPageShown(
     JNIEnv* env,
     const JavaParamRef<jobject>& obj,
     const JavaParamRef<jintArray>& jcategories,
     const JavaParamRef<jintArray>& jsuggestions_per_category) {
   std::vector<int> categories_int;
   JavaIntArrayToIntVector(env, jcategories, &categories_int);
   std::vector<int> suggestions_per_category_int;
   JavaIntArrayToIntVector(env, jsuggestions_per_category,
                           &suggestions_per_category_int);
   DCHECK_EQ(categories_int.size(), suggestions_per_category_int.size());
   std::vector<std::pair<Category, int>> suggestions_per_category;
   for (size_t i = 0; i < categories_int.size(); i++) {
     suggestions_per_category.push_back(
-        std::make_pair(CategoryFromIDValue(categories_int[i]),
+        std::make_pair(Category::FromIDValue(categories_int[i]),
                        suggestions_per_category_int[i]));
   }
   ntp_snippets::metrics::OnPageShown(suggestions_per_category);
   content_suggestions_service_->user_classifier()->OnEvent(
       ntp_snippets::UserClassifier::Metric::NTP_OPENED);
 }
 
 void NTPSnippetsBridge::OnSuggestionShown(JNIEnv* env,
                                           const JavaParamRef<jobject>& obj,
                                           jint global_position,
                                           jint j_category_id,
                                           jint category_position,
                                           jlong publish_timestamp_ms,
                                           jfloat score) {
   ntp_snippets::metrics::OnSuggestionShown(
-      global_position, CategoryFromIDValue(j_category_id), category_position,
+      global_position, Category::FromIDValue(j_category_id), category_position,
       base::Time::FromJavaTime(publish_timestamp_ms), score);
   if (global_position == 0) {
     content_suggestions_service_->user_classifier()->OnEvent(
         ntp_snippets::UserClassifier::Metric::SUGGESTIONS_SHOWN);
   }
 }
 
 void NTPSnippetsBridge::OnSuggestionOpened(JNIEnv* env,
                                            const JavaParamRef<jobject>& obj,
                                            jint global_position,
                                            jint j_category_id,
                                            jint category_position,
                                            jlong publish_timestamp_ms,
                                            jfloat score,
                                            int windowOpenDisposition) {
   ntp_snippets::metrics::OnSuggestionOpened(
-      global_position, CategoryFromIDValue(j_category_id), category_position,
+      global_position, Category::FromIDValue(j_category_id), category_position,
       base::Time::FromJavaTime(publish_timestamp_ms), score,
       static_cast<WindowOpenDisposition>(windowOpenDisposition));
+  content_suggestions_service_->section_ranker()->OnSuggestionOpened(

[Marc Treib, 2016/12/13 12:22:42]

It'd be nice not to expose the section ranker. Similar for the user classifier
actually. Maybe there should just be an OnSuggestionOpened method on
ContentSuggestionsService that does all this?

[vitaliii, 2016/12/14 08:59:38]

user_classifier is used in many places (e.g.
snippets_internals_message_handler.cc and RemoteSuggestionsProvider), so it is
not clear how not to expose it. Thus, for sure, the change should not be in this
CL.

[Marc Treib, 2016/12/14 10:24:30]

Agreed, I wasn't suggesting to do that in this CL.
What we could consider is making a ContentSuggestionsService::OnSuggestionOpened
method, so that we don't have to call two separate things here (neither of which
the bridge should have to know about).
(The metrics:: call should probably stay separate for now though - if we want to
pipe that through the service, then we should do the same for all the metrics
calls, and that's not for this CL.)

[vitaliii, 2016/12/15 15:30:12]

I added a TODO.

+      Category::FromIDValue(j_category_id));
   content_suggestions_service_->user_classifier()->OnEvent(
       ntp_snippets::UserClassifier::Metric::SUGGESTIONS_USED);
 }
 
 void NTPSnippetsBridge::OnSuggestionMenuOpened(JNIEnv* env,
                                                const JavaParamRef<jobject>& obj,
                                                jint global_position,
                                                jint j_category_id,
                                                jint category_position,
                                                jlong publish_timestamp_ms,
                                                jfloat score) {
   ntp_snippets::metrics::OnSuggestionMenuOpened(
-      global_position, CategoryFromIDValue(j_category_id), category_position,
+      global_position, Category::FromIDValue(j_category_id), category_position,
       base::Time::FromJavaTime(publish_timestamp_ms), score);
 }
 
 void NTPSnippetsBridge::OnMoreButtonShown(JNIEnv* env,
                                           const JavaParamRef<jobject>& obj,
                                           jint j_category_id,
                                           jint position) {
-  ntp_snippets::metrics::OnMoreButtonShown(CategoryFromIDValue(j_category_id),
+  ntp_snippets::metrics::OnMoreButtonShown(Category::FromIDValue(j_category_id),
                                            position);
 }
 
 void NTPSnippetsBridge::OnMoreButtonClicked(JNIEnv* env,
                                             const JavaParamRef<jobject>& obj,
                                             jint j_category_id,
                                             jint position) {
-  ntp_snippets::metrics::OnMoreButtonClicked(CategoryFromIDValue(j_category_id),
-                                             position);
+  ntp_snippets::metrics::OnMoreButtonClicked(
+      Category::FromIDValue(j_category_id), position);
   content_suggestions_service_->user_classifier()->OnEvent(
       ntp_snippets::UserClassifier::Metric::SUGGESTIONS_USED);
 }
 
 NTPSnippetsBridge::~NTPSnippetsBridge() {}
 
 void NTPSnippetsBridge::OnNewSuggestions(Category category) {
   if (observer_.is_null()) {
     return;
   }
@@ skipping 54 matching lines @@
     Category category,
     ntp_snippets::Status status,
     std::vector<ContentSuggestion> suggestions) {
   // TODO(fhorschig, dgn): Allow refetch or show notification acc. to status.
   JNIEnv* env = AttachCurrentThread();
   Java_SnippetsBridge_onMoreSuggestions(
       env, observer_, category.id(),
       ToJavaSuggestionList(env, category, suggestions));
 }
 
-Category NTPSnippetsBridge::CategoryFromIDValue(jint id) {
-  return content_suggestions_service_->category_factory()->FromIDValue(id);
-}
-
 // static
 bool NTPSnippetsBridge::Register(JNIEnv* env) {
   return RegisterNativesImpl(env);
 }