Add a Mojo Interface to Send Custom Proxy Connection Info to Browser

This interface is used in the chained CL to allow the PrefetchProxy
feature (the only consumer of the CustomProxy API) to observe the
connection failures, including Retry-After, and restrict further use of
the feature.

Bug: 1136114
Change-Id: Iea09da5f8a4f0b64dd76a0283490294176e7f090
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2587712Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Commit-Queue: Robert Ogden <robertogden@chromium.org>
Cr-Commit-Position: refs/heads/master@{#837170}

services/network/network_context.cc

@@ -1972,7 +1972,8 @@ URLRequestContextOwner NetworkContext::MakeURLRequestContext(
     std::unique_ptr<NetworkServiceProxyDelegate> proxy_delegate =
         std::make_unique<NetworkServiceProxyDelegate>(
             std::move(params_->initial_custom_proxy_config),
-            std::move(params_->custom_proxy_config_client_receiver));
+            std::move(params_->custom_proxy_config_client_receiver),
+            std::move(params_->custom_proxy_connection_observer_remote));
     proxy_delegate_ = proxy_delegate.get();
     builder.set_proxy_delegate(std::move(proxy_delegate));
   }

services/network/network_service_proxy_delegate.cc

@@ -3,6 +3,8 @@
 // found in the LICENSE file.
 
 #include "services/network/network_service_proxy_delegate.h"
+#include "base/bind.h"
+#include "base/memory/scoped_refptr.h"
 #include "net/base/url_util.h"
 #include "net/http/http_request_headers.h"
 #include "net/http/http_util.h"
@@ -96,13 +98,24 @@ void MergeRequestHeaders(net::HttpRequestHeaders* out,
 NetworkServiceProxyDelegate::NetworkServiceProxyDelegate(
     mojom::CustomProxyConfigPtr initial_config,
     mojo::PendingReceiver<mojom::CustomProxyConfigClient>
-        config_client_receiver)
+        config_client_receiver,
+    mojo::PendingRemote<mojom::CustomProxyConnectionObserver> observer_remote)
     : proxy_config_(std::move(initial_config)),
       receiver_(this, std::move(config_client_receiver)) {
   // Make sure there is always a valid proxy config so we don't need to null
   // check it.
-  if (!proxy_config_)
+  if (!proxy_config_) {
     proxy_config_ = mojom::CustomProxyConfig::New();
+  }
+
+  // |observer_remote| is an optional param for the NetworkContext.
+  if (observer_remote) {
+    observer_.Bind(std::move(observer_remote));
+    // Unretained is safe since |observer_| is owned by |this|.
+    observer_.set_disconnect_handler(
+        base::BindOnce(&NetworkServiceProxyDelegate::OnObserverDisconnect,
+                       base::Unretained(this)));
+  }
 }
 
 NetworkServiceProxyDelegate::~NetworkServiceProxyDelegate() {}
