Allow hostnames in socket.connect().

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

Index: chrome/browser/extensions/api/socket/socket_api.cc
diff --git a/chrome/browser/extensions/api/socket/socket_api.cc b/chrome/browser/extensions/api/socket/socket_api.cc
index 773c3ca8e96001e44dc7791b33c612aaaad6f797..055d8d040fd5487e42492ea6103a79cb29f6ca49 100644
--- a/chrome/browser/extensions/api/socket/socket_api.cc
+++ b/chrome/browser/extensions/api/socket/socket_api.cc
@@ -5,13 +5,19 @@
 #include "chrome/browser/extensions/api/socket/socket_api.h"
 
 #include "base/bind.h"
+#include "chrome/browser/browser_process.h"
 #include "chrome/browser/extensions/api/api_resource_controller.h"
+#include "chrome/browser/extensions/api/dns/host_resolver_wrapper.h"
 #include "chrome/browser/extensions/api/socket/socket.h"
 #include "chrome/browser/extensions/api/socket/tcp_socket.h"
 #include "chrome/browser/extensions/api/socket/udp_socket.h"
 #include "chrome/browser/extensions/extension_service.h"
+#include "chrome/browser/io_thread.h"
+#include "net/base/host_port_pair.h"
 #include "net/base/io_buffer.h"
 #include "net/base/ip_endpoint.h"
