chromeos: Reintroduce proxy resolution service error test.

chromeos/dbus/services/proxy_resolution_service_provider_unittest.cc

 // Copyright (c) 2012 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 "chromeos/dbus/services/proxy_resolution_service_provider.h"
 
 #include "base/bind.h"
 #include "base/bind_helpers.h"
 #include "base/logging.h"
 #include "base/macros.h"
@@ skipping 43 matching lines @@
       scoped_refptr<base::SingleThreadTaskRunner> network_task_runner)
       : network_task_runner_(network_task_runner) {
     proxy_info_.UseDirect();
   }
   ~TestProxyResolver() override = default;
 
   const net::ProxyInfo& proxy_info() const { return proxy_info_; }
   net::ProxyInfo* mutable_proxy_info() { return &proxy_info_; }
 
   void set_async(bool async) { async_ = async; }
+  void set_result(net::Error result) { result_ = result; }
 
   // net::ProxyResolver:
   int GetProxyForURL(const GURL& url,
                      net::ProxyInfo* results,
                      const net::CompletionCallback& callback,
                      std::unique_ptr<Request>* request,
                      const net::NetLogWithSource& net_log) override {
     CHECK(network_task_runner_->BelongsToCurrentThread());
     results->Use(proxy_info_);
     if (!async_)
-      return net::OK;
+      return result_;
 
     base::ThreadTaskRunnerHandle::Get()->PostTask(
-        FROM_HERE, base::Bind(callback, net::OK));
+        FROM_HERE, base::Bind(callback, result_));
     return net::ERR_IO_PENDING;
   }
 
  private:
   scoped_refptr<base::SingleThreadTaskRunner> network_task_runner_;
 
   // Proxy info for GetProxyForURL() to return.
   net::ProxyInfo proxy_info_;
 
   // If true, GetProxyForURL() replies asynchronously rather than synchronously.
   bool async_ = false;
 
+  // Final result for GetProxyForURL() to return.
+  net::Error result_ = net::OK;
+
   DISALLOW_COPY_AND_ASSIGN(TestProxyResolver);
 };
 
 // Trivial net::ProxyResolverFactory implementation that synchronously creates
 // net::ForwardingProxyResolvers that forward to a single passed-in resolver.
 class TestProxyResolverFactory : public net::ProxyResolverFactory {
  public:
   // Ownership of |resolver| remains with the caller. |resolver| must outlive
   // the forwarding resolvers returned by CreateProxyResolver().
   explicit TestProxyResolverFactory(net::ProxyResolver* resolver)
@@ skipping 43 matching lines @@
   scoped_refptr<net::URLRequestContextGetter> GetRequestContext() override {
     return context_getter_;
   }
 
  private:
   // Helper method for the constructor that initializes |proxy_service_| and
   // injects it into |context_getter_|'s context.
   void CreateProxyServiceOnNetworkThread() {
     CHECK(context_getter_->GetNetworkTaskRunner()->BelongsToCurrentThread());
 
-    // The config's autodetect property needs to be set in order for
-    // net::ProxyService to send requests to our resolver.
-    net::ProxyConfig config = net::ProxyConfig::CreateAutoDetect();
+    // Setting a mandatory PAC URL makes |proxy_service_| query
+    // |proxy_resolver_| and also lets us generate
+    // net::ERR_MANDATORY_PROXY_CONFIGURATION_FAILED errors.
+    net::ProxyConfig config;
+    config.set_pac_url(GURL("http://www.example.com"));
+    config.set_pac_mandatory(true);

[Daniel Erat, 2017/04/06 01:21:39]

is it legitimate to always set this, or should i only use it when i want to
trigger an error and use CreateAutoDetect() otherwise? (that'll require some
restructuring since this is currently initialized in the test class's c'tor.)

[eroman, 2017/04/07 23:54:12]

As written is good!

     proxy_service_ = base::MakeUnique<net::ProxyService>(
         base::MakeUnique<net::ProxyConfigServiceFixed>(config),
         base::MakeUnique<TestProxyResolverFactory>(proxy_resolver_),
         nullptr /* net_log */);
     context_getter_->GetURLRequestContext()->set_proxy_service(
         proxy_service_.get());
   }
 
   // Helper method for the destructor that resets |proxy_service_|.
   void DeleteProxyServiceOnNetworkThread() {
@@ skipping 209 matching lines @@
   std::string proxy_info, error;
   EXPECT_TRUE(reader.PopString(&proxy_info));
   EXPECT_TRUE(reader.PopString(&error));
   EXPECT_EQ(proxy_resolver_->proxy_info().ToPacString(), proxy_info);
   EXPECT_EQ("", error);
 
   // No signal should've been emitted.
   EXPECT_FALSE(signal);
 }
 
+TEST_F(ProxyResolutionServiceProviderTest, ResponseError) {
+  const char kSourceURL[] = "http://www.gmail.com/";
+  proxy_resolver_->set_result(net::ERR_FAILED);
+  std::unique_ptr<dbus::Response> response;
+  std::unique_ptr<SignalInfo> signal;
+  CallMethod(kSourceURL, false /* request_signal */, &response, &signal);
+
+  // The response should contain empty proxy info and a "mandatory proxy config
+  // failed" error (which the error from the resolver will be mapped to).
+  ASSERT_TRUE(response);
+  dbus::MessageReader reader(response.get());
+  std::string proxy_info, error;
+  EXPECT_TRUE(reader.PopString(&proxy_info));
+  EXPECT_TRUE(reader.PopString(&error));
+  EXPECT_EQ("DIRECT", proxy_info);

[Daniel Erat, 2017/04/06 01:21:39]

it's a bit surprising to me that the chrome os code returns proxy info even when
an error was received, but it looks like this is what the code did before i
started trying to clean it up. hopefully all the callers do the right thing...
:-/

[eroman, 2017/04/07 23:54:12]

.. that I can't comment on, not familiar with the  protocol being used here. As
long as callers check the error first then I guess it is fine :)

+  EXPECT_EQ(net::ErrorToString(net::ERR_MANDATORY_PROXY_CONFIGURATION_FAILED),
+            error);
+
+  // No signal should've been emitted.
+  EXPECT_FALSE(signal);
+}
+
 }  // namespace chromeos