@@ -124,7 +137,11 @@ void NetworkServiceProxyDelegate::OnResolveProxy(
 }
 
 void NetworkServiceProxyDelegate::OnFallback(const net::ProxyServer& bad_proxy,
-                                             int net_error) {}
+                                             int net_error) {
+  if (observer_) {
+    observer_->OnFallback(bad_proxy, net_error);
+  }
+}
 
 void NetworkServiceProxyDelegate::OnBeforeTunnelRequest(
     const net::ProxyServer& proxy_server,
@@ -136,6 +153,12 @@ void NetworkServiceProxyDelegate::OnBeforeTunnelRequest(
 net::Error NetworkServiceProxyDelegate::OnTunnelHeadersReceived(
     const net::ProxyServer& proxy_server,
     const net::HttpResponseHeaders& response_headers) {
+  if (observer_) {
+    // Copy the response headers since mojo expects a ref counted object.
+    observer_->OnTunnelHeadersReceived(
+        proxy_server, base::MakeRefCounted<net::HttpResponseHeaders>(
+                          response_headers.raw_headers()));
+  }
   return net::OK;
 }
 
@@ -204,4 +227,8 @@ bool NetworkServiceProxyDelegate::EligibleForProxy(
   return true;
 }
 
+void NetworkServiceProxyDelegate::OnObserverDisconnect() {
+  observer_.reset();
+}
+
 }  // namespace network

services/network/network_service_proxy_delegate.h

@@ -10,6 +10,7 @@
 #include "base/component_export.h"
 #include "base/macros.h"
 #include "mojo/public/cpp/bindings/receiver.h"
+#include "mojo/public/cpp/bindings/remote.h"
 #include "net/base/proxy_delegate.h"
 #include "services/network/public/mojom/network_context.mojom.h"
 
@@ -27,10 +28,12 @@ class COMPONENT_EXPORT(NETWORK_SERVICE) NetworkServiceProxyDelegate
     : public net::ProxyDelegate,
       public mojom::CustomProxyConfigClient {
  public:
-  explicit NetworkServiceProxyDelegate(
+  NetworkServiceProxyDelegate(
       mojom::CustomProxyConfigPtr initial_config,
       mojo::PendingReceiver<mojom::CustomProxyConfigClient>
-          config_client_receiver);
+          config_client_receiver,
+      mojo::PendingRemote<mojom::CustomProxyConnectionObserver>
+          observer_remote);
   ~NetworkServiceProxyDelegate() override;
 
   void SetProxyResolutionService(
@@ -62,6 +65,8 @@ class COMPONENT_EXPORT(NETWORK_SERVICE) NetworkServiceProxyDelegate
   bool EligibleForProxy(const net::ProxyInfo& proxy_info,
                         const std::string& method) const;
 
+  void OnObserverDisconnect();
+
   // mojom::CustomProxyConfigClient implementation:
   void OnCustomProxyConfigUpdated(
       mojom::CustomProxyConfigPtr proxy_config) override;
@@ -72,6 +77,7 @@ class COMPONENT_EXPORT(NETWORK_SERVICE) NetworkServiceProxyDelegate
 
   mojom::CustomProxyConfigPtr proxy_config_;
   mojo::Receiver<mojom::CustomProxyConfigClient> receiver_;
+  mojo::Remote<mojom::CustomProxyConnectionObserver> observer_;
 
   net::ProxyResolutionService* proxy_resolution_service_ = nullptr;
 

services/network/network_service_proxy_delegate_unittest.cc

@@ -6,8 +6,11 @@
 
 #include <string>
 
+#include "base/memory/scoped_refptr.h"
+#include "base/optional.h"
 #include "base/test/task_environment.h"
 #include "mojo/public/cpp/bindings/remote.h"
+#include "mojo/public/cpp/bindings/self_owned_receiver.h"
 #include "net/traffic_annotation/network_traffic_annotation_test_helper.h"
 #include "net/url_request/url_request_test_util.h"
 #include "testing/gmock/include/gmock/gmock.h"
@@ -32,6 +35,39 @@ MATCHER_P2(Contain,
   return arg.GetHeader(expected_name, &value) && value == expected_value;
 }
 
+class TestCustomProxyConnectionObserver
+    : public mojom::CustomProxyConnectionObserver {
+ public:
+  TestCustomProxyConnectionObserver() = default;
+  ~TestCustomProxyConnectionObserver() override = default;
+
+  const base::Optional<std::pair<net::ProxyServer, int>>& FallbackArgs() const {
+    return fallback_;
+  }
+
+  const base::Optional<
+      std::pair<net::ProxyServer, scoped_refptr<net::HttpResponseHeaders>>>&
+  HeadersReceivedArgs() const {
+    return headers_received_;
+  }
+
+  // mojom::CustomProxyConnectionObserver:
+  void OnFallback(const net::ProxyServer& bad_proxy, int net_error) override {
+    fallback_ = std::make_pair(bad_proxy, net_error);
+  }
+  void OnTunnelHeadersReceived(const net::ProxyServer& proxy_server,
+                               const scoped_refptr<net::HttpResponseHeaders>&
+                                   response_headers) override {
+    headers_received_ = std::make_pair(proxy_server, response_headers);
+  }
+
+ private:
+  base::Optional<std::pair<net::ProxyServer, int>> fallback_;
+  base::Optional<
+      std::pair<net::ProxyServer, scoped_refptr<net::HttpResponseHeaders>>>
+      headers_received_;
+};
+
 class NetworkServiceProxyDelegateTest : public testing::Test {
  public:
   NetworkServiceProxyDelegateTest() {}
@@ -44,9 +80,17 @@ class NetworkServiceProxyDelegateTest : public testing::Test {
  protected:
   std::unique_ptr<NetworkServiceProxyDelegate> CreateDelegate(
       mojom::CustomProxyConfigPtr config) {
+    std::unique_ptr<TestCustomProxyConnectionObserver> observer =
+        std::make_unique<TestCustomProxyConnectionObserver>();
+    observer_ = observer.get();
+
+    mojo::PendingRemote<mojom::CustomProxyConnectionObserver> observer_remote;
+    mojo::MakeSelfOwnedReceiver(
+        std::move(observer), observer_remote.InitWithNewPipeAndPassReceiver());
+
     auto delegate = std::make_unique<NetworkServiceProxyDelegate>(
         network::mojom::CustomProxyConfig::New(),
-        client_.BindNewPipeAndPassReceiver());
+        client_.BindNewPipeAndPassReceiver(), std::move(observer_remote));
     SetConfig(std::move(config));
     return delegate;
   }
@@ -61,8 +105,14 @@ class NetworkServiceProxyDelegateTest : public testing::Test {
     task_environment_.RunUntilIdle();
   }
 
+  void RunUntilIdle() { task_environment_.RunUntilIdle(); }
+
+  TestCustomProxyConnectionObserver* TestObserver() const { return observer_; }
+
  private:
   mojo::Remote<mojom::CustomProxyConfigClient> client_;
+  // Owned by the proxy delegate returned by |CreateDelegate|.
+  TestCustomProxyConnectionObserver* observer_ = nullptr;
   std::unique_ptr<net::TestURLRequestContext> context_;
   base::test::TaskEnvironment task_environment_;
 };
@@ -70,7 +120,7 @@ class NetworkServiceProxyDelegateTest : public testing::Test {
 TEST_F(NetworkServiceProxyDelegateTest, NullConfigDoesNotCrash) {
   mojo::Remote<mojom::CustomProxyConfigClient> client;
   auto delegate = std::make_unique<NetworkServiceProxyDelegate>(
-      nullptr, client.BindNewPipeAndPassReceiver());
+      nullptr, client.BindNewPipeAndPassReceiver(), mojo::NullRemote());
 
   net::HttpRequestHeaders headers;
   auto request = CreateRequest(GURL(kHttpUrl));
@@ -295,7 +345,8 @@ TEST_F(NetworkServiceProxyDelegateTest, InitialConfigUsedForProxy) {
   config->rules.ParseFromString("http=foo");
   mojo::Remote<mojom::CustomProxyConfigClient> client;
   auto delegate = std::make_unique<NetworkServiceProxyDelegate>(
-      std::move(config), client.BindNewPipeAndPassReceiver());
+      std::move(config), client.BindNewPipeAndPassReceiver(),
+      mojo::NullRemote());
 
   net::ProxyInfo result;
   result.UseDirect();
@@ -308,4 +359,41 @@ TEST_F(NetworkServiceProxyDelegateTest, InitialConfigUsedForProxy) {
   EXPECT_TRUE(result.proxy_list().Equals(expected_proxy_list));
 }
 
+TEST_F(NetworkServiceProxyDelegateTest, OnFallbackObserved) {
+  net::ProxyServer proxy(net::ProxyServer::SCHEME_HTTP,
+                         net::HostPortPair("proxy.com", 80));
+
+  auto config = mojom::CustomProxyConfig::New();
+  config->rules.ParseFromString("http=foo");
+  auto delegate = CreateDelegate(std::move(config));
+
+  EXPECT_FALSE(TestObserver()->FallbackArgs());
+  delegate->OnFallback(proxy, net::ERR_FAILED);
+  RunUntilIdle();
+  ASSERT_TRUE(TestObserver()->FallbackArgs());
+  EXPECT_EQ(TestObserver()->FallbackArgs()->first, proxy);
+  EXPECT_EQ(TestObserver()->FallbackArgs()->second, net::ERR_FAILED);
+}
+
+TEST_F(NetworkServiceProxyDelegateTest, OnTunnelHeadersReceivedObserved) {
+  net::ProxyServer proxy(net::ProxyServer::SCHEME_HTTP,
+                         net::HostPortPair("proxy.com", 80));
+  scoped_refptr<net::HttpResponseHeaders> headers =
+      base::MakeRefCounted<net::HttpResponseHeaders>(
+          "HTTP/1.1 200\nHello: World\n\n");
+
+  auto config = mojom::CustomProxyConfig::New();
+  config->rules.ParseFromString("http=foo");
+  auto delegate = CreateDelegate(std::move(config));
+
+  EXPECT_FALSE(TestObserver()->HeadersReceivedArgs());
+  EXPECT_EQ(net::OK, delegate->OnTunnelHeadersReceived(proxy, *headers));
+  RunUntilIdle();
+  ASSERT_TRUE(TestObserver()->HeadersReceivedArgs());
+  EXPECT_EQ(TestObserver()->HeadersReceivedArgs()->first, proxy);
+  // Compare raw header strings since the headers pointer is copied.
+  EXPECT_EQ(TestObserver()->HeadersReceivedArgs()->second->raw_headers(),
+            headers->raw_headers());
+}
+
 }  // namespace network

services/network/public/mojom/network_context.mojom

@@ -73,9 +73,7 @@ const uint32 kWebSocketOptionBlockThirdPartyCookies = 2;
 // The browser can provide a CustomProxyConfig to a CustomProxyConfigClient
 // running in the network service. The network service will use the given proxy
 // configuration if a request matches the proxy rules and all the other
-// criteria contained within it. This configuration allows the browser to
-// direct the network service to set headers on requests to the given proxies
-// before and/or after the caching layer.
+// criteria contained within it.
 struct CustomProxyConfig {
   // The custom proxy rules to use. Note that ftp:// requests are not
   // supported.
@@ -97,6 +95,21 @@ struct CustomProxyConfig {
   HttpRequestHeaders connect_tunnel_headers;
 };
 
+// Observer of custom proxy connections, especially for when errors occur.
+interface CustomProxyConnectionObserver {
+  // Called when use of |bad_proxy| fails due to |net_error|. |net_error| is the
+  // network error encountered, if any, and OK if the fallback was for a reason
+  // other than a network error (e.g. the proxy service was explicitly directed
+  // to skip a proxy).
+  OnFallback(proxy_resolver.mojom.ProxyServer bad_proxy, int32 net_error);
+
+  // Called when the response headers for the proxy tunnel request have been
+  // received.
+  OnTunnelHeadersReceived(
+      proxy_resolver.mojom.ProxyServer proxy_server,
+      HttpResponseHeaders response_headers);
+};
+
 // Parameters to specify how the net::CertVerifier and net::CertVerifyProc
 // objects should be instantiated.
 struct CertVerifierCreationParams {
@@ -335,6 +348,12 @@ struct NetworkContextParams {
   pending_receiver<CustomProxyConfigClient>?
       custom_proxy_config_client_receiver;
 
+  // If |initial_custom_proxy_config| or |custom_proxy_config_client_receiver|
+  // is set, information about custom proxy connections will be reported to this
+  // observer.
+  pending_remote<CustomProxyConnectionObserver>?
+      custom_proxy_connection_observer_remote;
+
   // If |proxy_config_client_request| is non-null, this is called during
   // periods of network activity, and can be used as a signal for polling-based
   // logic to determine the proxy config.