[NTP] Fix article suggestion clicks contributing to Most Visited tiles

chrome/android/java/src/org/chromium/chrome/browser/ntp/NewTabPage.java

Index: chrome/android/java/src/org/chromium/chrome/browser/ntp/NewTabPage.java
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/ntp/NewTabPage.java b/chrome/android/java/src/org/chromium/chrome/browser/ntp/NewTabPage.java
index ff1a75b8513ee3d3802bcc1f495e1c046197cf08..bf90f532037aa3966ddbf04947163e99b91562c2 100644
--- a/chrome/android/java/src/org/chromium/chrome/browser/ntp/NewTabPage.java
+++ b/chrome/android/java/src/org/chromium/chrome/browser/ntp/NewTabPage.java
@@ -45,6 +45,7 @@ import org.chromium.chrome.browser.multiwindow.MultiWindowUtils;
 import org.chromium.chrome.browser.ntp.LogoBridge.Logo;
 import org.chromium.chrome.browser.ntp.LogoBridge.LogoObserver;
 import org.chromium.chrome.browser.ntp.NewTabPageView.NewTabPageManager;
+import org.chromium.chrome.browser.ntp.snippets.KnownCategories;
 import org.chromium.chrome.browser.ntp.snippets.SnippetArticle;
 import org.chromium.chrome.browser.ntp.snippets.SnippetsBridge;
 import org.chromium.chrome.browser.ntp.snippets.SnippetsConfig;
@@ -68,6 +69,7 @@ import org.chromium.chrome.browser.tabmodel.TabModelUtils;
 import org.chromium.chrome.browser.tabmodel.document.TabDelegate;
 import org.chromium.chrome.browser.util.UrlUtilities;
 import org.chromium.content_public.browser.LoadUrlParams;
+import org.chromium.content_public.common.Referrer;
 import org.chromium.net.NetworkChangeNotifier;
 import org.chromium.ui.WindowOpenDisposition;
 import org.chromium.ui.base.DeviceFormFactor;
@@ -239,7 +241,7 @@ public class NewTabPage
             recordOpenedMostVisitedItem(item);
             String url = item.getUrl();
             if (!switchToExistingTab(url)) {
-                openUrl(WindowOpenDisposition.CURRENT_TAB, url);
+                openUrlMostVisited(WindowOpenDisposition.CURRENT_TAB, url);
             }
         }
 
@@ -248,7 +250,9 @@ public class NewTabPage
             if (mIsDestroyed) return;
             NewTabPageUma.recordAction(NewTabPageUma.ACTION_CLICKED_LEARN_MORE);
             String url = "https://support.google.com/chrome/?p=new_tab";
-            openUrl(WindowOpenDisposition.CURRENT_TAB, url);
+            // TODO(mastiz): Change this to LINK?
+            openUrl(WindowOpenDisposition.CURRENT_TAB,
+                    new LoadUrlParams(url, PageTransition.AUTO_BOOKMARK));
         }
 
         @TargetApi(Build.VERSION_CODES.LOLLIPOP)
@@ -312,26 +316,42 @@ public class NewTabPage
                     article.mPosition, article.mPublishTimestampMilliseconds, article.mScore,
                     windowOpenDisposition);
             NewTabPageUma.monitorContentSuggestionVisit(mTab, article.mCategory);
-            openUrl(windowOpenDisposition, article.mUrl);
+            LoadUrlParams loadUrlParams =
+                    new LoadUrlParams(article.mUrl, PageTransition.AUTO_BOOKMARK);
+
+            // For article suggestions, we set the referrer. This is exploited
+            // to filter out these history entries for NTP tiles.
+            if (article.mCategory == KnownCategories.ARTICLES) {

[Marc Treib, 2016/09/15 14:31:22]

In the longer term, we need to decide for which categories we want to set this -
at the very least, we should set it for all remote categories. But for now this
is okay.

[mastiz, 2016/09/15 14:37:56]

Acknowledged. I can a switch statement if you want.

[Marc Treib, 2016/09/15 14:50:55]

It can't be a switch, because remote categories aren't necessarily known.
Maybe just add a TODO for now?

[mastiz, 2016/09/15 15:06:16]

Done.

+                loadUrlParams.setReferrer(
+                        new Referrer("https://www.googleapis.com/auth/chrome-content-suggestions",

[Bernhard Bauer, 2016/09/15 15:10:52]

Can you make this a constant?

[Marc Treib, 2016/09/15 15:19:21]

Ah, actually one more nit: This is the scope for OAuth tokens; it's not the URL
for the endpoint itself. Is that on purpose? Maybe the actual endpoint
(https://chromecontentsuggestions-pa.googleapis.com/v1/suggestions/fetch) would
be slightly less of a lie? :D

[mastiz, 2016/09/15 15:31:57]

Acknowledged. Although you're right about the usefulness of curious users, as
discussed offline, this URL has been chosen because it's expected to be very
stable.

[mastiz, 2016/09/15 15:31:57]

Done.

+                                Referrer.REFERRER_POLICY_ALWAYS));
+            }
+
+            openUrl(windowOpenDisposition, loadUrlParams);
+        }
+
+        // TODO(mastiz): Merge with openMostVisitedItem().
+        private void openUrlMostVisited(int windowOpenDisposition, String url) {
+            openUrl(windowOpenDisposition, new LoadUrlParams(url, PageTransition.AUTO_BOOKMARK));
         }
 
