Refactor unix domain socket.

net/socket/unix_domain_client_socket_posix_unittest.cc

+// Copyright 2014 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 "net/socket/unix_domain_client_socket_posix.h"
+
+#include <unistd.h>
+
+#include "base/bind.h"
+#include "base/files/file_path.h"
+#include "base/files/scoped_temp_dir.h"
+#include "base/memory/scoped_ptr.h"
+#include "net/base/io_buffer.h"
+#include "net/base/net_errors.h"
+#include "net/base/test_completion_callback.h"
+#include "net/socket/unix_domain_server_socket_posix.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace net {
+
+namespace {
+
+const char kSocketFilename[] = "unix_domain_socket_for_testing";
+
+bool UserCanConnectCallback(bool allow_user, uid_t uid, gid_t gid) {
+  // Here peers are running in same process. Check if they are expected.
+  EXPECT_EQ(getuid(), uid);
+  EXPECT_EQ(getgid(), gid);
+  return allow_user;
+}
+
+UnixDomainServerSocket::AuthCallback CreateAuthCallback(bool allow_user) {
+  return base::Bind(&UserCanConnectCallback, allow_user);
+}
+
+// Reads data from |socket| until it fills |buf| at least up to |min_data_len|.
+// Returns length of data read, or a net error.
+int ReadSynchronously(StreamSocket* socket,
+                      IOBuffer* buf,
+                      int buf_len,
+                      int min_data_len) {
+  scoped_refptr<DrainableIOBuffer> read_buf(
+      new DrainableIOBuffer(buf, buf_len));
+  TestCompletionCallback read_callback;
+  // Iterate reading several times (but not infinite) until it reads buf
+  // at least up to |min_data_len|.
+  for (int retry_count = 10;
+       retry_count > 0 && (read_buf->BytesConsumed() < min_data_len ||
+                           // Try at least once when min_data_len == 0.
+                           min_data_len == 0);
+       --retry_count) {
+    int rv = socket->Read(read_buf, read_buf->BytesRemaining(),
+                          read_callback.callback());
+    EXPECT_GE(read_buf->BytesRemaining(), rv);
+    if (rv == ERR_IO_PENDING) {
+      // If |min_data_len| is 0, returns ERR_IO_PENDING to distinguish the case
+      // when some data has been read.
+      if (min_data_len == 0) {
+        // No data has been read because of for-loop condition.
+        DCHECK_EQ(0, read_buf->BytesConsumed());
+        return ERR_IO_PENDING;
+      }
+      rv = read_callback.WaitForResult();
+    }
+    EXPECT_NE(ERR_IO_PENDING, rv);
+    if (rv < 0)
+      return rv;
+    read_buf->DidConsume(rv);
+  }
+  EXPECT_LE(0, read_buf->BytesRemaining());
+  return read_buf->BytesConsumed();
+}
+
+// Writes data to |socket| until it completes writing |buf| up to |buf_len|.
+// Returns length of data written, or a net error.
+int WriteSynchronously(StreamSocket* socket,
+                       IOBuffer* buf,
+                       int buf_len) {
+  scoped_refptr<DrainableIOBuffer> write_buf(
+      new DrainableIOBuffer(buf, buf_len));
+  TestCompletionCallback write_callback;
+  // Iterate writing several times (but not infinite) until it writes buf fully.
+  for (int retry_count = 10;
+       retry_count > 0 && write_buf->BytesRemaining() > 0;
+       --retry_count) {
+    int rv = socket->Write(write_buf, write_buf->BytesRemaining(),
+                           write_callback.callback());
+    EXPECT_GE(write_buf->BytesRemaining(), rv);
+    if (rv == ERR_IO_PENDING)
+      rv = write_callback.WaitForResult();
+    EXPECT_NE(ERR_IO_PENDING, rv);
+    if (rv < 0)
+      return rv;
+    write_buf->DidConsume(rv);
+  }
+  EXPECT_LE(0, write_buf->BytesRemaining());
+  return write_buf->BytesConsumed();

[mmenke, 2014/07/15 15:25:56]

Still theoretically possible for these both to fail on conforming
implementations, since we assume we can completely write to one socket before
reading anything on the other.  I believe the old tests handle this by using a
second thread, which is a bit of a pain.

Do we care enough to fix that in this CL?  I'm not sure we do.

[byungchul, 2014/07/15 17:30:45]

Default socket write buffer is much bigger than the data amount written by these
unittests. So, write() must also succeed regardless of underlying kernel
implementation. read() could be split according to kernel.
I want more deterministic unittests, but using multiple thread sounds like less
deterministic unless they are synchronized. And, I don't see any difference of
synchronized multi-threading and single-threading here.

Basically, I feel that WriteSynchronously() is overkilling. I am willing to make
unittests for async write by reducing write/read socket buffer. But, not sure if
it could be deterministic because the data amount could be written without
pending would depend on internal buffer and minimum socket buffer size besides
socket buffers.

[mmenke, 2014/07/15 19:12:57]

My general feeling is that we should depend on public APIs, and their documented
behavior.  We should not be depending on behind-the-scenes logic.  This is
needed to ensure we don't break when the underlying APIs change to have still
compliant behavior, but that is different, or when running on platforms that
violate our assumptions about how API behaves in ways not officially specced
out.  It's also possible our assumptions may be wrong, under certain
circumstances.

[byungchul, 2014/07/15 22:01:18]

At least for write(), I think socket buffer size clearly documented. See "man 7
socket".

[mmenke, 2014/07/16 18:30:35]

And that applies to all supported platforms that this test will be run on?  It's
also unclear if the buffer is used as one would expect - that is, fully
allocated on socket creation, and write everything we can, up to that limit, on
the first write attempt (And all subsequent write attempts).

Hrm...So it looks like "TEST_F(TCPSocketTest, ReadWrite)" is doing just what
this test is now doing, which makes me more comfortable not worrying about the
(rather unlikely) case this code doesn't work on a particular platform.

+}
+
+}  // namespace
+
+class UnixDomainClientSocketTest : public testing::Test {
+ protected:
+  UnixDomainClientSocketTest() {
+    EXPECT_TRUE(temp_dir_.CreateUniqueTempDir());
+    socket_path_ = temp_dir_.path().Append(kSocketFilename).value();
+  }
+
+  base::ScopedTempDir temp_dir_;
+  std::string socket_path_;
+};
+
+TEST_F(UnixDomainClientSocketTest, Connect) {
+  const bool kUseAbstractNamespace = false;
+
+  UnixDomainServerSocket server_socket(CreateAuthCallback(true),
+                                       kUseAbstractNamespace);
+  EXPECT_EQ(OK, server_socket.ListenWithAddressAndPort(socket_path_, 0, 1));
+
+  scoped_ptr<StreamSocket> accepted_socket;
+  TestCompletionCallback accept_callback;
+  EXPECT_EQ(ERR_IO_PENDING,
+            server_socket.Accept(&accepted_socket, accept_callback.callback()));
+  EXPECT_FALSE(accepted_socket);
+
+  UnixDomainClientSocket client_socket(socket_path_, kUseAbstractNamespace);
+  EXPECT_FALSE(client_socket.IsConnected());
+
+  // Connect() returns OK synchronously before server gets accept event because
+  // kernel accepts the connection.

[mmenke, 2014/07/15 15:25:56]

Since we now work with accepting the socket async, may want to rephrase this to
something like:

"Connect() generally returns OK synchronously, but we support async responses,
just in case"

[byungchul, 2014/07/15 17:30:45]

Done.

+  TestCompletionCallback connect_callback;
+  int rv = client_socket.Connect(connect_callback.callback());
+  if (rv == ERR_IO_PENDING)
+    rv = connect_callback.WaitForResult();
+  EXPECT_EQ(OK, rv);
+  EXPECT_TRUE(client_socket.IsConnected());
+  // Server has not yet been notified of the connection.
+  EXPECT_FALSE(accepted_socket);
+
+  EXPECT_EQ(OK, accept_callback.WaitForResult());
+  EXPECT_TRUE(accepted_socket);
+  EXPECT_TRUE(accepted_socket->IsConnected());
+}
+
+TEST_F(UnixDomainClientSocketTest, ConnectWithAbstractNamespace) {
+  const bool kUseAbstractNamespace = true;
+
+  UnixDomainClientSocket client_socket(socket_path_, kUseAbstractNamespace);
+  EXPECT_FALSE(client_socket.IsConnected());
+
+#if defined(OS_ANDROID) || defined(OS_LINUX)
+  UnixDomainServerSocket server_socket(CreateAuthCallback(true),
+                                       kUseAbstractNamespace);
+  EXPECT_EQ(OK, server_socket.ListenWithAddressAndPort(socket_path_, 0, 1));
+
+  scoped_ptr<StreamSocket> accepted_socket;
+  TestCompletionCallback accept_callback;
+  EXPECT_EQ(ERR_IO_PENDING,
+            server_socket.Accept(&accepted_socket, accept_callback.callback()));
+  EXPECT_FALSE(accepted_socket);
+
+  TestCompletionCallback connect_callback;
+  int rv = client_socket.Connect(connect_callback.callback());
+  if (rv == ERR_IO_PENDING)
+    rv = connect_callback.WaitForResult();
+  EXPECT_EQ(OK, rv);
+  EXPECT_TRUE(client_socket.IsConnected());
+  // Server has not yet beend notified of the connection.
+  EXPECT_FALSE(accepted_socket);
+
+  EXPECT_EQ(OK, accept_callback.WaitForResult());
+  EXPECT_TRUE(accepted_socket);
+  EXPECT_TRUE(accepted_socket->IsConnected());
+#else
+  TestCompletionCallback connect_callback;
+  EXPECT_EQ(ERR_ADDRESS_INVALID,
+            client_socket.Connect(connect_callback.callback()));
+#endif
+}
+
+TEST_F(UnixDomainClientSocketTest, ConnectToNonExistentSocket) {
+  const bool kUseAbstractNamespace = false;
+
+  UnixDomainClientSocket client_socket(socket_path_, kUseAbstractNamespace);
+  EXPECT_FALSE(client_socket.IsConnected());
+
+  TestCompletionCallback connect_callback;
+  EXPECT_EQ(ERR_FILE_NOT_FOUND,
+            client_socket.Connect(connect_callback.callback()));

[mmenke, 2014/07/15 15:25:56]

As with the successful connection tests, should allow failures to happen
asynchronously (Goes for the rest of the tests, too).

[byungchul, 2014/07/15 17:30:45]

I have no idea how to make async connect() for unix domain socket. All
operations are handled by kernel and never goes out. Unless we have mock
SocketLibevent, I don't think it is possible. And, I don't think testing with
mock SocketLibevent because UnixDomainClient/ServerSocket is just a wrapper of
SocketLibevent.

[mmenke, 2014/07/15 17:44:41]

Not suggesting you make them, just that these tests pass if they happen.  Can do
same thing as the success tests:

int rv = client_socket.Connect(connect_callback.callback());
if (rv == ERR_IO_PENDING)
  rv = connect_callback.WaitForResult();
EXPECT_EQ(ERR_FILE_NOT_FOUND, rv);

[byungchul, 2014/07/15 18:07:01]

Oh, I see what you meant.
Here, async is hanlded only for OK cases. All error cases are detected
synchronously. I don't think it is necessary, but if you feel strongly, am
willing to do it. Please confirm again.

[mmenke, 2014/07/15 19:12:57]

I see no code that guarantees this error case will be detected synchronously. 
If we open the socket but it fails to connect, we handle sync and async connect
failures in socket_libevent.  Or does the socket fail to open, before we get
that far?  If so, is there documentation of that somewhere?

[byungchul, 2014/07/15 22:01:18]

Done.

+}
+
+TEST_F(UnixDomainClientSocketTest,
+       ConnectToNonExistentSocketWithAbstractNamespace) {
+  const bool kUseAbstractNamespace = true;
+
+  UnixDomainClientSocket client_socket(socket_path_, kUseAbstractNamespace);
+  EXPECT_FALSE(client_socket.IsConnected());
+
+  TestCompletionCallback connect_callback;
+#if defined(OS_ANDROID) || defined(OS_LINUX)
+  EXPECT_EQ(ERR_CONNECTION_REFUSED,
+            client_socket.Connect(connect_callback.callback()));
+#else
+  EXPECT_EQ(ERR_ADDRESS_INVALID,
+            client_socket.Connect(connect_callback.callback()));
+#endif
+}
+
+TEST_F(UnixDomainClientSocketTest, DisconnectFromClient) {
+  UnixDomainServerSocket server_socket(CreateAuthCallback(true), false);
+  EXPECT_EQ(OK, server_socket.ListenWithAddressAndPort(socket_path_, 0, 1));
+  scoped_ptr<StreamSocket> accepted_socket;
+  TestCompletionCallback accept_callback;
+  EXPECT_EQ(ERR_IO_PENDING,
+            server_socket.Accept(&accepted_socket, accept_callback.callback()));
+  UnixDomainClientSocket client_socket(socket_path_, false);
+  TestCompletionCallback connect_callback;
+  int rv = client_socket.Connect(connect_callback.callback());
+  if (rv == ERR_IO_PENDING)
+    rv = connect_callback.WaitForResult();
+  EXPECT_EQ(OK, rv);
+
+  EXPECT_EQ(OK, accept_callback.WaitForResult());
+  EXPECT_TRUE(accepted_socket->IsConnected());
+  EXPECT_TRUE(client_socket.IsConnected());
+
+  // Disconnect from client side.
+  client_socket.Disconnect();
+  EXPECT_FALSE(client_socket.IsConnected());
+  EXPECT_FALSE(accepted_socket->IsConnected());
+}
+
+TEST_F(UnixDomainClientSocketTest, DisconnectFromServer) {

[mmenke, 2014/07/15 15:25:56]

Don't think we have any tests where we get a closed message when trying to read
or write from a socket.  The reading test is pretty easy to do, not sure about
the writing one.

The main difference from this test is that the read completion callback gets
called with an error.

[byungchul, 2014/07/15 17:30:45]

Done for reading.

+  UnixDomainServerSocket server_socket(CreateAuthCallback(true), false);
+  EXPECT_EQ(OK, server_socket.ListenWithAddressAndPort(socket_path_, 0, 1));
+  scoped_ptr<StreamSocket> accepted_socket;
+  TestCompletionCallback accept_callback;
+  EXPECT_EQ(ERR_IO_PENDING,
+            server_socket.Accept(&accepted_socket, accept_callback.callback()));
+  UnixDomainClientSocket client_socket(socket_path_, false);
+  TestCompletionCallback connect_callback;
+  int rv = client_socket.Connect(connect_callback.callback());
+  if (rv == ERR_IO_PENDING)
+    rv = connect_callback.WaitForResult();
+  EXPECT_EQ(OK, rv);
+
+  EXPECT_EQ(OK, accept_callback.WaitForResult());
+  EXPECT_TRUE(accepted_socket->IsConnected());
+  EXPECT_TRUE(client_socket.IsConnected());
+
+  // Disconnect from server side.
+  accepted_socket->Disconnect();
+  EXPECT_FALSE(accepted_socket->IsConnected());
+  EXPECT_FALSE(client_socket.IsConnected());
+}
+
+TEST_F(UnixDomainClientSocketTest, ReadAfterWrite) {
+  UnixDomainServerSocket server_socket(CreateAuthCallback(true), false);
+  EXPECT_EQ(OK, server_socket.ListenWithAddressAndPort(socket_path_, 0, 1));
+  scoped_ptr<StreamSocket> accepted_socket;
+  TestCompletionCallback accept_callback;
+  EXPECT_EQ(ERR_IO_PENDING,
+            server_socket.Accept(&accepted_socket, accept_callback.callback()));
+  UnixDomainClientSocket client_socket(socket_path_, false);
+  TestCompletionCallback connect_callback;
+  int rv = client_socket.Connect(connect_callback.callback());
+  if (rv == ERR_IO_PENDING)
+    rv = connect_callback.WaitForResult();
+  EXPECT_EQ(OK, rv);
+
+  EXPECT_EQ(OK, accept_callback.WaitForResult());
+  EXPECT_TRUE(accepted_socket->IsConnected());
+  EXPECT_TRUE(client_socket.IsConnected());
+
+  // Send data from client to server.
+  const int write_data_size = 10;
+  scoped_refptr<IOBuffer> write_buffer(
+      new StringIOBuffer(std::string(write_data_size, 'd')));
+  EXPECT_EQ(write_data_size,
+            WriteSynchronously(&client_socket,
+                               write_buffer,
+                               write_data_size));
+
+  // The buffer is bigger than write data size.
+  const int read_buffer_size = write_data_size * 2;
+  scoped_refptr<IOBuffer> read_buffer(new IOBuffer(read_buffer_size));
+  EXPECT_EQ(write_data_size,
+            ReadSynchronously(accepted_socket.get(),
+                              read_buffer,
+                              read_buffer_size,
+                              write_data_size));
+  EXPECT_EQ(std::string(write_buffer->data(), write_data_size),
+            std::string(read_buffer->data(), write_data_size));
+
+  // Send data from server and client.
+  EXPECT_EQ(write_data_size,
+            WriteSynchronously(accepted_socket.get(),
+                               write_buffer,
+                               write_data_size));
+
+  // Read multiple times.
+  const int small_read_buffer_size = write_data_size / 3;
+  EXPECT_EQ(small_read_buffer_size,
+            ReadSynchronously(&client_socket,
+                              read_buffer,
+                              small_read_buffer_size,
+                              small_read_buffer_size));
+  EXPECT_EQ(std::string(write_buffer->data(), small_read_buffer_size),
+            std::string(read_buffer->data(), small_read_buffer_size));
+
+  EXPECT_EQ(write_data_size - small_read_buffer_size,
+            ReadSynchronously(&client_socket,
+                              read_buffer,
+                              read_buffer_size,
+                              write_data_size - small_read_buffer_size));
+  EXPECT_EQ(std::string(write_buffer->data() + small_read_buffer_size,
+                        write_data_size - small_read_buffer_size),
+            std::string(read_buffer->data(),
+                        write_data_size - small_read_buffer_size));
+
+  // No more data.
+  EXPECT_EQ(ERR_IO_PENDING,
+            ReadSynchronously(&client_socket,
+                              read_buffer,
+                              read_buffer_size,
+                              0));
+
+  // Disconnect from server side after read-write.
+  accepted_socket->Disconnect();
+  EXPECT_FALSE(accepted_socket->IsConnected());
+  EXPECT_FALSE(client_socket.IsConnected());
+}
+
+TEST_F(UnixDomainClientSocketTest, ReadBeforeWrite) {
+  UnixDomainServerSocket server_socket(CreateAuthCallback(true), false);
+  EXPECT_EQ(OK, server_socket.ListenWithAddressAndPort(socket_path_, 0, 1));
+  scoped_ptr<StreamSocket> accepted_socket;
+  TestCompletionCallback accept_callback;
+  EXPECT_EQ(ERR_IO_PENDING,
+            server_socket.Accept(&accepted_socket, accept_callback.callback()));
+  UnixDomainClientSocket client_socket(socket_path_, false);
+  TestCompletionCallback connect_callback;
+  int rv = client_socket.Connect(connect_callback.callback());
+  if (rv == ERR_IO_PENDING)
+    rv = connect_callback.WaitForResult();
+  EXPECT_EQ(OK, rv);
+
+  EXPECT_EQ(OK, accept_callback.WaitForResult());
+  EXPECT_TRUE(accepted_socket->IsConnected());
+  EXPECT_TRUE(client_socket.IsConnected());
+
+  // Wait for data from client.
+  const int write_data_size = 10;
+  const int read_buffer_size = write_data_size * 2;
+  const int small_read_buffer_size = write_data_size / 3;
+  // Read smaller than write data size first.
+  scoped_refptr<IOBuffer> read_buffer(new IOBuffer(read_buffer_size));
+  TestCompletionCallback read_callback;
+  EXPECT_EQ(ERR_IO_PENDING,
+            accepted_socket->Read(read_buffer, small_read_buffer_size,
+                                  read_callback.callback()));
+
+  scoped_refptr<IOBuffer> write_buffer(
+      new StringIOBuffer(std::string(write_data_size, 'd')));
+  EXPECT_EQ(write_data_size,
+            WriteSynchronously(&client_socket,
+                               write_buffer,
+                               write_data_size));
+
+  // First read completed.
+  EXPECT_EQ(small_read_buffer_size, read_callback.WaitForResult());
+  // Read remaining data.
+  EXPECT_EQ(write_data_size - small_read_buffer_size,
+            ReadSynchronously(accepted_socket.get(),
+                              read_buffer,
+                              read_buffer_size,
+                              write_data_size - small_read_buffer_size));
+  // No more data.
+  EXPECT_EQ(ERR_IO_PENDING,
+            ReadSynchronously(accepted_socket.get(),
+                              read_buffer,
+                              read_buffer_size,
+                              0));
+
+  // Disconnect from server side after read-write.
+  accepted_socket->Disconnect();
+  EXPECT_FALSE(accepted_socket->IsConnected());
+  EXPECT_FALSE(client_socket.IsConnected());
+}
+
+}  // namespace net