Eliminate mojo:resource_provider

services/catalog/public/cpp/resource_loader.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/public/cpp/resource_loader.h"
+
+#include <stddef.h>
+#include <utility>
+
+#include "base/bind.h"
+#include "base/files/file.h"
+#include "components/filesystem/public/interfaces/directory.mojom.h"
+#include "mojo/platform_handle/platform_handle_functions.h"
+#include "services/shell/public/cpp/connector.h"
+#include "services/shell/public/interfaces/connector.mojom.h"
+#include "services/shell/public/interfaces/interface_provider.mojom.h"
+
+namespace catalog {
+namespace {
+base::File GetFileFromHandle(mojo::ScopedHandle handle) {
+  CHECK(handle.is_valid());
+  MojoPlatformHandle platform_handle;
+  CHECK(MojoExtractPlatformHandle(handle.release().value(),
+                                  &platform_handle) == MOJO_RESULT_OK);
+  return base::File(platform_handle);
+}
+}
+
+ResourceLoader::ResourceLoader(shell::Connector* connector,

[sky, 2016/05/03 04:13:23]

This is a lot of expensive operations for the constructor. Seems like it should
be moved to a Load/Init function with a return value.

+                               const std::set<std::string>& paths) {
+  filesystem::DirectoryPtr directory;
+  connector->ConnectToInterface("mojo:catalog", &directory);
+
+  mojo::Array<filesystem::FileOpenDetailsPtr> deets(

[sky, 2016/05/03 04:13:23]

deets? You sure are going for obscure names lately.

+      mojo::Array<filesystem::FileOpenDetailsPtr>::New(paths.size()));
+  size_t i = 0;
+  for (const auto& path : paths) {
+    filesystem::FileOpenDetailsPtr open_details(
+        filesystem::FileOpenDetails::New());
+    open_details->path = path;
+    open_details->open_flags = filesystem::kFlagOpen | filesystem::kFlagRead;
+    deets[i++] = std::move(open_details);
+  }
+
+  mojo::Array<filesystem::FileOpenResultPtr> results(
+      mojo::Array<filesystem::FileOpenResultPtr>::New(paths.size()));
+  directory->OpenFileHandles(std::move(deets), &results);

[sky, 2016/05/03 04:13:23]

You need to check the return value. I don't think this code or TakeFile should
check. It's entirely possible during shutdown this code could end up failing
and/or not get everything.

+
+  CHECK_EQ(results.size(), paths.size());
+  for (const auto& result : results) {
+    resource_map_[result->path].reset(
+        new base::File(GetFileFromHandle(std::move(result->file_handle))));
+  }
+}
+
+ResourceLoader::~ResourceLoader() {
+}
+
+base::File ResourceLoader::TakeFile(const std::string& path) {
+  CHECK(resource_map_.count(path));
+  std::unique_ptr<base::File> file_wrapper(std::move(resource_map_[path]));
+  resource_map_.erase(path);
+  return std::move(*file_wrapper);
+}
+
+}  // namespace catalog