Migrate the site engagement service to use DidFinishNavigation.

chrome/browser/engagement/site_engagement_helper.cc

 // 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.
 
 #include "chrome/browser/engagement/site_engagement_helper.h"
 
 #include <utility>
 
 #include "base/time/time.h"
 #include "base/trace_event/trace_event.h"
 #include "chrome/browser/engagement/site_engagement_service.h"
 #include "chrome/browser/engagement/site_engagement_service_factory.h"
 #include "chrome/browser/prerender/prerender_contents.h"
 #include "chrome/browser/profiles/profile.h"
-#include "content/public/browser/navigation_details.h"
+#include "content/public/browser/navigation_handle.h"
 #include "content/public/browser/web_contents.h"
 
 namespace {
 
 int g_seconds_to_pause_engagement_detection = 10;
 int g_seconds_delay_after_navigation = 10;
 int g_seconds_delay_after_media_starts = 10;
 int g_seconds_delay_after_show = 5;
 
 }  // anonymous namespace
@@ skipping 123 matching lines @@
 
 void SiteEngagementHelper::MediaTracker::WasShown() {
   is_hidden_ = false;
 }
 
 void SiteEngagementHelper::MediaTracker::WasHidden() {
   is_hidden_ = true;
 }
 
 SiteEngagementHelper::~SiteEngagementHelper() {
-  content::WebContents* contents = web_contents();
-  if (contents) {
+  if (web_contents()) {
     input_tracker_.Stop();
     media_tracker_.Stop();
   }
 }
 
 SiteEngagementHelper::SiteEngagementHelper(content::WebContents* web_contents)
     : content::WebContentsObserver(web_contents),
       input_tracker_(this, web_contents),
       media_tracker_(this, web_contents),
       record_engagement_(false) {}
@@ skipping 20 matching lines @@
     Profile* profile =
         Profile::FromBrowserContext(contents->GetBrowserContext());
     SiteEngagementService* service =
         SiteEngagementServiceFactory::GetForProfile(profile);
 
     if (service)
       service->HandleMediaPlaying(contents->GetVisibleURL(), is_hidden);
   }
 }
 
-void SiteEngagementHelper::DidNavigateMainFrame(
-    const content::LoadCommittedDetails& details,
-    const content::FrameNavigateParams& params) {
-  // Ignore in-page navigations. However, do not stop input or media detection.
-  if (details.is_in_page)
-    return;
-
+void SiteEngagementHelper::DidFinishNavigation(
+    content::NavigationHandle* handle) {
   input_tracker_.Stop();
   media_tracker_.Stop();
-  record_engagement_ = params.url.SchemeIsHTTPOrHTTPS();
 
-  // Ignore all schemes except HTTP and HTTPS.
-  if (!record_engagement_)
+  // Ignore all schemes except HTTP and HTTPS, as well as uncommitted, non
+  // main-frame, same page, or error page navigations.
+  record_engagement_ = handle->GetURL().SchemeIsHTTPOrHTTPS();
+  if (!handle->HasCommitted() || !handle->IsInMainFrame() ||
+      handle->IsSamePage() || handle->IsErrorPage() || !record_engagement_) {

[calamity, 2016/05/12 07:58:14]

What are considered to be same pages and error pages?

[dominickn, 2016/05/12 08:03:06]

 - Same page navigations are things like fragment navigations or anchor link
navigations that stay on the same page. Makes sense not to count them as
navigations for engagement (they'll get user interaction points already)
 - Error page navigations are those which end with an error page (interstitial)
being shown to the user

     return;
+  }
 
   // Ignore prerender loads. This means that prerenders will not receive
   // navigation engagement. The implications are as follows:
   //
-  // - Instant search prerenders from the omnibox trigger DidNavigateMainFrame
+  // - Instant search prerenders from the omnibox trigger DidFinishNavigation
   //   twice: once for the prerender, and again when the page swaps in. The
   //   second trigger has transition GENERATED and receives navigation
   //   engagement.
   // - Prerenders initiated by <link rel="prerender"> (e.g. search results) are
   //   always assigned the LINK transition, which is ignored for navigation
   //   engagement.
   //
   // Prerenders trigger WasShown() when they are swapped in, so input engagement
   // will activate even if navigation engagement is not scored.
   if (prerender::PrerenderContents::FromWebContents(web_contents()) != nullptr)
     return;
 
   Profile* profile =
       Profile::FromBrowserContext(web_contents()->GetBrowserContext());
   SiteEngagementService* service =
       SiteEngagementServiceFactory::GetForProfile(profile);
 
   if (service)
-    service->HandleNavigation(params.url, params.transition);
+    service->HandleNavigation(handle->GetURL(), handle->GetPageTransition());
 
   input_tracker_.Start(
       base::TimeDelta::FromSeconds(g_seconds_delay_after_navigation));
 }
 
 void SiteEngagementHelper::WasShown() {
-  // Ensure that the input callbacks are registered when we come into view.
+  // Ensure that the input callbacks are registered when we come into view. This
+  // occurs when switching tabs, or when prerendered pages are swapped in.
   if (record_engagement_) {
     input_tracker_.Start(
         base::TimeDelta::FromSeconds(g_seconds_delay_after_show));
   }
 }
 
 void SiteEngagementHelper::WasHidden() {
   // Ensure that the input callbacks are not registered when hidden.
   input_tracker_.Stop();
 }
 
 // static
 void SiteEngagementHelper::SetSecondsBetweenUserInputCheck(int seconds) {
   g_seconds_to_pause_engagement_detection = seconds;
 }
 
 // static
 void SiteEngagementHelper::SetSecondsTrackingDelayAfterNavigation(int seconds) {
   g_seconds_delay_after_navigation = seconds;
 }
 
 // static
 void SiteEngagementHelper::SetSecondsTrackingDelayAfterShow(int seconds) {
   g_seconds_delay_after_show = seconds;
 }