Test that debug stub works with browser.

chrome/browser/nacl_host/nacl_process_host.cc

Index: chrome/browser/nacl_host/nacl_process_host.cc
===================================================================
--- chrome/browser/nacl_host/nacl_process_host.cc	(revision 163266)
+++ chrome/browser/nacl_host/nacl_process_host.cc	(working copy)
@@ -656,10 +656,26 @@
 
 #if defined(OS_POSIX)
 SocketDescriptor NaClProcessHost::GetDebugStubSocketHandle() {
-  SocketDescriptor s = net::TCPListenSocket::CreateAndBind("127.0.0.1",
-                                                           kDebugStubPort);
+  NaClBrowser* nacl_browser = NaClBrowser::GetInstance();

[Mark Seaborn, 2012/10/23 01:14:50]

Instead of adding HasDebugStubPortListener() etc., could you just add global
function pointer and do something like this here:

  if (g_replacement_socket_binder_for_testing)
    return g_replacement_socket_binder_for_testing()

The new code in nacl_browser.{h,cc} seems like overkill.

[halyavin, 2012/10/23 11:59:29]

Test lives in an object. A function can't call an object unless I introduce one
more global variable. Also global variables introduce inter-test dependencies.
Another test can fail, if previous test forgot to clear some global variable. So
I don't think this is a good idea.

[Mark Seaborn, 2012/12/04 21:46:42]

NaClBrowser::GetInstance() returns a global singleton argument, so the problem
still applies.  But maybe scoping the globals inside an object, as you've done,
is cleaner.

+  SocketDescriptor s;
+  LOG(INFO) << "Create debug stub socket";
+  if (nacl_browser->HasDebugStubPortListener()) {
+    LOG(INFO) << "allocate ephemeral port";

[Mark Seaborn, 2012/10/23 01:14:50]

These are too many LOG(INFO)s to commit.

[halyavin, 2012/10/23 11:59:29]

I inserted them for debugging and forgot to remove after upload. Done.

+    s = net::TCPListenSocket::CreateAndBind("127.0.0.1", 0);
+    int port = net::TCPListenSocket::GetBindedPort(s);
+    LOG(INFO) << "port number=" << port;
+    if (port < 0) {
+      LOG(ERROR) << "failed to open ephemeral server port for debug stub";
+      close(s);
+      return net::TCPListenSocket::kInvalidSocket;
+    }
+    nacl_browser->DebugStubNewPortAllocated(port);
+  } else {
+    s = net::TCPListenSocket::CreateAndBind("127.0.0.1", kDebugStubPort);
+  }
   if (listen(s, 1)) {
     LOG(ERROR) << "listen() failed on debug stub socket";
+    close(s);

[Mark Seaborn, 2012/10/23 01:14:50]

This is fixing an existing leak, right?  A separate change would be better, but
please at least note the fix in the commit message.

[halyavin, 2012/10/23 11:59:29]

Done.

     return net::TCPListenSocket::kInvalidSocket;
   }
   return s;