Add serialization to PairingRegistry.

remoting/protocol/pairing_registry_unittest.cc

 // Copyright (c) 2012 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.
 
 #include "remoting/protocol/pairing_registry.h"
 
 #include <stdlib.h>
 
 #include <algorithm>
 
@@ skipping 20 matching lines @@
   EXPECT_EQ(expected.client_id(), value);
 
   EXPECT_FALSE(actual.HasKey(PairingRegistry::kSharedSecretKey));
 }
 
 // SaveCallback that expects to be called with success = true.
 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 {
 namespace protocol {
 
 class PairingRegistryTest : public testing::Test {
  public:
   void CompareSecret(const std::string& expected,
                      PairingRegistry::Pairing actual) {
     EXPECT_EQ(expected, actual.shared_secret());
@@ skipping 120 matching lines @@
 
   // Re-read the list, and verify it is empty.
   registry->GetAllPairings(
       base::Bind(&PairingRegistryTest::set_pairings,
                  base::Unretained(this)));
   mock_delegate->RunCallback();
 
   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