Morph CapabilityFilter into caps::Capabilities, which supports capability classes (yet unimplemente…

mojo/services/catalog/builder_unittest.cc

Index: mojo/services/catalog/builder_unittest.cc
diff --git a/mojo/services/catalog/builder_unittest.cc b/mojo/services/catalog/builder_unittest.cc
index 4290a2f0c8ef94fef76cd27eed17681e429bc643..a5838f96949275c91c5a23a49480dcb9c57f1082 100644
--- a/mojo/services/catalog/builder_unittest.cc
+++ b/mojo/services/catalog/builder_unittest.cc
@@ -9,15 +9,27 @@
 #include "base/macros.h"
 #include "base/path_service.h"
 #include "base/values.h"
+#include "mojo/shell/public/cpp/capabilities.h"
 #include "mojo/shell/public/cpp/names.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
+namespace catalog {
+
 class BuilderTest : public testing::Test {
  public:
   BuilderTest() {}
   ~BuilderTest() override {}
 
  protected:
+  scoped_ptr<base::Value> ReadEntry(const std::string& manifest, Entry* entry) {
+    DCHECK(entry);
+    scoped_ptr<base::Value> value = ReadManifest(manifest);
+    base::DictionaryValue* dictionary = nullptr;
+    CHECK(value->GetAsDictionary(&dictionary));
+    *entry = BuildEntry(*dictionary);
+    return value;
+  }
+
   scoped_ptr<base::Value> ReadManifest(const std::string& manifest) {
     base::FilePath manifest_path;
     PathService::Get(base::DIR_SOURCE_ROOT, &manifest_path);
@@ -40,13 +52,9 @@ class BuilderTest : public testing::Test {
   DISALLOW_COPY_AND_ASSIGN(BuilderTest);
 };
 
-namespace catalog {
-
 TEST_F(BuilderTest, Simple) {
-  scoped_ptr<base::Value> value = ReadManifest("simple");
-  base::DictionaryValue* dictionary = nullptr;
-  CHECK(value->GetAsDictionary(&dictionary));
-  Entry entry = BuildEntry(*dictionary);
+  Entry entry;
+  ReadEntry("simple", &entry);
 
   EXPECT_EQ("mojo:foo", entry.name);
   EXPECT_EQ(mojo::GetNamePath(entry.name), entry.qualifier);
@@ -54,10 +62,8 @@ TEST_F(BuilderTest, Simple) {
 }
 
 TEST_F(BuilderTest, Instance) {
-  scoped_ptr<base::Value> value = ReadManifest("instance");
-  base::DictionaryValue* dictionary = nullptr;
-  CHECK(value->GetAsDictionary(&dictionary));
-  Entry entry = BuildEntry(*dictionary);
+  Entry entry;
+  ReadEntry("instance", &entry);
 
   EXPECT_EQ("mojo:foo", entry.name);
   EXPECT_EQ("bar", entry.qualifier);
@@ -65,19 +71,31 @@ TEST_F(BuilderTest, Instance) {
 }
 
 TEST_F(BuilderTest, Capabilities) {
-  scoped_ptr<base::Value> value = ReadManifest("capabilities");
-  base::DictionaryValue* dictionary = nullptr;
-  CHECK(value->GetAsDictionary(&dictionary));
-  Entry entry = BuildEntry(*dictionary);
+  Entry entry;
+  ReadEntry("capabilities", &entry);
 
   EXPECT_EQ("mojo:foo", entry.name);
   EXPECT_EQ("bar", entry.qualifier);
   EXPECT_EQ("Foo", entry.display_name);
-  CapabilityFilter filter;
-  AllowedInterfaces interfaces;
-  interfaces.insert("mojo::Bar");
-  filter["mojo:bar"] = interfaces;
-  EXPECT_EQ(filter, entry.capabilities);
+  mojo::caps::CapabilitySpec spec;
+  mojo::caps::CapabilityRequest request;
+  request.interfaces.insert("mojo::Bar");
+  spec.required["mojo:bar"] = request;
+  EXPECT_EQ(spec, entry.capabilities);
+}
+
+TEST_F(BuilderTest, Serialization) {
+  Entry entry;
+  scoped_ptr<base::Value> value = ReadEntry("serialization", &entry);
+
+  base::DictionaryValue* serialized = new base::DictionaryValue;

[sky, 2016/03/09 21:33:15]

I believe you leak here. Is there a reason SerializeEntry takes a
DictionaryValue** instead of returning a scoped_ptr<DictionaryValue>?

+  SerializeEntry(entry, &serialized);
+
+  // We can't just compare values, since during deserialization some of the
+  // lists get converted to std::sets, which are sorted, so Value::Equals will
+  // fail.
+  Entry reconstituted = BuildEntry(*serialized);
+  EXPECT_EQ(entry, reconstituted);
 }
 
 TEST_F(BuilderTest, Malformed) {