Allow extensions to redirect requests in onBeforeRequest.

BUG=60101
TEST=no

Review URL: http://codereview.chromium.org/6677148

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@81479 0039d316-1c4b-4281-b951-d872f2087c98

9 files changed, 174 insertions, 54 deletions

diff --git a/net/base/network_delegate.cc b/net/base/network_delegate.cc
index 2ed1ea5..d707c44 100644
--- a/net/base/network_delegate.cc
+++ b/net/base/network_delegate.cc
@@ -9,20 +9,21 @@
 namespace net {
 
 int NetworkDelegate::NotifyBeforeURLRequest(URLRequest* request,
-                                            CompletionCallback* callback) {
+                                            CompletionCallback* callback,
+                                            GURL* new_url) {
   DCHECK(CalledOnValidThread());
   DCHECK(request);
   DCHECK(callback);
-  return OnBeforeURLRequest(request, callback);
+  return OnBeforeURLRequest(request, callback, new_url);
 }
 
 int NetworkDelegate::NotifyBeforeSendHeaders(uint64 request_id,
-                                             HttpRequestHeaders* headers,
-                                             CompletionCallback* callback) {
+                                             CompletionCallback* callback,
+                                             HttpRequestHeaders* headers) {
   DCHECK(CalledOnValidThread());
   DCHECK(headers);
   DCHECK(callback);
-  return OnBeforeSendHeaders(request_id, headers, callback);
+  return OnBeforeSendHeaders(request_id, callback, headers);
 }
 
 void NetworkDelegate::NotifyResponseStarted(URLRequest* request) {
diff --git a/net/base/network_delegate.h b/net/base/network_delegate.h
index effc699..eafb212 100644
--- a/net/base/network_delegate.h
+++ b/net/base/network_delegate.h
@@ -9,6 +9,8 @@
 #include "base/threading/non_thread_safe.h"
 #include "net/base/completion_callback.h"
 
+class GURL;
+
 namespace net {
 
 // NOTE: Layering violations!
@@ -34,10 +36,11 @@ class NetworkDelegate : public base::NonThreadSafe {
   // checking on parameters. See the corresponding virtuals for explanations of
   // the methods and their arguments.
   int NotifyBeforeURLRequest(URLRequest* request,
-                             CompletionCallback* callback);
+                             CompletionCallback* callback,
+                             GURL* new_url);
   int NotifyBeforeSendHeaders(uint64 request_id,
-                              HttpRequestHeaders* headers,
-                              CompletionCallback* callback);
+                              CompletionCallback* callback,
+                              HttpRequestHeaders* headers);
   void NotifyResponseStarted(URLRequest* request);
   void NotifyReadCompleted(URLRequest* request, int bytes_read);
   void NotifyURLRequestDestroyed(URLRequest* request);
@@ -55,20 +58,22 @@ class NetworkDelegate : public base::NonThreadSafe {
   // member functions will be called by the respective public notification
   // member function, which will perform basic sanity checking.
 
-  // Called before a request is sent. The callback can be called at any time,
-  // but will have no effect if the request has already been cancelled or
+  // Called before a request is sent. Allows the delegate to rewrite the URL
+  // being fetched by modifying |new_url|. The callback can be called at any
+  // time, but will have no effect if the request has already been cancelled or
   // deleted. Returns a net status code, generally either OK to continue with
   // the request or ERR_IO_PENDING if the result is not ready yet.
   virtual int OnBeforeURLRequest(URLRequest* request,
-                                 CompletionCallback* callback) = 0;
+                                 CompletionCallback* callback,
+                                 GURL* new_url) = 0;
 
   // Called right before the HTTP headers are sent. Allows the delegate to
   // read/write |headers| before they get sent out. The callback can be called
   // at any time, but will have no effect if the transaction handling this
   // request has been cancelled. Returns a net status code.
   virtual int OnBeforeSendHeaders(uint64 request_id,
-                                  HttpRequestHeaders* headers,
-                                  CompletionCallback* callback) = 0;
+                                  CompletionCallback* callback,
+                                  HttpRequestHeaders* headers) = 0;
 
   // This corresponds to URLRequestDelegate::OnResponseStarted.
   virtual void OnResponseStarted(URLRequest* request) = 0;
diff --git a/net/http/http_cache_transaction.cc b/net/http/http_cache_transaction.cc
index c4293d9..a12a64f 100644
--- a/net/http/http_cache_transaction.cc
+++ b/net/http/http_cache_transaction.cc
@@ -908,7 +908,7 @@ int HttpCache::Transaction::DoNotifyBeforeSendHeaders() {
     // TODO(mpcomplete): need to be able to modify these headers.
     HttpRequestHeaders headers = request_->extra_headers;
     return cache_->GetSession()->network_delegate()->NotifyBeforeSendHeaders(
-        request_->request_id, &headers, cache_callback_);
+        request_->request_id, cache_callback_, &headers);
   }
 
   return OK;
