Add support for ProxyResolverErrorObserver to ProxyResolverMojo.

BUG=467832

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

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

chrome/browser/net/utility_process_mojo_proxy_resolver_factory.cc

@@ -64,6 +64,7 @@ UtilityProcessMojoProxyResolverFactory::CreateResolver(
     const mojo::String& pac_script,
     mojo::InterfaceRequest<net::interfaces::ProxyResolver> req,
     net::interfaces::HostResolverPtr host_resolver,
+    net::interfaces::ProxyResolverErrorObserverPtr error_observer,
     net::interfaces::ProxyResolverFactoryRequestClientPtr client) {
   DCHECK(thread_checker_.CalledOnValidThread());
   if (!resolver_factory_)
@@ -78,7 +79,8 @@ UtilityProcessMojoProxyResolverFactory::CreateResolver(
   idle_timer_.Stop();
   num_proxy_resolvers_++;
   resolver_factory_->CreateResolver(pac_script, req.Pass(),
-                                    host_resolver.Pass(), client.Pass());
+                                    host_resolver.Pass(), error_observer.Pass(),
+                                    client.Pass());
   return make_scoped_ptr(new base::ScopedClosureRunner(
       base::Bind(&UtilityProcessMojoProxyResolverFactory::OnResolverDestroyed,
                  base::Unretained(this))));

chrome/browser/net/utility_process_mojo_proxy_resolver_factory.h

@@ -33,6 +33,7 @@ class UtilityProcessMojoProxyResolverFactory
       const mojo::String& pac_script,
       mojo::InterfaceRequest<net::interfaces::ProxyResolver> req,
       net::interfaces::HostResolverPtr host_resolver,
+      net::interfaces::ProxyResolverErrorObserverPtr error_observer,
       net::interfaces::ProxyResolverFactoryRequestClientPtr client) override;
 
  private:

net/BUILD.gn

@@ -839,6 +839,8 @@ if (use_v8_in_net && !is_android) {
       "proxy/mojo_proxy_resolver_factory_impl.h",
       "proxy/mojo_proxy_resolver_impl.cc",
       "proxy/mojo_proxy_resolver_impl.h",
+      "proxy/proxy_resolver_error_observer_mojo.cc",
+      "proxy/proxy_resolver_error_observer_mojo.h",
     ]
 
     deps = [
@@ -1493,6 +1495,7 @@ if (!is_android && !is_mac) {
         "proxy/load_state_change_coalescer_unittest.cc",
         "proxy/mojo_proxy_resolver_factory_impl_unittest.cc",
         "proxy/mojo_proxy_resolver_impl_unittest.cc",
+        "proxy/proxy_resolver_error_observer_mojo_unittest.cc",
         "proxy/proxy_resolver_mojo_unittest.cc",
         "proxy/proxy_service_mojo_unittest.cc",
       ]

net/interfaces/proxy_resolver_service.mojom

@@ -42,11 +42,16 @@ interface ProxyResolverRequestClient {
   LoadStateChanged(int32 load_state);
 };
 
+interface ProxyResolverErrorObserver {
+  OnPacScriptError(int32 line_number, string error);
+};
+
 interface ProxyResolverFactory {
-  // TODO(amistry): Add NetLog and ProxyResolverErrorObserver.
+  // TODO(amistry): Add NetLog.
   CreateResolver(string pac_script,
                  ProxyResolver& resolver,
                  HostResolver host_resolver,
+                 ProxyResolverErrorObserver? error_observer,
                  ProxyResolverFactoryRequestClient client);
 };
 

net/net.gyp

@@ -319,6 +319,7 @@
               'proxy/load_state_change_coalescer_unittest.cc',
               'proxy/mojo_proxy_resolver_factory_impl_unittest.cc',
               'proxy/mojo_proxy_resolver_impl_unittest.cc',
+              'proxy/proxy_resolver_error_observer_mojo_unittest.cc',
               'proxy/proxy_resolver_mojo_unittest.cc',
               'proxy/proxy_service_mojo_unittest.cc',
             ],
@@ -923,6 +924,8 @@
             'proxy/mojo_proxy_resolver_factory_impl.h',
             'proxy/mojo_proxy_resolver_impl.cc',
             'proxy/mojo_proxy_resolver_impl.h',