+#include "net/base/net_errors.h"
+#include "net/base/net_log.h"
 
 namespace extensions {
 
@@ -84,28 +90,82 @@ void SocketDestroyFunction::Work() {
     error_ = kSocketNotFoundError;
 }
 
+SocketConnectFunction::SocketConnectFunction()
+    : io_thread_(g_browser_process->io_thread()),
+      request_handle_(new net::HostResolver::RequestHandle()),
+      addresses_(new net::AddressList) {
+}
+
+SocketConnectFunction::~SocketConnectFunction() {
+}
+
 bool SocketConnectFunction::Prepare() {
   EXTENSION_FUNCTION_VALIDATE(args_->GetInteger(0, &socket_id_));
-  EXTENSION_FUNCTION_VALIDATE(args_->GetString(1, &address_));
+  EXTENSION_FUNCTION_VALIDATE(args_->GetString(1, &hostname_));
   EXTENSION_FUNCTION_VALIDATE(args_->GetInteger(2, &port_));
   return true;
 }
 
 void SocketConnectFunction::AsyncWorkStart() {
+  StartDnsLookup();
+}
+
+void SocketConnectFunction::StartDnsLookup() {
+  net::HostResolver* host_resolver =
+      HostResolverWrapper::GetInstance()->GetHostResolver(
+          io_thread_->globals()->host_resolver.get());
+  DCHECK(host_resolver);
+
+  // Yes, we are passing zero as the port. There are some interesting but not
+  // presently relevant reasons why HostResolver asks for the port of the
+  // hostname you'd like to resolve, even though it doesn't use that value in
+  // determining its answer.
+  net::HostPortPair host_port_pair(hostname_, 0);
+
+  net::HostResolver::RequestInfo request_info(host_port_pair);
+  int resolve_result = host_resolver->Resolve(
+      request_info, addresses_.get(),
+      base::Bind(&SocketConnectFunction::OnDnsLookup, this),
+      request_handle_.get(), net::BoundNetLog());
+
+  // Balanced in OnDnsLookup.
+  AddRef();
+
+  if (resolve_result != net::ERR_IO_PENDING)
+    OnDnsLookup(resolve_result);
+}
+
+void SocketConnectFunction::OnDnsLookup(int resolve_result) {
+  if (resolve_result == net::OK) {
+    DCHECK(!addresses_->empty());
+    resolved_address_ = addresses_->front().ToStringWithoutPort();
+    StartConnect();
+  } else {
+    AddRef();  // Balanced in OnConnect().
+    OnConnect(resolve_result);
+  }
+  Release();  // Added in StartDnsLookup().

[Mihai Parparita -not on Chrome, 2012/06/07 00:29:44]

Do you need to the AddRef/Release in this method? As long as you're guaranteed
that you always start in StartDnsLookup and end up in OnConnect (even for
errors), then you can just have an AddRef in the former and a Release in the
latter, with no ref count changes in between.

[miket_OOO, 2012/06/07 17:36:13]

You're right. I simplified the code path in an earlier revision, but didn't
simplify the refcounting.

+}
+
+void SocketConnectFunction::StartConnect() {
   Socket* socket = controller()->GetSocket(socket_id_);
   if (!socket) {
     error_ = kSocketNotFoundError;
-    OnCompleted(-1);
+    OnConnect(-1);

[Mihai Parparita -not on Chrome, 2012/06/07 00:29:44]

If you do keep the current ref counting scheme, this also needs an AddRef call
to balance the Release in OnConnect.

[miket_OOO, 2012/06/07 17:36:13]

Doh. (Moot because I followed your earlier suggestion, but yes, thanks for
catching that.)

     return;
   }
 
-  socket->Connect(address_, port_,
-      base::Bind(&SocketConnectFunction::OnCompleted, this));
+  socket->Connect(resolved_address_, port_,
+                  base::Bind(&SocketConnectFunction::OnConnect, this));
+
+  // Balanced in OnConnect().
+  AddRef();
 }
 
-void SocketConnectFunction::OnCompleted(int result) {
+void SocketConnectFunction::OnConnect(int result) {
   result_.reset(Value::CreateIntegerValue(result));
   AsyncWorkCompleted();
+  Release();  // Added in StartConnect() or OnDnsLookup()'s error case.
 }
 
 bool SocketDisconnectFunction::Prepare() {
@@ -141,7 +201,7 @@ void SocketBindFunction::Work() {
 }
 
 SocketReadFunction::SocketReadFunction()
-  : params_(NULL) {
+    : params_(NULL) {
 }
 
 SocketReadFunction::~SocketReadFunction() {}
@@ -161,7 +221,7 @@ void SocketReadFunction::AsyncWorkStart() {
   }
 
   socket->Read(params_->buffer_size.get() ? *params_->buffer_size.get() : 4096,
-      base::Bind(&SocketReadFunction::OnCompleted, this));
+               base::Bind(&SocketReadFunction::OnCompleted, this));
 }
 
 void SocketReadFunction::OnCompleted(int bytes_read,
@@ -170,8 +230,8 @@ void SocketReadFunction::OnCompleted(int bytes_read,
   result->SetInteger(kResultCodeKey, bytes_read);
   if (bytes_read > 0) {
     result->Set(kDataKey,
-        base::BinaryValue::CreateWithCopiedBuffer(io_buffer->data(),
-            bytes_read));
+                base::BinaryValue::CreateWithCopiedBuffer(io_buffer->data(),
+                                                          bytes_read));
   } else {
     // BinaryValue does not support NULL buffer. Workaround it with new char[1].
     // http://crbug.com/127630
@@ -210,7 +270,7 @@ void SocketWriteFunction::AsyncWorkStart() {
   }
 
   socket->Write(io_buffer_, io_buffer_size_,
-      base::Bind(&SocketWriteFunction::OnCompleted, this));
+                base::Bind(&SocketWriteFunction::OnCompleted, this));
 }
 
 void SocketWriteFunction::OnCompleted(int bytes_written) {
@@ -222,7 +282,7 @@ void SocketWriteFunction::OnCompleted(int bytes_written) {
 }
 
 SocketRecvFromFunction::SocketRecvFromFunction()
-  : params_(NULL) {
+    : params_(NULL) {
 }
 
 SocketRecvFromFunction::~SocketRecvFromFunction() {}
@@ -242,7 +302,7 @@ void SocketRecvFromFunction::AsyncWorkStart() {
   }
 
   socket->RecvFrom(params_->buffer_size.get() ? *params_->buffer_size : 4096,
-      base::Bind(&SocketRecvFromFunction::OnCompleted, this));
+                   base::Bind(&SocketRecvFromFunction::OnCompleted, this));
 }
 
 void SocketRecvFromFunction::OnCompleted(int bytes_read,
@@ -253,8 +313,8 @@ void SocketRecvFromFunction::OnCompleted(int bytes_read,
   result->SetInteger(kResultCodeKey, bytes_read);
   if (bytes_read > 0) {
     result->Set(kDataKey,
-        base::BinaryValue::CreateWithCopiedBuffer(io_buffer->data(),
-            bytes_read));
+                base::BinaryValue::CreateWithCopiedBuffer(io_buffer->data(),
+                                                          bytes_read));
   } else {
     // BinaryValue does not support NULL buffer. Workaround it with new char[1].
     // http://crbug.com/127630
@@ -296,7 +356,7 @@ void SocketSendToFunction::AsyncWorkStart() {
   }
 
   socket->SendTo(io_buffer_, io_buffer_size_, address_, port_,
-      base::Bind(&SocketSendToFunction::OnCompleted, this));
+                 base::Bind(&SocketSendToFunction::OnCompleted, this));
 }
 
 void SocketSendToFunction::OnCompleted(int bytes_written) {
@@ -308,7 +368,7 @@ void SocketSendToFunction::OnCompleted(int bytes_written) {
 }
 
 SocketSetKeepAliveFunction::SocketSetKeepAliveFunction()
-  : params_(NULL) {
+    : params_(NULL) {
 }
 
 SocketSetKeepAliveFunction::~SocketSetKeepAliveFunction() {}
@@ -326,7 +386,7 @@ void SocketSetKeepAliveFunction::Work() {
     int delay = 0;
     if (params_->delay.get())
       delay = *params_->delay;
-      result = socket->SetKeepAlive(params_->enable, delay);
+    result = socket->SetKeepAlive(params_->enable, delay);
   } else {
     error_ = kSocketNotFoundError;
   }
@@ -334,7 +394,7 @@ void SocketSetKeepAliveFunction::Work() {
 }
 
 SocketSetNoDelayFunction::SocketSetNoDelayFunction()
-  : params_(NULL) {
+    : params_(NULL) {
 }
 
 SocketSetNoDelayFunction::~SocketSetNoDelayFunction() {}
@@ -349,9 +409,10 @@ void SocketSetNoDelayFunction::Work() {
   bool result = false;
   Socket* socket = controller()->GetSocket(params_->socket_id);
   if (socket)
-      result = socket->SetNoDelay(params_->no_delay);
+    result = socket->SetNoDelay(params_->no_delay);
   else
     error_ = kSocketNotFoundError;
   result_.reset(Value::CreateBooleanValue(result));
 }
+
 }  // namespace extensions