Skip to content

Commit

Permalink
Reference-count the data used by PAC scripts, so it is shared between…
Browse files Browse the repository at this point in the history 
… threads.

BUG=49396
Review URL: http://codereview.chromium.org/2836060

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@53095 0039d316-1c4b-4281-b951-d872f2087c98
  • Loading branch information
eroman@chromium.org committed Jul 20, 2010
1 parent 2890050 commit 2447640
Show file tree
Hide file tree
Showing 23 changed files with 364 additions and 253 deletions.
9 changes: 4 additions & 5 deletions net/http/http_network_transaction_unittest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5297,15 +5297,14 @@ class CapturingProxyResolver : public ProxyResolver {
NOTREACHED();
}

const std::vector<GURL>& resolved() const { return resolved_; }

private:
virtual int SetPacScript(const GURL& /*pac_url*/,
const string16& /*pac_script*/,
virtual int SetPacScript(const scoped_refptr<ProxyResolverScriptData>&,
CompletionCallback* /*callback*/) {
return OK;
}

const std::vector<GURL>& resolved() const { return resolved_; }

private:
std::vector<GURL> resolved_;

DISALLOW_COPY_AND_ASSIGN(CapturingProxyResolver);
Expand Down
2 changes: 2 additions & 0 deletions net/net.gyp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -434,6 +434,8 @@
'proxy/proxy_resolver_mac.h',
'proxy/proxy_resolver_request_context.h',
'proxy/proxy_resolver_script.h',
'proxy/proxy_resolver_script_data.cc',
'proxy/proxy_resolver_script_data.h',
'proxy/proxy_resolver_v8.cc',
'proxy/proxy_resolver_v8.h',
'proxy/proxy_resolver_winhttp.cc',
Expand Down
39 changes: 25 additions & 14 deletions net/proxy/init_proxy_resolver.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -63,13 +63,10 @@ int InitProxyResolver::Init(const ProxyConfig& config,
InitProxyResolver::UrlList InitProxyResolver::BuildPacUrlsFallbackList(
const ProxyConfig& config) const {
UrlList pac_urls;
if (config.auto_detect()) {
GURL pac_url = resolver_->expects_pac_bytes() ?
GURL("http://wpad/wpad.dat") : GURL();
pac_urls.push_back(pac_url);
}
if (config.auto_detect())
pac_urls.push_back(PacURL(true, GURL()));
if (config.has_pac_url())
pac_urls.push_back(config.pac_url());
pac_urls.push_back(PacURL(false, config.pac_url()));
return pac_urls;
}

Expand Down Expand Up @@ -123,18 +120,24 @@ int InitProxyResolver::DoFetchPacScript() {

next_state_ = STATE_FETCH_PAC_SCRIPT_COMPLETE;

const GURL& pac_url = current_pac_url();
const PacURL& pac_url = current_pac_url();

const GURL effective_pac_url =
pac_url.auto_detect ? GURL("http://wpad/wpad.dat") : pac_url.url;

net_log_.BeginEvent(
NetLog::TYPE_INIT_PROXY_RESOLVER_FETCH_PAC_SCRIPT,
new NetLogStringParameter("url", pac_url.spec()));
new NetLogStringParameter("url",
effective_pac_url.possibly_invalid_spec()));

if (!proxy_script_fetcher_) {
net_log_.AddEvent(NetLog::TYPE_INIT_PROXY_RESOLVER_HAS_NO_FETCHER, NULL);
return ERR_UNEXPECTED;
}

return proxy_script_fetcher_->Fetch(pac_url, &pac_script_, &io_callback_);
return proxy_script_fetcher_->Fetch(effective_pac_url,
&pac_script_,
&io_callback_);
}

int InitProxyResolver::DoFetchPacScriptComplete(int result) {
Expand All @@ -156,13 +159,21 @@ int InitProxyResolver::DoFetchPacScriptComplete(int result) {
int InitProxyResolver::DoSetPacScript() {
net_log_.BeginEvent(NetLog::TYPE_INIT_PROXY_RESOLVER_SET_PAC_SCRIPT, NULL);

const GURL& pac_url = current_pac_url();
const PacURL& pac_url = current_pac_url();

next_state_ = STATE_SET_PAC_SCRIPT_COMPLETE;

return resolver_->expects_pac_bytes() ?
resolver_->SetPacScriptByData(pac_script_, &io_callback_) :
resolver_->SetPacScriptByUrl(pac_url, &io_callback_);
scoped_refptr<ProxyResolverScriptData> script_data;

if (resolver_->expects_pac_bytes()) {
script_data = ProxyResolverScriptData::FromUTF16(pac_script_);
} else {
script_data = pac_url.auto_detect ?
ProxyResolverScriptData::ForAutoDetect() :
ProxyResolverScriptData::FromURL(pac_url.url);
}

return resolver_->SetPacScript(script_data, &io_callback_);
}

int InitProxyResolver::DoSetPacScriptComplete(int result) {
Expand Down Expand Up @@ -201,7 +212,7 @@ InitProxyResolver::State InitProxyResolver::GetStartState() const {
STATE_FETCH_PAC_SCRIPT : STATE_SET_PAC_SCRIPT;
}

const GURL& InitProxyResolver::current_pac_url() const {
const InitProxyResolver::PacURL& InitProxyResolver::current_pac_url() const {
DCHECK_LT(current_pac_url_index_, pac_urls_.size());
return pac_urls_[current_pac_url_index_];
}
Expand Down
12 changes: 10 additions & 2 deletions net/proxy/init_proxy_resolver.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,15 @@ class InitProxyResolver {
STATE_SET_PAC_SCRIPT,
STATE_SET_PAC_SCRIPT_COMPLETE,
};
typedef std::vector<GURL> UrlList;

struct PacURL {
PacURL(bool auto_detect, const GURL& url)
: auto_detect(auto_detect), url(url) {}
bool auto_detect;
GURL url;
};

typedef std::vector<PacURL> UrlList;

// Returns ordered list of PAC urls to try for |config|.
UrlList BuildPacUrlsFallbackList(const ProxyConfig& config) const;
Expand All @@ -83,7 +91,7 @@ class InitProxyResolver {
State GetStartState() const;

// Returns the current PAC URL we are fetching/testing.
const GURL& current_pac_url() const;
const PacURL& current_pac_url() const;

void DidCompleteInit();
void Cancel();
Expand Down
56 changes: 29 additions & 27 deletions net/proxy/init_proxy_resolver_unittest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -130,35 +130,37 @@ class RuleBasedProxyResolver : public ProxyResolver {
NOTREACHED();
}

virtual int SetPacScript(const GURL& pac_url,
const string16& pac_script,
CompletionCallback* callback) {
virtual int SetPacScript(
const scoped_refptr<ProxyResolverScriptData>& script_data,
CompletionCallback* callback) {

const GURL url =
script_data->type() == ProxyResolverScriptData::TYPE_SCRIPT_URL ?
script_data->url() : GURL();

const Rules::Rule& rule = expects_pac_bytes() ?
rules_->GetRuleByText(pac_script) :
rules_->GetRuleByUrl(pac_url);
rules_->GetRuleByText(script_data->utf16()) :
rules_->GetRuleByUrl(url);

int rv = rule.set_pac_error;
EXPECT_NE(ERR_UNEXPECTED, rv);

if (expects_pac_bytes())
EXPECT_EQ(rule.text(), pac_script);
else
EXPECT_EQ(rule.url, pac_url);

if (rv == OK) {
pac_script_ = pac_script;
pac_url_ = pac_url;
if (expects_pac_bytes()) {
EXPECT_EQ(rule.text(), script_data->utf16());
} else {
EXPECT_EQ(rule.url, url);
}

if (rv == OK)
script_data_ = script_data;
return rv;
}

const string16& pac_script() const { return pac_script_; }
const GURL& pac_url() const { return pac_url_; }
const ProxyResolverScriptData* script_data() const { return script_data_; }

private:
const Rules* rules_;
string16 pac_script_;
GURL pac_url_;
scoped_refptr<ProxyResolverScriptData> script_data_;
};

// Succeed using custom PAC script.
Expand All @@ -176,7 +178,7 @@ TEST(InitProxyResolverTest, CustomPacSucceeds) {
CapturingNetLog log(CapturingNetLog::kUnbounded);
InitProxyResolver init(&resolver, &fetcher, &log);
EXPECT_EQ(OK, init.Init(config, &callback));
EXPECT_EQ(rule.text(), resolver.pac_script());
EXPECT_EQ(rule.text(), resolver.script_data()->utf16());

// Check the NetLog was filled correctly.
EXPECT_EQ(6u, log.entries().size());
Expand Down Expand Up @@ -209,7 +211,7 @@ TEST(InitProxyResolverTest, CustomPacFails1) {
CapturingNetLog log(CapturingNetLog::kUnbounded);
InitProxyResolver init(&resolver, &fetcher, &log);
EXPECT_EQ(kFailedDownloading, init.Init(config, &callback));
EXPECT_EQ(string16(), resolver.pac_script());
EXPECT_EQ(NULL, resolver.script_data());

// Check the NetLog was filled correctly.
EXPECT_EQ(4u, log.entries().size());
Expand Down Expand Up @@ -237,7 +239,7 @@ TEST(InitProxyResolverTest, CustomPacFails2) {
TestCompletionCallback callback;
InitProxyResolver init(&resolver, &fetcher, NULL);
EXPECT_EQ(kFailedParsing, init.Init(config, &callback));
EXPECT_EQ(string16(), resolver.pac_script());
EXPECT_EQ(NULL, resolver.script_data());
}

// Fail downloading the custom PAC script, because the fetcher was NULL.
Expand All @@ -251,7 +253,7 @@ TEST(InitProxyResolverTest, HasNullProxyScriptFetcher) {
TestCompletionCallback callback;
InitProxyResolver init(&resolver, NULL, NULL);
EXPECT_EQ(ERR_UNEXPECTED, init.Init(config, &callback));
EXPECT_EQ(string16(), resolver.pac_script());
EXPECT_EQ(NULL, resolver.script_data());
}

// Succeeds in choosing autodetect (wpad).
Expand All @@ -268,7 +270,7 @@ TEST(InitProxyResolverTest, AutodetectSuccess) {
TestCompletionCallback callback;
InitProxyResolver init(&resolver, &fetcher, NULL);
EXPECT_EQ(OK, init.Init(config, &callback));
EXPECT_EQ(rule.text(), resolver.pac_script());
EXPECT_EQ(rule.text(), resolver.script_data()->utf16());
}

// Fails at WPAD (downloading), but succeeds in choosing the custom PAC.
Expand All @@ -287,7 +289,7 @@ TEST(InitProxyResolverTest, AutodetectFailCustomSuccess1) {
TestCompletionCallback callback;
InitProxyResolver init(&resolver, &fetcher, NULL);
EXPECT_EQ(OK, init.Init(config, &callback));
EXPECT_EQ(rule.text(), resolver.pac_script());
EXPECT_EQ(rule.text(), resolver.script_data()->utf16());
}

// Fails at WPAD (parsing), but succeeds in choosing the custom PAC.
Expand All @@ -307,7 +309,7 @@ TEST(InitProxyResolverTest, AutodetectFailCustomSuccess2) {
CapturingNetLog log(CapturingNetLog::kUnbounded);
InitProxyResolver init(&resolver, &fetcher, &log);
EXPECT_EQ(OK, init.Init(config, &callback));
EXPECT_EQ(rule.text(), resolver.pac_script());
EXPECT_EQ(rule.text(), resolver.script_data()->utf16());

// Check the NetLog was filled correctly.
// (Note that the Fetch and Set states are repeated since both WPAD and custom
Expand Down Expand Up @@ -355,7 +357,7 @@ TEST(InitProxyResolverTest, AutodetectFailCustomFails1) {
TestCompletionCallback callback;
InitProxyResolver init(&resolver, &fetcher, NULL);
EXPECT_EQ(kFailedDownloading, init.Init(config, &callback));
EXPECT_EQ(string16(), resolver.pac_script());
EXPECT_EQ(NULL, resolver.script_data());
}

// Fails at WPAD (downloading), and fails at custom PAC (parsing).
Expand All @@ -374,7 +376,7 @@ TEST(InitProxyResolverTest, AutodetectFailCustomFails2) {
TestCompletionCallback callback;
InitProxyResolver init(&resolver, &fetcher, NULL);
EXPECT_EQ(kFailedParsing, init.Init(config, &callback));
EXPECT_EQ(string16(), resolver.pac_script());
EXPECT_EQ(NULL, resolver.script_data());
}

// Fails at WPAD (parsing), but succeeds in choosing the custom PAC.
Expand All @@ -395,7 +397,7 @@ TEST(InitProxyResolverTest, AutodetectFailCustomSuccess2_NoFetch) {
TestCompletionCallback callback;
InitProxyResolver init(&resolver, &fetcher, NULL);
EXPECT_EQ(OK, init.Init(config, &callback));
EXPECT_EQ(rule.url, resolver.pac_url());
EXPECT_EQ(rule.url, resolver.script_data()->url());
}

} // namespace
Expand Down
25 changes: 11 additions & 14 deletions net/proxy/mock_proxy_resolver.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -59,19 +59,17 @@ class MockAsyncProxyResolverBase : public ProxyResolver {

class SetPacScriptRequest {
public:
SetPacScriptRequest(MockAsyncProxyResolverBase* resolver,
const GURL& pac_url,
const string16& pac_script,
CompletionCallback* callback)
SetPacScriptRequest(
MockAsyncProxyResolverBase* resolver,
const scoped_refptr<ProxyResolverScriptData>& script_data,
CompletionCallback* callback)
: resolver_(resolver),
pac_url_(pac_url),
pac_script_(pac_script),
script_data_(script_data),
callback_(callback),
origin_loop_(MessageLoop::current()) {
}

const GURL& pac_url() const { return pac_url_; }
const string16& pac_script() const { return pac_script_; }
const ProxyResolverScriptData* script_data() const { return script_data_; }

void CompleteNow(int rv) {
CompletionCallback* callback = callback_;
Expand All @@ -84,8 +82,7 @@ class MockAsyncProxyResolverBase : public ProxyResolver {

private:
MockAsyncProxyResolverBase* resolver_;
const GURL pac_url_;
const string16 pac_script_;
const scoped_refptr<ProxyResolverScriptData> script_data_;
CompletionCallback* callback_;
MessageLoop* origin_loop_;
};
Expand Down Expand Up @@ -114,12 +111,12 @@ class MockAsyncProxyResolverBase : public ProxyResolver {
RemovePendingRequest(request);
}

virtual int SetPacScript(const GURL& pac_url,
const string16& pac_script,
CompletionCallback* callback) {
virtual int SetPacScript(
const scoped_refptr<ProxyResolverScriptData>& script_data,
CompletionCallback* callback) {
DCHECK(!pending_set_pac_script_request_.get());
pending_set_pac_script_request_.reset(
new SetPacScriptRequest(this, pac_url, pac_script, callback));
new SetPacScriptRequest(this, script_data, callback));
// Finished when user calls SetPacScriptRequest::CompleteNow().
return ERR_IO_PENDING;
}
Expand Down
Loading

0 comments on commit 2447640

Please sign in to comment.