Add ProxyResolverFactoryV8Tracing.

This changes ProxyResolverV8Tracing to use the new ProxyResolver and
ProxyResolverFactory interfaces.

BUG=467403
NOPRESUBMIT=true

Review URL: https://codereview.chromium.org/1126513002

Cr-Commit-Position: refs/heads/master@{#330074}

net/data/proxy_resolver_v8_tracing_unittest/error_on_load.js

+// Copyright 2015 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.
+
+alert('Prepare to DIE!');
+var x = null;
+return x.split('-');  // Throws exception.
+
+function FindProxyForURL(url, host) {
+  return "PROXY i-approve-this-message:42";
+}

net/proxy/mojo_proxy_resolver_factory_impl.cc

@@ -10,6 +10,7 @@
 #include "net/base/net_errors.h"
 #include "net/dns/host_resolver_mojo.h"
 #include "net/proxy/mojo_proxy_resolver_impl.h"
+#include "net/proxy/proxy_resolver_error_observer.h"
 #include "net/proxy/proxy_resolver_factory.h"
 #include "net/proxy/proxy_resolver_v8.h"
 #include "net/proxy/proxy_resolver_v8_tracing.h"
@@ -18,30 +19,15 @@
 namespace net {
 namespace {
 
-class DefaultProxyResolverFactory : public LegacyProxyResolverFactory {
- public:
-  DefaultProxyResolverFactory(
-      HostResolver* host_resolver,
-      const ProxyResolver::LoadStateChangedCallback& callback)
-      : LegacyProxyResolverFactory(true),
-        host_resolver_(host_resolver),
-        callback_(callback) {}
-
-  scoped_ptr<ProxyResolver> CreateProxyResolver() override {
-    return make_scoped_ptr(new ProxyResolverV8Tracing(host_resolver_, nullptr,
-                                                      nullptr, callback_));
-  }
-
- private:
-  HostResolver* const host_resolver_;
-  const ProxyResolver::LoadStateChangedCallback callback_;
-};
+scoped_ptr<ProxyResolverErrorObserver> ReturnNullErrorObserver() {
+  return nullptr;
+}
 
 scoped_ptr<ProxyResolverFactory> CreateDefaultProxyResolver(
     HostResolver* host_resolver,
     const ProxyResolver::LoadStateChangedCallback& callback) {
-  return make_scoped_ptr(
-      new DefaultProxyResolverFactory(host_resolver, callback));
+  return make_scoped_ptr(new ProxyResolverFactoryV8Tracing(
+      host_resolver, nullptr, callback, base::Bind(&ReturnNullErrorObserver)));
 }
 
 class LoadStateChangeForwarder

net/proxy/network_delegate_error_observer.cc

@@ -74,6 +74,14 @@ NetworkDelegateErrorObserver::~NetworkDelegateErrorObserver() {
   core_->Shutdown();
 }
 
+// static
+scoped_ptr<ProxyResolverErrorObserver> NetworkDelegateErrorObserver::Create(
+    NetworkDelegate* network_delegate,
+    const scoped_refptr<base::SingleThreadTaskRunner>& origin_runner) {
+  return make_scoped_ptr(
+      new NetworkDelegateErrorObserver(network_delegate, origin_runner.get()));
+}
+
 void NetworkDelegateErrorObserver::OnPACScriptError(
     int line_number,
     const base::string16& error) {

net/proxy/network_delegate_error_observer.h

@@ -7,6 +7,7 @@
 
 #include "base/compiler_specific.h"
 #include "base/memory/ref_counted.h"
+#include "base/memory/scoped_ptr.h"
 #include "net/proxy/proxy_resolver_error_observer.h"
 
 namespace base {
@@ -26,6 +27,10 @@ class NET_EXPORT_PRIVATE NetworkDelegateErrorObserver
                                base::SingleThreadTaskRunner* origin_runner);
   ~NetworkDelegateErrorObserver() override;
 
+  static scoped_ptr<ProxyResolverErrorObserver> Create(
+      NetworkDelegate* network_delegate,
+      const scoped_refptr<base::SingleThreadTaskRunner>& origin_runner);
+
   // ProxyResolverErrorObserver implementation.
   void OnPACScriptError(int line_number, const base::string16& error) override;
 

net/proxy/proxy_resolver_v8_tracing.h

@@ -5,93 +5,62 @@
 #ifndef NET_PROXY_PROXY_RESOLVER_V8_TRACING_H_
 #define NET_PROXY_PROXY_RESOLVER_V8_TRACING_H_
 
+#include <set>
+
 #include "base/basictypes.h"
 #include "base/memory/ref_counted.h"
 #include "base/memory/scoped_ptr.h"
-#include "base/threading/non_thread_safe.h"
 #include "net/base/net_export.h"
 #include "net/proxy/proxy_resolver.h"
-
-namespace base {
-class Thread;
-class SingleThreadTaskRunner;
-}  // namespace base
+#include "net/proxy/proxy_resolver_factory.h"
 
 namespace net {
 
 class HostResolver;
 class NetLog;
 class ProxyResolverErrorObserver;
-class ProxyResolverV8;
 
-// ProxyResolverV8Tracing is a non-blocking ProxyResolver. It executes
+// ProxyResolverFactoryV8Tracing is a ProxyResolverFactory that returns
+// non-blocking ProxyResolver instances. Each ProxyResolver instance executes
 // ProxyResolverV8 on a single helper thread, and does some magic to avoid
 // blocking in DNS. For more details see the design document:
 // https://docs.google.com/a/google.com/document/d/16Ij5OcVnR3s0MH4Z5XkhI9VTPoMJdaBn9rKreAmGOdE/edit?pli=1
-class NET_EXPORT_PRIVATE ProxyResolverV8Tracing
-    : public ProxyResolver,
-      NON_EXPORTED_BASE(public base::NonThreadSafe) {
+class NET_EXPORT ProxyResolverFactoryV8Tracing : public ProxyResolverFactory {
  public:
-  // Constructs a ProxyResolver that will issue DNS requests through
-  // |host_resolver|, forward Javascript errors through |error_observer|, and
-  // log Javascript errors and alerts to |net_log|.
-  //
-  // Note that the constructor takes ownership of |error_observer|, whereas
-  // |host_resolver| and |net_log| are expected to outlive |this|.
-  ProxyResolverV8Tracing(HostResolver* host_resolver,
-                         ProxyResolverErrorObserver* error_observer,
-                         NetLog* net_log);
-
-  // Constructs a ProxyResolver that will issue DNS requests through
-  // |host_resolver|, forward Javascript errors through |error_observer|, and
-  // log Javascript errors and alerts to |net_log|. When the LoadState for a
-  // request changes, |on_load_state_changed| will be invoked with the
-  // RequestHandle for that request with the new LoadState.
-  //
-  // Note that the constructor takes ownership of |error_observer|, whereas
-  // |host_resolver| and |net_log| are expected to outlive |this|.
-  ProxyResolverV8Tracing(HostResolver* host_resolver,
-                         ProxyResolverErrorObserver* error_observer,
-                         NetLog* net_log,
-                         const LoadStateChangedCallback& on_load_state_changed);
-
-  ~ProxyResolverV8Tracing() override;
-
-  // ProxyResolver implementation:
-  int GetProxyForURL(const GURL& url,
-                     ProxyInfo* results,
-                     const CompletionCallback& callback,
-                     RequestHandle* request,
-                     const BoundNetLog& net_log) override;
-  void CancelRequest(RequestHandle request) override;
-  LoadState GetLoadState(RequestHandle request) const override;
-  void CancelSetPacScript() override;
-  int SetPacScript(const scoped_refptr<ProxyResolverScriptData>& script_data,
-                   const CompletionCallback& callback) override;
+  // Note that |host_resolver| and |net_log| are expected to outlive |this| and
+  // any ProxyResolver instances created using |this|. |error_observer_factory|
+  // will be invoked once per CreateProxyResolver() call to create a
+  // ProxyResolverErrorObserver to be used by the ProxyResolver instance
+  // returned by that call.
+  ProxyResolverFactoryV8Tracing(
+      HostResolver* host_resolver,
+      NetLog* net_log,
+      const ProxyResolver::LoadStateChangedCallback& callback,
+      const base::Callback<scoped_ptr<ProxyResolverErrorObserver>()>&
+          error_observer_factory);
+  ~ProxyResolverFactoryV8Tracing() override;
+
+  // ProxyResolverFactory override.
+  int CreateProxyResolver(
+      const scoped_refptr<ProxyResolverScriptData>& pac_script,
+      scoped_ptr<ProxyResolver>* resolver,
+      const CompletionCallback& callback,
+      scoped_ptr<Request>* request) override;
 
  private:
-  class Job;
-
-  // The worker thread on which the ProxyResolverV8 will be run.
-  scoped_ptr<base::Thread> thread_;
-  scoped_ptr<ProxyResolverV8> v8_resolver_;
-
-  // Non-owned host resolver, which is to be operated on the origin thread.
-  HostResolver* host_resolver_;
-
-  scoped_ptr<ProxyResolverErrorObserver> error_observer_;
-  NetLog* net_log_;
+  class CreateJob;
 
-  // The outstanding SetPacScript operation, or NULL.
-  scoped_refptr<Job> set_pac_script_job_;
+  void RemoveJob(CreateJob* job);
 
-  // The number of outstanding (non-cancelled) jobs.
-  int num_outstanding_callbacks_;
+  HostResolver* const host_resolver_;
+  NetLog* const net_log_;
+  const ProxyResolver::LoadStateChangedCallback load_state_changed_callback_;
+  const base::Callback<scoped_ptr<ProxyResolverErrorObserver>()>
+      error_observer_factory_;
 
-  // Invoked when the load state for a request changes.
-  const LoadStateChangedCallback on_load_state_changed_;
+  std::set<CreateJob*> jobs_;
 
-  DISALLOW_COPY_AND_ASSIGN(ProxyResolverV8Tracing);
+  DISALLOW_COPY_AND_ASSIGN(ProxyResolverFactoryV8Tracing);
 };
 
 }  // namespace net

net/proxy/proxy_service_v8.cc

@@ -14,38 +14,6 @@
 #include "net/proxy/proxy_service.h"
 
 namespace net {
-namespace {
-
-class ProxyResolverFactoryForV8Resolver : public LegacyProxyResolverFactory {
- public:
-  explicit ProxyResolverFactoryForV8Resolver(HostResolver* host_resolver,
-                                             NetLog* net_log,
-                                             NetworkDelegate* network_delegate)
-      : LegacyProxyResolverFactory(true),
-        host_resolver_(host_resolver),
-        net_log_(net_log),
-        network_delegate_(network_delegate) {}
-
-  // LegacyProxyResolverFactory override.
-  scoped_ptr<ProxyResolver> CreateProxyResolver() override {
-    DCHECK(thread_checker_.CalledOnValidThread());
-    ProxyResolverErrorObserver* error_observer =
-        new NetworkDelegateErrorObserver(
-            network_delegate_, base::ThreadTaskRunnerHandle::Get().get());
-    return make_scoped_ptr(
-        new ProxyResolverV8Tracing(host_resolver_, error_observer, net_log_));
-  }
-
- private:
-  HostResolver* const host_resolver_;
-  NetLog* const net_log_;
-  NetworkDelegate* const network_delegate_;
-
-  base::ThreadChecker thread_checker_;
-
-  DISALLOW_COPY_AND_ASSIGN(ProxyResolverFactoryForV8Resolver);
-};
-}  // namespace
 
 // static
 ProxyService* CreateProxyServiceUsingV8ProxyResolver(
@@ -60,11 +28,13 @@ ProxyService* CreateProxyServiceUsingV8ProxyResolver(
   DCHECK(dhcp_proxy_script_fetcher);
   DCHECK(host_resolver);
 
-  ProxyService* proxy_service =
-      new ProxyService(proxy_config_service,
-                       make_scoped_ptr(new ProxyResolverFactoryForV8Resolver(
-                           host_resolver, net_log, network_delegate)),
-                       net_log);
+  ProxyService* proxy_service = new ProxyService(
+      proxy_config_service,
+      make_scoped_ptr(new ProxyResolverFactoryV8Tracing(
+          host_resolver, net_log, ProxyResolver::LoadStateChangedCallback(),
+          base::Bind(&NetworkDelegateErrorObserver::Create, network_delegate,
+                     base::ThreadTaskRunnerHandle::Get()))),
+      net_log);
 
   // Configure fetchers to use for PAC script downloads and auto-detect.
   proxy_service->SetProxyScriptFetchers(proxy_script_fetcher,