From 82a3767c45e85b77fb41d4fc92fc49fcb879e75b Mon Sep 17 00:00:00 2001 From: "jochen@chromium.org" Date: Tue, 3 May 2011 12:02:41 +0000 Subject: [PATCH] Add a method for PAC script errors to the network delegate. Also add a wrapper class to avoid passing around raw NULL pointers, and a bridge so I can invoke the method from other than the IO thread BUG=48930 TEST=net unittests Review URL: http://codereview.chromium.org/6822026 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@83881 0039d316-1c4b-4281-b951-d872f2087c98 --- chrome/browser/net/chrome_network_delegate.cc | 4 + chrome/browser/net/chrome_network_delegate.h | 1 + chrome/browser/net/connection_tester.cc | 1 + chrome/browser/net/proxy_service_factory.cc | 3 +- net/base/network_delegate.cc | 6 ++ net/base/network_delegate.h | 5 ++ net/net.gyp | 4 + net/proxy/network_delegate_error_observer.cc | 79 +++++++++++++++++ net/proxy/network_delegate_error_observer.h | 39 +++++++++ ...etwork_delegate_error_observer_unittest.cc | 87 +++++++++++++++++++ net/proxy/proxy_resolver_error_observer.h | 27 ++++++ net/proxy/proxy_resolver_js_bindings.cc | 20 +++-- net/proxy/proxy_resolver_js_bindings.h | 10 ++- .../proxy_resolver_js_bindings_unittest.cc | 13 +-- net/proxy/proxy_resolver_perftest.cc | 4 +- net/proxy/proxy_resolver_v8.cc | 7 +- net/proxy/proxy_service.cc | 18 +++- net/proxy/proxy_service.h | 4 +- net/url_request/url_request_test_util.cc | 4 + net/url_request/url_request_test_util.h | 1 + 20 files changed, 313 insertions(+), 24 deletions(-) create mode 100644 net/proxy/network_delegate_error_observer.cc create mode 100644 net/proxy/network_delegate_error_observer.h create mode 100644 net/proxy/network_delegate_error_observer_unittest.cc create mode 100644 net/proxy/proxy_resolver_error_observer.h diff --git a/chrome/browser/net/chrome_network_delegate.cc b/chrome/browser/net/chrome_network_delegate.cc index 0613baa0e658..372a7fb098e0 100644 --- a/chrome/browser/net/chrome_network_delegate.cc +++ b/chrome/browser/net/chrome_network_delegate.cc @@ -132,3 +132,7 @@ net::URLRequestJob* ChromeNetworkDelegate::OnMaybeCreateURLRequestJob( return NULL; return protocol_handler_registry_->MaybeCreateJob(request); } + +void ChromeNetworkDelegate::OnPACScriptError(int line_number, + const string16& error) { +} diff --git a/chrome/browser/net/chrome_network_delegate.h b/chrome/browser/net/chrome_network_delegate.h index 4edc01a2a39d..a26bc318e4b5 100644 --- a/chrome/browser/net/chrome_network_delegate.h +++ b/chrome/browser/net/chrome_network_delegate.h @@ -55,6 +55,7 @@ class ChromeNetworkDelegate : public net::NetworkDelegate { virtual void OnHttpTransactionDestroyed(uint64 request_id); virtual net::URLRequestJob* OnMaybeCreateURLRequestJob( net::URLRequest* request); + virtual void OnPACScriptError(int line_number, const string16& error); scoped_refptr event_router_; const ProfileId profile_id_; diff --git a/chrome/browser/net/connection_tester.cc b/chrome/browser/net/connection_tester.cc index 47023e3dc727..84d45d9dfcf6 100644 --- a/chrome/browser/net/connection_tester.cc +++ b/chrome/browser/net/connection_tester.cc @@ -187,6 +187,7 @@ class ExperimentURLRequestContext : public net::URLRequestContext { 0u, new net::ProxyScriptFetcherImpl(proxy_request_context_), host_resolver(), + NULL, NULL)); return net::OK; diff --git a/chrome/browser/net/proxy_service_factory.cc b/chrome/browser/net/proxy_service_factory.cc index ad9afd0762ab..ba1cb9a7523d 100644 --- a/chrome/browser/net/proxy_service_factory.cc +++ b/chrome/browser/net/proxy_service_factory.cc @@ -89,7 +89,8 @@ net::ProxyService* ProxyServiceFactory::CreateProxyService( num_pac_threads, new net::ProxyScriptFetcherImpl(context), context->host_resolver(), - net_log); + net_log, + context->network_delegate()); } else { proxy_service = net::ProxyService::CreateUsingSystemProxyResolver( proxy_config_service, diff --git a/net/base/network_delegate.cc b/net/base/network_delegate.cc index d0184558ea19..f916572853f6 100644 --- a/net/base/network_delegate.cc +++ b/net/base/network_delegate.cc @@ -69,4 +69,10 @@ URLRequestJob* NetworkDelegate::MaybeCreateURLRequestJob(URLRequest* request) { return OnMaybeCreateURLRequestJob(request); } +void NetworkDelegate::NotifyPACScriptError(int line_number, + const string16& error) { + DCHECK(CalledOnValidThread()); + OnPACScriptError(line_number, error); +} + } // namespace net diff --git a/net/base/network_delegate.h b/net/base/network_delegate.h index 3cc7b456f386..f0f3325fd504 100644 --- a/net/base/network_delegate.h +++ b/net/base/network_delegate.h @@ -6,6 +6,7 @@ #define NET_BASE_NETWORK_DELEGATE_H_ #pragma once +#include "base/string16.h" #include "base/threading/non_thread_safe.h" #include "net/base/completion_callback.h" @@ -58,6 +59,8 @@ class NetworkDelegate : public base::NonThreadSafe { // URLRequestJobManager::CreateJob() as a general override mechanism. URLRequestJob* MaybeCreateURLRequestJob(URLRequest* request); + void NotifyPACScriptError(int line_number, const string16& error); + private: // This is the interface for subclasses of NetworkDelegate to implement. This // member functions will be called by the respective public notification @@ -108,6 +111,8 @@ class NetworkDelegate : public base::NonThreadSafe { // handle the request. virtual URLRequestJob* OnMaybeCreateURLRequestJob(URLRequest* request) = 0; + // Corresponds to ProxyResolverJSBindings::OnError. + virtual void OnPACScriptError(int line_number, const string16& error) = 0; }; } // namespace net diff --git a/net/net.gyp b/net/net.gyp index 8634fddf2f42..eb029fbbaec6 100644 --- a/net/net.gyp +++ b/net/net.gyp @@ -561,6 +561,8 @@ 'proxy/init_proxy_resolver.h', 'proxy/multi_threaded_proxy_resolver.cc', 'proxy/multi_threaded_proxy_resolver.h', + 'proxy/network_delegate_error_observer.cc', + 'proxy/network_delegate_error_observer.h', 'proxy/polling_proxy_config_service.cc', 'proxy/polling_proxy_config_service.h', 'proxy/proxy_bypass_rules.cc', @@ -581,6 +583,7 @@ 'proxy/proxy_list.cc', 'proxy/proxy_list.h', 'proxy/proxy_resolver.h', + 'proxy/proxy_resolver_error_observer.h', 'proxy/proxy_resolver_js_bindings.cc', 'proxy/proxy_resolver_js_bindings.h', 'proxy/proxy_resolver_mac.cc', @@ -989,6 +992,7 @@ 'http/url_security_manager_unittest.cc', 'proxy/init_proxy_resolver_unittest.cc', 'proxy/multi_threaded_proxy_resolver_unittest.cc', + 'proxy/network_delegate_error_observer_unittest.cc', 'proxy/proxy_bypass_rules_unittest.cc', 'proxy/proxy_config_service_linux_unittest.cc', 'proxy/proxy_config_service_win_unittest.cc', diff --git a/net/proxy/network_delegate_error_observer.cc b/net/proxy/network_delegate_error_observer.cc new file mode 100644 index 000000000000..f0f9fdd76822 --- /dev/null +++ b/net/proxy/network_delegate_error_observer.cc @@ -0,0 +1,79 @@ +// Copyright (c) 2011 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "base/message_loop.h" +#include "net/base/net_errors.h" +#include "net/base/network_delegate.h" +#include "net/proxy/network_delegate_error_observer.h" + +namespace net { + +// NetworkDelegateErrorObserver::Core ----------------------------------------- + +class NetworkDelegateErrorObserver::Core + : public base::RefCountedThreadSafe { + public: + Core(NetworkDelegate* network_delegate, MessageLoop* io_loop); + + void NotifyPACScriptError(int line_number, const string16& error); + + void Shutdown(); + + private: + friend class base::RefCountedThreadSafe; + + virtual ~Core(); + + NetworkDelegate* network_delegate_; + MessageLoop* const io_loop_; + + DISALLOW_COPY_AND_ASSIGN(Core); +}; + +NetworkDelegateErrorObserver::Core::Core(NetworkDelegate* network_delegate, + MessageLoop* io_loop) + : network_delegate_(network_delegate), + io_loop_(io_loop) { + DCHECK(io_loop_); +} + +NetworkDelegateErrorObserver::Core::~Core() {} + + +void NetworkDelegateErrorObserver::Core::NotifyPACScriptError( + int line_number, + const string16& error) { + if (MessageLoop::current() != io_loop_) { + io_loop_->PostTask( + FROM_HERE, + NewRunnableMethod(this, &Core::NotifyPACScriptError, + line_number, error)); + return; + } + if (network_delegate_) + network_delegate_->NotifyPACScriptError(line_number, error); +} + +void NetworkDelegateErrorObserver::Core::Shutdown() { + CHECK_EQ(MessageLoop::current(), io_loop_); + network_delegate_ = NULL; +} + +// NetworkDelegateErrorObserver ----------------------------------------------- + +NetworkDelegateErrorObserver::NetworkDelegateErrorObserver( + NetworkDelegate* network_delegate, + MessageLoop* io_loop) + : core_(new Core(network_delegate, io_loop)) {} + +NetworkDelegateErrorObserver::~NetworkDelegateErrorObserver() { + core_->Shutdown(); +} + +void NetworkDelegateErrorObserver::OnPACScriptError(int line_number, + const string16& error) { + core_->NotifyPACScriptError(line_number, error); +} + +} // namespace net diff --git a/net/proxy/network_delegate_error_observer.h b/net/proxy/network_delegate_error_observer.h new file mode 100644 index 000000000000..bca57be3b23a --- /dev/null +++ b/net/proxy/network_delegate_error_observer.h @@ -0,0 +1,39 @@ +// Copyright (c) 2011 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#ifndef NET_PROXY_NETWORK_DELEGATE_ERROR_OBSERVER_H_ +#define NET_PROXY_NETWORK_DELEGATE_ERROR_OBSERVER_H_ +#pragma once + +#include "base/memory/ref_counted.h" +#include "net/proxy/proxy_resolver_error_observer.h" + +class MessageLoop; + +namespace net { + +class NetworkDelegate; + +// An implementation of ProxyResolverErrorObserver that forwards PAC script +// errors to a NetworkDelegate object on the IO thread. +class NetworkDelegateErrorObserver : public ProxyResolverErrorObserver { + public: + NetworkDelegateErrorObserver(NetworkDelegate* network_delegate, + MessageLoop* io_loop); + virtual ~NetworkDelegateErrorObserver(); + + // ProxyResolverErrorObserver implementation. + virtual void OnPACScriptError(int line_number, const string16& error); + + private: + class Core; + + scoped_refptr core_; + + DISALLOW_COPY_AND_ASSIGN(NetworkDelegateErrorObserver); +}; + +} // namespace net + +#endif // NET_PROXY_NETWORK_DELEGATE_ERROR_OBSERVER_H_ diff --git a/net/proxy/network_delegate_error_observer_unittest.cc b/net/proxy/network_delegate_error_observer_unittest.cc new file mode 100644 index 000000000000..b54a11aa500e --- /dev/null +++ b/net/proxy/network_delegate_error_observer_unittest.cc @@ -0,0 +1,87 @@ +// Copyright (c) 2011 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "net/proxy/network_delegate_error_observer.h" + +#include "base/threading/thread.h" +#include "net/base/net_errors.h" +#include "net/base/network_delegate.h" +#include "testing/gtest/include/gtest/gtest.h" + +DISABLE_RUNNABLE_METHOD_REFCOUNT(net::NetworkDelegateErrorObserver); + +namespace net { + +namespace { +class TestNetworkDelegate : public net::NetworkDelegate { + public: + TestNetworkDelegate() : got_pac_error_(false) {} + virtual ~TestNetworkDelegate() {} + + bool got_pac_error() const { return got_pac_error_; } + + private: + // net::NetworkDelegate: + virtual int OnBeforeURLRequest(URLRequest* request, + CompletionCallback* callback, + GURL* new_url) { + return net::OK; + } + virtual int OnBeforeSendHeaders(uint64 request_id, + CompletionCallback* callback, + HttpRequestHeaders* headers) { + return net::OK; + } + virtual void OnRequestSent(uint64 request_id, + const HostPortPair& socket_address) {} + virtual void OnBeforeRedirect(URLRequest* request, + const GURL& new_location) {} + virtual void OnResponseStarted(URLRequest* request) {} + virtual void OnCompleted(URLRequest* request) {} + virtual void OnURLRequestDestroyed(URLRequest* request) {} + virtual void OnHttpTransactionDestroyed(uint64 request_id) {} + virtual URLRequestJob* OnMaybeCreateURLRequestJob(URLRequest* request) { + return NULL; + } + virtual void OnPACScriptError(int line_number, const string16& error) { + got_pac_error_ = true; + } + + bool got_pac_error_; +}; + +} // namespace + +// Check that the OnPACScriptError method can be called from an arbitrary +// thread. +TEST(NetworkDelegateErrorObserverTest, CallOnThread) { + base::Thread thread("test_thread"); + thread.Start(); + TestNetworkDelegate network_delegate; + NetworkDelegateErrorObserver + observer(&network_delegate, MessageLoop::current()); + thread.message_loop()->PostTask(FROM_HERE, + NewRunnableMethod(&observer, + &NetworkDelegateErrorObserver::OnPACScriptError, + 42, string16())); + thread.Stop(); + MessageLoop::current()->RunAllPending(); + ASSERT_TRUE(network_delegate.got_pac_error()); +} + +// Check that passing a NULL network delegate works. +TEST(NetworkDelegateErrorObserverTest, NoDelegate) { + base::Thread thread("test_thread"); + thread.Start(); + NetworkDelegateErrorObserver observer(NULL, MessageLoop::current()); + thread.message_loop()->PostTask(FROM_HERE, + NewRunnableMethod(&observer, + &NetworkDelegateErrorObserver::OnPACScriptError, + 42, string16())); + thread.Stop(); + MessageLoop::current()->RunAllPending(); + // Shouldn't have crashed until here... +} + +} // namespace net diff --git a/net/proxy/proxy_resolver_error_observer.h b/net/proxy/proxy_resolver_error_observer.h new file mode 100644 index 000000000000..ddb06948c448 --- /dev/null +++ b/net/proxy/proxy_resolver_error_observer.h @@ -0,0 +1,27 @@ +// Copyright (c) 2011 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#ifndef NET_PROXY_PROXY_RESOLVER_ERROR_OBSERVER_H_ +#define NET_PROXY_PROXY_RESOLVER_ERROR_OBSERVER_H_ +#pragma once + +#include "base/string16.h" + +namespace net { + +// Interface for observing JavaScript error messages from PAC scripts. +class ProxyResolverErrorObserver { + public: + ProxyResolverErrorObserver() {} + virtual ~ProxyResolverErrorObserver() {} + + virtual void OnPACScriptError(int line_number, const string16& error) = 0; + + private: + DISALLOW_COPY_AND_ASSIGN(ProxyResolverErrorObserver); +}; + +} // namespace net + +#endif // NET_PROXY_PROXY_RESOLVER_ERROR_OBSERVER_H_ diff --git a/net/proxy/proxy_resolver_js_bindings.cc b/net/proxy/proxy_resolver_js_bindings.cc index ae425541dd2e..18d4e9ebcb82 100644 --- a/net/proxy/proxy_resolver_js_bindings.cc +++ b/net/proxy/proxy_resolver_js_bindings.cc @@ -1,4 +1,4 @@ -// Copyright (c) 2010 The Chromium Authors. All rights reserved. +// Copyright (c) 2011 The Chromium Authors. All rights reserved. // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. @@ -14,6 +14,7 @@ #include "net/base/net_log.h" #include "net/base/net_util.h" #include "net/base/sys_addrinfo.h" +#include "net/proxy/proxy_resolver_error_observer.h" #include "net/proxy/proxy_resolver_request_context.h" namespace net { @@ -64,9 +65,12 @@ class AlertNetlogParams : public NetLog::EventParameters { // ProxyResolverJSBindings implementation. class DefaultJSBindings : public ProxyResolverJSBindings { public: - DefaultJSBindings(HostResolver* host_resolver, NetLog* net_log) + DefaultJSBindings(HostResolver* host_resolver, + NetLog* net_log, + ProxyResolverErrorObserver* error_observer) : host_resolver_(host_resolver), - net_log_(net_log) { + net_log_(net_log), + error_observer_(error_observer) { } // Handler for "alert(message)". @@ -150,6 +154,9 @@ class DefaultJSBindings : public ProxyResolverJSBindings { LogEventToCurrentRequestAndGlobally( NetLog::TYPE_PAC_JAVASCRIPT_ERROR, new ErrorNetlogParams(line_number, message)); + + if (error_observer_.get()) + error_observer_->OnPACScriptError(line_number, message); } virtual void Shutdown() { @@ -287,14 +294,17 @@ class DefaultJSBindings : public ProxyResolverJSBindings { HostResolver* const host_resolver_; NetLog* net_log_; + scoped_ptr error_observer_; }; } // namespace // static ProxyResolverJSBindings* ProxyResolverJSBindings::CreateDefault( - HostResolver* host_resolver, NetLog* net_log) { - return new DefaultJSBindings(host_resolver, net_log); + HostResolver* host_resolver, + NetLog* net_log, + ProxyResolverErrorObserver* error_observer) { + return new DefaultJSBindings(host_resolver, net_log, error_observer); } } // namespace net diff --git a/net/proxy/proxy_resolver_js_bindings.h b/net/proxy/proxy_resolver_js_bindings.h index 860be342e4d4..3b4719eccb0e 100644 --- a/net/proxy/proxy_resolver_js_bindings.h +++ b/net/proxy/proxy_resolver_js_bindings.h @@ -1,4 +1,4 @@ -// Copyright (c) 2010 The Chromium Authors. All rights reserved. +// Copyright (c) 2011 The Chromium Authors. All rights reserved. // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. @@ -14,6 +14,7 @@ namespace net { class HostResolver; class NetLog; +class ProxyResolverErrorObserver; struct ProxyResolverRequestContext; // Interface for the javascript bindings. @@ -64,8 +65,11 @@ class ProxyResolverJSBindings { // - Use the provided host resolver to service dnsResolve(). // // Note that |host_resolver| will be used in sync mode mode. - static ProxyResolverJSBindings* CreateDefault(HostResolver* host_resolver, - NetLog* net_log); + // Takes ownership of |error_observer| which might be NULL. + static ProxyResolverJSBindings* CreateDefault( + HostResolver* host_resolver, + NetLog* net_log, + ProxyResolverErrorObserver* error_observer); // Sets details about the currently executing FindProxyForURL() request. void set_current_request_context( diff --git a/net/proxy/proxy_resolver_js_bindings_unittest.cc b/net/proxy/proxy_resolver_js_bindings_unittest.cc index e19bdde33cf3..e4b309d8e1e4 100644 --- a/net/proxy/proxy_resolver_js_bindings_unittest.cc +++ b/net/proxy/proxy_resolver_js_bindings_unittest.cc @@ -115,7 +115,7 @@ TEST(ProxyResolverJSBindingsTest, DnsResolve) { // Get a hold of a DefaultJSBindings* (it is a hidden impl class). scoped_ptr bindings( - ProxyResolverJSBindings::CreateDefault(host_resolver.get(), NULL)); + ProxyResolverJSBindings::CreateDefault(host_resolver.get(), NULL, NULL)); std::string ip_address; @@ -142,7 +142,7 @@ TEST(ProxyResolverJSBindingsTest, MyIpAddress) { // Get a hold of a DefaultJSBindings* (it is a hidden impl class). scoped_ptr bindings( - ProxyResolverJSBindings::CreateDefault(host_resolver.get(), NULL)); + ProxyResolverJSBindings::CreateDefault(host_resolver.get(), NULL, NULL)); // Our IP address is always going to be 127.0.0.1, since we are using a // mock host resolver. @@ -169,7 +169,7 @@ TEST(ProxyResolverJSBindingsTest, RestrictAddressFamily) { // Get a hold of a DefaultJSBindings* (it is a hidden impl class). scoped_ptr bindings( - ProxyResolverJSBindings::CreateDefault(host_resolver.get(), NULL)); + ProxyResolverJSBindings::CreateDefault(host_resolver.get(), NULL, NULL)); // Make it so requests resolve to particular address patterns based on family: // IPV4_ONLY --> 192.168.1.* @@ -226,7 +226,7 @@ TEST(ProxyResolverJSBindingsTest, ExFunctionsReturnList) { // Get a hold of a DefaultJSBindings* (it is a hidden impl class). scoped_ptr bindings( - ProxyResolverJSBindings::CreateDefault(host_resolver.get(), NULL)); + ProxyResolverJSBindings::CreateDefault(host_resolver.get(), NULL, NULL)); std::string ip_addresses; @@ -243,7 +243,7 @@ TEST(ProxyResolverJSBindingsTest, PerRequestDNSCache) { // Get a hold of a DefaultJSBindings* (it is a hidden impl class). scoped_ptr bindings( - ProxyResolverJSBindings::CreateDefault(host_resolver.get(), NULL)); + ProxyResolverJSBindings::CreateDefault(host_resolver.get(), NULL, NULL)); std::string ip_address; @@ -295,7 +295,8 @@ TEST(ProxyResolverJSBindingsTest, NetLog) { // Get a hold of a DefaultJSBindings* (it is a hidden impl class). scoped_ptr bindings( - ProxyResolverJSBindings::CreateDefault(host_resolver.get(), &global_log)); + ProxyResolverJSBindings::CreateDefault( + host_resolver.get(), &global_log, NULL)); // Attach a capturing NetLog as the current request's log stream. CapturingNetLog log(CapturingNetLog::kUnbounded); diff --git a/net/proxy/proxy_resolver_perftest.cc b/net/proxy/proxy_resolver_perftest.cc index dda9587f9193..c3d65881e559 100644 --- a/net/proxy/proxy_resolver_perftest.cc +++ b/net/proxy/proxy_resolver_perftest.cc @@ -1,4 +1,4 @@ -// Copyright (c) 2010 The Chromium Authors. All rights reserved. +// Copyright (c) 2011 The Chromium Authors. All rights reserved. // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. @@ -193,7 +193,7 @@ TEST(ProxyResolverPerfTest, ProxyResolverMac) { TEST(ProxyResolverPerfTest, ProxyResolverV8) { net::ProxyResolverJSBindings* js_bindings = net::ProxyResolverJSBindings::CreateDefault( - new net::MockHostResolver, NULL); + new net::MockHostResolver, NULL, NULL); net::ProxyResolverV8 resolver(js_bindings); PacPerfSuiteRunner runner(&resolver, "ProxyResolverV8"); diff --git a/net/proxy/proxy_resolver_v8.cc b/net/proxy/proxy_resolver_v8.cc index 80a1937d30f3..9332043e6f18 100644 --- a/net/proxy/proxy_resolver_v8.cc +++ b/net/proxy/proxy_resolver_v8.cc @@ -1,4 +1,4 @@ -// Copyright (c) 2010 The Chromium Authors. All rights reserved. +// Copyright (c) 2011 The Chromium Authors. All rights reserved. // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. @@ -468,8 +468,11 @@ class ProxyResolverV8::Context { // At a minimum, the FindProxyForURL() function must be defined for this // to be a legitimiate PAC script. v8::Local function; - if (!GetFindProxyForURL(&function)) + if (!GetFindProxyForURL(&function)) { + js_bindings_->OnError( + -1, ASCIIToUTF16("FindProxyForURL() is undefined.")); return ERR_PAC_SCRIPT_FAILED; + } return OK; } diff --git a/net/proxy/proxy_service.cc b/net/proxy/proxy_service.cc index ca3629ebcdcd..75411c3892a1 100644 --- a/net/proxy/proxy_service.cc +++ b/net/proxy/proxy_service.cc @@ -17,6 +17,7 @@ #include "net/base/net_util.h" #include "net/proxy/init_proxy_resolver.h" #include "net/proxy/multi_threaded_proxy_resolver.h" +#include "net/proxy/network_delegate_error_observer.h" #include "net/proxy/proxy_config_service_fixed.h" #include "net/proxy/proxy_resolver.h" #include "net/proxy/proxy_resolver_js_bindings.h" @@ -168,7 +169,8 @@ class ProxyResolverFactoryForV8 : public ProxyResolverFactory { // |async_host_resolver| will only be operated on |io_loop|. ProxyResolverFactoryForV8(HostResolver* async_host_resolver, MessageLoop* io_loop, - NetLog* net_log) + NetLog* net_log, + NetworkDelegate* network_delegate) : ProxyResolverFactory(true /*expects_pac_bytes*/), async_host_resolver_(async_host_resolver), io_loop_(io_loop), @@ -181,8 +183,13 @@ class ProxyResolverFactoryForV8 : public ProxyResolverFactory { SyncHostResolverBridge* sync_host_resolver = new SyncHostResolverBridge(async_host_resolver_, io_loop_); + NetworkDelegateErrorObserver* error_observer = + new NetworkDelegateErrorObserver(network_delegate_, io_loop_); + + // ProxyResolverJSBindings takes ownership of |error_observer|. ProxyResolverJSBindings* js_bindings = - ProxyResolverJSBindings::CreateDefault(sync_host_resolver, net_log_); + ProxyResolverJSBindings::CreateDefault( + sync_host_resolver, net_log_, error_observer); // ProxyResolverV8 takes ownership of |js_bindings|. return new ProxyResolverV8(js_bindings); @@ -192,6 +199,7 @@ class ProxyResolverFactoryForV8 : public ProxyResolverFactory { HostResolver* const async_host_resolver_; MessageLoop* io_loop_; NetLog* net_log_; + NetworkDelegate* network_delegate_; }; // Creates ProxyResolvers using a platform-specific implementation. @@ -395,7 +403,8 @@ ProxyService* ProxyService::CreateUsingV8ProxyResolver( size_t num_pac_threads, ProxyScriptFetcher* proxy_script_fetcher, HostResolver* host_resolver, - NetLog* net_log) { + NetLog* net_log, + NetworkDelegate* network_delegate) { DCHECK(proxy_config_service); DCHECK(proxy_script_fetcher); DCHECK(host_resolver); @@ -407,7 +416,8 @@ ProxyService* ProxyService::CreateUsingV8ProxyResolver( new ProxyResolverFactoryForV8( host_resolver, MessageLoop::current(), - net_log); + net_log, + network_delegate); ProxyResolver* proxy_resolver = new MultiThreadedProxyResolver(sync_resolver_factory, num_pac_threads); diff --git a/net/proxy/proxy_service.h b/net/proxy/proxy_service.h index 5f6cbf2d5e81..4838954f78d5 100644 --- a/net/proxy/proxy_service.h +++ b/net/proxy/proxy_service.h @@ -27,6 +27,7 @@ namespace net { class HostResolver; class InitProxyResolver; +class NetworkDelegate; class ProxyResolver; class ProxyScriptFetcher; class URLRequestContext; @@ -174,7 +175,8 @@ class ProxyService : public NetworkChangeNotifier::IPAddressObserver, size_t num_pac_threads, ProxyScriptFetcher* proxy_script_fetcher, HostResolver* host_resolver, - NetLog* net_log); + NetLog* net_log, + NetworkDelegate* network_delegate); // Same as CreateUsingV8ProxyResolver, except it uses system libraries // for evaluating the PAC script if available, otherwise skips diff --git a/net/url_request/url_request_test_util.cc b/net/url_request/url_request_test_util.cc index f89458a65218..f8ab98a65565 100644 --- a/net/url_request/url_request_test_util.cc +++ b/net/url_request/url_request_test_util.cc @@ -305,3 +305,7 @@ net::URLRequestJob* TestNetworkDelegate::OnMaybeCreateURLRequestJob( net::URLRequest* request) { return NULL; } + +void TestNetworkDelegate::OnPACScriptError(int line_number, + const string16& error) { +} diff --git a/net/url_request/url_request_test_util.h b/net/url_request/url_request_test_util.h index 0fbf0529ffa4..3301bc328d09 100644 --- a/net/url_request/url_request_test_util.h +++ b/net/url_request/url_request_test_util.h @@ -215,6 +215,7 @@ class TestNetworkDelegate : public net::NetworkDelegate { virtual void OnHttpTransactionDestroyed(uint64 request_id); virtual net::URLRequestJob* OnMaybeCreateURLRequestJob( net::URLRequest* request); + virtual void OnPACScriptError(int line_number, const string16& error); int last_os_error_; int error_count_;