-        private void openUrl(int windowOpenDisposition, String url) {
+        private void openUrl(int windowOpenDisposition, LoadUrlParams loadUrlParams) {
             assert !mIsDestroyed;
             switch (windowOpenDisposition) {
                 case WindowOpenDisposition.CURRENT_TAB:
-                    mTab.loadUrl(new LoadUrlParams(url, PageTransition.AUTO_BOOKMARK));
+                    mTab.loadUrl(loadUrlParams);
                     break;
                 case WindowOpenDisposition.NEW_FOREGROUND_TAB:
-                    openUrlInNewTab(url, false);
+                    openUrlInNewTab(loadUrlParams, false);
                     break;
                 case WindowOpenDisposition.OFF_THE_RECORD:
-                    openUrlInNewTab(url, true);
+                    openUrlInNewTab(loadUrlParams, true);
                     break;
                 case WindowOpenDisposition.NEW_WINDOW:
-                    openUrlInNewWindow(url);
+                    openUrlInNewWindow(loadUrlParams);
                     break;
                 case WindowOpenDisposition.SAVE_TO_DISK:
-                    saveUrlForOffline(url);
+                    saveUrlForOffline(loadUrlParams.getUrl());
                     break;
                 default:
                     assert false;
@@ -363,15 +383,15 @@ public class NewTabPage
             switch (menuId) {
                 case ID_OPEN_IN_NEW_WINDOW:
                     // TODO(treib): Should we call recordOpenedMostVisitedItem here?
-                    openUrl(WindowOpenDisposition.NEW_WINDOW, item.getUrl());
+                    openUrlMostVisited(WindowOpenDisposition.NEW_WINDOW, item.getUrl());
                     return true;
                 case ID_OPEN_IN_NEW_TAB:
                     recordOpenedMostVisitedItem(item);
-                    openUrl(WindowOpenDisposition.NEW_FOREGROUND_TAB, item.getUrl());
+                    openUrlMostVisited(WindowOpenDisposition.NEW_FOREGROUND_TAB, item.getUrl());
                     return true;
                 case ID_OPEN_IN_INCOGNITO_TAB:
                     recordOpenedMostVisitedItem(item);
-                    openUrl(WindowOpenDisposition.OFF_THE_RECORD, item.getUrl());
+                    openUrlMostVisited(WindowOpenDisposition.OFF_THE_RECORD, item.getUrl());
                     return true;
                 case ID_REMOVE:
                     mMostVisitedSites.addBlacklistedUrl(item.getUrl());
@@ -392,16 +412,14 @@ public class NewTabPage
             return PrefServiceBridge.getInstance().isIncognitoModeEnabled();
         }
 
-        private void openUrlInNewWindow(String url) {
+        private void openUrlInNewWindow(LoadUrlParams loadUrlParams) {
             TabDelegate tabDelegate = new TabDelegate(false);
-            // TODO(treib): Should this use PageTransition.AUTO_BOOKMARK?
-            LoadUrlParams loadUrlParams = new LoadUrlParams(url);
             tabDelegate.createTabInOtherWindow(loadUrlParams, mActivity, mTab.getParentId());
         }
 
-        private void openUrlInNewTab(String url, boolean incognito) {
-            mTabModelSelector.openNewTab(new LoadUrlParams(url, PageTransition.AUTO_BOOKMARK),
-                    TabLaunchType.FROM_LONGPRESS_BACKGROUND, mTab, incognito);
+        private void openUrlInNewTab(LoadUrlParams loadUrlParams, boolean incognito) {
+            mTabModelSelector.openNewTab(
+                    loadUrlParams, TabLaunchType.FROM_LONGPRESS_BACKGROUND, mTab, incognito);
         }
 
         private void saveUrlForOffline(String url) {