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

There's a need to distinguish clicks on different elements on the NTP:
a) clicks on Most Visited tiles.
b) clicks on (newly introduced) article suggestions (aka snippets).

The first should contribute to Most Visited tiles (i.e. boost tiles
that have been clicked in the past). The second shouldn't.

We choose to achieve this by specifying a referrer for article
suggestion clicks. This exposes the referrer to third parties, which
has been discussed and considered a desirable feature.

The fix relies on such a workaround due to the current lack of
infrastructure to propagate opaque feature-specific data from upper
layers to navigation history (and sync).

The approach competes with more intrusive/controversial alternatives to
achieve the same:

1. Use page transition types (LINK vs AUTO_BOOKMARK) to distinguish
   tile clicks from article suggestion clicks: unfortunately both types
   have been used in the past (older versions of Chrome).

2. Introducing a new page transition type or qualifier: this can be
   considered a layering violation.

3. Introduce a dummy redirect by means of a data: schema page.

BUG=645895

Review-Url: https://codereview.chromium.org/2338133006
Cr-Commit-Position: refs/heads/master@{#419242}
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 ff1a75b..f5b0d27 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.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.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;
@@ -101,6 +103,9 @@
     private static final int CTA_IMAGE_CLICKED = 1;
     private static final int ANIMATED_LOGO_CLICKED = 2;
 
+    private static final String CHROME_CONTENT_SUGGESTIONS_REFERRER =
+            "https://www.googleapis.com/auth/chrome-content-suggestions";
+
     private static MostVisitedSites sMostVisitedSitesForTests;
 
     private final Tab mTab;
@@ -239,7 +244,7 @@
             recordOpenedMostVisitedItem(item);
             String url = item.getUrl();
             if (!switchToExistingTab(url)) {
-                openUrl(WindowOpenDisposition.CURRENT_TAB, url);
+                openUrlMostVisited(WindowOpenDisposition.CURRENT_TAB, url);
             }
         }
 
@@ -248,7 +253,9 @@
             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 +319,42 @@
                     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.
