An implementation of chrome.socket.secure().

chrome/browser/extensions/api/sockets_tcp/sockets_tcp_api.cc

Index: chrome/browser/extensions/api/sockets_tcp/sockets_tcp_api.cc
diff --git a/chrome/browser/extensions/api/sockets_tcp/sockets_tcp_api.cc b/chrome/browser/extensions/api/sockets_tcp/sockets_tcp_api.cc
index fbf02dc0a1e0561a3a3e7ed3c3f0399a2e7e4974..f2f71a8e08ad620d7c812cbb51559c71d34f624a 100644
--- a/chrome/browser/extensions/api/sockets_tcp/sockets_tcp_api.cc
+++ b/chrome/browser/extensions/api/sockets_tcp/sockets_tcp_api.cc
@@ -5,6 +5,7 @@
 #include "chrome/browser/extensions/api/sockets_tcp/sockets_tcp_api.h"
 
 #include "chrome/browser/extensions/api/socket/tcp_socket.h"
+#include "chrome/browser/extensions/api/socket/tls_socket.h"
 #include "chrome/browser/extensions/api/sockets_tcp/tcp_socket_event_dispatcher.h"
 #include "chrome/common/extensions/api/sockets/sockets_manifest_data.h"
 #include "content/public/common/socket_permission_request.h"
@@ -18,6 +19,9 @@ namespace {
 
 const char kSocketNotFoundError[] = "Socket not found";
 const char kPermissionError[] = "Does not have permission";
+const char kSecureSocketTypeError[] =
+    "Only TCP sockets are supported for TLS.";

[rpaquay, 2013/12/16 20:18:46]

This message should not be needed (use DCHEK instead), as all sockets used with
this API will be of type TCP.

[lally, 2013/12/16 22:20:07]

Ah, fixed.  I was actually trying to stop already-TLS sockets from trying to
re-secure, but didn't adjust the error messages or symbols.  Fixed.

+const char kSocketNotConnectedError[] = "Socket not connected";
 
 linked_ptr<SocketInfo> CreateSocketInfo(int socket_id,
                                         ResumableTCPSocket* socket) {
@@ -440,5 +444,71 @@ void SocketsTcpGetSocketsFunction::Work() {
   results_ = sockets_tcp::GetSockets::Results::Create(socket_infos);
 }
 
+SocketsTcpSecureFunction::SocketsTcpSecureFunction() {}
+SocketsTcpSecureFunction::~SocketsTcpSecureFunction() {}
+
+bool SocketsTcpSecureFunction::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 SocketsTcpSecureFunction::AsyncWorkStart() {
+  int result = net::ERR_INVALID_ARGUMENT;
+  DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::IO));
+
+  Socket* socket = GetSocket(params_->socket_id);
+  Profile* profile = GetProfile();
+  DCHECK(profile);
+
+  if (!socket) {
+    SetResult(new base::FundamentalValue(result));
+    error_ = kSocketNotFoundError;
+    AsyncWorkCompleted();
+    return;
+  }
+
+  TCPSocket* tcp_socket = static_cast<TCPSocket*>(socket);
+  if (socket->GetSocketType() != Socket::TYPE_TCP

[rpaquay, 2013/12/16 20:18:46]

The check for TYPE_TCP should be DCHECK, because it should always be the case. I
am not 100% sure what "ClientStream()" is supposed to check, but it seems like
it should be part of the DCHECK too (unless we are checking for client
connection established).

[lally, 2013/12/16 22:20:07]

I've added some comments and clarified this part.

+      || tcp_socket->ClientStream() == NULL) {
+    SetResult(new base::FundamentalValue(result));
+    error_ = kSecureSocketTypeError;
+    AsyncWorkCompleted();
+    return;
+  }
+
+  if (!socket->IsConnected()) {
+    SetResult(new base::FundamentalValue(result));
+    error_ = kSocketNotConnectedError;
+    AsyncWorkCompleted();
+    return;
+  }
+
+  TLSSocket::SecureTCPSocket(
+      socket, profile, url_request_getter_, extension_id(),
+      params_->options.get(),
+      base::Bind(&SocketsTcpSecureFunction::TlsConnectDone,
+                 this));
+}
+
+void SocketsTcpSecureFunction::TlsConnectDone(TLSSocket* sock, int result) {
+  SetResult(new base::FundamentalValue(result));

[rpaquay, 2013/12/16 20:18:46]

This call is not needed if you always use "results_ = " as noted below.

[lally, 2013/12/16 22:20:07]

Ok, done.  Thanks.

+  if (sock) {
+    SetSocket(params_->socket_id, sock);
+  } else {
+    RemoveSocket(params_->socket_id);
+  }
+
+  if (result != net::OK) {
+    error_ = net::ErrorToString(result);
+    results_ = api::socket::Secure::Results::Create(result);

[rpaquay, 2013/12/16 20:18:46]

1) move this out of the if block
2) RHS should be "api::sockets::tcp::Secure::Results::Create(result)";

[lally, 2013/12/16 22:20:07]

Done.

+  }
+  AsyncWorkCompleted();
+}
+
 }  // namespace api
 }  // namespace extensions