Service Manager: Miscellaneous catalog cleanup

services/catalog/reader.cc

 // 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 "services/catalog/reader.h"
 
 #include "base/base_paths.h"
 #include "base/bind.h"
 #include "base/files/file_enumerator.h"
 #include "base/files/file_util.h"
 #include "base/json/json_file_value_serializer.h"
 #include "base/json/json_reader.h"
 #include "base/memory/ptr_util.h"
 #include "base/path_service.h"
 #include "base/strings/string_util.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/task_runner_util.h"
 #include "base/threading/thread_task_runner_handle.h"
 #include "base/values.h"
 #include "services/catalog/constants.h"
 #include "services/catalog/entry.h"
+#include "services/catalog/entry_cache.h"
 #include "services/catalog/manifest_provider.h"
 #include "services/catalog/public/interfaces/constants.mojom.h"
 #include "services/service_manager/public/interfaces/constants.mojom.h"
 
 namespace catalog {
 namespace {
 
 #if defined(OS_WIN)
 const char kServiceExecutableExtension[] = ".service.exe";
 #else
@@ skipping 113 matching lines @@
   std::unique_ptr<Entry> entry = CreateEntryForManifestAt(manifest_path,
                                                           package_dir);
   if (!entry) {
     entry.reset(new Entry(mojo_name));
     entry->set_path(GetExecutablePath(
         package_dir.AppendASCII(kPackagesDirName), mojo_name));
   }
   return entry;
 }
 
-void AddEntryToCache(EntryCache* cache, std::unique_ptr<Entry> entry) {
-  std::vector<std::unique_ptr<Entry>> children = entry->TakeChildren();
-  for (auto& child : children)
-    AddEntryToCache(cache, std::move(child));
-  (*cache)[entry->name()] = std::move(entry);
-}
-
-void DoNothing(service_manager::mojom::ResolveResultPtr) {}
+void IgnoreResolveResult(service_manager::mojom::ResolveResultPtr,
+                         service_manager::mojom::ResolveResultPtr) {}
 
 void LoadCatalogManifestIntoCache(const base::Value* root,
                                   const base::FilePath& package_dir,
                                   EntryCache* cache) {
   DCHECK(root);
   const base::DictionaryValue* catalog = nullptr;
   if (!root->GetAsDictionary(&catalog)) {
     LOG(ERROR) << "Catalog manifest is not a dictionary value.";
     return;
   }
@@ skipping 42 matching lines @@
       LOG(ERROR) << "Catalog entry for \"" << it.key() << "\" has an invalid "
                  << "\"manifest\" value.";
       continue;
     }
 
     DCHECK(!(is_embedded && !executable_path.empty()));
 
     auto entry = ProcessManifest(
         manifest, is_embedded ? base::FilePath() : package_dir,
         executable_path);
-    if (entry)
-      AddEntryToCache(cache, std::move(entry));
-    else
+    if (entry) {
+      bool added = cache->AddRootEntry(std::move(entry));
+      DCHECK(added);
+    } else {
       LOG(ERROR) << "Failed to read manifest entry for \"" << it.key() << "\".";
+    }
   }
 }
 
 }  // namespace
 
 Reader::Reader(std::unique_ptr<base::Value> static_manifest,
                EntryCache* cache)
     : using_static_catalog_(true),
       manifest_provider_(nullptr),
       weak_factory_(this) {
@@ skipping 22 matching lines @@
 
 Reader::~Reader() {}
 
 void Reader::Read(const base::FilePath& package_dir,
                   EntryCache* cache,
                   const base::Closure& read_complete_closure) {
   file_task_runner_->PostTask(
       FROM_HERE,
       base::Bind(&ScanDir, package_dir,
                  base::Bind(&Reader::OnReadManifest, weak_factory_.GetWeakPtr(),
-                            cache, base::Bind(&DoNothing)),
+                            cache, base::Bind(&IgnoreResolveResult)),
                  base::ThreadTaskRunnerHandle::Get(),
                  read_complete_closure));
 }
 
 void Reader::CreateEntryForName(
-    const std::string& mojo_name,
+    const std::string& name,
     EntryCache* cache,
     const CreateEntryForNameCallback& entry_created_callback) {
   if (manifest_provider_) {
     std::unique_ptr<base::Value> manifest_root =
-        manifest_provider_->GetManifest(mojo_name);
+        manifest_provider_->GetManifest(name);
     if (manifest_root) {
       base::PostTaskAndReplyWithResult(
           file_task_runner_.get(), FROM_HERE,
           base::Bind(&ProcessUniqueManifest, base::Passed(&manifest_root),
                      system_package_dir_),
           base::Bind(&Reader::OnReadManifest, weak_factory_.GetWeakPtr(), cache,
                      entry_created_callback));
       return;
     }
   } else if (using_static_catalog_) {
     // A Reader using a static catalog manifest does not support dynamic
     // discovery or introduction of new catalog entries.
-    entry_created_callback.Run(service_manager::mojom::ResolveResultPtr());
+    entry_created_callback.Run(service_manager::mojom::ResolveResultPtr(),
+                               service_manager::mojom::ResolveResultPtr());
     return;
   }
 
   base::FilePath manifest_path_override;
   {
-    auto override_iter = manifest_path_overrides_.find(mojo_name);
+    auto override_iter = manifest_path_overrides_.find(name);
     if (override_iter != manifest_path_overrides_.end())
       manifest_path_override = override_iter->second;
   }
 
   std::string package_name_override;
   {
-    auto override_iter = package_name_overrides_.find(mojo_name);
+    auto override_iter = package_name_overrides_.find(name);
     if (override_iter != package_name_overrides_.end())
       package_name_override = override_iter->second;
   }
   base::PostTaskAndReplyWithResult(
       file_task_runner_.get(), FROM_HERE,
-      base::Bind(&ReadManifest, system_package_dir_, mojo_name,
+      base::Bind(&ReadManifest, system_package_dir_, name,
                  package_name_override, manifest_path_override),
       base::Bind(&Reader::OnReadManifest, weak_factory_.GetWeakPtr(), cache,
                   entry_created_callback));
 }
 
 void Reader::OverridePackageName(const std::string& service_name,
                                  const std::string& package_name) {
   package_name_overrides_.insert(std::make_pair(service_name, package_name));
 }
 
 void Reader::OverrideManifestPath(const std::string& service_name,
                                   const base::FilePath& path) {
   manifest_path_overrides_.insert(std::make_pair(service_name, path));
 }
 
 Reader::Reader(ManifestProvider* manifest_provider)
     : using_static_catalog_(false),
       manifest_provider_(manifest_provider),
       weak_factory_(this) {
   PathService::Get(base::DIR_MODULE, &system_package_dir_);
 }
 
 void Reader::OnReadManifest(
     EntryCache* cache,
     const CreateEntryForNameCallback& entry_created_callback,
     std::unique_ptr<Entry> entry) {
   if (!entry)
     return;
-  service_manager::mojom::ResolveResultPtr result =
-      service_manager::mojom::ResolveResult::From(*entry);
-  AddEntryToCache(cache, std::move(entry));
-  entry_created_callback.Run(std::move(result));
+
+  std::string name = entry->name();
+  cache->AddRootEntry(std::move(entry));
+
+  // NOTE: It's currently possible to end up with a duplicate entry, in which
+  // case the above call to AddRootEntry() may actually discard |entry|. We
+  // therefore look up the Entry by name here to get resolution metadata.
+
+  const Entry* resolved_entry = cache->GetEntry(name);
+  DCHECK(resolved_entry);
+  entry_created_callback.Run(
+      service_manager::mojom::ResolveResult::From(resolved_entry),
+      service_manager::mojom::ResolveResult::From(resolved_entry->parent()));
 }
 
 }  // namespace catalog