Remove the use of ProvisionalChangeToMainFrameUrl from prerender code.

chrome/browser/prerender/prerender_tab_helper.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 "chrome/browser/prerender/prerender_tab_helper.h"
 
 #include "base/bind.h"
 #include "base/metrics/histogram.h"
 #include "base/time/time.h"
 #include "chrome/browser/prerender/prerender_histograms.h"
 #include "chrome/browser/prerender/prerender_local_predictor.h"
 #include "chrome/browser/prerender/prerender_manager.h"
 #include "chrome/browser/prerender/prerender_manager_factory.h"
 #include "chrome/browser/profiles/profile.h"
 #include "components/password_manager/core/browser/password_manager.h"
 #include "content/public/browser/navigation_details.h"
 #include "content/public/browser/navigation_entry.h"
 #include "content/public/browser/render_frame_host.h"
+#include "content/public/browser/resource_request_details.h"
 #include "content/public/browser/web_contents.h"
 #include "content/public/common/frame_navigate_params.h"
 
 using content::WebContents;
 
 DEFINE_WEB_CONTENTS_USER_DATA_KEY(prerender::PrerenderTabHelper);
 
 namespace prerender {
 
 namespace {
@@ skipping 45 matching lines @@
       prerender_manager->IsWebContentsPrerendering(web_contents, &origin_)) {
     navigation_type_ = NAVIGATION_TYPE_PRERENDERED;
   } else {
     navigation_type_ = NAVIGATION_TYPE_NORMAL;
   }
 }
 
 PrerenderTabHelper::~PrerenderTabHelper() {
 }
 
-void PrerenderTabHelper::ProvisionalChangeToMainFrameUrl(
-    const GURL& url,
-    content::RenderFrameHost* render_frame_host) {
-  url_ = url;
+void PrerenderTabHelper::DidGetRedirectForResourceRequest(

[davidben, 2014/09/17 15:08:30]

For redirects, the difference between this hook and the previous is that on
occurs when the browser receives the redirect and the other is after the
renderer processes it. (It may reject the redirect for some reason. But there
aren't many places where this happens for a top-level navigation. The only ones
I can think of are some CSP rules.)

I do not believe that will have a meaningful difference on this code which is
just metrics anyway, but I'm less familiar with it. (As I recall, these metrics
don't quite measure things right anyway.)

[tburkard, 2014/09/18 09:36:04]

What do you mean by "previous" here?

[Avi (use Gerrit), 2014/09/18 21:00:21]

The difference between ProvisionalChangeToMainFrameUrl and
DidGetRedirectForResourceRequest. Those apparently get called at different
times.

[davidben, 2014/09/18 22:14:21]

Sorry, that was poorly phrased. Yeah, that was what I meant by "previous".

+    content::RenderViewHost* render_view_host,
+    const content::ResourceRedirectDetails& details) {
+  if (details.resource_type != content::RESOURCE_TYPE_MAIN_FRAME)
+    return;
+
+  url_ = details.new_url;
   RecordEvent(EVENT_MAINFRAME_CHANGE);
-  RecordEventIfLoggedInURL(EVENT_MAINFRAME_CHANGE_DOMAIN_LOGGED_IN, url);
+  RecordEventIfLoggedInURL(EVENT_MAINFRAME_CHANGE_DOMAIN_LOGGED_IN, url_);
   PrerenderManager* prerender_manager = MaybeGetPrerenderManager();
   if (!prerender_manager)
     return;
   if (prerender_manager->IsWebContentsPrerendering(web_contents(), NULL))
     return;
-  ReportTabHelperURLSeenToLocalPredictor(prerender_manager, url,
+  ReportTabHelperURLSeenToLocalPredictor(prerender_manager, url_,
                                          web_contents());
 }
 
 void PrerenderTabHelper::DidCommitProvisionalLoadForFrame(
     content::RenderFrameHost* render_frame_host,
     const GURL& validated_url,
     content::PageTransition transition_type) {
   if (render_frame_host->GetParent())
     return;
   RecordEvent(EVENT_MAINFRAME_COMMIT);
@@ skipping 42 matching lines @@
       RecordPerceivedPageLoadTime(
           now - pplt_load_start_, fraction_elapsed_at_swapin);
     }
   }
 
   // Reset the PPLT metric.
   pplt_load_start_ = base::TimeTicks();
   actual_load_start_ = base::TimeTicks();
 }
 
 void PrerenderTabHelper::DidStartProvisionalLoadForFrame(

[davidben, 2014/09/17 15:08:30]

[Verified that, for the initial ProvisionalChangeToMainFrameUrl, duplicating the
block here is equivalent.]

     content::RenderFrameHost* render_frame_host,
     const GURL& validated_url,
     bool is_error_page,
     bool is_iframe_srcdoc) {
   if (render_frame_host->GetParent())
     return;
 
   // Record PPLT state for the beginning of a new navigation.
   pplt_load_start_ = base::TimeTicks::Now();
   actual_load_start_ = base::TimeTicks();
 
   if (next_load_is_control_prerender_) {
     DCHECK_EQ(NAVIGATION_TYPE_NORMAL, navigation_type_);
     navigation_type_ = NAVIGATION_TYPE_WOULD_HAVE_BEEN_PRERENDERED;
     origin_ = next_load_origin_;
     next_load_is_control_prerender_ = false;
     next_load_origin_ = ORIGIN_NONE;
   }
+
+  url_ = validated_url;
+  RecordEvent(EVENT_MAINFRAME_CHANGE);
+  RecordEventIfLoggedInURL(EVENT_MAINFRAME_CHANGE_DOMAIN_LOGGED_IN, url_);
+  PrerenderManager* prerender_manager = MaybeGetPrerenderManager();
+  if (!prerender_manager)
+    return;
+  if (prerender_manager->IsWebContentsPrerendering(web_contents(), NULL))
+    return;
+  ReportTabHelperURLSeenToLocalPredictor(prerender_manager, url_,
+                                         web_contents());

[davidben, 2014/09/17 15:08:30]

Maybe split this and the other block into their own function.

[Avi (use Gerrit), 2014/09/18 21:00:21]

Done.

 }
 
 void PrerenderTabHelper::PasswordSubmitted(const autofill::PasswordForm& form) {
   PrerenderManager* prerender_manager = MaybeGetPrerenderManager();
   if (prerender_manager) {
     prerender_manager->RecordLikelyLoginOnURL(form.origin);
     RecordEvent(EVENT_LOGIN_ACTION_ADDED);
     if (form.password_value.empty())
       RecordEvent(EVENT_LOGIN_ACTION_ADDED_PW_EMPTY);
   }
@@ skipping 80 matching lines @@
   prerender_manager->RecordPerceivedPageLoadTime(
       origin_, navigation_type_, perceived_page_load_time,
       fraction_plt_elapsed_at_swap_in, url_);
 
   // Reset state for the next navigation.
   navigation_type_ = NAVIGATION_TYPE_NORMAL;
   origin_ = ORIGIN_NONE;
 }
 
 }  // namespace prerender