+            'proxy/proxy_resolver_error_observer_mojo.cc',
+            'proxy/proxy_resolver_error_observer_mojo.h',
           ],
           'dependencies': [
             'mojo_type_converters',

net/net.gypi

@@ -1475,6 +1475,7 @@
       'proxy/proxy_config_unittest.cc',
       'proxy/proxy_info_unittest.cc',
       'proxy/proxy_list_unittest.cc',
+      'proxy/proxy_resolver_error_observer_mojo_unittest.cc',
       'proxy/proxy_resolver_mojo_unittest.cc',
       'proxy/proxy_resolver_factory_unittest.cc',
       'proxy/proxy_resolver_v8_tracing_unittest.cc',

net/proxy/in_process_mojo_proxy_resolver_factory.cc

@@ -30,9 +30,10 @@ InProcessMojoProxyResolverFactory::CreateResolver(
     const mojo::String& pac_script,
     mojo::InterfaceRequest<interfaces::ProxyResolver> req,
     interfaces::HostResolverPtr host_resolver,
+    interfaces::ProxyResolverErrorObserverPtr error_observer,
     interfaces::ProxyResolverFactoryRequestClientPtr client) {
   factory_->CreateResolver(pac_script, req.Pass(), host_resolver.Pass(),
-                           client.Pass());
+                           error_observer.Pass(), client.Pass());
   return nullptr;
 }
 

net/proxy/in_process_mojo_proxy_resolver_factory.h

@@ -25,6 +25,7 @@ class InProcessMojoProxyResolverFactory : public MojoProxyResolverFactory {
       const mojo::String& pac_script,
       mojo::InterfaceRequest<interfaces::ProxyResolver> req,
       interfaces::HostResolverPtr host_resolver,
+      interfaces::ProxyResolverErrorObserverPtr error_observer,
       interfaces::ProxyResolverFactoryRequestClientPtr client) override;
 
  private:

net/proxy/mojo_proxy_resolver_factory.h

@@ -24,6 +24,7 @@ class MojoProxyResolverFactory {
       const mojo::String& pac_script,
       mojo::InterfaceRequest<interfaces::ProxyResolver> req,
       interfaces::HostResolverPtr host_resolver,
+      interfaces::ProxyResolverErrorObserverPtr error_observer,
       interfaces::ProxyResolverFactoryRequestClientPtr client) = 0;
 
  protected:

net/proxy/mojo_proxy_resolver_factory_impl.cc

@@ -10,7 +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_error_observer_mojo.h"
 #include "net/proxy/proxy_resolver_factory.h"
 #include "net/proxy/proxy_resolver_v8.h"
 #include "net/proxy/proxy_resolver_v8_tracing.h"
