[NTP::SectionOrder] Propagate category rank from UI through the bridge.

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 329 matching lines @@
   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_rank,
                                            jint category_position,
                                            jlong publish_timestamp_ms,
                                            jfloat score,
                                            int windowOpenDisposition) {
+  DCHECK_GT(category_rank, 0);

[Marc Treib, 2017/01/16 11:23:05]

Is this 1-based?!

[vitaliii, 2017/01/16 11:36:17]

Yes, is this unusual?
It is not called "index".
I agree that "position" would be ambiguous.

Moreover, 
https://en.wikipedia.org/wiki/Mean_reciprocal_rank
implies that rank should 1-based.

[Marc Treib, 2017/01/16 12:28:52]

Well, most things we deal with are 0-based, including all of our other
histograms. So yes, I find this quite unusual.
If there's no particular reason for making it 1-based, then I'd much prefer 0.
If you really want to keep it as is, then lgtm, but please clearly mention this
in the histogram description and in comments on member variables.

[vitaliii, 2017/01/16 12:48:37]

I like the consistency.
However, I do not want to have 0-based rank variable, so I would go for "index"
instead.

tschumann@, it was your idea to use "rank", do you mind 0-based "index" instead?

[tschumann, 2017/01/16 13:45:41]

+mastiz: What's the usual way ranks get handled in logs data etc. Are they
0-based or 1-based?

I'm against introducing a different name. I'm open to internally handle it zero
based, but I wonder if that wouldn't make things unnecessarily complicated as
the histograms are 1-based again anyways.

[mastiz, 2017/01/16 15:22:53]

The term 'rank' suggests 1-based, otherwise logs usually use 'position' or
'index' which is zero-based (say, average click position).

   ntp_snippets::metrics::OnSuggestionOpened(
       global_position, Category::FromIDValue(j_category_id), category_position,
       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_->user_classifier()->OnEvent(
@@ skipping 119 matching lines @@
   JNIEnv* env = AttachCurrentThread();
   Java_SnippetsBridge_onMoreSuggestions(
       env, bridge_, category.id(),
       ToJavaSuggestionList(env, category, suggestions));
 }
 
 // static
 bool NTPSnippetsBridge::Register(JNIEnv* env) {
   return RegisterNativesImpl(env);
 }