+            // TODO(mastiz): Extend this with support for other categories.
+            if (article.mCategory == KnownCategories.ARTICLES) {
+                loadUrlParams.setReferrer(new Referrer(
+                        CHROME_CONTENT_SUGGESTIONS_REFERRER, 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;
@@ -363,15 +386,15 @@
             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 +415,14 @@
             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) {
diff --git a/chrome/browser/history/history_tab_helper.cc b/chrome/browser/history/history_tab_helper.cc
index c633e3c..9e19218 100644
--- a/chrome/browser/history/history_tab_helper.cc
+++ b/chrome/browser/history/history_tab_helper.cc
@@ -30,6 +30,14 @@
 
 DEFINE_WEB_CONTENTS_USER_DATA_KEY(HistoryTabHelper);
 
+namespace {
+
+// Referrer used for clicks on article suggestions on the NTP.
+const char kChromeContentSuggestionsReferrer[] =
+    "https://www.googleapis.com/auth/chrome-content-suggestions";
+
+}  // namespace
+
 HistoryTabHelper::HistoryTabHelper(WebContents* web_contents)
     : content::WebContentsObserver(web_contents),
       received_page_title_(false) {
@@ -58,10 +66,16 @@
     bool did_replace_entry,
     int nav_entry_id,
     const content::FrameNavigateParams& params) {
+  // Clicks on content suggestions on the NTP should not contribute to the
+  // Most Visited tiles in the NTP.
+  const bool consider_for_ntp_most_visited =
+      params.referrer.url != GURL(kChromeContentSuggestionsReferrer);
+
   history::HistoryAddPageArgs add_page_args(
       params.url, timestamp, history::ContextIDForWebContents(web_contents()),
       nav_entry_id, params.referrer.url, params.redirects, params.transition,
-      history::SOURCE_BROWSED, did_replace_entry);
+      history::SOURCE_BROWSED, did_replace_entry,
+      consider_for_ntp_most_visited);
   if (ui::PageTransitionIsMainFrame(params.transition) &&
       virtual_url != params.url) {
     // Hack on the "virtual" URL so that it will appear in history. For some
diff --git a/chrome/browser/supervised_user/supervised_user_navigation_observer.cc b/chrome/browser/supervised_user/supervised_user_navigation_observer.cc
index e354bbb..8211965 100644
--- a/chrome/browser/supervised_user/supervised_user_navigation_observer.cc
+++ b/chrome/browser/supervised_user/supervised_user_navigation_observer.cc
@@ -68,7 +68,7 @@
   history::HistoryAddPageArgs add_page_args(
       url, timestamp, history::ContextIDForWebContents(web_contents_), 0, url,
       history::RedirectList(), ui::PAGE_TRANSITION_BLOCKED,
-      history::SOURCE_BROWSED, false);
+      history::SOURCE_BROWSED, false, true);
 
   // Add the entry to the history database.
   Profile* profile =
diff --git a/chrome/browser/ui/webui/web_dialog_web_contents_delegate_unittest.cc b/chrome/browser/ui/webui/web_dialog_web_contents_delegate_unittest.cc
index e419d3a..df6506a8 100644
--- a/chrome/browser/ui/webui/web_dialog_web_contents_delegate_unittest.cc
+++ b/chrome/browser/ui/webui/web_dialog_web_contents_delegate_unittest.cc
@@ -66,7 +66,7 @@
   EXPECT_TRUE(test_web_contents_delegate_->IsPopupOrPanel(NULL));
   history::HistoryAddPageArgs should_add_args(
       GURL(), base::Time::Now(), 0, 0, GURL(), history::RedirectList(),
-      ui::PAGE_TRANSITION_TYPED, history::SOURCE_SYNCED, false);
+      ui::PAGE_TRANSITION_TYPED, history::SOURCE_SYNCED, false, true);
   test_web_contents_delegate_->NavigationStateChanged(
       NULL, content::InvalidateTypes(0));
   test_web_contents_delegate_->ActivateContents(NULL);
diff --git a/components/history/core/browser/history_backend.cc b/components/history/core/browser/history_backend.cc
index d92049f..0b3f6cd 100644
--- a/components/history/core/browser/history_backend.cc
+++ b/components/history/core/browser/history_backend.cc
@@ -522,7 +522,7 @@
     // Update the segment for this visit. KEYWORD_GENERATED visits should not
     // result in changing most visited, so we don't update segments (most
     // visited db).
-    if (!is_keyword_generated) {
+    if (!is_keyword_generated && request.consider_for_ntp_most_visited) {
       UpdateSegments(request.url, from_visit_id, last_ids.second, t,
                      request.time);
 
@@ -592,9 +592,10 @@
       last_ids = AddPageVisit(redirects[redirect_index], request.time,
                               last_ids.second, t, request.visit_source);
       if (t & ui::PAGE_TRANSITION_CHAIN_START) {
-        // Update the segment for this visit.
-        UpdateSegments(redirects[redirect_index], from_visit_id,
-                       last_ids.second, t, request.time);
+        if (request.consider_for_ntp_most_visited) {
+          UpdateSegments(redirects[redirect_index], from_visit_id,
+                         last_ids.second, t, request.time);
+        }
 
         // Update the visit_details for this visit.
         UpdateVisitDuration(from_visit_id, request.time);
diff --git a/components/history/core/browser/history_backend_unittest.cc b/components/history/core/browser/history_backend_unittest.cc
index 8f4b22f..b89d0a7 100644
--- a/components/history/core/browser/history_backend_unittest.cc
+++ b/components/history/core/browser/history_backend_unittest.cc
@@ -357,7 +357,7 @@
     history::HistoryAddPageArgs request(
         redirects.back(), time, context_id, nav_entry_id, GURL(),
         redirects, transition, history::SOURCE_BROWSED,
-        true);
+        true, true);
     backend_->AddPage(request);
   }
 
@@ -383,7 +383,7 @@
     HistoryAddPageArgs request(
         url2, time, dummy_context_id, 0, url1,
         redirects, ui::PAGE_TRANSITION_CLIENT_REDIRECT,
-        history::SOURCE_BROWSED, did_replace);
+        history::SOURCE_BROWSED, did_replace, true);
     backend_->AddPage(request);
 
     *transition1 = GetTransition(url1);
@@ -779,7 +779,7 @@
   HistoryAddPageArgs request(url, visit_time, NULL, 0, GURL(),
                              history::RedirectList(),
                              ui::PAGE_TRANSITION_KEYWORD_GENERATED,
-                             history::SOURCE_BROWSED, false);
+                             history::SOURCE_BROWSED, false, true);
   backend_->AddPage(request);
 
   // Check that a row was added.
@@ -921,7 +921,7 @@
   HistoryAddPageArgs request(url, visit_time, NULL, 0, GURL(),
                              history::RedirectList(),
                              ui::PAGE_TRANSITION_KEYWORD_GENERATED,
-                             history::SOURCE_BROWSED, false);
+                             history::SOURCE_BROWSED, false, true);
   backend_->AddPage(request);
 
   // A row should have been added for the url.
