[PlzNavigate] Use WebContentsGetter in PrerenderResourceThrottle

chrome/browser/prerender/prerender_resource_throttle.cc

Index: chrome/browser/prerender/prerender_resource_throttle.cc
diff --git a/chrome/browser/prerender/prerender_resource_throttle.cc b/chrome/browser/prerender/prerender_resource_throttle.cc
index 8efbcfe11533015a1f2fffdc107b91f18ad0531c..dbc6c6559b2c8986d37c1ecbba0bc76a9d3f3416 100644
--- a/chrome/browser/prerender/prerender_resource_throttle.cc
+++ b/chrome/browser/prerender/prerender_resource_throttle.cc
@@ -12,14 +12,13 @@
 #include "chrome/browser/prerender/prerender_manager.h"
 #include "chrome/browser/prerender/prerender_util.h"
 #include "content/public/browser/browser_thread.h"
-#include "content/public/browser/render_frame_host.h"
-#include "content/public/browser/resource_request_info.h"
 #include "content/public/browser/web_contents.h"
 #include "net/http/http_response_headers.h"
 #include "net/url_request/redirect_info.h"
 #include "net/url_request/url_request.h"
 
 using content::BrowserThread;
+using content::ResourceRequestInfo;
 using content::ResourceType;
 
 namespace prerender {
@@ -49,7 +48,8 @@ class PrerenderThrottleInfo
     : public base::RefCountedThreadSafe<PrerenderThrottleInfo,
                                         BrowserThread::DeleteOnUIThread> {
  public:
-  PrerenderThrottleInfo() : manager_(nullptr) {}
+  PrerenderThrottleInfo()
+      : mode_(NO_PRERENDER), origin_(ORIGIN_NONE), manager_(nullptr) {}
 
   void Set(PrerenderMode mode, Origin origin, PrerenderManager* manager) {
     DCHECK_CURRENTLY_ON(BrowserThread::UI);
@@ -94,8 +94,8 @@ void PrerenderResourceThrottle::WillStartRequest(bool* defer) {
   BrowserThread::PostTask(
       BrowserThread::UI, FROM_HERE,
       base::Bind(&PrerenderResourceThrottle::WillStartRequestOnUI, AsWeakPtr(),
-                 request_->method(), info->GetResourceType(),
-                 info->GetChildID(), info->GetRenderFrameID(), request_->url(),
+                 request_->method(), info->GetResourceType(), request_->url(),
+                 info->GetWebContentsGetterForRequest(),
                  prerender_throttle_info_));
 }
 
@@ -112,8 +112,8 @@ void PrerenderResourceThrottle::WillRedirectRequest(
       BrowserThread::UI, FROM_HERE,
       base::Bind(&PrerenderResourceThrottle::WillRedirectRequestOnUI,
                  AsWeakPtr(), header, info->GetResourceType(), info->IsAsync(),
-                 IsNoStoreResponse(*request_), info->GetChildID(),
-                 info->GetRenderFrameID(), redirect_info.new_url));
+                 IsNoStoreResponse(*request_), redirect_info.new_url,
+                 info->GetWebContentsGetterForRequest()));
 }
 
 void PrerenderResourceThrottle::WillProcessResponse(bool* defer) {
@@ -131,7 +131,6 @@ void PrerenderResourceThrottle::WillProcessResponse(bool* defer) {
       base::Bind(&PrerenderResourceThrottle::WillProcessResponseOnUI,
                  content::IsResourceTypeFrame(info->GetResourceType()),
                  IsNoStoreResponse(*request_), redirect_count,
-                 info->GetChildID(), info->GetRenderFrameID(),
                  prerender_throttle_info_));
 }
 
