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

11 files changed, 285 insertions, 23 deletions

diff --git a/net/proxy/network_delegate_error_observer.cc b/net/proxy/network_delegate_error_observer.cc
new file mode 100644
index 0000000..f0f9fdd
--- /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<NetworkDelegateErrorObserver::Core> {
+ public:
+  Core(NetworkDelegate* network_delegate, MessageLoop* io_loop);
+
+  void NotifyPACScriptError(int line_number, const string16& error);
+
+  void Shutdown();
+
+ private:
+  friend class base::RefCountedThreadSafe<NetworkDelegateErrorObserver::Core>;
+
+  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 0000000..bca57be
--- /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> 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 0000000..b54a11a
--- /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 0000000..ddb0694
--- /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 ae42554..18d4e9e 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<ProxyResolverErrorObserver> 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 860be34..3b4719e 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 e19bdde..e4b309d 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<ProxyResolverJSBindings> 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<ProxyResolverJSBindings> 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<ProxyResolverJSBindings> 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<ProxyResolverJSBindings> 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<ProxyResolverJSBindings> 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<ProxyResolverJSBindings> 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 dda9587..c3d6588 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 80a1937..9332043 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<v8::Value> 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 ca3629e..75411c3 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 5f6cbf2..4838954 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