Service Manager: Miscellaneous catalog cleanup

services/service_manager/service_manager.cc

 // Copyright 2014 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/service_manager/service_manager.h"
 
 #include <stdint.h>
 
 #include <utility>
 
@@ skipping 796 matching lines @@
   // Remove the mapping.
   auto it = service_factories_.find(which);
   DCHECK(it != service_factories_.end());
   service_factories_.erase(it);
 }
 
 void ServiceManager::OnGotResolvedName(std::unique_ptr<ConnectParams> params,
                                        mojom::ServicePtr service,
                                        bool has_source_instance,
                                        base::WeakPtr<Instance> source_instance,
-                                       mojom::ResolveResultPtr result) {
+                                       mojom::ResolveResultPtr result,
+                                       mojom::ResolveResultPtr parent) {
   // If this request was originated by a specific Instance and that Instance is
   // no longer around, we ignore this response.
   if (has_source_instance && !source_instance)
     return;
 
   // If name resolution failed, we drop the connection.
   if (!result) {
     LOG(ERROR) << "Failed to resolve service name: " << params->target().name();
     if (!params->connect_callback().is_null()) {
       params->connect_callback().Run(
           mojom::ConnectResult::INVALID_ARGUMENT, "");
     }
     return;
   }
 
   std::string instance_name = params->target().instance();
-  if (instance_name == params->target().name() &&
-      result->qualifier != result->resolved_name) {
-    instance_name = result->qualifier;

[Ben Goodger (Google), 2017/01/07 19:29:55]

so this means it's not possible to specify an instance name in manifest now?

i think that's ok since we don't use it for anything yet?

-  }
-  // |result->interface_provider_specs| can be empty when there is no manifest,
-  // e.g. for URL types not resolvable by the resolver.
+
+  // |result->interface_provider_specs| can be empty when there is no manifest.
   InterfaceProviderSpec connection_spec = GetPermissiveInterfaceProviderSpec();
   auto it = result->interface_provider_specs.find(
       mojom::kServiceManager_ConnectorSpec);
   if (it != result->interface_provider_specs.end())
     connection_spec = it->second;
 
   const Identity original_target(params->target());
   const std::string user_id =
       HasCapability(connection_spec, kCapability_AllUsers)
           ? base::GenerateGUID() : params->target().user_id();
@@ skipping 34 matching lines @@
     // is already holding a corresponding ServiceRequest.
     instance->StartWithService(std::move(service));
   } else if (!client_process_connection.is_null()) {
     // Likewise if a ClientProcessConnection was given via Connect(), it
     // provides the Service proxy to use.
     instance->StartWithClientProcessConnection(
         std::move(client_process_connection));
   } else {
     // Otherwise we create a new Service pipe.
     mojom::ServiceRequest request(&service);
-    CHECK(!result->package_path.empty());
 
     // The catalog was unable to read a manifest for this service. We can't do
     // anything more.
     // TODO(beng): There may be some cases where it's valid to have an empty
     // spec, so we should probably include a return value in |result|.
     if (result->interface_provider_specs.empty()) {
       LOG(ERROR)
           << "Error: The catalog was unable to read a manifest for service \""
           << result->name << "\".";
       if (!params->connect_callback().is_null())
         params->connect_callback().Run(mojom::ConnectResult::ACCESS_DENIED, "");
       return;
     }
 
-    if (target.name() != result->resolved_name) {
-      // This service is part of a package.
+    if (parent) {
+      // This service is provided by another service via a ServiceFactory.
       std::string target_user_id = target.user_id();
       std::string factory_instance_name = instance_name;
-      bool instance_per_child = result->package_spec.has_value() &&
-          HasCapability(result->package_spec.value(),
-                        kCapability_InstancePerChild);
-      if (instance_per_child) {
+
+      auto spec_iter = parent->interface_provider_specs.find(
+          mojom::kServiceManager_ConnectorSpec);
+      if (spec_iter != parent->interface_provider_specs.end() &&
+              HasCapability(spec_iter->second, kCapability_InstancePerChild)) {
         // If configured to start a new instance, create a random instance name
         // for the factory so that we don't reuse an existing process.
         factory_instance_name = base::GenerateGUID();
       } else {
         // Use the original user ID so the existing embedder factory can
         // be found and used to create the new service.
         target_user_id = original_target.user_id();
         Identity packaged_service_target(target);
         packaged_service_target.set_user_id(original_target.user_id());
         instance->set_identity(packaged_service_target);
       }
       instance->StartWithService(std::move(service));
 
-      Identity factory(result->resolved_name, target_user_id,
-                       factory_instance_name);
+      Identity factory(parent->name, target_user_id, factory_instance_name);
       CreateServiceWithFactory(factory, target.name(), std::move(request));
     } else {
       base::FilePath package_path;
       if (!service_overrides_ || !service_overrides_->GetExecutablePathOverride(
             target.name(), &package_path)) {
         package_path = result->package_path;
       }
+      DCHECK(!package_path.empty());
 
       if (!instance->StartWithFilePath(package_path)) {
         OnInstanceError(instance);
         if (!params->connect_callback().is_null()) {
           params->connect_callback().Run(
               mojom::ConnectResult::INVALID_ARGUMENT, "");
         }
         return;
       }
     }
   }
 
   // Now that the instance has a Service, we can connect to it.
   bool connected = instance->ConnectToService(&params);
   DCHECK(connected);
 }
 
 base::WeakPtr<ServiceManager> ServiceManager::GetWeakPtr() {
   return weak_ptr_factory_.GetWeakPtr();
 }
 
 }  // namespace service_manager