diff --git a/net/http/http_network_transaction.cc b/net/http/http_network_transaction.cc
index f7d3acc..5651a41 100644
--- a/net/http/http_network_transaction.cc
+++ b/net/http/http_network_transaction.cc
@@ -771,7 +771,7 @@ int HttpNetworkTransaction::DoBuildRequest() {
 
   if (session_->network_delegate()) {
     return session_->network_delegate()->NotifyBeforeSendHeaders(
-        request_->request_id, &request_headers_, delegate_callback_);
+        request_->request_id, delegate_callback_, &request_headers_);
   }
 
   return OK;
diff --git a/net/url_request/url_request.cc b/net/url_request/url_request.cc
index 5c437c2..3d60137 100644
--- a/net/url_request/url_request.cc
+++ b/net/url_request/url_request.cc
@@ -23,6 +23,7 @@
 #include "net/url_request/url_request_job.h"
 #include "net/url_request/url_request_job_manager.h"
 #include "net/url_request/url_request_netlog_params.h"
+#include "net/url_request/url_request_redirect_job.h"
 
 using base::Time;
 using std::string;
@@ -120,7 +121,9 @@ URLRequest::URLRequest(const GURL& url, Delegate* delegate)
       redirect_limit_(kMaxRedirects),
       final_upload_progress_(0),
       priority_(LOWEST),
