Add serialization to PairingRegistry.

remoting/protocol/pairing_registry_unittest.cc

Index: remoting/protocol/pairing_registry_unittest.cc
diff --git a/remoting/protocol/pairing_registry_unittest.cc b/remoting/protocol/pairing_registry_unittest.cc
index b113f1369e7937c8a18af51570f8b6883d38e735..407a08768960a2be637a5b69bd7293d6880c1442 100644
--- a/remoting/protocol/pairing_registry_unittest.cc
+++ b/remoting/protocol/pairing_registry_unittest.cc
@@ -38,6 +38,15 @@ void ExpectSaveSuccess(bool success) {
   EXPECT_TRUE(success);
 }
 
+void ExpectClientName(const std::string& expected,
+                      PairingRegistry::Pairing actual) {
+  EXPECT_EQ(expected, actual.client_name());
+}
+
+void ExpectNoPairings(scoped_ptr<base::ListValue> pairings) {
+  EXPECT_TRUE(pairings->empty());
+}
+
 }  // namespace
 
 namespace remoting {
@@ -178,5 +187,34 @@ TEST_F(PairingRegistryTest, ClearAllPairings) {
   EXPECT_TRUE(pairings_->empty());
 }
 
+TEST_F(PairingRegistryTest, SerializedRequests) {
+  MockPairingRegistryDelegate* mock_delegate =
+      new MockPairingRegistryDelegate();
+  scoped_ptr<PairingRegistry::Delegate> delegate(mock_delegate);
+  mock_delegate->set_run_save_callback_automatically(false);
+
+  scoped_refptr<PairingRegistry> registry(new PairingRegistry(delegate.Pass()));
+  PairingRegistry::Pairing pairing_1 = registry->CreatePairing("client1");
+  PairingRegistry::Pairing pairing_2 = registry->CreatePairing("client2");
+  registry->GetPairing(pairing_1.client_id(),
+                       base::Bind(ExpectClientName, "client1"));
+  registry->GetPairing(pairing_2.client_id(),
+                       base::Bind(ExpectClientName, "client2"));
+  registry->DeletePairing(pairing_2.client_id(), base::Bind(ExpectSaveSuccess));
+  registry->GetPairing(pairing_1.client_id(),
+                       base::Bind(ExpectClientName, "client1"));
+  registry->GetPairing(pairing_2.client_id(),
+                       base::Bind(ExpectClientName, ""));
+  registry->ClearAllPairings(base::Bind(ExpectSaveSuccess));

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

optional: I know I originally wrote this myself in the test cases I added, but I
don't think base::Bind makes sense here - we're not binding anything. Maybe
remove it, or replace it with an explicit Callback ctor:
SaveCallback(ExpectSaveSuccess) ?

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

Bind is needed to wrap a bare function. See callback.h for details.

+  registry->GetAllPairings(base::Bind(ExpectNoPairings));
+  PairingRegistry::Pairing pairing_3 = registry->CreatePairing("client3");
+  registry->GetPairing(pairing_3.client_id(),
+                       base::Bind(ExpectClientName, "client3"));
+
+  while (mock_delegate->HasCallback()) {
+    mock_delegate->RunCallback();

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

I think we should verify the number of times this gets called. Otherwise a
PairingRegistry that fails to queue some requests might still pass this test as
the expectations would not be triggered.
Something like this:

for (int i = 0; i < ??; i++) {
  ASSERT_TRUE(mock_delegate->HasCallback());
  mock_delegate->RunCallback();
}
EXPECT_FALSE(mock_delegate->HasCallback());

Is there a better alternative that doesn't involve knowing how many callbacks
there should be?

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

This test shouldn't care how many *delegate* callbacks are made, but you're
correct that we need to verify that the correct number of pairing registry
callbacks were made. I've added a simple counter for that purpose and cleaned up
another unit test that was using a boolean and a string for a similar purpose.

+  }
+}
+
 }  // namespace protocol
 }  // namespace remoting