@@ -953,7 +953,7 @@
       ui::PAGE_TRANSITION_TYPED | ui::PAGE_TRANSITION_FORWARD_BACK);
   HistoryAddPageArgs back_request(url, visit_time, NULL, 0, GURL(),
                                   history::RedirectList(), back_transition,
-                                  history::SOURCE_BROWSED, false);
+                                  history::SOURCE_BROWSED, false, true);
   backend_->AddPage(back_request);
   url_id = backend_->db()->GetRowForURL(url, &row);
   ASSERT_NE(0, url_id);
@@ -1482,19 +1482,19 @@
   HistoryAddPageArgs request1(url, base::Time::Now(), NULL, 0, GURL(),
                              history::RedirectList(),
                              ui::PAGE_TRANSITION_KEYWORD_GENERATED,
-                             history::SOURCE_BROWSED, false);
+                             history::SOURCE_BROWSED, false, true);
   backend_->AddPage(request1);
   // Assume this page is synced.
   HistoryAddPageArgs request2(url, base::Time::Now(), NULL, 0, GURL(),
                              history::RedirectList(),
                              ui::PAGE_TRANSITION_LINK,
-                             history::SOURCE_SYNCED, false);
+                             history::SOURCE_SYNCED, false, true);
   backend_->AddPage(request2);
   // Assume this page is browsed again.
   HistoryAddPageArgs request3(url, base::Time::Now(), NULL, 0, GURL(),
                              history::RedirectList(),
                              ui::PAGE_TRANSITION_TYPED,
-                             history::SOURCE_BROWSED, false);
+                             history::SOURCE_BROWSED, false, true);
   backend_->AddPage(request3);
 
   // Three visits should be added with proper sources.
@@ -3806,4 +3806,34 @@
   EXPECT_FALSE(mem_backend_->db()->GetKeywordSearchTermRow(row2.id(), NULL));
 }
 
+TEST_F(HistoryBackendTest, QueryMostVisitedURLs) {
+  ASSERT_TRUE(backend_.get());
+
+  // Pairs from page transitions to consider_for_ntp_most_visited.
+  std::vector<std::pair<ui::PageTransition, bool>> pages;
+  pages.emplace_back(ui::PAGE_TRANSITION_AUTO_BOOKMARK, true);   // good.
+  pages.emplace_back(ui::PAGE_TRANSITION_AUTO_BOOKMARK, false);  // bad.
+  pages.emplace_back(ui::PAGE_TRANSITION_LINK, true);            // bad.
+  pages.emplace_back(ui::PAGE_TRANSITION_TYPED, false);          // bad.
+  pages.emplace_back(ui::PAGE_TRANSITION_TYPED, true);           // good.
+
+  for (size_t i = 0; i < pages.size(); ++i) {
+    HistoryAddPageArgs args;
+    args.url = GURL("http://example" + base::SizeTToString(i + 1) + ".com");
+    args.time = base::Time::Now() - base::TimeDelta::FromDays(i + 1);
+    args.transition = pages[i].first;
+    args.consider_for_ntp_most_visited = pages[i].second;
+    backend_->AddPage(args);
+  }
+
+  MostVisitedURLList most_visited;
+  backend_->QueryMostVisitedURLs(100, 100, &most_visited);
+
+  const base::string16 kSomeTitle;  // Ignored by equality operator.
+  EXPECT_THAT(
+      most_visited,
+      ElementsAre(MostVisitedURL(GURL("http://example1.com"), kSomeTitle),
+                  MostVisitedURL(GURL("http://example5.com"), kSomeTitle)));
+}
+
 }  // namespace history
