Add serialization to PairingRegistry.

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 f6536a2e72efc23bb13f8545805e8d1a4dd56b8e..a01e753164d493eb87d9933a5bc31be081dd0f1b 100644
--- a/remoting/protocol/pairing_registry.h
+++ b/remoting/protocol/pairing_registry.h
@@ -6,6 +6,7 @@
 #define REMOTING_PROTOCOL_PAIRING_REGISTRY_H_
 
 #include <map>
+#include <queue>
 #include <string>
 #include <vector>
 
@@ -127,8 +128,27 @@ class PairingRegistry : public base::RefCountedThreadSafe<PairingRegistry>,
 
   virtual ~PairingRegistry();
 
+  // Helper method for unit tests.
   void AddPairing(const Pairing& pairing);
+
+  // Private implementation functions for each of the public methods. These
+  // are queued as closures by the corresponding public method if there is
+  // a request being serviced when it is called. The |callback| parameters
+  // include the original caller-supplied callback wrapped in one of the
+  // Invoke*CallbackAndScheduleNext methods defined below.
+  void SavePairingAndScheduleNext(Pairing pairing,

[Jamie, 2013/07/18 00:15:07]

I'm not convinced by the naming of these methods. They don't schedule the next
request in the queue directly (this happens because they are provided with a
suitable callback), but that's the key difference between them and the
corresponding public methods. I'm open to suggestions for a better naming
convention.

+                                  const SaveCallback& callback);
+  void GetPairingAndScheduleNext(const std::string& client_id,
+                                 const GetPairingCallback& callback);
+  void GetAllPairingsAndScheduleNext(const GetAllPairingsCallback& callback);
+  void DeletePairingAndScheduleNext(const std::string& client_id,
+                                    const SaveCallback& callback);
+  void ClearAllPairingsAndScheduleNext(const SaveCallback& callback);
+
+  // Worker functions for each of the public methods, passed as a callback to
+  // the delegate.
   void MergePairingAndSave(const Pairing& pairing,
+                           const SaveCallback& callback,
                            const std::string& pairings_json);
   void DoGetPairing(const std::string& client_id,
                     const GetPairingCallback& callback,
@@ -139,6 +159,22 @@ class PairingRegistry : public base::RefCountedThreadSafe<PairingRegistry>,
                        const SaveCallback& callback,
                        const std::string& pairings_json);
 
+  // "Trampoline" callbacks that schedule the next pending request and then
+  // invoke the original caller-supplied callback.
+  void InvokeLoadCallbackAndScheduleNext(
+      const LoadCallback& callback, const std::string& pairings_json);
+  void InvokeSaveCallbackAndScheduleNext(
+      const SaveCallback& callback, bool success);
+  void InvokeGetPairingCallbackAndScheduleNext(
+      const GetPairingCallback& callback, Pairing pairing);
+  void InvokeGetAllPairingsCallbackAndScheduleNext(
+      const GetAllPairingsCallback& callback,
+      scoped_ptr<base::ListValue> pairings);
+
+  // Queue management methods.
+  void ServiceOrQueueRequest(const base::Closure& request);
+  void ServiceNextRequest();
+
   // Translate between the structured and serialized forms of the pairing data.
   static PairedClients DecodeJson(const std::string& pairings_json);
   static std::string EncodeJson(const PairedClients& clients);
@@ -148,6 +184,9 @@ class PairingRegistry : public base::RefCountedThreadSafe<PairingRegistry>,
 
   scoped_ptr<Delegate> delegate_;
 
+  std::queue<base::Closure> pending_requests_;
+  bool servicing_request_;

[Lambros, 2013/07/18 20:34:51]

Suggestion: if you keep the currently-serviced request on the queue, and only
pop it off after completion, I think you won't need the separate flag - you can
just test the queue for emptiness.

[Jamie, 2013/07/18 22:44:56]

Done.

+
   DISALLOW_COPY_AND_ASSIGN(PairingRegistry);
 };