Convert PrerenderContents to use the new navigation callbacks.

chrome/browser/prerender/prerender_contents.cc

Index: chrome/browser/prerender/prerender_contents.cc
diff --git a/chrome/browser/prerender/prerender_contents.cc b/chrome/browser/prerender/prerender_contents.cc
index aea147d28bf082ac2336cfc8edae770afc03b79b..788597db744632523200fdf1df1c28c26a362a9a 100644
--- a/chrome/browser/prerender/prerender_contents.cc
+++ b/chrome/browser/prerender/prerender_contents.cc
@@ -32,6 +32,7 @@
 #include "components/history/core/browser/history_types.h"
 #include "content/public/browser/browser_child_process_host.h"
 #include "content/public/browser/browser_thread.h"
+#include "content/public/browser/navigation_handle.h"
 #include "content/public/browser/notification_service.h"
 #include "content/public/browser/render_frame_host.h"
 #include "content/public/browser/render_process_host.h"
@@ -42,6 +43,7 @@
 #include "content/public/browser/web_contents.h"
 #include "content/public/browser/web_contents_delegate.h"
 #include "content/public/common/frame_navigate_params.h"
+#include "net/http/http_response_headers.h"
 #include "services/service_manager/public/cpp/interface_registry.h"
 #include "ui/base/page_transition_types.h"
 #include "ui/gfx/geometry/size.h"
@@ -549,22 +551,21 @@ void PrerenderContents::DocumentLoadedInFrame(
     NotifyPrerenderDomContentLoaded();
 }
 
-void PrerenderContents::DidStartProvisionalLoadForFrame(
-    content::RenderFrameHost* render_frame_host,
-    const GURL& validated_url,
-    bool is_error_page) {
-  if (!render_frame_host->GetParent()) {
-    if (!CheckURL(validated_url))
-      return;
+void PrerenderContents::DidStartNavigation(
+    content::NavigationHandle* navigation_handle) {
+  if (!navigation_handle->IsInMainFrame() || navigation_handle->IsSamePage())
+    return;
 
-    // Usually, this event fires if the user clicks or enters a new URL.
-    // Neither of these can happen in the case of an invisible prerender.
-    // So the cause is: Some JavaScript caused a new URL to be loaded.  In that
-    // case, the spinner would start again in the browser, so we must reset
-    // has_stopped_loading_ so that the spinner won't be stopped.
-    has_stopped_loading_ = false;
-    has_finished_loading_ = false;
-  }
+  if (!CheckURL(navigation_handle->GetURL()))
+    return;
+
+  // Usually, this event fires if the user clicks or enters a new URL.
+  // Neither of these can happen in the case of an invisible prerender.
+  // So the cause is: Some JavaScript caused a new URL to be loaded.  In that
+  // case, the spinner would start again in the browser, so we must reset
+  // has_stopped_loading_ so that the spinner won't be stopped.
+  has_stopped_loading_ = false;
+  has_finished_loading_ = false;
 }
 
 void PrerenderContents::DidFinishLoad(
@@ -574,9 +575,29 @@ void PrerenderContents::DidFinishLoad(
     has_finished_loading_ = true;
 }
 
-void PrerenderContents::DidNavigateMainFrame(
-    const content::LoadCommittedDetails& details,
-    const content::FrameNavigateParams& params) {
+void PrerenderContents::DidFinishNavigation(
+    content::NavigationHandle* navigation_handle) {
+  if (!navigation_handle->IsInMainFrame() ||
+      !navigation_handle->HasCommitted() ||
+      navigation_handle->IsErrorPage()) {

[pasko, 2017/01/25 17:34:28]

I do not know the peculiarities of the new navigation API, so a question:

does it happen in practice that IsErrorPage() is true, and we did not destroy
the prerender contents based on the response code before that?

[jam, 2017/01/25 18:13:18]

IsErrorPage() is added to maintain the same behavior of DidFinishNavigation as
DidNavigateMainFrame (since the latter doesn't fire it for errors).

For PrerenderContents, in case of an error, we actually don't get an
IsErrorPage() to true unless in some races. The timing is that there are two
DidStartNavigation/DidFinishNavigation sets of callbacks. The first one is for
the load that doesn't find the page. It's not the error page yet so
IsErrorPage() is false. The second pair is for the loading of the error page, so
IsErrorPage() is true. The issue is that the renderer sends the prefetch
complete message that you added recently between these two pairs, so usually
with the thread hopping/loading times the second pair is not seen.

+    return;
+  }
+
+  if (navigation_handle->GetResponseHeaders() &&
+      navigation_handle->GetResponseHeaders()->response_code() >= 400) {
+    // Maintain same behavior as old navigation API when the URL is unreachable
+    // and leads to an error page. While there will be a subsequent navigation
+    // that has navigation_handle->IsErrorPage(), it'll be too late to wait for
+    // it as the renderer side will consider this prerender complete. This
+    // object would therefore have been destructed already and so instead look
+    // for the error response code now.
+    // Also maintain same final status code that previous navigation API
+    // returned, which was reached because the URL for the error page was
+    // kUnreachableWebDataURL and that was interpreted as unsupported scheme.
+    Destroy(FINAL_STATUS_UNSUPPORTED_SCHEME);
+    return;
+  }
+
   // If the prerender made a second navigation entry, abort the prerender. This
   // avoids having to correctly implement a complex history merging case (this
   // interacts with location.replace) and correctly synchronize with the
@@ -590,13 +611,13 @@ void PrerenderContents::DidNavigateMainFrame(
     return;
   }
 
-  // Add each redirect as an alias. |params.url| is included in
-  // |params.redirects|.
+  // Add each redirect as an alias. |navigation_handle->GetURL()| is included in
+  // |navigation_handle->GetRedirectChain()|.
   //
   // TODO(davidben): We do not correctly patch up history for renderer-initated
   // navigations which add history entries. http://crbug.com/305660.
-  for (size_t i = 0; i < params.redirects.size(); i++) {
-    if (!AddAliasURL(params.redirects[i]))
+  for (const auto& redirect : navigation_handle->GetRedirectChain()) {
+    if (!AddAliasURL(redirect))
       return;
   }
 }