diff --git a/components/history/core/browser/history_service.cc b/components/history/core/browser/history_service.cc
index a7e47e12..5e62f47 100644
--- a/components/history/core/browser/history_service.cc
+++ b/components/history/core/browser/history_service.cc
@@ -378,7 +378,7 @@
   DCHECK(thread_checker_.CalledOnValidThread());
   AddPage(HistoryAddPageArgs(url, time, context_id, nav_entry_id, referrer,
                              redirects, transition, visit_source,
-                             did_replace_entry));
+                             did_replace_entry, true));
 }
 
 void HistoryService::AddPage(const GURL& url,
@@ -386,7 +386,8 @@
                              VisitSource visit_source) {
   DCHECK(thread_checker_.CalledOnValidThread());
   AddPage(HistoryAddPageArgs(url, time, nullptr, 0, GURL(), RedirectList(),
-                             ui::PAGE_TRANSITION_LINK, visit_source, false));
+                             ui::PAGE_TRANSITION_LINK, visit_source, false,
+                             true));
 }
 
 void HistoryService::AddPage(const HistoryAddPageArgs& add_page_args) {
diff --git a/components/history/core/browser/history_types.cc b/components/history/core/browser/history_types.cc
index 5d9ea3c8..c6d3edac 100644
--- a/components/history/core/browser/history_types.cc
+++ b/components/history/core/browser/history_types.cc
@@ -254,7 +254,8 @@
                          RedirectList(),
                          ui::PAGE_TRANSITION_LINK,
                          SOURCE_BROWSED,
-                         false) {
+                         false,
+                         true) {
 }
 
 HistoryAddPageArgs::HistoryAddPageArgs(const GURL& url,
@@ -265,7 +266,8 @@
                                        const RedirectList& redirects,
                                        ui::PageTransition transition,
                                        VisitSource source,
-                                       bool did_replace_entry)
+                                       bool did_replace_entry,
+                                       bool consider_for_ntp_most_visited)
     : url(url),
       time(time),
       context_id(context_id),
@@ -274,7 +276,8 @@
       redirects(redirects),
       transition(transition),
       visit_source(source),
-      did_replace_entry(did_replace_entry) {
+      did_replace_entry(did_replace_entry),
+      consider_for_ntp_most_visited(consider_for_ntp_most_visited) {
 }
 
 HistoryAddPageArgs::HistoryAddPageArgs(const HistoryAddPageArgs& other) =
diff --git a/components/history/core/browser/history_types.h b/components/history/core/browser/history_types.h
index 9795ded3..64f34c30 100644
--- a/components/history/core/browser/history_types.h
+++ b/components/history/core/browser/history_types.h
@@ -332,7 +332,7 @@
 
   RedirectList redirects;
 
-  bool operator==(const MostVisitedURL& other) {
+  bool operator==(const MostVisitedURL& other) const {
     return url == other.url;
   }
 };
@@ -372,7 +372,7 @@
   //   HistoryAddPageArgs(
   //       GURL(), base::Time(), NULL, 0, GURL(),
   //       RedirectList(), ui::PAGE_TRANSITION_LINK,
-  //       SOURCE_BROWSED, false)
+  //       SOURCE_BROWSED, false, true)
   HistoryAddPageArgs();
   HistoryAddPageArgs(const GURL& url,
                      base::Time time,
@@ -382,7 +382,8 @@
                      const RedirectList& redirects,
                      ui::PageTransition transition,
                      VisitSource source,
-                     bool did_replace_entry);
+                     bool did_replace_entry,
+                     bool consider_for_ntp_most_visited);
   HistoryAddPageArgs(const HistoryAddPageArgs& other);
   ~HistoryAddPageArgs();
 
@@ -395,6 +396,11 @@
   ui::PageTransition transition;
   VisitSource visit_source;
   bool did_replace_entry;
+  // Specifies whether a page visit should contribute to the Most Visited tiles
+  // in the New Tab Page. Note that setting this to true (most common case)
+  // doesn't guarantee it's relevant for Most Visited, since other requirements
+  // exist (e.g. certain page transition types).
+  bool consider_for_ntp_most_visited;
 };
 
 // TopSites -------------------------------------------------------------------