Make the mapping from client id -> secret asynchronous.

remoting/protocol/pairing_registry.h

Index: remoting/protocol/pairing_registry.h
diff --git a/remoting/protocol/pairing_registry.h b/remoting/protocol/pairing_registry.h
index f96097b93504a3a32d2fd9247986ea209133dc7c..2fc87491743bbba0c19a1429a33b9e3c64b9bb39 100644
--- a/remoting/protocol/pairing_registry.h
+++ b/remoting/protocol/pairing_registry.h
@@ -37,24 +37,32 @@ class PairingRegistry : public base::RefCountedThreadSafe<PairingRegistry>,
   // 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>;
@@ -62,7 +70,6 @@ class PairingRegistry : public base::RefCountedThreadSafe<PairingRegistry>,
   virtual ~PairingRegistry();
 
   scoped_ptr<Delegate> delegate_;
-  PairedClients paired_clients_;
 
   DISALLOW_COPY_AND_ASSIGN(PairingRegistry);
 };
@@ -71,8 +78,11 @@ class PairingRegistry : public base::RefCountedThreadSafe<PairingRegistry>,
 // 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