Disable the http cache when fetching PAC scripts. Also adds an extra log stat...

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
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=17572

[eroman, 2009-05-29 22:21:44]

http://codereview.chromium.org/118032/diff/11/1002
File net/proxy/proxy_script_fetcher.cc (left):

http://codereview.chromium.org/118032/diff/11/1002#oldcode225
Line 225: if (logging::GetMinLogLevel() <= logging::LOG_INFO) {
Got rid of this guard, it was ugly and doesn't really help any (performance is
negligeable).

http://codereview.chromium.org/118032/diff/11/1002
File net/proxy/proxy_script_fetcher.cc (right):

http://codereview.chromium.org/118032/diff/11/1002#newcode222
Line 222: << request->response_headers()->GetStatusLine();
This was some feedback you gave me a while ago.

[wtc, 2009-05-29 23:31:39]

LGTM.

http://codereview.chromium.org/118032/diff/11/1002
File net/proxy/proxy_script_fetcher.cc (right):

http://codereview.chromium.org/118032/diff/11/1002#newcode159
Line 159: cur_request_->set_load_flags(LOAD_BYPASS_PROXY | LOAD_DISABLE_CACHE);
I wonder if we should only set the LOAD_DISABLE_CACHE
flag after a network change.  Perhaps that's premature
optimization?

http://codereview.chromium.org/118032/diff/11/1003
File net/proxy/proxy_script_fetcher_unittest.cc (right):

http://codereview.chromium.org/118032/diff/11/1003#newcode261
Line 261: EXPECT_TRUE(server->WaitToFinish(20000));
Does the server's destructor wait until the server's
shut down?  If so, we can just do server = NULL here.

[eroman, 2009-05-29 23:59:15]

http://codereview.chromium.org/118032/diff/11/1002
File net/proxy/proxy_script_fetcher.cc (right):

http://codereview.chromium.org/118032/diff/11/1002#newcode159
Line 159: cur_request_->set_load_flags(LOAD_BYPASS_PROXY | LOAD_DISABLE_CACHE);
On 2009/05/29 23:31:39, wtc wrote:
> I wonder if we should only set the LOAD_DISABLE_CACHE
> flag after a network change.  Perhaps that's premature
> optimization?

The scenario that might be tricker to support is detecting if the network was
changed while chrome wasn't running -- in this case, when chrome starts up on
new network we wouldn't want to use the PAC script found in the disk cache. By
not saving into the cache we avoid this problem.

I reckon this current approach is robust. Once we have a mechanism to be
notified of network changes, we can revisit.

http://codereview.chromium.org/118032/diff/11/1003
File net/proxy/proxy_script_fetcher_unittest.cc (right):

http://codereview.chromium.org/118032/diff/11/1003#newcode261
Line 261: EXPECT_TRUE(server->WaitToFinish(20000));
On 2009/05/29 23:31:39, wtc wrote:
> Does the server's destructor wait until the server's
> shut down?  If so, we can just do server = NULL here.

The destructor does not block until the server has finished exitting. (If I
don't call that wait there is a race in the test).