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

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

 // Copyright 2015 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;
 
 import android.annotation.TargetApi;
 import android.content.Context;
 import android.graphics.Canvas;
 import android.graphics.Point;
@@ skipping 27 matching lines @@
 import org.chromium.chrome.browser.favicon.FaviconHelper;
 import org.chromium.chrome.browser.favicon.FaviconHelper.FaviconImageCallback;
 import org.chromium.chrome.browser.favicon.FaviconHelper.IconAvailabilityCallback;
 import org.chromium.chrome.browser.favicon.LargeIconBridge;
 import org.chromium.chrome.browser.favicon.LargeIconBridge.LargeIconCallback;
 import org.chromium.chrome.browser.metrics.StartupMetrics;
 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;
 import org.chromium.chrome.browser.offlinepages.OfflinePageBridge;
 import org.chromium.chrome.browser.preferences.PrefServiceBridge;
 import org.chromium.chrome.browser.profiles.MostVisitedSites;
 import org.chromium.chrome.browser.profiles.MostVisitedSites.MostVisitedURLsObserver;
 import org.chromium.chrome.browser.profiles.Profile;
 import org.chromium.chrome.browser.search_engines.TemplateUrlService;
 import org.chromium.chrome.browser.search_engines.TemplateUrlService.TemplateUrlServiceObserver;
 import org.chromium.chrome.browser.snackbar.Snackbar;
 import org.chromium.chrome.browser.snackbar.SnackbarManager.SnackbarController;
 import org.chromium.chrome.browser.sync.SyncSessionsMetrics;
 import org.chromium.chrome.browser.tab.EmptyTabObserver;
 import org.chromium.chrome.browser.tab.Tab;
 import org.chromium.chrome.browser.tab.TabObserver;
 import org.chromium.chrome.browser.tabmodel.TabModel;
 import org.chromium.chrome.browser.tabmodel.TabModel.TabLaunchType;
 import org.chromium.chrome.browser.tabmodel.TabModelSelector;
 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;
 import org.chromium.ui.base.PageTransition;
 
 import java.util.HashSet;
 import java.util.Set;
 import java.util.concurrent.TimeUnit;
 
 import jp.tomorrowkey.android.gifplayer.BaseGifImage;
@@ skipping 151 matching lines @@
             mMostVisitedSites.recordOpenedMostVisitedItem(
                     item.getIndex(), item.getTileType(), item.getSource());
         }
 
         @Override
         public void openMostVisitedItem(MostVisitedItem item) {
             if (mIsDestroyed) return;
             recordOpenedMostVisitedItem(item);
             String url = item.getUrl();
             if (!switchToExistingTab(url)) {
-                openUrl(WindowOpenDisposition.CURRENT_TAB, url);
+                openUrlMostVisited(WindowOpenDisposition.CURRENT_TAB, url);
             }
         }
 
         @Override
         public void onLearnMoreClicked() {
             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)
         private boolean switchToExistingTab(String url) {
             String matchPattern = CommandLine.getInstance().getSwitchValue(
                     ChromeSwitches.NTP_SWITCH_TO_EXISTING_TAB);
             boolean matchByHost;
             if ("url".equals(matchPattern)) {
                 matchByHost = false;
             } else if ("host".equals(matchPattern)) {
@@ skipping 43 matching lines @@
         public void trackSnippetCategoryActionClick(int category, int position) {
             mSnippetsBridge.onMoreButtonClicked(category, position);
         }
 
         @Override
         public void openSnippet(int windowOpenDisposition, SnippetArticle article) {
             mSnippetsBridge.onSuggestionOpened(article.mGlobalPosition, article.mCategory,
                     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);
         }
 
-        private void openUrl(int windowOpenDisposition, String url) {
+        // 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, 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;
             }
         }
 
         @Override
         public void onCreateContextMenu(ContextMenu menu, OnMenuItemClickListener listener) {
             if (mIsDestroyed) return;
             if (isOpenInNewWindowEnabled()) {
@@ skipping 11 matching lines @@
             menu.add(Menu.NONE, ID_REMOVE, Menu.NONE, R.string.remove)
                     .setOnMenuItemClickListener(listener);
         }
 
         @Override
         public boolean onMenuItemClick(int menuId, MostVisitedItem item) {
             if (mIsDestroyed) return false;
             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());
                     showMostVisitedItemRemovedSnackbar(item.getUrl());
                     return true;
                 default:
                     return false;
             }
         }
 
         @Override
         public boolean isOpenInNewWindowEnabled() {
             return MultiWindowUtils.getInstance().isOpenInOtherWindowSupported(mActivity);
         }
 
         @Override
         public boolean isOpenInIncognitoEnabled() {
             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) {
             OfflinePageBridge.getForProfile(mProfile)
                     .savePageLater(url, "ntp_suggestions", true /* userRequested */);
         }
 
         @Override
         public void navigateToBookmarks() {
             if (mIsDestroyed) return;
@@ skipping 499 matching lines @@
     @Override
     public boolean shouldCaptureThumbnail() {
         return mNewTabPageView.shouldCaptureThumbnail();
     }
 
     @Override
     public void captureThumbnail(Canvas canvas) {
         mNewTabPageView.captureThumbnail(canvas);
     }
 }