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

components/ntp_snippets/content_suggestions_metrics.h

 // 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.
 
 #ifndef COMPONENTS_NTP_SNIPPETS_CONTENT_SUGGESTIONS_METRICS_H_
 #define COMPONENTS_NTP_SNIPPETS_CONTENT_SUGGESTIONS_METRICS_H_
 
 #include <utility>
 #include <vector>
 
 #include "base/time/time.h"
 #include "components/ntp_snippets/category.h"
 #include "ui/base/window_open_disposition.h"
 
 namespace ntp_snippets {
 namespace metrics {
 
 void OnPageShown(
     const std::vector<std::pair<Category, int>>& suggestions_per_category);
 
 // Should only be called once per NTP for each suggestion.
 void OnSuggestionShown(int global_position,
                        Category category,
-                       int category_position,
+                       int position_in_category,
                        base::Time publish_date,
                        base::Time last_background_fetch_time,
                        float score);

[Marc Treib, 2017/01/16 10:45:42]

Hm. I'm wondering whether we should pack most of these params into a common
struct (even if some of them would be unused in some cases).
As it is, it's super easy to mess up some parameter, since these all take some
subset of (global_position, category_rank, position_in_category), all of which
are ints.

Not for this CL, but WDYT about this in general?

[tschumann, 2017/01/16 11:02:22]

Seems like a good idea. However, I'd be in favor of identifying a set of
required properties and enforce they are always set -- otherwise, we get a
pretty loose contract and it gets really hard to say which field is set where.

[Marc Treib, 2017/01/16 11:18:54]

Oh, absolutely! I was thinking global_position, category, category_rank,
position_in_category, publish_date, score. The contract would be that all those
must always be set (even if not all of the methods actually make use of all of
them (yet)).

[vitaliii, 2017/01/18 09:19:20]

I agree, I added a TODO and crbug.com/682160.

 
 void OnSuggestionOpened(int global_position,
                         Category category,
-                        int category_position,
+                        int category_rank,
+                        int position_in_category,
                         base::Time publish_date,
                         float score,
                         WindowOpenDisposition disposition);
 
 void OnSuggestionMenuOpened(int global_position,
                             Category category,
-                            int category_position,
+                            int position_in_category,
                             base::Time publish_date,
                             float score);
 
 void OnSuggestionDismissed(int global_position,
                            Category category,
-                           int category_position,
+                           int position_in_category,
                            bool visited);
 
 void OnSuggestionTargetVisited(Category category, base::TimeDelta visit_time);
 
 // Should only be called once per NTP for each "more" button.
 void OnMoreButtonShown(Category category, int position);
 
 void OnMoreButtonClicked(Category category, int position);
 
 void OnCategoryDismissed(Category category);
 
 }  // namespace metrics
 }  // namespace ntp_snippets
 
 #endif  // COMPONENTS_NTP_SNIPPETS_CONTENT_SUGGESTIONS_SERVICE_H_