Disable the http cache when fetching PAC scripts. Also adds an extra log statement for when response is non-200.

The cache is disabled to avoid problems when switching networks (wouldn't want to re-use cached response from old network after switching to new network). 

This is only a partial piece for 12293 (still needs to re-trigger auto-detect on network change).

BUG=12293

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

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

4 files changed, 46 insertions, 10 deletions

diff --git a/net/data/proxy_script_fetcher_unittest/cacheable_1hr.pac b/net/data/proxy_script_fetcher_unittest/cacheable_1hr.pac
new file mode 100644
index 0000000..f71b53a
--- /dev/null
+++ b/net/data/proxy_script_fetcher_unittest/cacheable_1hr.pac
@@ -0,0 +1 @@
+-cacheable_1hr.pac-
diff --git a/net/data/proxy_script_fetcher_unittest/cacheable_1hr.pac.mock-http-headers b/net/data/proxy_script_fetcher_unittest/cacheable_1hr.pac.mock-http-headers
new file mode 100644
index 0000000..5b64f73
--- /dev/null
+++ b/net/data/proxy_script_fetcher_unittest/cacheable_1hr.pac.mock-http-headers
@@ -0,0 +1,3 @@
+HTTP/1.1 200 OK
+Content-Type: application/x-javascript-config
+Cache-Control: public, max-age=3600
diff --git a/net/proxy/proxy_script_fetcher.cc b/net/proxy/proxy_script_fetcher.cc
index 87732c8..ec0c8b1 100644
--- a/net/proxy/proxy_script_fetcher.cc
+++ b/net/proxy/proxy_script_fetcher.cc
@@ -12,6 +12,7 @@
 #include "net/base/io_buffer.h"
 #include "net/base/load_flags.h"
 #include "net/base/net_errors.h"
+#include "net/http/http_response_headers.h"
 #include "net/url_request/url_request.h"
 
 // TODO(eroman):
@@ -152,7 +153,10 @@ void ProxyScriptFetcherImpl::Fetch(const GURL& url,
 
   // Make sure that the PAC script is downloaded using a direct connection,
   // to avoid circular dependencies (fetching is a part of proxy resolution).
-  cur_request_->set_load_flags(LOAD_BYPASS_PROXY);
+  // Also disable the use of the disk cache. The cache is disabled so that if
+  // the user switches networks we don't potentially use the cached response
+  // from old network when we should in fact be re-fetching on the new network.
+  cur_request_->set_load_flags(LOAD_BYPASS_PROXY | LOAD_DISABLE_CACHE);
 
   // Save the caller's info for notification on completion.
   callback_ = callback;
@@ -214,6 +218,8 @@ void ProxyScriptFetcherImpl::OnResponseStarted(URLRequest* request) {
     // NOTE about status codes: We are like Firefox 3 in this respect.
     // {IE 7, Safari 3, Opera 9.5} do not care about the status code.
     if (request->GetResponseCode() != 200) {
+      LOG(INFO) << "Fetched PAC script had (bad) status line: "
+                << request->response_headers()->GetStatusLine();
       result_code_ = ERR_PAC_STATUS_NOT_OK;
       request->Cancel();
       return;
@@ -222,13 +228,11 @@ void ProxyScriptFetcherImpl::OnResponseStarted(URLRequest* request) {
     // NOTE about mime types: We do not enforce mime types on PAC files.
     // This is for compatibility with {IE 7, Firefox 3, Opera 9.5}. We will
     // however log mismatches to help with debugging.
-    if (logging::GetMinLogLevel() <= logging::LOG_INFO) {
-      std::string mime_type;
-      cur_request_->GetMimeType(&mime_type);
-      if (!IsPacMimeType(mime_type)) {
-        LOG(INFO) << "Fetched PAC script does not have a proper mime type: "
-                  << mime_type;
-      }
+    std::string mime_type;
+    cur_request_->GetMimeType(&mime_type);
+    if (!IsPacMimeType(mime_type)) {
+      LOG(INFO) << "Fetched PAC script does not have a proper mime type: "
+                << mime_type;
     }
   }
 
diff --git a/net/proxy/proxy_script_fetcher_unittest.cc b/net/proxy/proxy_script_fetcher_unittest.cc
index 7d0cb83..3614cfe 100644
--- a/net/proxy/proxy_script_fetcher_unittest.cc
+++ b/net/proxy/proxy_script_fetcher_unittest.cc
@@ -8,6 +8,8 @@
 #include "base/compiler_specific.h"
 #include "base/path_service.h"
 #include "net/base/net_util.h"
+#include "net/disk_cache/disk_cache.h"
+#include "net/http/http_cache.h"
 #include "net/url_request/url_request_unittest.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "testing/platform_test.h"
@@ -29,8 +31,10 @@ class RequestContext : public URLRequestContext {
   RequestContext() {
     net::ProxyConfig no_proxy;
     proxy_service_ = net::ProxyService::CreateFixed(no_proxy);
-    http_transaction_factory_ = net::HttpNetworkLayer::CreateFactory(
-        proxy_service_);
+
+    http_transaction_factory_ =
+        new net::HttpCache(net::HttpNetworkLayer::CreateFactory(proxy_service_),
+                           disk_cache::CreateInMemoryCacheBackend(0));
   }
   ~RequestContext() {
     delete http_transaction_factory_;
@@ -240,6 +244,30 @@ TEST_F(ProxyScriptFetcherTest, ContentDisposition) {
   EXPECT_EQ("-downloadable.pac-\n", result.bytes);
 }
 
+TEST_F(ProxyScriptFetcherTest, NoCache) {
+  scoped_refptr<HTTPTestServer> server =
+      HTTPTestServer::CreateServer(kDocRoot, NULL);
+  ASSERT_TRUE(NULL != server.get());
+  SynchFetcher pac_fetcher;
+
+  // Fetch a PAC script whose HTTP headers make it cacheable for 1 hour.
+  GURL url = server->TestServerPage("files/cacheable_1hr.pac");
+  FetchResult result = pac_fetcher.Fetch(url);
+  EXPECT_EQ(net::OK, result.code);
+  EXPECT_EQ("-cacheable_1hr.pac-\n", result.bytes);
+
+  // Now kill the HTTP server.
+  server->SendQuit();
+  EXPECT_TRUE(server->WaitToFinish(20000));
+  server = NULL;
+
+  // Try to fetch the file again -- if should fail, since the server is not
+  // running anymore. (If it were instead being loaded from cache, we would
+  // get a success.
+  result = pac_fetcher.Fetch(url);
+  EXPECT_EQ(net::ERR_CONNECTION_REFUSED, result.code);
+}
+
 TEST_F(ProxyScriptFetcherTest, TooLarge) {
   scoped_refptr<HTTPTestServer> server =
       HTTPTestServer::CreateServer(kDocRoot, NULL);