@@ -19,15 +19,18 @@
 namespace net {
 namespace {
 
-scoped_ptr<ProxyResolverErrorObserver> ReturnNullErrorObserver() {
-  return nullptr;
+scoped_ptr<ProxyResolverErrorObserver> ReturnErrorObserver(
+    scoped_ptr<ProxyResolverErrorObserver> error_observer) {
+  return error_observer;
 }
 
 scoped_ptr<ProxyResolverFactory> CreateDefaultProxyResolver(
     HostResolver* host_resolver,
+    scoped_ptr<ProxyResolverErrorObserver> error_observer,
     const ProxyResolver::LoadStateChangedCallback& callback) {
   return make_scoped_ptr(new ProxyResolverFactoryV8Tracing(
-      host_resolver, nullptr, callback, base::Bind(&ReturnNullErrorObserver)));
+      host_resolver, nullptr, callback,
+      base::Bind(&ReturnErrorObserver, base::Passed(&error_observer))));
 }
 
 class LoadStateChangeForwarder
@@ -107,6 +110,7 @@ class MojoProxyResolverFactoryImpl::Job : public mojo::ErrorHandler {
       const MojoProxyResolverFactoryImpl::Factory& proxy_resolver_factory,
       mojo::InterfaceRequest<interfaces::ProxyResolver> request,
       interfaces::HostResolverPtr host_resolver,
+      interfaces::ProxyResolverErrorObserverPtr error_observer,
       interfaces::ProxyResolverFactoryRequestClientPtr client);
   ~Job() override;
 
@@ -134,6 +138,7 @@ MojoProxyResolverFactoryImpl::Job::Job(
     const MojoProxyResolverFactoryImpl::Factory& proxy_resolver_factory,
     mojo::InterfaceRequest<interfaces::ProxyResolver> request,
     interfaces::HostResolverPtr host_resolver,
+    interfaces::ProxyResolverErrorObserverPtr error_observer,
     interfaces::ProxyResolverFactoryRequestClientPtr client)
     : parent_(factory),
       host_resolver_(new HostResolverMojo(
@@ -144,6 +149,7 @@ MojoProxyResolverFactoryImpl::Job::Job(
       proxy_request_(request.Pass()),
       factory_(proxy_resolver_factory.Run(
           host_resolver_.get(),
+          ProxyResolverErrorObserverMojo::Create(error_observer.Pass()),
           base::Bind(&LoadStateChangeForwarder::OnLoadStateChanged,
                      load_state_change_forwarder_))),
       client_ptr_(client.Pass()) {
@@ -195,13 +201,14 @@ void MojoProxyResolverFactoryImpl::CreateResolver(
     const mojo::String& pac_script,
     mojo::InterfaceRequest<interfaces::ProxyResolver> request,
     interfaces::HostResolverPtr host_resolver,
+    interfaces::ProxyResolverErrorObserverPtr error_observer,
     interfaces::ProxyResolverFactoryRequestClientPtr client) {
   // The Job will call RemoveJob on |this| when either the create request
   // finishes or |request| or |client| encounters a connection error.
   jobs_.insert(new Job(
       this, ProxyResolverScriptData::FromUTF8(pac_script.To<std::string>()),
       proxy_resolver_impl_factory_, request.Pass(), host_resolver.Pass(),
-      client.Pass()));
+      error_observer.Pass(), client.Pass()));
 }
 
 void MojoProxyResolverFactoryImpl::RemoveJob(Job* job) {

net/proxy/mojo_proxy_resolver_factory_impl.h

@@ -14,12 +14,14 @@
 
 namespace net {
 class HostResolver;
+class ProxyResolverErrorObserver;
 class ProxyResolverFactory;
 
 class MojoProxyResolverFactoryImpl : public interfaces::ProxyResolverFactory {
  public:
   using Factory = base::Callback<scoped_ptr<net::ProxyResolverFactory>(
       HostResolver*,
+      scoped_ptr<ProxyResolverErrorObserver>,
       const ProxyResolver::LoadStateChangedCallback&)>;
 
   explicit MojoProxyResolverFactoryImpl(
@@ -38,6 +40,7 @@ class MojoProxyResolverFactoryImpl : public interfaces::ProxyResolverFactory {
       const mojo::String& pac_script,
       mojo::InterfaceRequest<interfaces::ProxyResolver> request,
       interfaces::HostResolverPtr host_resolver,
+      interfaces::ProxyResolverErrorObserverPtr error_observer,
       interfaces::ProxyResolverFactoryRequestClientPtr client) override;
 
   void RemoveJob(Job* job);

net/proxy/mojo_proxy_resolver_factory_impl_unittest.cc

@@ -7,6 +7,7 @@
 #include "base/strings/utf_string_conversions.h"
 #include "net/base/test_completion_callback.h"
 #include "net/proxy/mock_proxy_resolver.h"
+#include "net/proxy/proxy_resolver_error_observer.h"
 #include "net/test/event_waiter.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "third_party/mojo/src/mojo/public/cpp/bindings/binding.h"
@@ -75,6 +76,7 @@ class MojoProxyResolverFactoryImplTest
 
   scoped_ptr<ProxyResolverFactory> CreateFakeProxyResolverFactory(
       HostResolver* host_resolver,
+      scoped_ptr<ProxyResolverErrorObserver> error_observer,
       const ProxyResolver::LoadStateChangedCallback& callback) {
     EXPECT_TRUE(host_resolver);
     EXPECT_FALSE(callback.is_null());
@@ -105,12 +107,14 @@ TEST_F(MojoProxyResolverFactoryImplTest, DisconnectHostResolver) {
   interfaces::HostResolverPtr host_resolver;
   mojo::InterfaceRequest<interfaces::HostResolver> host_resolver_request =
       mojo::GetProxy(&host_resolver);
+  interfaces::ProxyResolverErrorObserverPtr error_observer;
+  mojo::GetProxy(&error_observer);
   interfaces::ProxyResolverFactoryRequestClientPtr client_ptr;
   mojo::Binding<ProxyResolverFactoryRequestClient> client_binding(
       this, mojo::GetProxy(&client_ptr));
-  factory_->CreateResolver(mojo::String::From(kScriptData),
-                           mojo::GetProxy(&proxy_resolver),
-                           host_resolver.Pass(), client_ptr.Pass());
+  factory_->CreateResolver(
+      mojo::String::From(kScriptData), mojo::GetProxy(&proxy_resolver),
+      host_resolver.Pass(), error_observer.Pass(), client_ptr.Pass());
   proxy_resolver.set_error_handler(this);
   waiter_.WaitForEvent(RESOLVER_CREATED);
   EXPECT_EQ(0, instances_destroyed_);
@@ -136,12 +140,14 @@ TEST_F(MojoProxyResolverFactoryImplTest, DisconnectProxyResolverClient) {
       mojo::GetProxy(&host_resolver);
   mojo::Binding<interfaces::HostResolver> binding(nullptr, &host_resolver);
   binding.set_error_handler(this);
+  interfaces::ProxyResolverErrorObserverPtr error_observer;
+  mojo::GetProxy(&error_observer);
   interfaces::ProxyResolverFactoryRequestClientPtr client_ptr;
   mojo::Binding<ProxyResolverFactoryRequestClient> client_binding(
       this, mojo::GetProxy(&client_ptr));
-  factory_->CreateResolver(mojo::String::From(kScriptData),
-                           mojo::GetProxy(&proxy_resolver),
-                           host_resolver.Pass(), client_ptr.Pass());
+  factory_->CreateResolver(
+      mojo::String::From(kScriptData), mojo::GetProxy(&proxy_resolver),
+      host_resolver.Pass(), error_observer.Pass(), client_ptr.Pass());
   proxy_resolver.set_error_handler(this);
   waiter_.WaitForEvent(RESOLVER_CREATED);
   EXPECT_EQ(0, instances_destroyed_);
@@ -165,12 +171,14 @@ TEST_F(MojoProxyResolverFactoryImplTest, DisconnectBoth) {
   interfaces::HostResolverPtr host_resolver;
   mojo::InterfaceRequest<interfaces::HostResolver> host_resolver_request =
       mojo::GetProxy(&host_resolver);
+  interfaces::ProxyResolverErrorObserverPtr error_observer;
+  mojo::GetProxy(&error_observer);
   interfaces::ProxyResolverFactoryRequestClientPtr client_ptr;
   mojo::Binding<ProxyResolverFactoryRequestClient> client_binding(
       this, mojo::GetProxy(&client_ptr));
-  factory_->CreateResolver(mojo::String::From(kScriptData),
-                           mojo::GetProxy(&proxy_resolver),
-                           host_resolver.Pass(), client_ptr.Pass());
+  factory_->CreateResolver(
+      mojo::String::From(kScriptData), mojo::GetProxy(&proxy_resolver),
+      host_resolver.Pass(), error_observer.Pass(), client_ptr.Pass());
   proxy_resolver.set_error_handler(this);
   waiter_.WaitForEvent(RESOLVER_CREATED);
   EXPECT_EQ(0, instances_destroyed_);
@@ -195,12 +203,14 @@ TEST_F(MojoProxyResolverFactoryImplTest, Error) {
   interfaces::HostResolverPtr host_resolver;
   mojo::InterfaceRequest<interfaces::HostResolver> host_resolver_request =
       mojo::GetProxy(&host_resolver);
+  interfaces::ProxyResolverErrorObserverPtr error_observer;
+  mojo::GetProxy(&error_observer);
   interfaces::ProxyResolverFactoryRequestClientPtr client_ptr;
   mojo::Binding<ProxyResolverFactoryRequestClient> client_binding(
       this, mojo::GetProxy(&client_ptr));
-  factory_->CreateResolver(mojo::String::From(kScriptData),
-                           mojo::GetProxy(&proxy_resolver),
-                           host_resolver.Pass(), client_ptr.Pass());
+  factory_->CreateResolver(
+      mojo::String::From(kScriptData), mojo::GetProxy(&proxy_resolver),
+      host_resolver.Pass(), error_observer.Pass(), client_ptr.Pass());
   proxy_resolver.set_error_handler(this);
   waiter_.WaitForEvent(RESOLVER_CREATED);
   EXPECT_EQ(0, instances_destroyed_);
@@ -220,12 +230,14 @@ TEST_F(MojoProxyResolverFactoryImplTest,
   interfaces::HostResolverPtr host_resolver;
   mojo::InterfaceRequest<interfaces::HostResolver> host_resolver_request =
       mojo::GetProxy(&host_resolver);
+  interfaces::ProxyResolverErrorObserverPtr error_observer;
+  mojo::GetProxy(&error_observer);
   interfaces::ProxyResolverFactoryRequestClientPtr client_ptr;
   mojo::Binding<ProxyResolverFactoryRequestClient> client_binding(
       this, mojo::GetProxy(&client_ptr));
-  factory_->CreateResolver(mojo::String::From(kScriptData),
-                           mojo::GetProxy(&proxy_resolver),
-                           host_resolver.Pass(), client_ptr.Pass());
+  factory_->CreateResolver(
+      mojo::String::From(kScriptData), mojo::GetProxy(&proxy_resolver),
+      host_resolver.Pass(), error_observer.Pass(), client_ptr.Pass());
   proxy_resolver.set_error_handler(this);
   waiter_.WaitForEvent(RESOLVER_CREATED);
   EXPECT_EQ(0, instances_destroyed_);
@@ -247,12 +259,14 @@ TEST_F(MojoProxyResolverFactoryImplTest,
       mojo::GetProxy(&host_resolver);
   mojo::Binding<interfaces::HostResolver> binding(nullptr, &host_resolver);
   binding.set_error_handler(this);
+  interfaces::ProxyResolverErrorObserverPtr error_observer;
+  mojo::GetProxy(&error_observer);
   interfaces::ProxyResolverFactoryRequestClientPtr client_ptr;
   mojo::Binding<ProxyResolverFactoryRequestClient> client_binding(
       this, mojo::GetProxy(&client_ptr));
-  factory_->CreateResolver(mojo::String::From(kScriptData),
-                           mojo::GetProxy(&proxy_resolver),
-                           host_resolver.Pass(), client_ptr.Pass());
+  factory_->CreateResolver(
+      mojo::String::From(kScriptData), mojo::GetProxy(&proxy_resolver),
+      host_resolver.Pass(), error_observer.Pass(), client_ptr.Pass());
   proxy_resolver.set_error_handler(this);
   waiter_.WaitForEvent(RESOLVER_CREATED);
   EXPECT_EQ(0, instances_destroyed_);
@@ -271,12 +285,14 @@ TEST_F(MojoProxyResolverFactoryImplTest,
       mojo::GetProxy(&host_resolver);
   mojo::Binding<interfaces::HostResolver> binding(nullptr, &host_resolver);
   binding.set_error_handler(this);
+  interfaces::ProxyResolverErrorObserverPtr error_observer;
+  mojo::GetProxy(&error_observer);
   interfaces::ProxyResolverFactoryRequestClientPtr client_ptr;
   mojo::Binding<ProxyResolverFactoryRequestClient> client_binding(
       this, mojo::GetProxy(&client_ptr));
-  factory_->CreateResolver(mojo::String::From(kScriptData),
-                           mojo::GetProxy(&proxy_resolver),
-                           host_resolver.Pass(), client_ptr.Pass());
+  factory_->CreateResolver(
+      mojo::String::From(kScriptData), mojo::GetProxy(&proxy_resolver),
+      host_resolver.Pass(), error_observer.Pass(), client_ptr.Pass());
   proxy_resolver.set_error_handler(this);
   client_binding.set_error_handler(this);
   waiter_.WaitForEvent(RESOLVER_CREATED);

net/proxy/proxy_resolver_error_observer_mojo.cc

@@ -0,0 +1,46 @@
+// 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.
+
+#include "net/proxy/proxy_resolver_error_observer_mojo.h"
+
+#include "base/bind.h"
+#include "base/location.h"
+#include "base/thread_task_runner_handle.h"
+#include "mojo/common/common_type_converters.h"
+
+namespace net {
+
+// static
+scoped_ptr<ProxyResolverErrorObserver> ProxyResolverErrorObserverMojo::Create(
+    interfaces::ProxyResolverErrorObserverPtr error_observer) {
+  if (!error_observer)
+    return nullptr;
+
+  return scoped_ptr<ProxyResolverErrorObserver>(
+      new ProxyResolverErrorObserverMojo(error_observer.Pass()));
+}
+
+void ProxyResolverErrorObserverMojo::OnPACScriptError(
+    int line_number,
+    const base::string16& error) {
+  if (!task_runner_->RunsTasksOnCurrentThread()) {
+    task_runner_->PostTask(
+        FROM_HERE, base::Bind(&ProxyResolverErrorObserverMojo::OnPACScriptError,
+                              weak_this_, line_number, error));
+    return;
+  }
+  error_observer_->OnPacScriptError(line_number, mojo::String::From(error));
+}
+
+ProxyResolverErrorObserverMojo::ProxyResolverErrorObserverMojo(
+    interfaces::ProxyResolverErrorObserverPtr error_observer)
+    : error_observer_(error_observer.Pass()),
+      task_runner_(base::ThreadTaskRunnerHandle::Get()),
+      weak_factory_(this) {
+  weak_this_ = weak_factory_.GetWeakPtr();
+}
+
+ProxyResolverErrorObserverMojo::~ProxyResolverErrorObserverMojo() = default;
+
+}  // namespace net