[NTP::SectionOrder] Add category position metric for opened suggestions.

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 <algorithm>
 #include <utility>
 #include <vector>
 
 #include "base/android/callback_android.h"
 #include "base/android/jni_android.h"
 #include "base/android/jni_array.h"
 #include "base/android/jni_string.h"
 #include "base/callback.h"
+#include "base/stl_util.h"
 #include "base/time/time.h"
 #include "chrome/browser/history/history_service_factory.h"
 #include "chrome/browser/ntp_snippets/content_suggestions_service_factory.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/profiles/profile_android.h"
 #include "chrome/browser/profiles/profile_manager.h"
 #include "components/history/core/browser/history_service.h"
 #include "components/ntp_snippets/content_suggestion.h"
 #include "components/ntp_snippets/content_suggestions_metrics.h"
 #include "components/ntp_snippets/pref_names.h"
@@ skipping 74 matching lines @@
   ntp_snippets::ContentSuggestionsService* content_suggestions_service =
       ContentSuggestionsServiceFactory::GetForProfile(
           ProfileManager::GetLastUsedProfile());
   // Can maybe be null in some cases? (Incognito profile?) crbug.com/647920
   if (!content_suggestions_service) {
     return nullptr;
   }
   return content_suggestions_service->remote_suggestions_scheduler();
 }
 
+int GetCategoryPosition(
+    Category category,
+    const ntp_snippets::ContentSuggestionsService* service) {
+  std::vector<Category> ordered_categories = service->GetCategories();
+  DCHECK(base::ContainsValue(ordered_categories, category));
+  auto it =
+      std::find(ordered_categories.begin(), ordered_categories.end(), category);
+  return it - ordered_categories.begin();
+}

[noyau (Ping after 24h), 2017/01/05 14:38:04]

Can this lookup can be done by the metrics code in component instead of here to
avoid code duplication with non android UI?

[vitaliii, 2017/01/05 15:16:59]

Done.

[jkrcal, 2017/01/05 15:18:08]

I am not sure this is the right way, since metrics code would need a pointer to
ContentSuggestionsService in OnSuggestionOpened(). To me, this seems even more
ugly than code duplication.

Do you insist?

[vitaliii, 2017/01/05 15:47:37]

Acknowledged.

+
 }  // namespace
 
 static jlong Init(JNIEnv* env,
                   const JavaParamRef<jobject>& obj,
                   const JavaParamRef<jobject>& j_profile) {
   NTPSnippetsBridge* snippets_bridge = new NTPSnippetsBridge(env, j_profile);
   return reinterpret_cast<intptr_t>(snippets_bridge);
 }
 
 // Initiates a background fetch for remote suggestions.
@@ skipping 137 matching lines @@
       const base::android::JavaParamRef<jobject>& obj) {
   content_suggestions_service_->ReloadSuggestions();
 }
 
 void NTPSnippetsBridge::DismissSuggestion(
     JNIEnv* env,
     const JavaParamRef<jobject>& obj,
     const JavaParamRef<jstring>& jurl,
     jint global_position,
     jint j_category_id,
-    jint category_position,
+    jint position_in_category,
     const JavaParamRef<jstring>& id_within_category) {
   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,
+          [](int global_position, Category category, int position_in_category,
              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, position_in_category, visited);
           },
-          global_position, category, category_position),
+          global_position, category, position_in_category),
       &tracker_);
 }
 
 void NTPSnippetsBridge::DismissCategory(JNIEnv* env,
                                         const JavaParamRef<jobject>& obj,
                                         jint j_category_id) {
   Category category = Category::FromIDValue(j_category_id);
 
   content_suggestions_service_->DismissCategory(category);
 
@@ skipping 25 matching lines @@
   }
   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,
+                                          jint position_in_category,
                                           jlong publish_timestamp_ms,
                                           jfloat score) {
   PrefService* pref_service = ProfileManager::GetLastUsedProfile()->GetPrefs();
   base::Time last_background_fetch_time =
       base::Time::FromInternalValue(pref_service->GetInt64(
           ntp_snippets::prefs::kLastSuccessfulBackgroundFetchTime));
 
   ntp_snippets::metrics::OnSuggestionShown(
-      global_position, Category::FromIDValue(j_category_id), category_position,
-      base::Time::FromJavaTime(publish_timestamp_ms),
+      global_position, Category::FromIDValue(j_category_id),
+      position_in_category, base::Time::FromJavaTime(publish_timestamp_ms),
       last_background_fetch_time, 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,
+                                           jint position_in_category,
                                            jlong publish_timestamp_ms,
                                            jfloat score,
                                            int windowOpenDisposition) {
+  const Category category = Category::FromIDValue(j_category_id);
   ntp_snippets::metrics::OnSuggestionOpened(
-      global_position, Category::FromIDValue(j_category_id), category_position,
-      base::Time::FromJavaTime(publish_timestamp_ms), score,
-      static_cast<WindowOpenDisposition>(windowOpenDisposition));
+      global_position, category,
+      GetCategoryPosition(category, content_suggestions_service_),
+      position_in_category, base::Time::FromJavaTime(publish_timestamp_ms),
+      score, static_cast<WindowOpenDisposition>(windowOpenDisposition));
   // TODO(vitaliii): Add ContentSuggestionsService::OnSuggestionOpened and
   // notify the ranker and the classifier there instead. Do not expose both of
   // them at all. See crbug.com/674080.
-  content_suggestions_service_->category_ranker()->OnSuggestionOpened(
-      Category::FromIDValue(j_category_id));
+  content_suggestions_service_->category_ranker()->OnSuggestionOpened(category);
   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,
+                                               jint position_in_category,
                                                jlong publish_timestamp_ms,
                                                jfloat score) {
   ntp_snippets::metrics::OnSuggestionMenuOpened(
-      global_position, Category::FromIDValue(j_category_id), category_position,
-      base::Time::FromJavaTime(publish_timestamp_ms), score);
+      global_position, Category::FromIDValue(j_category_id),
+      position_in_category, 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(Category::FromIDValue(j_category_id),
                                            position);
 }
 
@@ skipping 80 matching lines @@
   JNIEnv* env = AttachCurrentThread();
   Java_SnippetsBridge_onMoreSuggestions(
       env, observer_, category.id(),
       ToJavaSuggestionList(env, category, suggestions));
 }
 
 // static
 bool NTPSnippetsBridge::Register(JNIEnv* env) {
   return RegisterNativesImpl(env);
 }