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

components/history/core/browser/history_backend.cc

 // Copyright (c) 2012 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.
 
 #include "components/history/core/browser/history_backend.h"
 
 #include <algorithm>
 #include <functional>
 #include <list>
 #include <map>
@@ skipping 53 matching lines @@
       VisitDatabase (stores a list of visits for the URLs)
       VisitSegmentDatabase (stores groups of URLs for the most visited view).
 
     ExpireHistoryBackend (manages deleting things older than 3 months)
 */
 
 namespace history {
 
 namespace {
 
+// Referrer used for clicks on article suggestions on the NTP.
+const char kChromeContentSuggestionsReferrer[] =
+    "https://www.googleapis.com/auth/chrome-content-suggestions";

[sky, 2016/09/15 17:46:26]

I like this approach much better, but I don't want to bake into history_backend
this url. Can you add a boolean to HistoryAddPageArgs indicating if the page
should contribute to the ntp and change HistoryTabHelper to set the value based
on the url?

[mastiz, 2016/09/15 18:12:07]

Would you be fine if we instead verify the referrer is empty? This is what the
server-side implementation does, and after all, I expect TYPED and AUTO_BOOKMARK
always contain an empty referer? (based on the fact, that referrer sanitization
requires http[s] schema)

I avoided this in the first place to reduce the risk for regressions.

+
 void RunUnlessCanceled(
     const base::Closure& closure,
     const base::CancelableTaskTracker::IsCanceledCallback& is_canceled) {
   if (!is_canceled.Run())
     closure.Run();
 }
 
 }  // namespace
 
 // How long we'll wait to do a commit, so that things are batched together.
@@ skipping 224 matching lines @@
     if (visit_set.find(visit_id) != visit_set.end()) {
       NOTREACHED() << "Loop in referer chain, giving up";
       break;
     }
     visit_set.insert(visit_id);
   }
   return 0;
 }
 
 SegmentID HistoryBackend::UpdateSegments(const GURL& url,
+                                         const GURL& referrer,
                                          VisitID from_visit,
                                          VisitID visit_id,
                                          ui::PageTransition transition_type,
                                          const Time ts) {
   if (!db_)
     return 0;
 
   // We only consider main frames.
   if (!ui::PageTransitionIsMainFrame(transition_type))
     return 0;
 
   SegmentID segment_id = 0;
 
   // Are we at the beginning of a new segment?
   // Note that navigating to an existing entry (with back/forward) reuses the
   // same transition type.  We are not adding it as a new segment in that case
   // because if this was the target of a redirect, we might end up with
   // 2 entries for the same final URL. Ex: User types google.net, gets
   // redirected to google.com. A segment is created for google.net. On
   // google.com users navigates through a link, then press back. That last
   // navigation is for the entry google.com transition typed. We end up adding
   // a segment for that one as well. So we end up with google.net and google.com
   // in the segment table, showing as 2 entries in the NTP.
   // Note also that we should still be updating the visit count for that segment
   // which we are not doing now. It should be addressed when
   // http://crbug.com/96860 is fixed.
   if ((ui::PageTransitionCoreTypeIs(transition_type,
                                     ui::PAGE_TRANSITION_TYPED) ||
-       ui::PageTransitionCoreTypeIs(transition_type,
-                                    ui::PAGE_TRANSITION_AUTO_BOOKMARK)) &&
+       (ui::PageTransitionCoreTypeIs(transition_type,
+                                     ui::PAGE_TRANSITION_AUTO_BOOKMARK) &&
+        referrer != GURL(kChromeContentSuggestionsReferrer))) &&
       (transition_type & ui::PAGE_TRANSITION_FORWARD_BACK) == 0) {
     // If so, create or get the segment.
     std::string segment_name = db_->ComputeSegmentName(url);
     URLID url_id = db_->GetRowForURL(url, nullptr);
     if (!url_id)
       return 0;
 
     segment_id = db_->GetSegmentNamed(segment_name);
     if (!segment_id) {
       segment_id = db_->CreateSegment(url_id, segment_name);
@@ skipping 158 matching lines @@
         ui::PAGE_TRANSITION_CHAIN_END);
 
     // No redirect case (one element means just the page itself).
     last_ids = AddPageVisit(request.url, request.time, last_ids.second, t,
                             request.visit_source);
 
     // 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) {
-      UpdateSegments(request.url, from_visit_id, last_ids.second, t,
-                     request.time);
+      UpdateSegments(request.url, request.referrer, from_visit_id,
+                     last_ids.second, t, request.time);
 
       // Update the referrer's duration.
       UpdateVisitDuration(from_visit_id, request.time);
     }
   } else {
     // Redirect case. Add the redirect chain.
 
     ui::PageTransition redirect_info = ui::PAGE_TRANSITION_CHAIN_START;
 
     RedirectList redirects = request.redirects;
@@ skipping 48 matching lines @@
         t = ui::PageTransitionFromInt(t | ui::PAGE_TRANSITION_CHAIN_END);
       }
 
       // Record all redirect visits with the same timestamp. We don't display
       // them anyway, and if we ever decide to, we can reconstruct their order
       // from the redirect chain.
       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);
+        UpdateSegments(redirects[redirect_index], request.referrer,
+                       from_visit_id, last_ids.second, t, request.time);
 
         // Update the visit_details for this visit.
         UpdateVisitDuration(from_visit_id, request.time);
       }
 
       // Subsequent transitions in the redirect list must all be server
       // redirects.
       redirect_info = ui::PAGE_TRANSITION_SERVER_REDIRECT;
     }
 
@@ skipping 2040 matching lines @@
   // transaction is currently open.
   db_->CommitTransaction();
   db_->Vacuum();
   db_->BeginTransaction();
   db_->GetStartDate(&first_recorded_time_);
 
   return true;
 }
 
 }  // namespace history