Make the mapping from client id -> secret asynchronous.

remoting/protocol/pairing_registry.h

 // Copyright 2013 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 REMOTING_PROTOCOL_PAIRING_REGISTRY_H_
 #define REMOTING_PROTOCOL_PAIRING_REGISTRY_H_
 
 #include <map>
 #include <string>
 
@@ skipping 19 matching lines @@
  public:
   struct Pairing {
     std::string client_id;
     std::string client_name;
     std::string shared_secret;
   };
 
   // Mapping from client id to pairing information.
   typedef std::map<std::string, Pairing> PairedClients;
 
+  // Delegate::GetPairing callback.
+  typedef base::Callback<void(Pairing)> GetPairingCallback;
+
   // Interface representing the persistent storage back-end.
   class Delegate {
    public:
     virtual ~Delegate() {}
 
-    // Save pairing information to persistent storage. Must not block.
-    virtual void Save(const PairedClients& paired_clients) = 0;
+    // Add pairing information to persistent storage. Must not block.

[rmsousa, 2013/06/13 04:26:19]

This interface isn't safe anymore, now that there isn't a local cache. Assuming
"must not block" means that it will post to an IO thread to do the actual write
to disk, it is possible that CreatePairing would succeed, and then an attempt to
connect with those credentials immediately afterwards would fail (either because
the disk write failed, or, depending on the IO threading setup, because
GetPairing returned before the write).

It seems that this should also be asynchronous. It would finish writing, and
*then* call a callback passed by clientSession that sends the new pairing. You'd
need to add a weakptrfactory to clientsession, though.

Of course, because right now the webapp doesn't really notify the user of the
pairing response, or show the user the saved pairings, the failure cases would
look exactly the same to the user either way - the user would've requested
pairing, and it wouldn't work in the next connection. So this is only relevant
once there is some client UI for showing pairings.

[Jamie, 2013/06/13 17:22:15]

The client will never show pairing information, only the host. So, the failure
mode you describe is always benign. However, I don't think adding an
asynchronous callback is particularly difficult, so it's probably worth doing it
as a follow-up, perhaps once all the platform specific delegates are
implemented.

+    virtual void AddPairing(const Pairing& new_paired_client) = 0;
+
+    // Retrieve the pairing for the specified client id. If none is found,
+    // invoke the callback with a callback in which the client_id is empty.

[rmsousa, 2013/06/13 04:26:19]

nit: it says client_id is empty in the description, but the test in the
authenticator code is for paired_secret.

[Jamie, 2013/06/13 17:22:15]

Done.

+    virtual void GetPairing(const std::string& client_id,
+                            const GetPairingCallback& callback) = 0;
   };
 
-  explicit PairingRegistry(scoped_ptr<Delegate> delegate,
-                           const PairedClients& paired_clients);
+  explicit PairingRegistry(scoped_ptr<Delegate> delegate);
 
   // Create a pairing for a new client and save it to disk.
-  const Pairing& CreatePairing(const std::string& client_name);
+  Pairing CreatePairing(const std::string& client_name);
 
-  // Look up the shared secret for the specified client id. Returns an empty
-  // string if the client id is not known.
-  std::string GetSecret(const std::string &client_id) const;
+  // Get the pairing for the specified client id. See the corresponding
+  // Delegate method for details.
+  void GetPairing(const std::string& client_id,
+                  const GetPairingCallback& callback);
 
  private:
   friend class base::RefCountedThreadSafe<PairingRegistry>;
 
   virtual ~PairingRegistry();
 
   scoped_ptr<Delegate> delegate_;
-  PairedClients paired_clients_;
 
   DISALLOW_COPY_AND_ASSIGN(PairingRegistry);
 };
 
 // Temporary delegate that just logs NOTIMPLEMENTED for Load/Save.
 // TODO(jamiewalch): Delete once Delegates are implemented for all platforms.
 class NotImplementedPairingRegistryDelegate : public PairingRegistry::Delegate {
  public:
-  virtual void Save(
-      const PairingRegistry::PairedClients& paired_clients) OVERRIDE;
+  virtual void AddPairing(
+      const PairingRegistry::Pairing& paired_clients) OVERRIDE;
+  virtual void GetPairing(
+      const std::string& client_id,
+      const PairingRegistry::GetPairingCallback& callback) OVERRIDE;
 };
 
 }  // namespace protocol
 }  // namespace remoting
 
 #endif  // REMOTING_PROTOCOL_PAIRING_REGISTRY_H_