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

@@ -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) {
+}

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<ExtensionEventRouterForwarder> event_router_;
   const ProfileId profile_id_;

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;

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,

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

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

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',

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

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_

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

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_

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