ProxyResolvingSocket[Factory]: Add SocketObserver support.

Bug: 875032
Cq-Include-Trybots: luci.chromium.try:linux_mojo
Change-Id: I087bd6a9b62f05ccb853db4e5b04639fab0e4e83
Reviewed-on: https://chromium-review.googlesource.com/1238795Reviewed-by: Tom Sepez <tsepez@chromium.org>
Reviewed-by: Matt Menke <mmenke@chromium.org>
Reviewed-by: Nicolas Zea <zea@chromium.org>
Commit-Queue: Maks Orlovich <morlovich@chromium.org>
Cr-Commit-Position: refs/heads/master@{#593307}

google_apis/gcm/base/socket_stream_unittest.cc

@@ -225,7 +225,7 @@ void GCMSocketStreamTest::OpenConnection() {
   mojo_socket_factory_ptr_->CreateProxyResolvingSocket(
       kDestination, true /* use_tls */,
       net::MutableNetworkTrafficAnnotationTag(TRAFFIC_ANNOTATION_FOR_TESTS),
-      mojo::MakeRequest(&mojo_socket_ptr_),
+      mojo::MakeRequest(&mojo_socket_ptr_), nullptr /* observer */,
       base::BindLambdaForTesting(
           [&](int result, const base::Optional<net::IPEndPoint>& local_addr,
               const base::Optional<net::IPEndPoint>& peer_addr,

google_apis/gcm/engine/connection_factory_impl.cc

@@ -343,7 +343,7 @@ void ConnectionFactoryImpl::StartConnection() {
   socket_factory_->CreateProxyResolvingSocket(
       current_endpoint, true /* use_tls */,
       net::MutableNetworkTrafficAnnotationTag(traffic_annotation),
-      mojo::MakeRequest(&socket_),
+      mojo::MakeRequest(&socket_), nullptr /* observer */,
       base::BindOnce(&ConnectionFactoryImpl::OnConnectDone,
                      base::Unretained(this)));
 }

google_apis/gcm/engine/connection_handler_impl_unittest.cc

@@ -239,7 +239,7 @@ void GCMConnectionHandlerImplTest::BuildSocket(const ReadList& read_list,
   mojo_socket_factory_ptr_->CreateProxyResolvingSocket(
       kDestination, true /* use_tls */,
       net::MutableNetworkTrafficAnnotationTag(TRAFFIC_ANNOTATION_FOR_TESTS),
-      mojo::MakeRequest(&mojo_socket_ptr_),
+      mojo::MakeRequest(&mojo_socket_ptr_), nullptr /* observer */,
       base::BindLambdaForTesting(
           [&](int result, const base::Optional<net::IPEndPoint>& local_addr,
               const base::Optional<net::IPEndPoint>& peer_addr,

services/network/proxy_resolving_socket_factory_mojo.cc

@@ -27,11 +27,12 @@ void ProxyResolvingSocketFactoryMojo::CreateProxyResolvingSocket(
     bool use_tls,
     const net::MutableNetworkTrafficAnnotationTag& traffic_annotation,
     mojom::ProxyResolvingSocketRequest request,
+    mojom::SocketObserverPtr observer,
     CreateProxyResolvingSocketCallback callback) {
   auto socket = std::make_unique<ProxyResolvingSocketMojo>(
       factory_impl_.CreateSocket(url, use_tls),
       static_cast<net::NetworkTrafficAnnotationTag>(traffic_annotation),
-      &tls_socket_factory_);
+      std::move(observer), &tls_socket_factory_);
   ProxyResolvingSocketMojo* socket_raw = socket.get();
   proxy_resolving_socket_bindings_.AddBinding(std::move(socket),
                                               std::move(request));

services/network/proxy_resolving_socket_factory_mojo.h

@@ -34,6 +34,7 @@ class COMPONENT_EXPORT(NETWORK_SERVICE) ProxyResolvingSocketFactoryMojo
       bool use_tls,
       const net::MutableNetworkTrafficAnnotationTag& traffic_annotation,
       mojom::ProxyResolvingSocketRequest request,
+      mojom::SocketObserverPtr observer,
       CreateProxyResolvingSocketCallback callback) override;
 
  private:

services/network/proxy_resolving_socket_mojo.cc

@@ -16,8 +16,10 @@ namespace network {
 ProxyResolvingSocketMojo::ProxyResolvingSocketMojo(
     std::unique_ptr<ProxyResolvingClientSocket> socket,
     const net::NetworkTrafficAnnotationTag& traffic_annotation,
+    mojom::SocketObserverPtr observer,
     TLSSocketFactory* tls_socket_factory)
-    : tls_socket_factory_(tls_socket_factory),
+    : observer_(std::move(observer)),
+      tls_socket_factory_(tls_socket_factory),
       socket_(std::move(socket)),
       traffic_annotation_(traffic_annotation) {}
 
@@ -112,8 +114,16 @@ void ProxyResolvingSocketMojo::OnConnectCompleted(int result) {
            std::move(send_pipe.producer_handle));
 }
 
-void ProxyResolvingSocketMojo::OnNetworkReadError(int net_error) {}
-void ProxyResolvingSocketMojo::OnNetworkWriteError(int net_error) {}
+void ProxyResolvingSocketMojo::OnNetworkReadError(int net_error) {
+  if (observer_)
+    observer_->OnReadError(net_error);
+}
+
+void ProxyResolvingSocketMojo::OnNetworkWriteError(int net_error) {
+  if (observer_)
+    observer_->OnWriteError(net_error);
+}
+
 void ProxyResolvingSocketMojo::OnShutdown() {
   socket_data_pump_ = nullptr;
   if (!pending_upgrade_to_tls_callback_.is_null())

services/network/proxy_resolving_socket_mojo.h

@@ -28,6 +28,7 @@ class COMPONENT_EXPORT(NETWORK_SERVICE) ProxyResolvingSocketMojo
   ProxyResolvingSocketMojo(
       std::unique_ptr<ProxyResolvingClientSocket> socket,
       const net::NetworkTrafficAnnotationTag& traffic_annotation,
+      mojom::SocketObserverPtr observer,
       TLSSocketFactory* tls_socket_factory);
   ~ProxyResolvingSocketMojo() override;
   void Connect(
@@ -54,6 +55,7 @@ class COMPONENT_EXPORT(NETWORK_SERVICE) ProxyResolvingSocketMojo
   const net::StreamSocket* BorrowSocket() override;
   std::unique_ptr<net::StreamSocket> TakeSocket() override;
 
+  mojom::SocketObserverPtr observer_;
   TLSSocketFactory* tls_socket_factory_;
   std::unique_ptr<ProxyResolvingClientSocket> socket_;
   const net::NetworkTrafficAnnotationTag traffic_annotation_;

services/network/proxy_resolving_socket_mojo_unittest.cc

@@ -13,6 +13,7 @@
 #include "base/test/bind_test_util.h"
 #include "base/test/scoped_task_environment.h"
 #include "mojo/public/cpp/bindings/strong_binding.h"
+#include "mojo/public/cpp/system/data_pipe_utils.h"
 #include "net/base/net_errors.h"
 #include "net/base/test_completion_callback.h"
 #include "net/dns/mock_host_resolver.h"
@@ -20,6 +21,7 @@
 #include "net/socket/socket_test_util.h"
 #include "net/traffic_annotation/network_traffic_annotation_test_helper.h"
 #include "net/url_request/url_request_test_util.h"
+#include "services/network/mojo_socket_test_util.h"
 #include "services/network/proxy_resolving_socket_factory_mojo.h"
 #include "services/network/proxy_resolving_socket_mojo.h"
 #include "services/network/socket_factory.h"
@@ -105,6 +107,7 @@ class ProxyResolvingSocketTestBase {
 
   int CreateSocketSync(
       mojom::ProxyResolvingSocketRequest request,
+      mojom::SocketObserverPtr socket_observer,
       net::IPEndPoint* peer_addr_out,
       const GURL& url,
       mojo::ScopedDataPipeConsumerHandle* receive_pipe_handle_out,
@@ -114,7 +117,7 @@ class ProxyResolvingSocketTestBase {
     factory_ptr_->CreateProxyResolvingSocket(
         url, use_tls_,
         net::MutableNetworkTrafficAnnotationTag(TRAFFIC_ANNOTATION_FOR_TESTS),
-        std::move(request),
+        std::move(request), std::move(socket_observer),
         base::BindLambdaForTesting(
             [&](int result, const base::Optional<net::IPEndPoint>& local_addr,
                 const base::Optional<net::IPEndPoint>& peer_addr,
@@ -203,10 +206,11 @@ TEST_P(ProxyResolvingSocketTest, ConnectToProxy) {
     mojo::ScopedDataPipeConsumerHandle client_socket_receive_handle;
     mojo::ScopedDataPipeProducerHandle client_socket_send_handle;
     net::IPEndPoint actual_remote_addr;
-    EXPECT_EQ(net::OK,
-              CreateSocketSync(mojo::MakeRequest(&socket), &actual_remote_addr,
-                               kDestination, &client_socket_receive_handle,
-                               &client_socket_send_handle));
+    EXPECT_EQ(net::OK, CreateSocketSync(mojo::MakeRequest(&socket),
+                                        nullptr /* socket_observer*/,
+                                        &actual_remote_addr, kDestination,
+                                        &client_socket_receive_handle,
+                                        &client_socket_send_handle));
     // Consume all read data.
     base::RunLoop().RunUntilIdle();
     if (!is_direct) {
@@ -248,9 +252,10 @@ TEST_P(ProxyResolvingSocketTest, ConnectError) {
     mojom::ProxyResolvingSocketPtr socket;
     mojo::ScopedDataPipeConsumerHandle client_socket_receive_handle;
     mojo::ScopedDataPipeProducerHandle client_socket_send_handle;
-    int status = CreateSocketSync(mojo::MakeRequest(&socket), nullptr,
-                                  kDestination, &client_socket_receive_handle,
-                                  &client_socket_send_handle);
+    int status = CreateSocketSync(
+        mojo::MakeRequest(&socket), nullptr /* socket_observer*/,
+        nullptr /* peer_addr_out */, kDestination,
+        &client_socket_receive_handle, &client_socket_send_handle);
     if (test.is_direct) {
       EXPECT_EQ(net::ERR_FAILED, status);
     } else {
@@ -292,10 +297,11 @@ TEST_P(ProxyResolvingSocketTest, BasicReadWrite) {
   mojo::ScopedDataPipeConsumerHandle client_socket_receive_handle;
   mojo::ScopedDataPipeProducerHandle client_socket_send_handle;
   const GURL kDestination("http://example.com");
-  EXPECT_EQ(net::OK,
-            CreateSocketSync(mojo::MakeRequest(&socket), nullptr, kDestination,
-                             &client_socket_receive_handle,
-                             &client_socket_send_handle));
+  EXPECT_EQ(net::OK, CreateSocketSync(mojo::MakeRequest(&socket),
+                                      nullptr /* socket_observer */,
+                                      nullptr /* peer_addr_out */, kDestination,
+                                      &client_socket_receive_handle,
+                                      &client_socket_send_handle));
   // Loop kNumIterations times to test that writes can follow reads, and reads
   // can follow writes.
   for (int j = 0; j < kNumIterations; ++j) {
@@ -346,7 +352,7 @@ TEST_F(ProxyResolvingSocketMojoTest, SocketDestroyedBeforeConnectCompletes) {
   factory()->CreateProxyResolvingSocket(
       kDestination, false,
       net::MutableNetworkTrafficAnnotationTag(TRAFFIC_ANNOTATION_FOR_TESTS),
-      mojo::MakeRequest(&socket),
+      mojo::MakeRequest(&socket), nullptr /* observer */,
       base::BindLambdaForTesting(
           [&](int result, const base::Optional<net::IPEndPoint>& local_addr,
               const base::Optional<net::IPEndPoint>& peer_addr,
@@ -359,4 +365,43 @@ TEST_F(ProxyResolvingSocketMojoTest, SocketDestroyedBeforeConnectCompletes) {
   EXPECT_EQ(net::ERR_ABORTED, net_error);
 }
 
+TEST_F(ProxyResolvingSocketMojoTest, SocketObserver) {
+  Init("DIRECT");
+
+  const char kMsg[] = "message!";
+  const char kMsgLen = strlen(kMsg);
+
+  std::vector<net::MockRead> reads = {
+      net::MockRead(kMsg),
+      net::MockRead(net::ASYNC, net::ERR_CONNECTION_ABORTED)};
+  std::vector<net::MockWrite> writes = {
+      net::MockWrite(net::ASYNC, net::ERR_TIMED_OUT)};
+
+  net::StaticSocketDataProvider data_provider(reads, writes);
+  data_provider.set_connect_data(net::MockConnect(net::ASYNC, net::OK));
+  mock_client_socket_factory()->AddSocketDataProvider(&data_provider);
+
+  const GURL kDestination("http://example.com");
+
+  mojom::ProxyResolvingSocketPtr socket;
+  mojo::ScopedDataPipeConsumerHandle client_socket_receive_handle;
+  mojo::ScopedDataPipeProducerHandle client_socket_send_handle;
+  TestSocketObserver test_observer;
+
+  int status = CreateSocketSync(
+      mojo::MakeRequest(&socket), test_observer.GetObserverPtr(),
+      nullptr /* peer_addr_out */, kDestination, &client_socket_receive_handle,
+      &client_socket_send_handle);
+  EXPECT_EQ(net::OK, status);
+
+  EXPECT_EQ(kMsg, Read(&client_socket_receive_handle, kMsgLen));
+  EXPECT_EQ(net::ERR_CONNECTION_ABORTED, test_observer.WaitForReadError());
+
+  EXPECT_TRUE(mojo::BlockingCopyFromString(kMsg, client_socket_send_handle));
+  EXPECT_EQ(net::ERR_TIMED_OUT, test_observer.WaitForWriteError());
+
+  EXPECT_TRUE(data_provider.AllReadDataConsumed());
+  EXPECT_TRUE(data_provider.AllWriteDataConsumed());
+}
+
 }  // namespace network

services/network/public/mojom/proxy_resolving_socket.mojom

@@ -56,7 +56,8 @@ interface ProxyResolvingSocketFactory {
   // when the implementation of this factory goes away.
   CreateProxyResolvingSocket(url.mojom.Url url, bool use_tls,
       MutableNetworkTrafficAnnotationTag traffic_annotation,
-      ProxyResolvingSocket& socket)
+      ProxyResolvingSocket& socket,
+      SocketObserver? observer)
      => (int32 result,
          net.interfaces.IPEndPoint? local_addr,
          net.interfaces.IPEndPoint? peer_addr,

services/network/tls_client_socket_unittest.cc

@@ -167,7 +167,7 @@ class TLSClientSocketTestBase {
     proxy_resolving_factory_->CreateProxyResolvingSocket(
         url, false /* use_tls */,
         net::MutableNetworkTrafficAnnotationTag(TRAFFIC_ANNOTATION_FOR_TESTS),
-        std::move(request),
+        std::move(request), nullptr /* observer */,
         base::BindLambdaForTesting(
             [&](int result,
                 const base::Optional<net::IPEndPoint>& actual_local_addr,