Skip to content

Commit

Permalink
Introduce an artificial 2 second delay after network IP address chang…
Browse files Browse the repository at this point in the history 
…es before re-running proxy auto-config.

During this time network requests will be stalled.

This is to work around a problem where DNS gives transient failures shortly after IP address changes.

BUG=50779
TEST=On a linux laptop switch between wireless networks while using auto-detect. When you switch to a network that contains the host 'wpad' verify that when InitProxyResolver runs it does not get a DNS error resolving 'wpad'. (Use about:net-internals to view this information).

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

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@57471 0039d316-1c4b-4281-b951-d872f2087c98
  • Loading branch information
eroman@chromium.org committed Aug 26, 2010
1 parent 924cad8 commit 1b8740f
Show file tree
Hide file tree
Showing 7 changed files with 213 additions and 13 deletions.
5 changes: 5 additions & 0 deletions net/base/net_log_event_type_list.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,11 @@ EVENT_TYPE(HOST_RESOLVER_IMPL_JOB)
// The start/end of auto-detect + custom PAC URL configuration.
EVENT_TYPE(INIT_PROXY_RESOLVER)

// The start/end of when proxy autoconfig was artificially paused following
// a network change event. (We wait some amount of time after being told of
// network changes to avoid hitting spurious errors during auto-detect).
EVENT_TYPE(INIT_PROXY_RESOLVER_WAIT)

// The start/end of download of a PAC script. This could be the well-known
// WPAD URL (if testing auto-detect), or a custom PAC URL.
//
Expand Down
43 changes: 42 additions & 1 deletion net/proxy/init_proxy_resolver.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,17 +36,23 @@ InitProxyResolver::~InitProxyResolver() {
}

