Blimp: add support for SSL connections.

blimp/client/session/assignment_source.h

 // Copyright 2016 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #ifndef BLIMP_CLIENT_SESSION_ASSIGNMENT_SOURCE_H_
 #define BLIMP_CLIENT_SESSION_ASSIGNMENT_SOURCE_H_
 
 #include <string>
+#include <vector>
 
 #include "base/callback.h"
+#include "base/files/file_path.h"

[Ryan Sleevi, 2016/02/19 22:56:08]

Unused

[Kevin M, 2016/02/22 22:53:31]

Done.

+#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"

[Ryan Sleevi, 2016/02/19 22:56:08]

Unused

[Kevin M, 2016/02/22 22:53:31]

Done.

+#include "net/cert/x509_certificate.h"

[Ryan Sleevi, 2016/02/19 22:56:08]

Can forward declare

[Kevin M, 2016/02/22 22:53:31]

Done.

 
 namespace base {
 class SingleThreadTaskRunner;
 }
 
 namespace blimp {
 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();
+
+  std::vector<net::IPAddress> ip_addresses;

[Ryan Sleevi, 2016/02/19 22:56:08]

STYLE: Document this. It took me a while of reading the code to figure out how
this is used, and I'm not entirely sure it's correct/desirable.

That is, you're seemingly using this "as if" it was resolved - that is,
addresses have a non-deterministic order in which they'll be attempted. Further,
you only ever use one IP address - it seems like it'd make your later
construction of the AddressList easier if you just stored a single
net::IPAddress  here (and then later constructed the AddressList using
https://code.google.com/p/chromium/codesearch#chromium/src/net/base/address_l...
)

Alternatively, if you really do want that behaviour, then it seems better to
store both the TCP addresses and the SSL addresses themselves in individual
net::AddressLists here (which encompass both the net::IPAddress and the port)

[Kevin M, 2016/02/22 22:53:31]

Agreed that multiple addresses contribute to ambiguity. I'll switch to one
IPEndpoint host+port pair with a protocol enum, so it's 100% clear what the
endpoint should be. In the future we might want to support multiple connection
types (e.g. favor QUIC connections first, then fallback to TLS) but we can add
that capability pretty easily. No sense in overdesigning up front.

+  uint16_t tcp_port = 0;
+  uint16_t ssl_port = 0;
   std::string client_token;
+  scoped_refptr<net::X509Certificate> cert;
 };
 
 // AssignmentSource provides functionality to find out how a client should
 // connect to an engine.
 class BLIMP_CLIENT_EXPORT AssignmentSource {
  public:
   typedef const base::Callback<void(const Assignment&)> AssignmentCallback;
 
   // The |main_task_runner| should be the task runner for the UI thread because
   // this will in some cases be used to trigger user interaction on the UI
   // thread.
   AssignmentSource(
       const scoped_refptr<base::SingleThreadTaskRunner>& main_task_runner);
-  virtual ~AssignmentSource();
+  ~AssignmentSource();
 
   // Retrieves a valid assignment for the client and posts the result to the
   // given callback.
   void GetAssignment(const AssignmentCallback& callback);
 
  private:
+  // Returns the TaskRunner for the |io_thread_|.
+  scoped_refptr<base::SingleThreadTaskRunner> GetIOTaskRunner();
+
   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.
+  scoped_ptr<base::Thread> io_thread_;
+
   DISALLOW_COPY_AND_ASSIGN(AssignmentSource);
 };
 
 }  // namespace client
 }  // namespace blimp
 
 #endif  // BLIMP_CLIENT_SESSION_ASSIGNMENT_SOURCE_H_