@@ -148,13 +147,12 @@ void PrerenderResourceThrottle::WillStartRequestOnUI(
     const base::WeakPtr<PrerenderResourceThrottle>& throttle,
     const std::string& method,
     ResourceType resource_type,
-    int render_process_id,
-    int render_frame_id,
     const GURL& url,
+    const ResourceRequestInfo::WebContentsGetter& web_contents_getter,
     scoped_refptr<PrerenderThrottleInfo> prerender_throttle_info) {
   bool cancel = false;
   PrerenderContents* prerender_contents =
-      PrerenderContentsFromRenderFrame(render_process_id, render_frame_id);
+      PrerenderContentsFromGetter(web_contents_getter);
   if (prerender_contents) {
     DCHECK(prerender_throttle_info);
     prerender_throttle_info->Set(prerender_contents->prerender_mode(),
@@ -172,7 +170,8 @@ void PrerenderResourceThrottle::WillStartRequestOnUI(
         prerender_contents->Destroy(FINAL_STATUS_INVALID_HTTP_METHOD);
       }
       cancel = true;
-    } else if (!PrerenderManager::DoesSubresourceURLHaveValidScheme(url)) {
+    } else if (!PrerenderManager::DoesSubresourceURLHaveValidScheme(url) &&
+               resource_type != content::RESOURCE_TYPE_MAIN_FRAME) {

[droger, 2016/12/15 09:46:21]

This is the one line fix for the failing tests.

The test here is not intended for the main resource (see the line above, using a
subresource-related check).

The check for the main resource is done elsewhere (in PrerenderContents and in
WillRedirectRequestOnUI below for redirects).

The reason why it was failing the test is a bit complex. Here are some
explanations for posterity:

This was making the test fail because it would cause
FINAL_STATUS_UNSUPPORTED_SCHEME when requesting chrome://crash instead of
letting it go through and crashing the rendrerer.

This was only happening with plz navigate because of an ordering issue: with plz
navigate this code is called before the renderer is invoked, whereas with the
old flow the renderer is invoked first (and crashes before running this code).

Note that chrome://crash is able to go through in the test, because the check
for unsupported URLs in PrerenderContents is specifically disabled by
TestPrerenderContents.
So in real life, prerendering chrome://crash would not crash the prerenderer, it
only can do so in the test.

[pasko, 2016/12/15 15:25:45]

Thanks. That's very informative! Can we put a comment in the body of this branch
referencing a the bug with this description?

Like:
// Destroying the prerender for unsupported scheme only for non-main resource to
// allow chrome://crash to actually crash in the *RendererCrash tests instead of
// being intercepted here. The unsupported scheme for the main resource is
// checked in X. See http://crbug.com/673771.

[droger, 2016/12/15 16:25:01]

Done.

Note also that DoesSubresourceURLHaveValidScheme() is only intended to be called
on sub resources.

If we wanted to check main resources, we would call DoesURLHaveValidScheme(). We
don't do it though, for the reasons explained above.

       prerender_contents->Destroy(FINAL_STATUS_UNSUPPORTED_SCHEME);
       ReportUnsupportedPrerenderScheme(url);
       cancel = true;
@@ -200,12 +199,11 @@ void PrerenderResourceThrottle::WillRedirectRequestOnUI(
     ResourceType resource_type,
     bool async,
     bool is_no_store,
-    int render_process_id,
-    int render_frame_id,
-    const GURL& new_url) {
+    const GURL& new_url,
+    const ResourceRequestInfo::WebContentsGetter& web_contents_getter) {
   bool cancel = false;
   PrerenderContents* prerender_contents =
-      PrerenderContentsFromRenderFrame(render_process_id, render_frame_id);
+      PrerenderContentsFromGetter(web_contents_getter);
   if (prerender_contents) {
     prerender_contents->prerender_manager()->RecordPrefetchResponseReceived(
         prerender_contents->origin(),
@@ -245,8 +243,6 @@ void PrerenderResourceThrottle::WillProcessResponseOnUI(
     bool is_main_resource,
     bool is_no_store,
     int redirect_count,
-    int render_process_id,
-    int render_frame_id,
     scoped_refptr<PrerenderThrottleInfo> prerender_throttle_info) {
   DCHECK(prerender_throttle_info);
   if (!prerender_throttle_info->manager())
@@ -263,15 +259,11 @@ void PrerenderResourceThrottle::WillProcessResponseOnUI(
 }
 
 // static
-PrerenderContents* PrerenderResourceThrottle::PrerenderContentsFromRenderFrame(
-    int render_process_id, int render_frame_id) {
+PrerenderContents* PrerenderResourceThrottle::PrerenderContentsFromGetter(
+    const ResourceRequestInfo::WebContentsGetter& web_contents_getter) {
   if (g_prerender_contents_for_testing)
     return g_prerender_contents_for_testing;
-  content::RenderFrameHost* rfh = content::RenderFrameHost::FromID(
-      render_process_id, render_frame_id);
-  content::WebContents* web_contents =
-      content::WebContents::FromRenderFrameHost(rfh);
-  return PrerenderContents::FromWebContents(web_contents);
+  return PrerenderContents::FromWebContents(web_contents_getter.Run());
 }
 
 }  // namespace prerender