[Android] Redirect std{in,out,err} to sockets for layout tests.

content/shell/browser/layout_test/layout_test_android.cc

Index: content/shell/browser/layout_test/layout_test_android.cc
diff --git a/content/shell/browser/layout_test/layout_test_android.cc b/content/shell/browser/layout_test/layout_test_android.cc
index 45bc77f5eab784a93162c617a5a896b9cc0dad22..a601811f7b7c93142971b37b8ee29adee6fefa96 100644
--- a/content/shell/browser/layout_test/layout_test_android.cc
+++ b/content/shell/browser/layout_test/layout_test_android.cc
@@ -4,6 +4,8 @@
 
 #include "content/shell/browser/layout_test/layout_test_android.h"
 
+#include <fcntl.h>
+#include <iostream>
 #include <memory>
 
 #include "base/android/context_utils.h"
@@ -13,28 +15,26 @@
 #include "base/command_line.h"
 #include "base/files/file_path.h"
 #include "base/message_loop/message_loop.h"
+#include "base/strings/string_number_conversions.h"
+#include "base/synchronization/waitable_event.h"
+#include "base/test/test_support_android.h"
+#include "content/public/browser/browser_thread.h"
 #include "content/public/test/nested_message_pump_android.h"
+#include "content/shell/browser/layout_test/blink_test_controller.h"
+#include "content/shell/common/layout_test/layout_test_switches.h"
 #include "content/shell/common/shell_switches.h"
 #include "jni/ShellLayoutTestUtils_jni.h"
