Migrate extensions UDP socket API tests to a C++ UDP echo server.

In particular, migrate SocketsUdpApiTest.SocketsUdpExtension
and SocketApiTest.SocketUDPExtension.

This CL introduces a new C++ TestUdpEchoServer class, which echoes
back the data it receives over an ephemeral UDP socket, and makes
those two tests use it.

They had been the only consumers of the SpawnedTestServer's UDP_ECHO
mode, so once this lands, that mode can be removed from the test
server.

The reason for migrating tests off of SpawnedTestServer is that
it has been a source of flaky failures and timeouts for years,
so everything that can use an in-process C++ server should be
doing so.

Bug: 492672
Change-Id: Iec532fc94ee9c33c612fca3d9392388b594707e6
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2513787Reviewed-by: Reilly Grant <reillyg@chromium.org>
Commit-Queue: Matt Menke <mmenke@chromium.org>
Cr-Commit-Position: refs/heads/master@{#823307}

chrome/browser/extensions/api/socket/socket_apitest.cc

@@ -11,10 +11,10 @@
 #include "chrome/test/base/in_process_browser_test.h"
 #include "content/public/test/browser_test.h"
 #include "extensions/browser/api/socket/socket_api.h"
+#include "extensions/browser/api/sockets_udp/test_udp_echo_server.h"
 #include "extensions/test/extension_test_message_listener.h"
 #include "extensions/test/result_catcher.h"
 #include "net/dns/mock_host_resolver.h"
-#include "net/test/embedded_test_server/embedded_test_server.h"
 
 using extensions::Extension;
 using extensions::ResultCatcher;
@@ -35,13 +35,10 @@ class SocketApiTest : public extensions::ExtensionApiTest {
 }  // namespace
 
 IN_PROC_BROWSER_TEST_F(SocketApiTest, SocketUDPExtension) {
-  std::unique_ptr<net::SpawnedTestServer> test_server(
-      new net::SpawnedTestServer(
-          net::SpawnedTestServer::TYPE_UDP_ECHO,
-          base::FilePath(FILE_PATH_LITERAL("net/data"))));
-  EXPECT_TRUE(test_server->Start());
+  extensions::TestUdpEchoServer udp_echo_server;
+  net::HostPortPair host_port_pair;
+  ASSERT_TRUE(udp_echo_server.Start(&host_port_pair));
 
-  net::HostPortPair host_port_pair = test_server->host_port_pair();
   int port = host_port_pair.port();
   ASSERT_GT(port, 0);
 

chrome/test/BUILD.gn

@@ -2145,6 +2145,7 @@ if (!is_android) {
         # should be deleted and this line removed. See the
         # chrome_extensions_browsertests target for more.
         "//extensions:chrome_extensions_browsertests",
+        "//extensions/browser:test_support",
         "//extensions/common/api",
       ]
 

chrome/test/data/extensions/api_test/socket/api/background.js

@@ -9,13 +9,9 @@ const tcpRequest = "POST /echo HTTP/1.1\r\n" +
     "0100000005320000005";
     const tcpExpectedResponsePattern = /\n0100000005320000005$/;
 
-// UDP tests use net/tools/testserver/testserver.py, which is picky about the
-// format of what it calls its UDP "echo" messages. One might go so far as to
-// mutter to oneself that it isn't an echo server at all.
-//
-// The response is based on the request but obfuscated using a random key.
-const udpRequest = "0100000005320000005hello";
-const udpExpectedResponsePattern = /0100000005320000005.{11}/;
+// UDP tests use a server that just echoes back the request.
+const udpRequest = "0100000005320000005";
+const udpExpectedResponsePattern = /^0100000005320000005$/;
 
 const socket = chrome.socket;
 var address;

extensions/browser/BUILD.gn

@@ -543,6 +543,8 @@ source_set("browser_tests") {
 source_set("test_support") {
   testonly = true
   sources = [
+    "api/sockets_udp/test_udp_echo_server.cc",
+    "api/sockets_udp/test_udp_echo_server.h",
     "browsertest_util.cc",
     "browsertest_util.h",
     "preload_check_test_util.cc",

extensions/browser/api/sockets_udp/sockets_udp_apitest.cc

@@ -6,6 +6,7 @@
 #include "base/strings/stringprintf.h"
 #include "build/build_config.h"
 #include "extensions/browser/api/sockets_udp/sockets_udp_api.h"
+#include "extensions/browser/api/sockets_udp/test_udp_echo_server.h"
 #include "extensions/browser/api_test_utils.h"
 #include "extensions/common/extension.h"
 #include "extensions/common/extension_builder.h"
@@ -14,7 +15,6 @@
 #include "extensions/test/extension_test_message_listener.h"
 #include "extensions/test/result_catcher.h"
 #include "net/dns/mock_host_resolver.h"
-#include "net/test/spawned_test_server/spawned_test_server.h"
 
 namespace extensions {
 
@@ -59,13 +59,10 @@ IN_PROC_BROWSER_TEST_F(SocketsUdpApiTest, SocketsUdpCreateGood) {
 #endif
 
 IN_PROC_BROWSER_TEST_F(SocketsUdpApiTest, MAYBE_SocketsUdpExtension) {
-  std::unique_ptr<net::SpawnedTestServer> test_server(
-      new net::SpawnedTestServer(
-          net::SpawnedTestServer::TYPE_UDP_ECHO,
-          base::FilePath(FILE_PATH_LITERAL("net/data"))));
-  EXPECT_TRUE(test_server->Start());
+  TestUdpEchoServer udp_echo_server;
+  net::HostPortPair host_port_pair;
+  ASSERT_TRUE(udp_echo_server.Start(&host_port_pair));
 
-  net::HostPortPair host_port_pair = test_server->host_port_pair();
   int port = host_port_pair.port();
   ASSERT_TRUE(port > 0);
 

extensions/browser/api/sockets_udp/test_udp_echo_server.cc

+// Copyright 2020 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 "extensions/browser/api/sockets_udp/test_udp_echo_server.h"
+
+#include "base/bind.h"
+#include "base/logging.h"
+#include "base/memory/ref_counted.h"
+#include "base/run_loop.h"
+#include "base/task/post_task.h"
+#include "net/base/host_port_pair.h"
+#include "net/base/io_buffer.h"
+#include "net/base/ip_address.h"
+#include "net/base/ip_endpoint.h"
+#include "net/base/net_errors.h"
+#include "net/log/net_log_source.h"
+#include "net/socket/udp_server_socket.h"
+
+namespace extensions {
+
+// Size of UDP buffer.  Max allowed read size.
+static const size_t kIOBufferSize = 4096;
+
+class TestUdpEchoServer::Core {
+ public:
+  Core() = default;
+  ~Core() = default;
+
+  Core(const Core&) = delete;
+  Core& operator=(const Core&) = delete;
+
+  void Start(net::HostPortPair* host_port_pair, int* result) {
+    udp_server_socket_ = std::make_unique<net::UDPServerSocket>(
+        nullptr /* net_log */, net::NetLogSource());
+    io_buffer_ = base::MakeRefCounted<net::IOBufferWithSize>(kIOBufferSize);
+
+    *result = udp_server_socket_->Listen(
+        net::IPEndPoint(net::IPAddress::IPv4Localhost(), 0 /* port */));
+    if (*result != net::OK) {
+      LOG(ERROR) << "UDP listen failed.";
+      udp_server_socket_.reset();
+      return;
+    }
+
+    net::IPEndPoint local_address;
+    *result = udp_server_socket_->GetLocalAddress(&local_address);
+    if (*result != net::OK) {
+      LOG(ERROR) << "Failed to get local address.";
+      udp_server_socket_.reset();
+      return;
+    }
+
+    *host_port_pair = net::HostPortPair::FromIPEndPoint(local_address);
+    ReadLoop();
+  }
+
+  void ReadLoop() {
+    int result = udp_server_socket_->RecvFrom(
+        io_buffer_.get(), io_buffer_->size(), &recv_from_address_,
+        base::BindOnce(&Core::OnReadComplete, base::Unretained(this)));
+    if (result != net::ERR_IO_PENDING)
+      OnReadComplete(result);
+  }
+
+  void OnReadComplete(int result) {
+    if (result < 0) {
+      // Consider read errors fatal, to avoid getting into a read error loop.
+      LOG(ERROR) << "Error reading from socket: " << net::ErrorToString(result);
+      return;
+    }
+
+    int send_result = udp_server_socket_->SendTo(
+        io_buffer_.get(), result, recv_from_address_,
+        base::BindOnce(&Core::OnSendComplete, base::Unretained(this), result));
+    if (send_result != net::ERR_IO_PENDING)
+      OnSendComplete(result, send_result);
+  }
+
+  void OnSendComplete(int expected_result, int result) {
+    // Don't consider write errors to be fatal.
+    if (result < 0) {
+      LOG(ERROR) << "Error writing to socket: " << net::ErrorToString(result);
+    } else if (result != expected_result) {
+      LOG(ERROR) << "Failed to write entire message. Expected "
+                 << expected_result << ", but wrote " << result;
+    }
+    ReadLoop();
+  }
+
+ private:
+  std::unique_ptr<net::UDPServerSocket> udp_server_socket_;
+
+  scoped_refptr<net::IOBufferWithSize> io_buffer_;
+  net::IPEndPoint recv_from_address_;
+};
+
+TestUdpEchoServer::TestUdpEchoServer() = default;
+
+TestUdpEchoServer::~TestUdpEchoServer() {
+  if (io_thread_) {
+    io_thread_->task_runner()->DeleteSoon(FROM_HERE, std::move(core_));
+    base::RunLoop run_loop;
+    io_thread_->task_runner()->PostTask(FROM_HERE, run_loop.QuitClosure());
+    run_loop.Run();
+    io_thread_.reset();
+  }
+}
+
+bool TestUdpEchoServer::Start(net::HostPortPair* host_port_pair) {
+  DCHECK(!io_thread_);
+
+  core_ = std::make_unique<Core>();
+
+  base::Thread::Options thread_options;
+  thread_options.message_pump_type = base::MessagePumpType::IO;
+  io_thread_ = std::make_unique<base::Thread>("EmbeddedTestServer IO Thread");
+  CHECK(io_thread_->StartWithOptions(thread_options));
+  CHECK(io_thread_->WaitUntilThreadStarted());
+
+  base::RunLoop run_loop;
+  int result = net::ERR_UNEXPECTED;
+  io_thread_->task_runner()->PostTaskAndReply(
+      FROM_HERE,
+      base::BindOnce(&TestUdpEchoServer::Core::Start,
+                     base::Unretained(core_.get()), host_port_pair, &result),
+      run_loop.QuitClosure());
+  run_loop.Run();
+
+  return result == net::OK;
+}
+
+}  // namespace extensions

extensions/browser/api/sockets_udp/test_udp_echo_server.h

+// Copyright 2020 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.
+
+#ifndef EXTENSIONS_BROWSER_API_SOCKETS_UDP_TEST_UDP_ECHO_SERVER_H_
+#define EXTENSIONS_BROWSER_API_SOCKETS_UDP_TEST_UDP_ECHO_SERVER_H_
+
+#include <memory>
+
+#include "base/compiler_specific.h"
+#include "base/threading/thread.h"
+#include "net/base/host_port_pair.h"
+
+namespace net {
+class HostPortPair;
+}  // namespace net
+
+namespace extensions {
+
+// Test UDP server that echos back everything it receives.
+class TestUdpEchoServer {
+ public:
+  TestUdpEchoServer();
+
+  // Destroying the server shuts it down.
+  ~TestUdpEchoServer();
+
+  TestUdpEchoServer(const TestUdpEchoServer&) = delete;
+  TestUdpEchoServer& operator=(const TestUdpEchoServer&) = delete;
+
+  // Starts the echo server, and returns an error on failure. Sets
+  // |host_port_pair| to the the host and port the server is listening on.
+  // |host_port_pair| must not be null. Spins the current message loop while
+  // waiting for the server to start.
+  bool Start(net::HostPortPair* host_port_pair) WARN_UNUSED_RESULT;
+
+ private:
+  // Class that does all the work. Created on the test server's thread, but
+  // otherwise lives an IO thread, where it is also destroyed.
+  class Core;
+
+  std::unique_ptr<base::Thread> io_thread_;
+
+  std::unique_ptr<Core> core_;
+};
+
+}  // namespace extensions
+
+#endif  // EXTENSIONS_BROWSER_API_SOCKETS_UDP_TEST_UDP_ECHO_SERVER_H_

extensions/test/data/sockets_udp/api/background.js

@@ -2,13 +2,7 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-// net/tools/testserver/testserver.py is picky about the format of what it
-// calls its "echo" messages. One might go so far as to mutter to oneself that
-// it isn't an echo server at all.
-//
-// The response is based on the request but obfuscated using a random key.
 const request = "0100000005320000005hello";
-var expectedResponsePattern = /0100000005320000005.{11}/;
 
 var address;
 var bytesSent = 0;
@@ -141,10 +135,9 @@ var testSending = function() {
     console.log("received bytes on from echo server: " + info.data.byteLength +
       "(" + info.socketId + ")");
     if (localSocketId == info.socketId) {
-      arrayBuffer2String(info.data, function(s) {
-        dataAsString = s;  // save this for error reporting
-        var match = !!s.match(expectedResponsePattern);
-        chrome.test.assertTrue(match, "Received data does not match.");
+      arrayBuffer2String(info.data, function(response) {
+        dataAsString = response;  // save this for error reporting
+        chrome.test.assertEq(request, response);
         chrome.sockets.udp.close(localSocketId, function () {
           chrome.sockets.udp.onReceive.removeListener(onReceive);
           chrome.sockets.udp.onReceiveError.removeListener(onReceiveError);