Moves manifest parsing to a new class, Reader.

mojo/services/catalog/reader.cc

Index: mojo/services/catalog/reader.cc
diff --git a/mojo/services/catalog/reader.cc b/mojo/services/catalog/reader.cc
new file mode 100644
index 0000000000000000000000000000000000000000..335bdf959e88f49b7c0c0f701f1d1189cc30b2e1
--- /dev/null
+++ b/mojo/services/catalog/reader.cc
@@ -0,0 +1,86 @@
+// Copyright 2016 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 "mojo/services/catalog/reader.h"
+
+#include "base/json/json_file_value_serializer.h"
+#include "base/location.h"
+#include "base/task_runner_util.h"
+#include "mojo/services/catalog/entry.h"
+#include "mojo/shell/public/cpp/names.h"
+
+namespace catalog {
+namespace {
+
+scoped_ptr<base::Value> ReadManifest(const base::FilePath& manifest_path) {
+  JSONFileValueDeserializer deserializer(manifest_path);
+  int error = 0;
+  std::string message;
+  // TODO(beng): probably want to do more detailed error checking. This should
+  //             be done when figuring out if to unblock connection completion.
+  return deserializer.Deserialize(&error, &message);
+}
+
+}  // namespace
+
+Reader::Reader(const base::FilePath& package_path,
+               base::TaskRunner* file_task_runner)
+    : package_path_(package_path),
+      file_task_runner_(file_task_runner),
+      weak_factory_(this) {}
+Reader::~Reader() {}
+
+void Reader::Read(const std::string& name,
+                  const ReadManifestCallback& callback) {
+  base::FilePath manifest_path = GetManifestPath(name);
+  if (manifest_path.empty()) {
+    callback.Run(nullptr);
+    return;
+  }
+
+  std::string type = mojo::GetNameType(name);
+  CHECK(type == "mojo" || type == "exe");
+  base::PostTaskAndReplyWithResult(
+      file_task_runner_, FROM_HERE, base::Bind(&ReadManifest, manifest_path),
+      base::Bind(&Reader::OnReadManifest, weak_factory_.GetWeakPtr(),
+                 name, callback));
+}
+
+base::FilePath Reader::GetManifestPath(const std::string& name) const {
+  // TODO(beng): think more about how this should be done for exe targets.
+  std::string type = mojo::GetNameType(name);
+  std::string path = mojo::GetNamePath(name);
+  if (type == "mojo")
+    return package_path_.AppendASCII(path + "/manifest.json");
+  else if (type == "exe")
+    return package_path_.AppendASCII(path + "_manifest.json");
+  return base::FilePath();
+}
+
+// static
+void Reader::OnReadManifest(base::WeakPtr<Reader> reader,

[sky, 2016/03/22 23:47:46]

Is there a reason you need this static? It doesn't seem like OnReadManifestImpl
needs to be a member function at all since it doesn't need any state from
Reader.

+                            const std::string& name,
+                            const ReadManifestCallback& callback,
+                            scoped_ptr<base::Value> manifest) {
+  if (!reader) {
+    // The Reader was destroyed, we're likely in shutdown. Run the callback so
+    // we don't trigger a DCHECK.
+    callback.Run(nullptr);
+    return;
+  }
+  reader->OnReadManifestImpl(callback, std::move(manifest));
+}
+
+void Reader::OnReadManifestImpl(const ReadManifestCallback& callback,
+                                scoped_ptr<base::Value> manifest) {
+  scoped_ptr<Entry> entry;
+  if (manifest) {
+    const base::DictionaryValue* dictionary = nullptr;
+    CHECK(manifest->GetAsDictionary(&dictionary));
+    entry = Entry::Deserialize(*dictionary);
+  }
+  callback.Run(std::move(entry));
+}
+
+}  // namespace catalog