Turn off safebrowsing for PREFETCH requests

chrome/browser/renderer_host/safe_browsing_resource_handler.cc

 // Copyright (c) 2011 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/renderer_host/safe_browsing_resource_handler.h"
 
 #include "base/logging.h"
 #include "chrome/browser/renderer_host/global_request_id.h"
 #include "chrome/browser/renderer_host/resource_dispatcher_host.h"
 #include "chrome/browser/renderer_host/resource_message_filter.h"
 #include "chrome/common/resource_response.h"
+#include "net/base/io_buffer.h"
+#include "net/base/load_flags.h"
 #include "net/base/net_errors.h"
-#include "net/base/io_buffer.h"
 
 // Maximum time in milliseconds to wait for the safe browsing service to
 // verify a URL. After this amount of time the outstanding check will be
 // aborted, and the URL will be treated as if it were safe.
 static const int kCheckUrlTimeoutMs = 5000;
 
 // TODO(eroman): Downgrade these CHECK()s to DCHECKs once there is more
 //               unit test coverage.
 
 SafeBrowsingResourceHandler::SafeBrowsingResourceHandler(
@@ skipping 141 matching lines @@
 }
 
 void SafeBrowsingResourceHandler::StartDisplayingBlockingPage(
     const GURL& url,
     SafeBrowsingService::UrlCheckResult result) {
   CHECK(state_ == STATE_NONE);
   CHECK(defer_state_ != DEFERRED_NONE);
   CHECK(deferred_request_id_ != -1);
 
   state_ = STATE_DISPLAYING_BLOCKING_PAGE;
   AddRef();  // Balanced in OnBlockingPageComplete().

[cbentzel, 2011/02/22 15:56:04]

Should you just move this after the PREFETCH test so you don't need to balance
with a Release() there?

Also, you could set the state_ after that block as well, so you don't need to
reclear to STATE_NONE inside of the block.

[gavinp, 2011/02/22 22:51:33]

Mooted by moving the code over to OnBrowseUrlCheckResult.

 
   // Grab the original url of this request as well.
   GURL original_url;
   net::URLRequest* request = rdh_->GetURLRequest(
       GlobalRequestID(render_process_host_id_, deferred_request_id_));
+

[lzheng, 2011/02/22 19:07:18]

Instead of putting this logic here, can you move it to OnBrowseUrlCheckResult
where StartDisplayingBlockingPage is called? I feel it fits better there. And
you don't need to deal with state_ and Release there. It does not seem to need
to call ClearDefrredReqeustInfo() too (unless I missed something).

[gavinp, 2011/02/22 22:51:33]

Good point Lei!  Done.  However, the original reason I put the code there was to
use the "request" automatic.  In the version I'm uploading, I just make a
redundant call to request, which does an STL map lookup: probably not awful.

I removed the call to ClearDeferredRequestInfo().  Every path except the one in
OnBrowseUrlCheckResult was calling it, and I think I was overcautious.

+  if (request->load_flags() & net::LOAD_PREFETCH) {
+    // Don't prefetch resources that fail safe browsing, disallow
+    // them.
+    rdh_->CancelRequest(render_process_host_id_, deferred_request_id_, false);
+    next_handler_->OnResponseCompleted(

[cbentzel, 2011/02/22 15:56:04]

Why do you need to call next_handler_->OnResponseCompleted()?

In the OnBlockingPageComplete, it just does rdh_->CancelRequest(). 

[gavinp, 2011/02/22 22:51:33]

Without the call here, the load of the current page never completes; it spins
forever.  In OnBlockPageComplete, the case is different because we're displaying
the interstitial, and not the page itself: and furthermore we know we'll never
navigate to it.

This does seem like a bug though: a page should show as complete as whether a
never-ending prefetch finishes or not.  Created Bug 73778, and added a TODO here
to track this.

+        deferred_request_id_,
+        net::URLRequestStatus(net::URLRequestStatus::FAILED, net::ERR_ABORTED),
+        std::string());
+    state_ = STATE_NONE;
+    ClearDeferredRequestInfo();
+    Release();
+    return;
+  }
+
   if (request)
     original_url = request->original_url();
   else
     original_url = url;
 
   safe_browsing_->DisplayBlockingPage(
       url, original_url, redirect_urls_, resource_type_,
       result, this, render_process_host_id_, render_view_id_);
 }
 
@@ skipping 106 matching lines @@
     rdh_->FollowDeferredRedirect(render_process_host_id_, request_id,
                                  false, GURL());
   }
 }
 
 void SafeBrowsingResourceHandler::ClearDeferredRequestInfo() {
   deferred_request_id_ = -1;
   deferred_url_ = GURL();
   deferred_redirect_response_ = NULL;
 }