An implementation of chrome.socket.secure().

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 da173c2952ae873f8068a4db783838efd5bf2225..8aca5856d350d75d3da3d278ee33e5ca45b78345 100644
--- a/chrome/browser/extensions/api/socket/socket_api.cc
+++ b/chrome/browser/extensions/api/socket/socket_api.cc
@@ -12,6 +12,7 @@
 #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/tls_socket.h"
 #include "chrome/browser/extensions/api/socket/udp_socket.h"
 #include "chrome/browser/extensions/extension_system.h"
 #include "chrome/browser/io_thread.h"
@@ -24,6 +25,7 @@
 #include "net/base/net_errors.h"
 #include "net/base/net_log.h"
 #include "net/base/net_util.h"
+#include "net/url_request/url_request_context_getter.h"
 
 namespace extensions {
 
@@ -46,6 +48,9 @@ const char kMulticastSocketTypeError[] =
     "Only UDP socket supports multicast.";
 const char kWildcardAddress[] = "*";
 const int kWildcardPort = 0;
+const char kSecureSocketTypeError[] =
+    "Only TCP sockets are supported for TLS.";
+const char kSocketNotConnectedError[] = "Socket not connected";
 
 SocketAsyncApiFunction::SocketAsyncApiFunction() {
 }
@@ -76,6 +81,11 @@ Socket* SocketAsyncApiFunction::GetSocket(int api_resource_id) {
   return manager_->Get(extension_->id(), api_resource_id);
 }
 
+void SocketAsyncApiFunction::SetSocket(int api_resource_id,
+                                       Socket* socket) {
+  manager_->Set(extension_->id(), api_resource_id, socket);
+}
+
 base::hash_set<int>* SocketAsyncApiFunction::GetSocketIds() {
   return manager_->GetResourceIds(extension_->id());
 }
@@ -239,6 +249,7 @@ void SocketConnectFunction::AfterDnsLookup(int lookup_result) {
 }
 
 void SocketConnectFunction::StartConnect() {
+  socket_->set_hostname(hostname_);
   socket_->Connect(resolved_address_, port_,
                    base::Bind(&SocketConnectFunction::OnConnect, this));
 }
@@ -896,4 +907,69 @@ void SocketGetJoinedGroupsFunction::Work() {
   SetResult(values);
 }
 
+SocketSecureFunction::SocketSecureFunction() {}
+SocketSecureFunction::~SocketSecureFunction() {}
+
+bool SocketSecureFunction::Prepare() {
+  DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI));
+  params_ = api::socket::Secure::Params::Create(*args_);
+  EXTENSION_FUNCTION_VALIDATE(params_.get());
+  url_request_getter_ = GetProfile()->GetRequestContext();
+  return true;
+}
+
+// Override the regular implementation, which would call AsyncWorkCompleted
+// immediately after Work().
+void SocketSecureFunction::AsyncWorkStart() {
+  int result = net::ERR_INVALID_ARGUMENT;

[Ryan Sleevi, 2013/12/17 00:37:40]

You never assign to result, but I think it makes this code less readable to
constantly have to check and ensure this.

I'd recommend updating lines 929, 938, and 945 with an explicit
net::ERR_INVALID_ARGUMENT parameter.

[lally, 2014/01/09 18:47:16]

Done.

+  DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::IO));

[Ryan Sleevi, 2013/12/17 00:37:40]

style nit: Normally we put pre-condition checks first in the function (eg:
consistent with line 914)

[lally, 2014/01/09 18:47:16]

Done.

+
+  Socket* socket = GetSocket(params_->socket_id);
+  if (!socket) {
+    SetResult(new base::FundamentalValue(result));
+    error_ = kSocketNotFoundError;
+    AsyncWorkCompleted();
+    return;
+  }
+
+  TCPSocket* tcp_socket = static_cast<TCPSocket*>(socket);
+  if (socket->GetSocketType() != Socket::TYPE_TCP
+      || tcp_socket->ClientStream() == NULL) {

[Ryan Sleevi, 2013/12/17 00:37:40]

style: Wrap after binary operators, not before (
http://www.chromium.org/developers/coding-style#Code_Formatting )


DANGER: You do the cast before the comparison. Seems like this could be
rewritten as

if (socket->GetSocketType() != Socket::TYPE_TCP ||
    static_cast<TCPSocket*>(socket)->ClientStream() == NULL) {

}

[lally, 2014/01/09 18:47:16]

Done.

+    SetResult(new base::FundamentalValue(result));
+    error_ = kSecureSocketTypeError;
+    AsyncWorkCompleted();
+    return;
+  }
+
+  if (!socket->IsConnected()) {
+    SetResult(new base::FundamentalValue(result));
+    error_ = kSocketNotConnectedError;
+    AsyncWorkCompleted();
+    return;
+  }
+
+  Profile* profile = GetProfile();
+  DCHECK(profile);
+
+  TLSSocket::SecureTCPSocket(
+      socket, profile, url_request_getter_, extension_id(),
+      params_->options.get(), base::Bind(&SocketSecureFunction::TlsConnectDone,
+                                         this));
+}
+
+void SocketSecureFunction::TlsConnectDone(TLSSocket* sock, int result) {
+  if (sock) {
+    SetSocket(params_->socket_id, sock);
+  } else {
+    RemoveSocket(params_->socket_id);

[Ryan Sleevi, 2013/12/17 00:37:40]

naming: sock -> socket

DESIGN: I find this conditional confusing, with respect to |result|, in that
it's not clear if sock != NULL and result != net::OK is a valid result.

It seems like sock == NULL if result != net::OK, and sock != NULL if result ==
net::OK

[lally, 2014/01/09 18:47:16]

Ah, fair enough.  socket can only be non-null if result == net::OK.  I've got a
DCHECK in now to make sure that's true.  The error path now runs if either
(socket == NULL || result != net::OK).

+  }
+
+  results_ = api::socket::Secure::Results::Create(result);
+  if (result != net::OK) {
+    error_ = net::ErrorToString(result);
+  }
+  url_request_getter_->Release();
+  AsyncWorkCompleted();
+}
+
 }  // namespace extensions