📰 Tweak the suggestion ranks for UMA to handle fetchMore

chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/SuggestionsSection.java

 // 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.
 
 package org.chromium.chrome.browser.ntp.cards;
 
 import org.chromium.base.Callback;
 import org.chromium.base.Log;
 import org.chromium.base.VisibleForTesting;
 import org.chromium.chrome.browser.ntp.NewTabPage.DestructionObserver;
 import org.chromium.chrome.browser.ntp.NewTabPageView.NewTabPageManager;
+import org.chromium.chrome.browser.ntp.cards.SectionList.SuggestionRanker;
 import org.chromium.chrome.browser.ntp.snippets.CategoryInt;
 import org.chromium.chrome.browser.ntp.snippets.CategoryStatus.CategoryStatusEnum;
 import org.chromium.chrome.browser.ntp.snippets.SectionHeader;
 import org.chromium.chrome.browser.ntp.snippets.SnippetArticle;
 import org.chromium.chrome.browser.ntp.snippets.SnippetArticleViewHolder;
 import org.chromium.chrome.browser.ntp.snippets.SnippetsBridge;
 import org.chromium.chrome.browser.ntp.snippets.SuggestionsSource;
 import org.chromium.chrome.browser.offlinepages.ClientId;
 import org.chromium.chrome.browser.offlinepages.OfflinePageBridge;
 import org.chromium.chrome.browser.offlinepages.OfflinePageItem;
 
 import java.util.ArrayList;
 import java.util.Iterator;
 import java.util.List;
 
 /**
  * A group of suggestions, with a header, a status card, and a progress indicator. This is
  * responsible for tracking whether its suggestions have been saved offline.
  */
 public class SuggestionsSection extends InnerNode {
     private static final String TAG = "NtpCards";
 
     private final Delegate mDelegate;
     private final SuggestionsCategoryInfo mCategoryInfo;
     private final OfflinePageBridge mOfflinePageBridge;
+    private final SuggestionRanker mSuggestionRanker;
 
     // Children
     private final SectionHeader mHeader;
     private final SuggestionsList mSuggestionsList;
     private final StatusItem mStatus;
     private final ActionItem mMoreButton;
     private final ProgressItem mProgressIndicator;
 
     private boolean mIsNtpDestroyed;
 
     // Keep track of impressions of the suggestions so that we replace only suggestions that
     // have not been impressed, yet. We keep track of the first suggestion separately as the
     // first is often impressed in the form of a peeking card and we still want to be able to
     // replace something in this case.
     private boolean mFirstSuggestionSeen;
     private boolean mSubsequentSuggestionsSeen;
 
     /**
      * Delegate interface that allows dismissing this section without introducing
      * a circular dependency.
      */
     public interface Delegate {
         void dismissSection(SuggestionsSection section);
     }
 
-    public SuggestionsSection(Delegate delegate, NewTabPageManager manager,
+    public SuggestionsSection(Delegate delegate, NewTabPageManager manager, SuggestionRanker ranker,
             OfflinePageBridge offlinePageBridge, SuggestionsCategoryInfo info) {
         mDelegate = delegate;
         mCategoryInfo = info;
         mOfflinePageBridge = offlinePageBridge;
+        mSuggestionRanker = ranker;
 
         mHeader = new SectionHeader(info.getTitle());
         mSuggestionsList = new SuggestionsList(manager, info);
         mStatus = StatusItem.createNoSuggestionsItem(info);
         mMoreButton = new ActionItem(this);
         mProgressIndicator = new ProgressItem();
         addChildren(mHeader, mSuggestionsList, mStatus, mMoreButton, mProgressIndicator);
 
         setupOfflinePageBridgeObserver(manager);
         refreshChildrenVisibility();
@@ skipping 164 matching lines @@
     }
 
     @Override
     public void onItemRangeRemoved(TreeNode child, int index, int count) {
         super.onItemRangeRemoved(child, index, count);
         if (child == mSuggestionsList) refreshChildrenVisibility();
     }
 
     @Override
     public void onBindViewHolder(NewTabPageViewHolder holder, int position, List<Object> payloads) {
+        @ItemViewType
+        int viewType = getItemViewType(position);
+        if (viewType == ItemViewType.SNIPPET) {
+            childSeen(position);
+        } else if (viewType == ItemViewType.ACTION) {
+            mSuggestionRanker.rankItem((ActionItem) getChildForPosition(position), this);

[Bernhard Bauer, 2017/01/17 17:42:31]

Could we have the ActionItem itself call this?

[dgn, 2017/01/17 18:46:24]

Done.

+        }
         super.onBindViewHolder(holder, position, payloads);
-        childSeen(position);
     }
 
     /**
      * Sets the child at position {@code position} as being seen by the user.
      * @param position Position in the list being shown (the first suggestion being at index 1,
      * as at index 0, there is a non-suggestion).
      */
     @VisibleForTesting
     public void childSeen(int position) {

[Bernhard Bauer, 2017/01/17 17:42:31]

If this is only called for suggestions now, rename it accordingly?

[dgn, 2017/01/17 18:46:24]

Reverted since actionItem calls its ranker itself.

         Log.d(TAG, "childSeen: position %d in category %d", position, mCategoryInfo.getCategory());
         assert getStartingOffsetForChild(mSuggestionsList) == 1;
-        if (getItemViewType(position) == ItemViewType.SNIPPET) {
-            // We assume all non-snippet cards come after all cards of type SNIPPET.
-            int positionAmongSuggestions = position - 1;
-            if (positionAmongSuggestions == 0) {
-                mFirstSuggestionSeen = true;
-            } else if (positionAmongSuggestions > 0) {
-                mSubsequentSuggestionsSeen = true;
-            }
+        assert getItemViewType(position) == ItemViewType.SNIPPET;
+
+        // We assume all non-snippet cards come after all cards of type SNIPPET.
+        int positionAmongSuggestions = position - 1;
+        if (positionAmongSuggestions == 0) {
+            mFirstSuggestionSeen = true;
+        } else if (positionAmongSuggestions > 0) {
+            mSubsequentSuggestionsSeen = true;
         }
     }
 
     /**
      * Removes a suggestion. Does nothing if the ID is unknown.
      * @param idWithinCategory The ID of the suggestion to remove.
      */
     public void removeSuggestionById(String idWithinCategory) {
         int i = 0;
         for (SnippetArticle suggestion : mSuggestionsList) {
@@ skipping 60 matching lines @@
                 Log.d(TAG, "setSuggestions: removing all suggestions");
                 mSuggestionsList.clear();
             }
         }
 
         mProgressIndicator.setVisible(SnippetsBridge.isCategoryLoading(status));
 
         mSuggestionsList.addAll(suggestions);
 
         for (SnippetArticle article : suggestions) {
+            mSuggestionRanker.rankItem(article);
             if (!article.requiresExactOfflinePage()) {
                 updateSnippetOfflineAvailability(article);
             }
         }
 
         refreshChildrenVisibility();
     }
 
     private void updateSnippetOfflineAvailability(final SnippetArticle article) {
         // This method is not applicable to articles for which the exact offline id must specified.
@@ skipping 79 matching lines @@
     @VisibleForTesting
     ActionItem getActionItem() {
         return mMoreButton;
     }
 
     @VisibleForTesting
     StatusItem getStatusItem() {
         return mStatus;
     }
 }