-      identifier_(GenerateURLRequestIdentifier()) {
+      identifier_(GenerateURLRequestIdentifier()),
+      ALLOW_THIS_IN_INITIALIZER_LIST(
+          before_request_callback_(this, &URLRequest::BeforeRequestComplete)) {
   SIMPLE_STATS_COUNTER("URLRequestCount");
 
   // Sanity check out environment.
@@ -134,9 +137,6 @@ URLRequest::~URLRequest() {
   if (context_ && context_->network_delegate())
     context_->network_delegate()->NotifyURLRequestDestroyed(this);
 
-  if (before_request_callback_)
-    before_request_callback_->Cancel();
-
   Cancel();
 
   if (job_)
@@ -362,22 +362,41 @@ GURL URLRequest::GetSanitizedReferrer() const {
 void URLRequest::Start() {
   response_info_.request_time = Time::Now();
 
+  // Only notify the delegate for the initial request.
   if (context_ && context_->network_delegate()) {
-    before_request_callback_ = new CancelableCompletionCallback<URLRequest>(
-        this, &URLRequest::BeforeRequestComplete);
     if (context_->network_delegate()->NotifyBeforeURLRequest(
-            this, before_request_callback_) == net::ERR_IO_PENDING) {
-      before_request_callback_->AddRef();  // balanced in BeforeRequestComplete
+            this, &before_request_callback_, &delegate_redirect_url_) ==
+            net::ERR_IO_PENDING) {
       net_log_.BeginEvent(NetLog::TYPE_URL_REQUEST_BLOCKED_ON_EXTENSION, NULL);
       return;  // paused
     }
   }
 
-  StartJob(URLRequestJobManager::GetInstance()->CreateJob(this));
+  StartInternal();
 }
 
 ///////////////////////////////////////////////////////////////////////////////
 
+void URLRequest::BeforeRequestComplete(int error) {
+  DCHECK(!job_);
+  DCHECK_NE(ERR_IO_PENDING, error);
+
+  net_log_.EndEvent(NetLog::TYPE_URL_REQUEST_BLOCKED_ON_EXTENSION, NULL);
+  if (error != OK) {
+    StartJob(new URLRequestErrorJob(this, error));
+  } else if (!delegate_redirect_url_.is_empty()) {
+    GURL new_url;
+    new_url.Swap(&delegate_redirect_url_);
+    StartJob(new URLRequestRedirectJob(this, new_url));
+  } else {
+    StartInternal();
+  }
+}
+
+void URLRequest::StartInternal() {
+  StartJob(URLRequestJobManager::GetInstance()->CreateJob(this));
+}
+
 void URLRequest::StartJob(URLRequestJob* job) {
   DCHECK(!is_pending_);
   DCHECK(!job_);
@@ -404,18 +423,6 @@ void URLRequest::StartJob(URLRequestJob* job) {
   job_->Start();
 }
 
-void URLRequest::BeforeRequestComplete(int error) {
-  DCHECK(!job_);
-
-  net_log_.EndEvent(NetLog::TYPE_URL_REQUEST_BLOCKED_ON_EXTENSION, NULL);
-  before_request_callback_->Release();  // balanced in Start
-  if (error != OK) {
-    StartJob(new URLRequestErrorJob(this, error));
-  } else {
-    StartJob(URLRequestJobManager::GetInstance()->CreateJob(this));
-  }
-}
-
 void URLRequest::Restart() {
   // Should only be called if the original job didn't make any progress.
   DCHECK(job_ && !job_->has_response_started());
@@ -628,7 +635,7 @@ int URLRequest::Redirect(const GURL& location, int http_status_code) {
     final_upload_progress_ = job_->GetUploadProgress();
 
   PrepareToRestart();
-  Start();
+  StartInternal();
   return OK;
 }
 
diff --git a/net/url_request/url_request.h b/net/url_request/url_request.h
index 40f7d8a..b25b842 100644
--- a/net/url_request/url_request.h
+++ b/net/url_request/url_request.h
@@ -584,6 +584,15 @@ class URLRequest : public base::NonThreadSafe {
   friend class URLRequestJob;
   typedef std::map<const void*, linked_ptr<UserData> > UserDataMap;
 
+  void StartInternal();
+
+  // Resumes or blocks a request paused by the NetworkDelegate::OnBeforeRequest
+  // handler. If |blocked| is true, the request is blocked and an error page is
+  // returned indicating so. This should only be called after Start is called
+  // and OnBeforeRequest returns true (signalling that the request should be
+  // paused).
+  void BeforeRequestComplete(int error);
+
   void StartJob(URLRequestJob* job);
 
   // Restarting involves replacing the current job with a new one such as what
@@ -600,13 +609,6 @@ class URLRequest : public base::NonThreadSafe {
   // passed values.
   void DoCancel(int os_error, const SSLInfo& ssl_info);
 
-  // Resumes or blocks a request paused by the NetworkDelegate::OnBeforeRequest
-  // handler. If |blocked| is true, the request is blocked and an error page is
-  // returned indicating so. This should only be called after Start is called
-  // and OnBeforeRequest returns true (signalling that the request should be
-  // paused).
-  void BeforeRequestComplete(int error);
-
   // Contextual information used for this request (can be NULL). This contains
   // most of the dependencies which are shared between requests (disk cache,
   // cookie store, socket pool, etc.)
@@ -620,6 +622,7 @@ class URLRequest : public base::NonThreadSafe {
   GURL url_;
   GURL original_url_;
   GURL first_party_for_cookies_;
+  GURL delegate_redirect_url_;
   std::string method_;  // "GET", "POST", etc. Should be all uppercase.
   std::string referrer_;
   HttpRequestHeaders extra_request_headers_;
@@ -663,8 +666,7 @@ class URLRequest : public base::NonThreadSafe {
 
   // Callback passed to the network delegate to notify us when a blocked request
   // is ready to be resumed or canceled.
-  scoped_refptr< CancelableCompletionCallback<URLRequest> >
-      before_request_callback_;
+  CompletionCallbackImpl<URLRequest> before_request_callback_;
 
   DISALLOW_COPY_AND_ASSIGN(URLRequest);
 };
diff --git a/net/url_request/url_request_test_util.cc b/net/url_request/url_request_test_util.cc
index aedda63..a91460a 100644
--- a/net/url_request/url_request_test_util.cc
+++ b/net/url_request/url_request_test_util.cc
@@ -248,20 +248,25 @@ void TestDelegate::OnResponseCompleted(net::URLRequest* request) {
 
 TestNetworkDelegate::TestNetworkDelegate()
   : last_os_error_(0),
-    error_count_(0) {
+    error_count_(0),
+    created_requests_(0),
+    destroyed_requests_(0) {
 }
 
 TestNetworkDelegate::~TestNetworkDelegate() {}
 
 int TestNetworkDelegate::OnBeforeURLRequest(
-    net::URLRequest* request, net::CompletionCallback* callback) {
+    net::URLRequest* request,
+    net::CompletionCallback* callback,
+    GURL* new_url) {
+  created_requests_++;
   return net::OK;
 }
 
 int TestNetworkDelegate::OnBeforeSendHeaders(
     uint64 request_id,
-    net::HttpRequestHeaders* headers,
-    net::CompletionCallback* callback) {
+    net::CompletionCallback* callback,
+    net::HttpRequestHeaders* headers) {
   return net::OK;
 }
 
@@ -281,6 +286,7 @@ void TestNetworkDelegate::OnReadCompleted(net::URLRequest* request,
 }
 
 void TestNetworkDelegate::OnURLRequestDestroyed(net::URLRequest* request) {
+  destroyed_requests_++;
 }
 
 net::URLRequestJob* TestNetworkDelegate::OnMaybeCreateURLRequestJob(
diff --git a/net/url_request/url_request_test_util.h b/net/url_request/url_request_test_util.h
index 4d77d80..cef5449 100644
--- a/net/url_request/url_request_test_util.h
+++ b/net/url_request/url_request_test_util.h
@@ -190,14 +190,17 @@ class TestNetworkDelegate : public net::NetworkDelegate {
 
   int last_os_error() const { return last_os_error_; }
   int error_count() const { return error_count_; }
+  int created_requests() const { return created_requests_; }
+  int destroyed_requests() const { return destroyed_requests_; }
 
- private:
+ protected:
   // net::NetworkDelegate:
   virtual int OnBeforeURLRequest(net::URLRequest* request,
-                                 net::CompletionCallback* callback);
+                                 net::CompletionCallback* callback,
+                                 GURL* new_url);
   virtual int OnBeforeSendHeaders(uint64 request_id,
-                                  net::HttpRequestHeaders* headers,
-                                  net::CompletionCallback* callback);
+                                  net::CompletionCallback* callback,
+                                  net::HttpRequestHeaders* headers);
   virtual void OnResponseStarted(net::URLRequest* request);
   virtual void OnReadCompleted(net::URLRequest* request, int bytes_read);
   virtual void OnURLRequestDestroyed(net::URLRequest* request);
@@ -206,6 +209,8 @@ class TestNetworkDelegate : public net::NetworkDelegate {
 
   int last_os_error_;
   int error_count_;
+  int created_requests_;
+  int destroyed_requests_;
 };
 
 #endif  // NET_URL_REQUEST_URL_REQUEST_TEST_UTIL_H_
diff --git a/net/url_request/url_request_unittest.cc b/net/url_request/url_request_unittest.cc
index 394f813..7ecc847 100644
--- a/net/url_request/url_request_unittest.cc
+++ b/net/url_request/url_request_unittest.cc
@@ -53,6 +53,13 @@
 
 using base::Time;
 
+// We don't need a refcount because we are guaranteed the test will not proceed
+// until its task is run.
+namespace net {
+class BlockingNetworkDelegate;
+}  // namespace net
+DISABLE_RUNNABLE_METHOD_REFCOUNT(net::BlockingNetworkDelegate);
+
 namespace net {
 
 namespace {
@@ -118,6 +125,38 @@ void CheckSSLInfo(const SSLInfo& ssl_info) {
 
 }  // namespace
 
+// A network delegate that blocks requests, optionally cancelling or redirecting
+// them.
+class BlockingNetworkDelegate : public TestNetworkDelegate {
+ public:
+  BlockingNetworkDelegate() : callback_retval_(net::OK) {}
+
+  void set_callback_retval(int retval) { callback_retval_ = retval; }
+  void set_redirect_url(const GURL& url) { redirect_url_ = url; }
+
+ private:
+  // TestNetworkDelegate:
+  virtual int OnBeforeURLRequest(net::URLRequest* request,
+                                 net::CompletionCallback* callback,
+                                 GURL* new_url) {
+    TestNetworkDelegate::OnBeforeURLRequest(request, callback, new_url);
+
+    if (!redirect_url_.is_empty())
+      *new_url = redirect_url_;
+    MessageLoop::current()->PostTask(FROM_HERE,
+        NewRunnableMethod(this, &BlockingNetworkDelegate::DoCallback,
+                          callback));
+    return net::ERR_IO_PENDING;
+  }
+
+  void DoCallback(net::CompletionCallback* callback) {
+    callback->Run(callback_retval_);
+  }
+
+  int callback_retval_;
+  GURL redirect_url_;
+};
+
 // Inherit PlatformTest since we require the autorelease pool on Mac OS X.f
 class URLRequestTest : public PlatformTest {
  public:
@@ -263,6 +302,61 @@ TEST_F(URLRequestTestHTTP, NetworkDelegateTunnelConnectionFailed) {
   }
 }
 
+// Tests that the network delegate can block and cancel a request.
+TEST_F(URLRequestTestHTTP, NetworkDelegateCancelRequest) {
+  ASSERT_TRUE(test_server_.Start());
+
+  TestDelegate d;
+  BlockingNetworkDelegate network_delegate;
+  network_delegate.set_callback_retval(ERR_EMPTY_RESPONSE);
+
+  {
+    TestURLRequest r(test_server_.GetURL(""), &d);
+    scoped_refptr<TestURLRequestContext> context(
+        new TestURLRequestContext(test_server_.host_port_pair().ToString()));
+    context->set_network_delegate(&network_delegate);
+    r.set_context(context);
+
+    r.Start();
+    MessageLoop::current()->Run();
+
+    EXPECT_EQ(URLRequestStatus::FAILED, r.status().status());
+    EXPECT_EQ(ERR_EMPTY_RESPONSE, r.status().os_error());
+    EXPECT_EQ(1, network_delegate.created_requests());
+    EXPECT_EQ(0, network_delegate.destroyed_requests());
+  }
+  EXPECT_EQ(1, network_delegate.destroyed_requests());
+}
+
+// Tests that the network delegate can block and redirect a request to a new
+// URL.
+TEST_F(URLRequestTestHTTP, NetworkDelegateRedirectRequest) {
+  ASSERT_TRUE(test_server_.Start());
+
+  TestDelegate d;
+  BlockingNetworkDelegate network_delegate;
+  GURL redirect_url(test_server_.GetURL("simple.html"));
+  network_delegate.set_redirect_url(redirect_url);
+
+  {
+    TestURLRequest r(test_server_.GetURL("empty.html"), &d);
+    scoped_refptr<TestURLRequestContext> context(
+        new TestURLRequestContext(test_server_.host_port_pair().ToString()));
+    context->set_network_delegate(&network_delegate);
+    r.set_context(context);
+
+    r.Start();
+    MessageLoop::current()->Run();
+
+    EXPECT_EQ(URLRequestStatus::SUCCESS, r.status().status());
+    EXPECT_EQ(0, r.status().os_error());
+    EXPECT_EQ(redirect_url, r.url());
+    EXPECT_EQ(1, network_delegate.created_requests());
+    EXPECT_EQ(0, network_delegate.destroyed_requests());
+  }
+  EXPECT_EQ(1, network_delegate.destroyed_requests());
+}
+
 // In this unit test, we're using the HTTPTestServer as a proxy server and
 // issuing a CONNECT request with the magic host name "www.server-auth.com".
 // The HTTPTestServer will return a 401 response, which we should balk at.