Add support to URLFetcher for stopping on redirects. Use this in AlternateNavURLFetcher in hopes of faster and more reliable detection of intranet sites.

Note that this does increase the risk that we'll start showing infobars all the time for users with ISPs that redirect bogus addresses to search landing pages.  In particular, if such ISPs use multi-stage redirects, rather than redirecting "bad" URLs directly, we'll start showing infobars where we wouldn't before.  I consider this a fairly low risk.

BUG=105312
TEST=On a new profile on the corp network, type in a valid "go/xxx" address (without scheme) and hit enter on the default choice (to search for the string).  Verify you get an infobar offering to navigate.
Review URL: https://chromiumcodereview.appspot.com/10543017

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

7 files changed, 126 insertions, 32 deletions

diff --git a/content/common/net/url_fetcher_core.cc b/content/common/net/url_fetcher_core.cc
index 34dfdbb..6393d47 100644
--- a/content/common/net/url_fetcher_core.cc
+++ b/content/common/net/url_fetcher_core.cc
@@ -273,6 +273,8 @@ URLFetcherCore::URLFetcherCore(net::URLFetcher* fetcher,
       num_retries_(0),
       was_cancelled_(false),
       response_destination_(STRING),
+      stop_on_redirect_(false),
+      stopped_on_redirect_(false),
       automatically_retry_on_5xx_(true),
       max_retries_(0),
       current_upload_bytes_(-1),
@@ -386,6 +388,10 @@ void URLFetcherCore::SetURLRequestUserData(
   url_request_create_data_callback_ = create_data_callback;
 }
 
+void URLFetcherCore::SetStopOnRedirect(bool stop_on_redirect) {
+  stop_on_redirect_ = true;
+}
+
 void URLFetcherCore::SetAutomaticallyRetryOn5xx(bool retry) {
   automatically_retry_on_5xx_ = retry;
 }
@@ -507,6 +513,21 @@ bool URLFetcherCore::GetResponseAsFilePath(bool take_ownership,
   return true;
 }
 
+void URLFetcherCore::OnReceivedRedirect(net::URLRequest* request,
+                                        const GURL& new_url,
+                                        bool* defer_redirect) {
+  DCHECK_EQ(request, request_.get());
+  DCHECK(io_message_loop_proxy_->BelongsToCurrentThread());
+  if (stop_on_redirect_) {
+    stopped_on_redirect_ = true;
+    url_ = new_url;
+    response_code_ = request_->GetResponseCode();
+    was_fetched_via_proxy_ = request_->was_fetched_via_proxy();
+    request->Cancel();
+    OnReadCompleted(request, 0);
+  }
+}
+
 void URLFetcherCore::OnResponseStarted(net::URLRequest* request) {
   DCHECK_EQ(request, request_.get());
   DCHECK(io_message_loop_proxy_->BelongsToCurrentThread());
@@ -526,7 +547,8 @@ void URLFetcherCore::OnReadCompleted(net::URLRequest* request,
   DCHECK(request == request_);
   DCHECK(io_message_loop_proxy_->BelongsToCurrentThread());
 
-  url_ = request->url();
+  if (!stopped_on_redirect_)
+    url_ = request->url();
   net::URLRequestThrottlerManager* throttler_manager =
       request->context()->throttler_manager();
   if (throttler_manager) {
diff --git a/content/common/net/url_fetcher_core.h b/content/common/net/url_fetcher_core.h
index 211dbe1..f970111 100644
--- a/content/common/net/url_fetcher_core.h
+++ b/content/common/net/url_fetcher_core.h
@@ -89,6 +89,7 @@ class URLFetcherCore
   void SetURLRequestUserData(
       const void* key,
       const net::URLFetcher::CreateDataCallback& create_data_callback);
+  void SetStopOnRedirect(bool stop_on_redirect);
   void SetAutomaticallyRetryOn5xx(bool retry);
   void SetMaxRetries(int max_retries);
   int GetMaxRetries() const;
@@ -119,10 +120,12 @@ class URLFetcherCore
                              FilePath* out_response_path);
 
   // Overridden from net::URLRequest::Delegate:
-  virtual void OnResponseStarted(
-      net::URLRequest* request) OVERRIDE;
-  virtual void OnReadCompleted(
-      net::URLRequest* request, int bytes_read) OVERRIDE;
+  virtual void OnReceivedRedirect(net::URLRequest* request,
+                                  const GURL& new_url,
+                                  bool* defer_redirect) OVERRIDE;
+  virtual void OnResponseStarted(net::URLRequest* request) OVERRIDE;
+  virtual void OnReadCompleted(net::URLRequest* request,
+                               int bytes_read) OVERRIDE;
 
   net::URLFetcherDelegate* delegate() const { return delegate_; }
   static void CancelAll();
@@ -170,8 +173,8 @@ class URLFetcherCore
    public:
     FileWriter(URLFetcherCore* core,
                scoped_refptr<base::MessageLoopProxy> file_message_loop_proxy);
-
     ~FileWriter();
+
     void CreateFileAtPath(const FilePath& file_path);
     void CreateTempFile();
 
@@ -370,6 +373,15 @@ class URLFetcherCore
   // Path to the file where the response is written.
   FilePath response_destination_file_path_;
 
+  // By default any server-initiated redirects are automatically followed.  If
+  // this flag is set to true, however, a redirect will halt the fetch and call
+  // back to to the delegate immediately.
+  bool stop_on_redirect_;
+  // True when we're actually stopped due to a redirect halted by the above.  We
+  // use this to ensure that |url_| is set to the redirect destination rather
+  // than the originally-fetched URL.
+  bool stopped_on_redirect_;
+
   // If |automatically_retry_on_5xx_| is false, 5xx responses will be
   // propagated to the observer, if it is true URLFetcher will automatically
   // re-execute the request, after the back-off delay has expired.
diff --git a/content/common/net/url_fetcher_impl.cc b/content/common/net/url_fetcher_impl.cc
index 4ed60b3..b502f5f 100644
--- a/content/common/net/url_fetcher_impl.cc
+++ b/content/common/net/url_fetcher_impl.cc
@@ -133,6 +133,10 @@ void URLFetcherImpl::SetURLRequestUserData(
   core_->SetURLRequestUserData(key, create_data_callback);
 }
 
+void URLFetcherImpl::SetStopOnRedirect(bool stop_on_redirect) {
+  core_->SetStopOnRedirect(stop_on_redirect);
+}
+
 void URLFetcherImpl::SetAutomaticallyRetryOn5xx(bool retry) {
   core_->SetAutomaticallyRetryOn5xx(retry);
 }
diff --git a/content/common/net/url_fetcher_impl.h b/content/common/net/url_fetcher_impl.h
index f661012..8b4e0fb 100644
--- a/content/common/net/url_fetcher_impl.h
+++ b/content/common/net/url_fetcher_impl.h
@@ -61,6 +61,7 @@ class CONTENT_EXPORT URLFetcherImpl : public net::URLFetcher {
   virtual void SetURLRequestUserData(
       const void* key,
       const CreateDataCallback& create_data_callback) OVERRIDE;
+  virtual void SetStopOnRedirect(bool stop_on_redirect) OVERRIDE;
   virtual void SetAutomaticallyRetryOn5xx(bool retry) OVERRIDE;
   virtual void SetMaxRetries(int max_retries) OVERRIDE;
   virtual int GetMaxRetries() const OVERRIDE;
diff --git a/content/common/net/url_fetcher_impl_unittest.cc b/content/common/net/url_fetcher_impl_unittest.cc
index d8bacd0..1b93aae 100644
--- a/content/common/net/url_fetcher_impl_unittest.cc
+++ b/content/common/net/url_fetcher_impl_unittest.cc
@@ -86,8 +86,14 @@ class URLFetcherTest : public testing::Test,
   virtual void CreateFetcher(const GURL& url);
 
   // net::URLFetcherDelegate
+  // Subclasses that override this should either call this function or
+  // CleanupAfterFetchComplete() at the end of their processing, depending on
+  // whether they want to check for a non-empty HTTP 200 response or not.
   virtual void OnURLFetchComplete(const net::URLFetcher* source) OVERRIDE;
 
+  // Deletes |fetcher| and terminates the message loop.
+  void CleanupAfterFetchComplete();
+
   scoped_refptr<base::MessageLoopProxy> io_message_loop_proxy() {
     return io_message_loop_proxy_;
   }
@@ -140,6 +146,10 @@ void URLFetcherTest::OnURLFetchComplete(const net::URLFetcher* source) {
   EXPECT_TRUE(source->GetResponseAsString(&data));
   EXPECT_FALSE(data.empty());
 
+  CleanupAfterFetchComplete();
+}
+
+void URLFetcherTest::CleanupAfterFetchComplete() {
   delete fetcher_;  // Have to delete this here and not in the destructor,
                     // because the destructor won't necessarily run on the
                     // same thread that CreateFetcher() did.
@@ -220,6 +230,24 @@ class URLFetcherSocketAddressTest : public URLFetcherTest {
   uint16 expected_port_;
 };
 
+// Version of URLFetcherTest that tests stopping on a redirect.
+class URLFetcherStopOnRedirectTest : public URLFetcherTest {
+ public:
+  URLFetcherStopOnRedirectTest();
+  virtual ~URLFetcherStopOnRedirectTest();
+
+  // URLFetcherTest override.
+  virtual void CreateFetcher(const GURL& url) OVERRIDE;
+  // net::URLFetcherDelegate
+  virtual void OnURLFetchComplete(const net::URLFetcher* source) OVERRIDE;
+
+ protected:
+  // The URL we should be redirected to.
+  static const char* kRedirectTarget;
+
+  bool callback_called_;  // Set to true in OnURLFetchComplete().
+};
+
 // Version of URLFetcherTest that tests overload protection.
 class URLFetcherProtectTest : public URLFetcherTest {
  public:
@@ -409,9 +437,8 @@ void URLFetcherDownloadProgressCancelTest::OnURLFetchDownloadProgress(
     const net::URLFetcher* source, int64 current, int64 total) {
   EXPECT_FALSE(cancelled_);
   if (!cancelled_) {
-    delete fetcher_;
     cancelled_ = true;
-    io_message_loop_proxy()->PostTask(FROM_HERE, MessageLoop::QuitClosure());
+    CleanupAfterFetchComplete();
   }
 }
 
@@ -419,8 +446,7 @@ void URLFetcherDownloadProgressCancelTest::OnURLFetchComplete(
     const net::URLFetcher* source) {
   // Should have been cancelled.
   ADD_FAILURE();
-  delete fetcher_;
-  io_message_loop_proxy()->PostTask(FROM_HERE, MessageLoop::QuitClosure());
+  CleanupAfterFetchComplete();
 }
 
 void URLFetcherUploadProgressTest::CreateFetcher(const GURL& url) {
@@ -469,6 +495,34 @@ void URLFetcherSocketAddressTest::OnURLFetchComplete(
   URLFetcherTest::OnURLFetchComplete(source);
 }
 
+// static
+const char* URLFetcherStopOnRedirectTest::kRedirectTarget =
+    "http://redirect.target.com";
+
+URLFetcherStopOnRedirectTest::URLFetcherStopOnRedirectTest()
+    : callback_called_(false) {
+}
+
+URLFetcherStopOnRedirectTest::~URLFetcherStopOnRedirectTest() {
+}
+
+void URLFetcherStopOnRedirectTest::CreateFetcher(const GURL& url) {
+  fetcher_ = new URLFetcherImpl(url, net::URLFetcher::GET, this);
+  fetcher_->SetRequestContext(new ThrottlingTestURLRequestContextGetter(
+      io_message_loop_proxy(), request_context()));
+  fetcher_->SetStopOnRedirect(true);
+  fetcher_->Start();
+}
+
+void URLFetcherStopOnRedirectTest::OnURLFetchComplete(
+    const net::URLFetcher* source) {
+  callback_called_ = true;
+  EXPECT_EQ(GURL(kRedirectTarget), source->GetURL());
+  EXPECT_EQ(net::URLRequestStatus::CANCELED, source->GetStatus().status());
+  EXPECT_EQ(301, source->GetResponseCode());
+  CleanupAfterFetchComplete();
+}
+
 void URLFetcherProtectTest::CreateFetcher(const GURL& url) {
   fetcher_ = new URLFetcherImpl(url, net::URLFetcher::GET, this);
   fetcher_->SetRequestContext(new ThrottlingTestURLRequestContextGetter(
@@ -478,8 +532,7 @@ void URLFetcherProtectTest::CreateFetcher(const GURL& url) {
   fetcher_->Start();
 }
 
-void URLFetcherProtectTest::OnURLFetchComplete(
-    const net::URLFetcher* source) {
+void URLFetcherProtectTest::OnURLFetchComplete(const net::URLFetcher* source) {
   const TimeDelta one_second = TimeDelta::FromMilliseconds(1000);
   if (source->GetResponseCode() >= 500) {
     // Now running ServerUnavailable test.
@@ -489,8 +542,7 @@ void URLFetcherProtectTest::OnURLFetchComplete(
     std::string data;
     EXPECT_TRUE(source->GetResponseAsString(&data));
     EXPECT_FALSE(data.empty());
-    delete fetcher_;
-    io_message_loop_proxy()->PostTask(FROM_HERE, MessageLoop::QuitClosure());
+    CleanupAfterFetchComplete();
   } else {
     // Now running Overload test.
     static int count = 0;
@@ -538,8 +590,7 @@ void URLFetcherProtectTestPassedThrough::OnURLFetchComplete(
     ADD_FAILURE();
   }
 
-  delete fetcher_;
-  io_message_loop_proxy()->PostTask(FROM_HERE, MessageLoop::QuitClosure());
+  CleanupAfterFetchComplete();
 }
 
 
@@ -566,10 +617,7 @@ void URLFetcherBadHTTPSTest::OnURLFetchComplete(
   std::string data;
   EXPECT_TRUE(source->GetResponseAsString(&data));
   EXPECT_TRUE(data.empty());
-
-  // The rest is the same as URLFetcherTest::OnURLFetchComplete.
-  delete fetcher_;
-  io_message_loop_proxy()->PostTask(FROM_HERE, MessageLoop::QuitClosure());
+  CleanupAfterFetchComplete();
 }
 
 void URLFetcherCancelTest::CreateFetcher(const GURL& url) {
@@ -590,8 +638,7 @@ void URLFetcherCancelTest::OnURLFetchComplete(
     const net::URLFetcher* source) {
   // We should have cancelled the request before completion.
   ADD_FAILURE();
-  delete fetcher_;
-  io_message_loop_proxy()->PostTask(FROM_HERE, MessageLoop::QuitClosure());
+  CleanupAfterFetchComplete();
 }
 
 void URLFetcherCancelTest::CancelRequest() {
@@ -615,13 +662,7 @@ void URLFetcherMultipleAttemptTest::OnURLFetchComplete(
     fetcher_->Start();
   } else {
     EXPECT_EQ(data, data_);
-    delete fetcher_;  // Have to delete this here and not in the destructor,
-                      // because the destructor won't necessarily run on the
-                      // same thread that CreateFetcher() did.
-
-    io_message_loop_proxy()->PostTask(FROM_HERE, MessageLoop::QuitClosure());
-    // If the current message loop is not the IO loop, it will be shut down when
-    // the main loop returns and this thread subsequently goes out of scope.
+    CleanupAfterFetchComplete();
   }
 }
 
@@ -663,9 +704,7 @@ void URLFetcherFileTest::OnURLFetchComplete(const net::URLFetcher* source) {
     EXPECT_TRUE(fetcher_->FileErrorOccurred(&error_code));
     EXPECT_EQ(expected_file_error_, error_code);
   }
-  delete fetcher_;
-
-  io_message_loop_proxy()->PostTask(FROM_HERE, MessageLoop::QuitClosure());
+  CleanupAfterFetchComplete();
 }
 
 TEST_F(URLFetcherTest, SameThreadsTest) {
@@ -821,6 +860,18 @@ TEST_F(URLFetcherSocketAddressTest, SocketAddress) {
   // The actual tests are in the URLFetcherSocketAddressTest fixture.
 }
 
+TEST_F(URLFetcherStopOnRedirectTest, StopOnRedirect) {
+  net::TestServer test_server(net::TestServer::TYPE_HTTP,
+                              net::TestServer::kLocalhost,
+                              FilePath(kDocRoot));
+  ASSERT_TRUE(test_server.Start());
+
+  CreateFetcher(
+      test_server.GetURL(std::string("server-redirect?") + kRedirectTarget));
+  MessageLoop::current()->Run();
+  EXPECT_TRUE(callback_called_);
+}
+
 TEST_F(URLFetcherProtectTest, Overload) {
   net::TestServer test_server(net::TestServer::TYPE_HTTP,
                               net::TestServer::kLocalhost,
diff --git a/content/public/test/test_url_fetcher_factory.h b/content/public/test/test_url_fetcher_factory.h
index 62c0735..66706ff 100644
--- a/content/public/test/test_url_fetcher_factory.h
+++ b/content/public/test/test_url_fetcher_factory.h
@@ -85,6 +85,7 @@ class TestURLFetcher : public net::URLFetcher {
   virtual void SetURLRequestUserData(
       const void* key,
       const CreateDataCallback& create_data_callback) OVERRIDE;
+  virtual void SetStopOnRedirect(bool stop_on_redirect) OVERRIDE;
   virtual void SetAutomaticallyRetryOn5xx(bool retry) OVERRIDE;
   virtual void SetMaxRetries(int max_retries) OVERRIDE;
   virtual int GetMaxRetries() const OVERRIDE;
diff --git a/content/test/test_url_fetcher_factory.cc b/content/test/test_url_fetcher_factory.cc
index 5f174e5..1886dd8 100644
--- a/content/test/test_url_fetcher_factory.cc
+++ b/content/test/test_url_fetcher_factory.cc
@@ -98,6 +98,9 @@ void TestURLFetcher::SetURLRequestUserData(
     const CreateDataCallback& create_data_callback) {
 }
 
+void TestURLFetcher::SetStopOnRedirect(bool stop_on_redirect) {
+}
+
 void TestURLFetcher::SetAutomaticallyRetryOn5xx(bool retry) {
 }