+#include "net/base/ip_address.h"
+#include "net/base/ip_endpoint.h"
+#include "net/base/net_errors.h"
+#include "net/base/sockaddr_storage.h"
+#include "net/socket/socket_posix.h"
 #include "url/gurl.h"
 
 using base::android::ScopedJavaLocalRef;
 
 namespace {
 
-base::FilePath GetTestFilesDirectory(JNIEnv* env) {
-  ScopedJavaLocalRef<jstring> directory =
-      content::Java_ShellLayoutTestUtils_getApplicationFilesDirectory(
-          env, base::android::GetApplicationContext());
-  return base::FilePath(ConvertJavaStringToUTF8(directory));
-}
-
-void EnsureCreateFIFO(const base::FilePath& path) {
-  unlink(path.value().c_str());
-  CHECK(base::android::CreateFIFO(path, 0666))
-    << "Unable to create the Android's FIFO: " << path.value().c_str();
-}
-
 std::unique_ptr<base::MessagePump> CreateMessagePumpForUI() {
   return std::unique_ptr<base::MessagePump>(
       new content::NestedMessagePumpAndroid());
@@ -44,34 +44,138 @@ std::unique_ptr<base::MessagePump> CreateMessagePumpForUI() {
 
 namespace content {
 
-void EnsureInitializeForAndroidLayoutTests() {
-  JNIEnv* env = base::android::AttachCurrentThread();
+ScopedAndroidConfiguration::ScopedAndroidConfiguration() : sockets_() {}

[Peter Beverloo, 2016/12/06 19:24:06]

nit - no need to explicitly construct |sockets_|. The compiler does that for
you. I'd use:

  ScopedAndroidConfiguration::ScopedAndroidConfiguration() = default;

[jbudorick, 2016/12/08 02:04:13]

Done.

 
-  bool success = base::MessageLoop::InitMessagePumpForUIFactory(
-      &CreateMessagePumpForUI);
-  CHECK(success) << "Unable to initialize the message pump for Android.";
+void ConnectCompleted(base::Closure socket_connected, int rv) {

[Peter Beverloo, 2016/12/06 19:24:06]

nit: please keep the functions that aren't declared in the header file in the
anonymous namespace. That avoids accidental namespace pollution.

(It's not unreasonable that somebody else may also try to define e.g.
content::RedirectStream(), causing odd linking errors.)

[jbudorick, 2016/12/08 02:04:13]

Done.

+  CHECK_EQ(net::OK, rv) << " Failed to redirect to socket: "
+                        << net::ErrorToString(rv);
+  socket_connected.Run();
+}
 
-  // Android will need three FIFOs to communicate with the Blink test runner,
-  // one for each of [stdout, stderr, stdin].
-  base::FilePath files_dir(GetTestFilesDirectory(env));
+void CreateAndConnectSocket(
+    uint16_t port,
+    base::Callback<void(std::unique_ptr<net::SocketPosix>)> socket_open,
+    base::Callback<void(int)> socket_connected) {

[Peter Beverloo, 2016/12/06 19:24:06]

nit - avoid the base::Callback refcount churn by marking the base::Callback
parameters as const& (here and elsewhere)

[jbudorick, 2016/12/08 02:04:13]

Done.

+  net::SockaddrStorage storage;
+  net::IPAddress address;
+  CHECK(address.AssignFromIPLiteral("127.0.0.1"))
+      << "Failed to create IPAddress from IP literal 127.0.0.1.";
+  net::IPEndPoint endpoint(address, port);
+  CHECK(endpoint.ToSockAddr(storage.addr, &storage.addr_len))
+      << "Failed to convert " << endpoint.ToString() << " to sockaddr.";
+
+  std::unique_ptr<net::SocketPosix> socket(new net::SocketPosix);
+
+  int result = net::OK;
+  result = socket->Open(AF_INET);

[Peter Beverloo, 2016/12/06 19:24:06]

nit - you can combine these lines:

  int result = socket->Open(AF_INET);

[jbudorick, 2016/12/08 02:04:13]

I imagine there was some reason I had them split at some point ... at least, I
hope there was.

Done.

+  CHECK_EQ(net::OK, result) << "Failed to open socket for "
+                            << endpoint.ToString() << ": "
+                            << net::ErrorToString(result);
+
+  // Set the socket as blocking.
+  const int flags = fcntl(socket->socket_fd(), F_GETFL);

[Peter Beverloo, 2016/12/06 19:24:06]

nit - check for the error condition for the fcntl calls:

  CHECK_NE(flags, -1);

[jbudorick, 2016/12/08 02:04:13]

Done.

+  if (flags & O_NONBLOCK) {
+    fcntl(socket->socket_fd(), F_SETFL, flags & ~O_NONBLOCK);
+  }
+
+  net::CompletionCallback connect_completed = base::Bind(
+      &ConnectCompleted, base::Bind(socket_connected, socket->socket_fd()));
+  result = socket->Connect(storage, connect_completed);
+  if (result != net::ERR_IO_PENDING) {

[Peter Beverloo, 2016/12/06 19:24:06]

nit - what about other values for |result|? Should we CHECK that |result| is
either net::ERR_IO_PENDING or net::OK?

[jbudorick, 2016/12/08 02:04:13]

These currently get checked in the callback s.t. they get handled in either
case.

+    connect_completed.Run(result);
+  }
+  socket_open.Run(std::move(socket));
+}
 
-  base::FilePath stdout_fifo(files_dir.Append(FILE_PATH_LITERAL("test.fifo")));
-  EnsureCreateFIFO(stdout_fifo);
+void RedirectStdout(int fd) {
+  CHECK_NE(dup2(fd, STDOUT_FILENO), -1) << "Failed to dup2 stdout: "
+                                        << strerror(errno);
+}
 
-  base::FilePath stderr_fifo(
-      files_dir.Append(FILE_PATH_LITERAL("stderr.fifo")));
-  EnsureCreateFIFO(stderr_fifo);
+void RedirectStdin(int fd) {
+  CHECK_NE(dup2(fd, STDIN_FILENO), -1) << "Failed to dup2 stdin: "
+                                       << strerror(errno);
+}
 
-  base::FilePath stdin_fifo(files_dir.Append(FILE_PATH_LITERAL("stdin.fifo")));
-  EnsureCreateFIFO(stdin_fifo);
+void RedirectStderr(int fd) {
+  CHECK_NE(dup2(fd, STDERR_FILENO), -1) << "Failed to dup2 stderr: "
+                                        << strerror(errno);
+}
 
-  // Redirecting stdout needs to happen before redirecting stdin, which needs
-  // to happen before redirecting stderr.
-  success = base::android::RedirectStream(stdout, stdout_fifo, "w") &&
-            base::android::RedirectStream(stdin, stdin_fifo, "r") &&
-            base::android::RedirectStream(stderr, stderr_fifo, "w");
+void FinishRedirection(base::Callback<void(int)> redirect,
+                       base::WaitableEvent* event,
+                       int fd) {
+  redirect.Run(fd);
+  event->Signal();
+}
+
+void RedirectStream(
+    uint16_t port,
+    base::Callback<void(std::unique_ptr<net::SocketPosix>)> socket_open,
+    base::Callback<void(base::WaitableEvent*, int)> finish_redirection) {
+  base::WaitableEvent redirected(
+      base::WaitableEvent::ResetPolicy::MANUAL,
+      base::WaitableEvent::InitialState::NOT_SIGNALED);
+  content::BrowserThread::PostTask(
+      content::BrowserThread::IO, FROM_HERE,
+      base::Bind(&CreateAndConnectSocket, port, socket_open,
+                 base::Bind(finish_redirection, &redirected)));
+  content::ScopedAllowWaitForAndroidLayoutTests allow_wait;

[Peter Beverloo, 2016/12/06 19:24:06]

micro nit (4x in this file) - you're in content::, no need to specifically
qualify that

[jbudorick, 2016/12/08 02:04:13]

Done.

+  while (!redirected.IsSignaled())
+    redirected.Wait();
+}
+
+void ScopedAndroidConfiguration::RedirectStreams() {
+  base::Callback<void(std::unique_ptr<net::SocketPosix>)> add_socket =
+      base::Bind(&ScopedAndroidConfiguration::AddSocket,
+                 base::Unretained(this));

[Peter Beverloo, 2016/12/06 19:24:06]

nit: add a comment explaining that it's safe to use base::Unretained() because
the code's executed "synchronously".

[jbudorick, 2016/12/08 02:04:12]

Done.

+
+  std::string stdout_port_str =
+      base::CommandLine::ForCurrentProcess()->GetSwitchValueNative(
+          switches::kAndroidStdoutPort);
+  unsigned stdout_port = 0;
+  if (base::StringToUint(stdout_port_str, &stdout_port)) {
+    RedirectStream(base::checked_cast<uint16_t>(stdout_port), add_socket,
+                   base::Bind(&FinishRedirection, base::Bind(&RedirectStdout)));
+  }
+
+  std::string stdin_port_str =
+      base::CommandLine::ForCurrentProcess()->GetSwitchValueNative(
+          switches::kAndroidStdinPort);
+  unsigned stdin_port = 0;
+  if (base::StringToUint(stdin_port_str, &stdin_port)) {
+    RedirectStream(base::checked_cast<uint16_t>(stdin_port), add_socket,
+                   base::Bind(&FinishRedirection, base::Bind(&RedirectStdin)));
+  }
+
+  std::string stderr_port_str =
+      base::CommandLine::ForCurrentProcess()->GetSwitchValueNative(
+          switches::kAndroidStderrPort);
+  unsigned stderr_port = 0;
+  if (base::StringToUint(stderr_port_str, &stderr_port)) {
+    RedirectStream(base::checked_cast<uint16_t>(stderr_port), add_socket,
+                   base::Bind(&FinishRedirection, base::Bind(&RedirectStderr)));
+  }
+}
 
-  CHECK(success) << "Unable to initialize the Android FIFOs.";
+void ScopedAndroidConfiguration::AddSocket(
+    std::unique_ptr<net::SocketPosix> socket) {
+  sockets_.push_back(std::move(socket));
+}
+
+ScopedAndroidConfiguration::~ScopedAndroidConfiguration() {}
+
+void ScopedAndroidConfiguration::Initialize() {
+  JNIEnv* env = base::android::AttachCurrentThread();
+  ScopedJavaLocalRef<jstring> jtest_data_dir =
+      content::Java_ShellLayoutTestUtils_getIsolatedTestRoot(env);
+  base::FilePath test_data_dir(
+      base::android::ConvertJavaStringToUTF8(env, jtest_data_dir));
+  base::InitAndroidTestPaths(test_data_dir);
+
+  bool success =
+      base::MessageLoop::InitMessagePumpForUIFactory(&CreateMessagePumpForUI);
+  CHECK(success) << "Unable to initialize the message pump for Android.";
 }
 
 }  // namespace content