Blimp: add support for SSL connections.

blimp/client/session/assignment_source.h

Index: blimp/client/session/assignment_source.h
diff --git a/blimp/client/session/assignment_source.h b/blimp/client/session/assignment_source.h
index 9891d4a2d49c0d91cf1ffecfc4f2966bb9fd75ac..7533e96ef48febc1485b366c8d5269546d9d9435 100644
--- a/blimp/client/session/assignment_source.h
+++ b/blimp/client/session/assignment_source.h
@@ -6,10 +6,15 @@
 #define BLIMP_CLIENT_SESSION_ASSIGNMENT_SOURCE_H_
 
 #include <string>
+#include <vector>
 
 #include "base/callback.h"
+#include "base/files/file_path.h"
+#include "base/threading/thread.h"
 #include "blimp/client/blimp_client_export.h"
-#include "net/base/ip_endpoint.h"
+#include "net/base/address_list.h"
+#include "net/base/hash_value.h"
+#include "net/cert/x509_certificate.h"
 
 namespace base {
 class SingleThreadTaskRunner;
@@ -21,8 +26,14 @@ namespace client {
 // An Assignment contains the configuration data needed for a client
 // to connect to the engine.
 struct BLIMP_CLIENT_EXPORT Assignment {
-  net::IPEndPoint ip_endpoint;
+  Assignment();
+  ~Assignment();

[Wez, 2016/02/12 02:31:14]

IIUC you only need these because the struct is no longer purely POD?

[Kevin M, 2016/02/13 00:44:18]

Correct

+
+  std::vector<net::IPAddress> ip_addresses;
+  int16_t tcp_port;
+  int16_t ssl_port;

[Wez, 2016/02/12 02:31:13]

Can you inline initialize these?

These need to be uint16_t, otherwise they can't hold the full potential port
range; your command-line parsing code uses uint16_t internally, so you're
implicitly reinterpreting when you assign from one to the other - I'm a little
surprising the compiler didn't complain!

[Kevin M, 2016/02/13 00:44:18]

vOn 2016/02/12 02:31:13, Wez wrote:
Done
Hmm, yes - why no warning? <_<

Done.

   std::string client_token;
+  scoped_refptr<net::X509Certificate> cert;
 };
 
 // AssignmentSource provides functionality to find out how a client should
@@ -43,8 +54,25 @@ class BLIMP_CLIENT_EXPORT AssignmentSource {
   void GetAssignment(const AssignmentCallback& callback);
 
  private:
+  // Reads the contents of |path| into a string.
+  // Sends the contents to |callback| on the IO thread.

[Wez, 2016/02/12 02:31:13]

This comment looks to be incorrect?

As far as I can tell there is no reason for either of these functions to even
exist in this class, so having them here just increases the risk of future code
changes breaking things, with no net benefit.

You're just trying to turn a GetAssignment call into a file-read on the
IO-thread followed by a decode of the PEM file (does that also have to be on the
IO thread?), so all you need is for GetAssignment to PostTask a static function
with all the necessary data, including the calling task-runner and callback,
bound to it.

Better still, you could use PostTaskAndReply to do some of the thread-hopping
for you.

[Kevin M, 2016/02/13 00:44:18]

Done.

+  void ReadFromDisk(base::FilePath path,

[Wez, 2016/02/12 02:31:13]

|path| should be const& here

[Kevin M, 2016/02/13 00:44:18]

Done.

+                    const base::Callback<void(const std::string&)>& callback);
+
+  // Parses a certificate from PEM-encoded |cert_str| and attaches it to
+  // |assignment|. Returns the populated assignment object via |callback|.
+  void ParseCertForAssignment(Assignment assignment,

[Wez, 2016/02/12 02:31:14]

nit: It's really strange to be passing Assignment by-value; it'll end up being
copied (expensive, since it's non-POD) and then be mutable by the recipient.

As noted, though, it doesn't look like you actually need this method at all?

[Kevin M, 2016/02/13 00:44:18]

Passing around Assignments as scoped_ptr now.

+                              const AssignmentCallback& callback,
+                              const std::string& cert_str);
+
   scoped_refptr<base::SingleThreadTaskRunner> main_task_runner_;
 
+  // Thread for executing cert file reads.
+  // TODO(kmarshall): Use BlimpClientSession IO thread task runner.
+  // dtrainor@ will land a change that moves AssignmentSource into the
+  // BlimpClientSession, which manages its own IO thread.
+  base::Thread io_thread_;
+
   DISALLOW_COPY_AND_ASSIGN(AssignmentSource);
 };