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

chrome/browser/android/ntp/ntp_snippets_bridge.cc

Index: chrome/browser/android/ntp/ntp_snippets_bridge.cc
diff --git a/chrome/browser/android/ntp/ntp_snippets_bridge.cc b/chrome/browser/android/ntp/ntp_snippets_bridge.cc
index 1674d00ad9ba3b2e66c6bf13b4463ead1ec16829..3b8eaee0a311bbde3d04eeae28db93510c0f6c0c 100644
--- a/chrome/browser/android/ntp/ntp_snippets_bridge.cc
+++ b/chrome/browser/android/ntp/ntp_snippets_bridge.cc
@@ -171,12 +171,8 @@ 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));
 }
 
@@ -216,7 +212,7 @@ 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(
@@ -225,7 +221,7 @@ base::android::ScopedJavaLocalRef<jobject> NTPSnippetsBridge::GetCategoryInfo(
     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);
   }
@@ -241,7 +237,7 @@ 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));
@@ -255,7 +251,7 @@ void NTPSnippetsBridge::FetchSuggestionImage(
     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));
@@ -270,7 +266,7 @@ void NTPSnippetsBridge::Fetch(
   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()),
@@ -286,7 +282,7 @@ void NTPSnippetsBridge::DismissSuggestion(
     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)));
@@ -308,7 +304,7 @@ void NTPSnippetsBridge::DismissSuggestion(
 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);
 
@@ -335,7 +331,7 @@ void NTPSnippetsBridge::OnPageShown(
   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);
@@ -351,7 +347,7 @@ void NTPSnippetsBridge::OnSuggestionShown(JNIEnv* env,
                                           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(
@@ -368,9 +364,11 @@ void NTPSnippetsBridge::OnSuggestionOpened(JNIEnv* env,
                                            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);
 }
@@ -383,7 +381,7 @@ void NTPSnippetsBridge::OnSuggestionMenuOpened(JNIEnv* env,
                                                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);
 }
 
@@ -391,7 +389,7 @@ 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);
 }
 
@@ -399,8 +397,8 @@ 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);
 }
@@ -475,10 +473,6 @@ void NTPSnippetsBridge::OnSuggestionsFetched(
       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);