int InitProxyResolver::Init(const ProxyConfig& config,
const base::TimeDelta wait_delay,
CompletionCallback* callback) {
DCHECK_EQ(STATE_NONE, next_state_);
DCHECK(callback);
DCHECK(config.MayRequirePACResolver());

net_log_.BeginEvent(NetLog::TYPE_INIT_PROXY_RESOLVER, NULL);

// Save the |wait_delay| as a non-negative value.
wait_delay_ = wait_delay;
if (wait_delay_ < base::TimeDelta())
wait_delay_ = base::TimeDelta();

pac_urls_ = BuildPacUrlsFallbackList(config);
DCHECK(!pac_urls_.empty());

next_state_ = GetStartState();
next_state_ = STATE_WAIT;

int rv = DoLoop(OK);
if (rv == ERR_IO_PENDING)
Expand Down Expand Up @@ -86,6 +92,13 @@ int InitProxyResolver::DoLoop(int result) {
State state = next_state_;
next_state_ = STATE_NONE;
switch (state) {
case STATE_WAIT:
DCHECK_EQ(OK, rv);
rv = DoWait();
break;
case STATE_WAIT_COMPLETE:
rv = DoWaitComplete(rv);
break;
case STATE_FETCH_PAC_SCRIPT:
DCHECK_EQ(OK, rv);
rv = DoFetchPacScript();
Expand Down Expand Up @@ -115,6 +128,27 @@ void InitProxyResolver::DoCallback(int result) {
user_callback_->Run(result);
}

int InitProxyResolver::DoWait() {
next_state_ = STATE_WAIT_COMPLETE;

// If no waiting is required, continue on to the next state.
if (wait_delay_.ToInternalValue() == 0)
return OK;

// Otherwise wait the specified amount of time.
wait_timer_.Start(wait_delay_, this, &InitProxyResolver::OnWaitTimerFired);
net_log_.BeginEvent(NetLog::TYPE_INIT_PROXY_RESOLVER_WAIT, NULL);
return ERR_IO_PENDING;
}

int InitProxyResolver::DoWaitComplete(int result) {
DCHECK_EQ(OK, result);
if (wait_delay_.ToInternalValue() != 0)
net_log_.EndEvent(NetLog::TYPE_INIT_PROXY_RESOLVER_WAIT, NULL);
next_state_ = GetStartState();
return OK;
}

int InitProxyResolver::DoFetchPacScript() {
DCHECK(resolver_->expects_pac_bytes());

Expand Down Expand Up @@ -217,6 +251,10 @@ const InitProxyResolver::PacURL& InitProxyResolver::current_pac_url() const {
return pac_urls_[current_pac_url_index_];
}

void InitProxyResolver::OnWaitTimerFired() {
OnIOCompletion(OK);
}

void InitProxyResolver::DidCompleteInit() {
net_log_.EndEvent(NetLog::TYPE_INIT_PROXY_RESOLVER, NULL);
}
Expand All @@ -227,6 +265,9 @@ void InitProxyResolver::Cancel() {
net_log_.AddEvent(NetLog::TYPE_CANCELLED, NULL);

switch (next_state_) {
case STATE_WAIT_COMPLETE:
wait_timer_.Stop();
break;
case STATE_FETCH_PAC_SCRIPT_COMPLETE:
proxy_script_fetcher_->Cancel();
break;
Expand Down
14 changes: 14 additions & 0 deletions net/proxy/init_proxy_resolver.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@
#include <vector>

#include "base/string16.h"
#include "base/time.h"
#include "base/timer.h"
#include "googleurl/src/gurl.h"
#include "net/base/completion_callback.h"
#include "net/base/net_log.h"
Expand Down Expand Up @@ -47,12 +49,17 @@ class InitProxyResolver {
~InitProxyResolver();

// Apply the PAC settings of |config| to |resolver_|.
// If |wait_delay| is positive, the initialization will pause for this
// amount of time before getting started.
int Init(const ProxyConfig& config,
const base::TimeDelta wait_delay,
CompletionCallback* callback);

private:
enum State {
STATE_NONE,
STATE_WAIT,
STATE_WAIT_COMPLETE,
STATE_FETCH_PAC_SCRIPT,
STATE_FETCH_PAC_SCRIPT_COMPLETE,
STATE_SET_PAC_SCRIPT,
Expand All @@ -75,6 +82,9 @@ class InitProxyResolver {
int DoLoop(int result);
void DoCallback(int result);

int DoWait();
int DoWaitComplete(int result);

int DoFetchPacScript();
int DoFetchPacScriptComplete(int result);

Expand All @@ -94,6 +104,7 @@ class InitProxyResolver {
// Returns the current PAC URL we are fetching/testing.
const PacURL& current_pac_url() const;

void OnWaitTimerFired();
void DidCompleteInit();
void Cancel();

Expand All @@ -113,6 +124,9 @@ class InitProxyResolver {

BoundNetLog net_log_;

base::TimeDelta wait_delay_;
base::OneShotTimer<InitProxyResolver> wait_timer_;

DISALLOW_COPY_AND_ASSIGN(InitProxyResolver);
};

Expand Down
93 changes: 83 additions & 10 deletions net/proxy/init_proxy_resolver_unittest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,7 @@ TEST(InitProxyResolverTest, CustomPacSucceeds) {
TestCompletionCallback callback;
CapturingNetLog log(CapturingNetLog::kUnbounded);
InitProxyResolver init(&resolver, &fetcher, &log);
EXPECT_EQ(OK, init.Init(config, &callback));
EXPECT_EQ(OK, init.Init(config, base::TimeDelta(), &callback));
EXPECT_EQ(rule.text(), resolver.script_data()->utf16());

// Check the NetLog was filled correctly.
Expand Down Expand Up @@ -210,7 +210,8 @@ TEST(InitProxyResolverTest, CustomPacFails1) {
TestCompletionCallback callback;
CapturingNetLog log(CapturingNetLog::kUnbounded);
InitProxyResolver init(&resolver, &fetcher, &log);
EXPECT_EQ(kFailedDownloading, init.Init(config, &callback));
EXPECT_EQ(kFailedDownloading,
init.Init(config, base::TimeDelta(), &callback));
EXPECT_EQ(NULL, resolver.script_data());

// Check the NetLog was filled correctly.
Expand Down Expand Up @@ -238,7 +239,7 @@ TEST(InitProxyResolverTest, CustomPacFails2) {

TestCompletionCallback callback;
InitProxyResolver init(&resolver, &fetcher, NULL);
EXPECT_EQ(kFailedParsing, init.Init(config, &callback));
EXPECT_EQ(kFailedParsing, init.Init(config, base::TimeDelta(), &callback));
EXPECT_EQ(NULL, resolver.script_data());
}

Expand All @@ -252,7 +253,7 @@ TEST(InitProxyResolverTest, HasNullProxyScriptFetcher) {

TestCompletionCallback callback;
InitProxyResolver init(&resolver, NULL, NULL);
EXPECT_EQ(ERR_UNEXPECTED, init.Init(config, &callback));
EXPECT_EQ(ERR_UNEXPECTED, init.Init(config, base::TimeDelta(), &callback));
EXPECT_EQ(NULL, resolver.script_data());
}

Expand All @@ -269,7 +270,7 @@ TEST(InitProxyResolverTest, AutodetectSuccess) {

TestCompletionCallback callback;
InitProxyResolver init(&resolver, &fetcher, NULL);
EXPECT_EQ(OK, init.Init(config, &callback));
EXPECT_EQ(OK, init.Init(config, base::TimeDelta(), &callback));
EXPECT_EQ(rule.text(), resolver.script_data()->utf16());
}

Expand All @@ -288,7 +289,7 @@ TEST(InitProxyResolverTest, AutodetectFailCustomSuccess1) {

TestCompletionCallback callback;
InitProxyResolver init(&resolver, &fetcher, NULL);
EXPECT_EQ(OK, init.Init(config, &callback));
EXPECT_EQ(OK, init.Init(config, base::TimeDelta(), &callback));
EXPECT_EQ(rule.text(), resolver.script_data()->utf16());
}

Expand All @@ -308,7 +309,7 @@ TEST(InitProxyResolverTest, AutodetectFailCustomSuccess2) {
TestCompletionCallback callback;
CapturingNetLog log(CapturingNetLog::kUnbounded);
InitProxyResolver init(&resolver, &fetcher, &log);
EXPECT_EQ(OK, init.Init(config, &callback));
EXPECT_EQ(OK, init.Init(config, base::TimeDelta(), &callback));
EXPECT_EQ(rule.text(), resolver.script_data()->utf16());

// Check the NetLog was filled correctly.
Expand Down Expand Up @@ -356,7 +357,8 @@ TEST(InitProxyResolverTest, AutodetectFailCustomFails1) {

TestCompletionCallback callback;
InitProxyResolver init(&resolver, &fetcher, NULL);
EXPECT_EQ(kFailedDownloading, init.Init(config, &callback));
EXPECT_EQ(kFailedDownloading,
init.Init(config, base::TimeDelta(), &callback));
EXPECT_EQ(NULL, resolver.script_data());
}

Expand All @@ -375,7 +377,7 @@ TEST(InitProxyResolverTest, AutodetectFailCustomFails2) {

TestCompletionCallback callback;
InitProxyResolver init(&resolver, &fetcher, NULL);
EXPECT_EQ(kFailedParsing, init.Init(config, &callback));
EXPECT_EQ(kFailedParsing, init.Init(config, base::TimeDelta(), &callback));
EXPECT_EQ(NULL, resolver.script_data());
}

Expand All @@ -396,9 +398,80 @@ TEST(InitProxyResolverTest, AutodetectFailCustomSuccess2_NoFetch) {

TestCompletionCallback callback;
InitProxyResolver init(&resolver, &fetcher, NULL);
EXPECT_EQ(OK, init.Init(config, &callback));
EXPECT_EQ(OK, init.Init(config, base::TimeDelta(), &callback));
EXPECT_EQ(rule.url, resolver.script_data()->url());
}

// This is a copy-paste of CustomPacFails1, with the exception that we give it
// a 1 millisecond delay. This means it will now complete asynchronously.
// Moreover, we test the NetLog to make sure it logged the pause.
TEST(InitProxyResolverTest, CustomPacFails1_WithPositiveDelay) {
Rules rules;
RuleBasedProxyResolver resolver(&rules, true /*expects_pac_bytes*/);
RuleBasedProxyScriptFetcher fetcher(&rules);

ProxyConfig config;
config.set_pac_url(GURL("http://custom/proxy.pac"));

rules.AddFailDownloadRule("http://custom/proxy.pac");

TestCompletionCallback callback;
CapturingNetLog log(CapturingNetLog::kUnbounded);
InitProxyResolver init(&resolver, &fetcher, &log);
EXPECT_EQ(ERR_IO_PENDING,
init.Init(config, base::TimeDelta::FromMilliseconds(1),
&callback));

EXPECT_EQ(kFailedDownloading, callback.WaitForResult());
EXPECT_EQ(NULL, resolver.script_data());

// Check the NetLog was filled correctly.
EXPECT_EQ(6u, log.entries().size());
EXPECT_TRUE(LogContainsBeginEvent(
log.entries(), 0, NetLog::TYPE_INIT_PROXY_RESOLVER));
EXPECT_TRUE(LogContainsBeginEvent(
log.entries(), 1, NetLog::TYPE_INIT_PROXY_RESOLVER_WAIT));
EXPECT_TRUE(LogContainsEndEvent(
log.entries(), 2, NetLog::TYPE_INIT_PROXY_RESOLVER_WAIT));
EXPECT_TRUE(LogContainsBeginEvent(
log.entries(), 3, NetLog::TYPE_INIT_PROXY_RESOLVER_FETCH_PAC_SCRIPT));
EXPECT_TRUE(LogContainsEndEvent(
log.entries(), 4, NetLog::TYPE_INIT_PROXY_RESOLVER_FETCH_PAC_SCRIPT));
EXPECT_TRUE(LogContainsEndEvent(
log.entries(), 5, NetLog::TYPE_INIT_PROXY_RESOLVER));
}

// This is a copy-paste of CustomPacFails1, with the exception that we give it
// a -5 second delay instead of a 0 ms delay. This change should have no effect
// so the rest of the test is unchanged.
TEST(InitProxyResolverTest, CustomPacFails1_WithNegativeDelay) {
Rules rules;
RuleBasedProxyResolver resolver(&rules, true /*expects_pac_bytes*/);
RuleBasedProxyScriptFetcher fetcher(&rules);

ProxyConfig config;
config.set_pac_url(GURL("http://custom/proxy.pac"));

rules.AddFailDownloadRule("http://custom/proxy.pac");

TestCompletionCallback callback;
CapturingNetLog log(CapturingNetLog::kUnbounded);
InitProxyResolver init(&resolver, &fetcher, &log);
EXPECT_EQ(kFailedDownloading,
init.Init(config, base::TimeDelta::FromSeconds(-5), &callback));
EXPECT_EQ(NULL, resolver.script_data());

// Check the NetLog was filled correctly.
EXPECT_EQ(4u, log.entries().size());
EXPECT_TRUE(LogContainsBeginEvent(
log.entries(), 0, NetLog::TYPE_INIT_PROXY_RESOLVER));
EXPECT_TRUE(LogContainsBeginEvent(
log.entries(), 1, NetLog::TYPE_INIT_PROXY_RESOLVER_FETCH_PAC_SCRIPT));
EXPECT_TRUE(LogContainsEndEvent(
log.entries(), 2, NetLog::TYPE_INIT_PROXY_RESOLVER_FETCH_PAC_SCRIPT));
EXPECT_TRUE(LogContainsEndEvent(
log.entries(), 3, NetLog::TYPE_INIT_PROXY_RESOLVER));
}

} // namespace
} // namespace net
Loading

0 comments on commit 1b8740f

Please sign in to comment.