diff options
author | sky <sky@chromium.org> | 2015-05-11 17:28:56 -0700 |
---|---|---|
committer | Commit bot <commit-bot@chromium.org> | 2015-05-12 00:29:05 +0000 |
commit | 6e4a8779b6f3cf90d4d69356f499db025e891c5a (patch) | |
tree | 9b01ea08d8b3ef40fec14ddf0e3b8515230d3b2f | |
parent | bf21714b4e5432f50f1ca9d7190109e7d322e917 (diff) | |
download | chromium_src-6e4a8779b6f3cf90d4d69356f499db025e891c5a.zip chromium_src-6e4a8779b6f3cf90d4d69356f499db025e891c5a.tar.gz chromium_src-6e4a8779b6f3cf90d4d69356f499db025e891c5a.tar.bz2 |
Makes ResourceLoader own handles and return Files
This way it won't leak if destroyed. Also cleaned up html_viewer's use
of ResourceLoader.
R=ben@chromium.org
TBR=ben@chromium.org
BUG=none
TEST=none
Review URL: https://codereview.chromium.org/1132083003
Cr-Commit-Position: refs/heads/master@{#329295}
-rw-r--r-- | components/html_viewer/BUILD.gn | 11 | ||||
-rw-r--r-- | components/html_viewer/html_viewer.cc | 41 | ||||
-rw-r--r-- | components/html_viewer/html_viewer_resources.grd | 11 | ||||
-rw-r--r-- | components/resource_provider/public/cpp/resource_loader.cc | 17 | ||||
-rw-r--r-- | components/resource_provider/public/cpp/resource_loader.h | 16 | ||||
-rw-r--r-- | components/resource_provider/resource_provider_apptest.cc | 8 |
6 files changed, 76 insertions, 28 deletions
diff --git a/components/html_viewer/BUILD.gn b/components/html_viewer/BUILD.gn index b09c8c3..2696229 100644 --- a/components/html_viewer/BUILD.gn +++ b/components/html_viewer/BUILD.gn @@ -7,6 +7,7 @@ import("//mojo/mojo_application_package.gni") import("//testing/test.gni") import("//third_party/mojo/src/mojo/public/mojo.gni") import("//third_party/mojo/src/mojo/public/mojo_application.gni") +import("//tools/grit/grit_rule.gni") import("//tools/grit/repack.gni") # Repack this here. @@ -45,6 +46,14 @@ action("generate_blink_resource_map") { ] } +grit("html_viewer_resources_grit") { + source = "html_viewer_resources.grd" + outputs = [ + "grit/html_viewer_resources.h", + "html_viewer_resources.pak", + "html_viewer_resources.rc", + ] +} source_set("lib") { sources = [ "$target_gen_dir/blink_resource_map.cc", @@ -166,6 +175,7 @@ mojo_application_package("html_viewer") { "ui_setup_android.h", ] deps = [ + ":html_viewer_resources_grit", ":lib", "//components/resource_provider/public/cpp", "//components/resource_provider/public/interfaces", @@ -181,6 +191,7 @@ mojo_application_package("html_viewer") { resources = [ "$root_out_dir/icudtl.dat", "$root_out_dir/ui_test.pak", + "$target_gen_dir/html_viewer_resources.pak", ] if (v8_use_external_startup_data) { diff --git a/components/html_viewer/html_viewer.cc b/components/html_viewer/html_viewer.cc index e8707da..dc83c77 100644 --- a/components/html_viewer/html_viewer.cc +++ b/components/html_viewer/html_viewer.cc @@ -55,7 +55,7 @@ using mojo::URLResponsePtr; using resource_provider::ResourceLoader; namespace html_viewer { - +namespace { // Switches for html_viewer. // Enable MediaRenderer in media pipeline instead of using the internal @@ -70,6 +70,17 @@ const char kIsHeadless[] = "is-headless"; size_t kDesiredMaxMemory = 20 * 1024 * 1024; +// Paths resources are loaded from. +const char kResourceIcudtl[] = "icudtl.dat"; +const char kResourceResourcesPak[] = "html_viewer_resources.pak"; +const char kResourceUIPak[] = "ui_test.pak"; +#if defined(V8_USE_EXTERNAL_STARTUP_DATA) +const char kResourceNativesBlob[] = "natives_blob.bin"; +const char kResourceSnapshotBlob[] = "snapshot_blob.bin"; +#endif + +} // namespace + class HTMLViewer; class HTMLViewerApplication : public mojo::Application { @@ -182,13 +193,13 @@ class HTMLViewer : public mojo::ApplicationDelegate, private: // Overridden from ApplicationDelegate: void Initialize(mojo::ApplicationImpl* app) override { - // TODO(sky): make this typesafe and not leak. std::set<std::string> paths; - paths.insert("icudtl.dat"); - paths.insert("ui_test.pak"); + paths.insert(kResourceResourcesPak); + paths.insert(kResourceIcudtl); + paths.insert(kResourceUIPak); #if defined(V8_USE_EXTERNAL_STARTUP_DATA) - paths.insert("natives_blob.bin"); - paths.insert("snapshot_blob.bin"); + paths.insert(kResourceNativesBlob); + paths.insert(kResourceSnapshotBlob); #endif ResourceLoader resource_loader(app->shell(), paths); if (!resource_loader.BlockUntilLoaded()) { @@ -198,8 +209,6 @@ class HTMLViewer : public mojo::ApplicationDelegate, return; } - ResourceLoader::ResourceMap resource_map(resource_loader.resource_map()); - ui_setup_.reset(new UISetup); base::DiscardableMemoryAllocator::SetInstance( &discardable_memory_allocator_); @@ -210,12 +219,15 @@ class HTMLViewer : public mojo::ApplicationDelegate, #if defined(V8_USE_EXTERNAL_STARTUP_DATA) // Note: this requires file system access. CHECK(gin::V8Initializer::LoadV8SnapshotFromFD( - resource_map["natives_blob.bin"], 0u, 0u, - resource_map["snapshot_blob.bin"], 0u, 0u)); + resource_loader.ReleaseFile(kResourceNativesBlob).TakePlatformFile(), + 0u, 0u, + resource_loader.ReleaseFile(kResourceSnapshotBlob).TakePlatformFile(), + 0u, 0u)); #endif blink::initialize(blink_platform_.get()); base::i18n::InitializeICUWithFileDescriptor( - resource_map["icudtl.dat"], base::MemoryMappedFile::Region::kWholeFile); + resource_loader.ReleaseFile(kResourceIcudtl).TakePlatformFile(), + base::MemoryMappedFile::Region::kWholeFile); ui::RegisterPathProvider(); @@ -232,14 +244,11 @@ class HTMLViewer : public mojo::ApplicationDelegate, is_headless_ = command_line->HasSwitch(kIsHeadless); if (!is_headless_) { - // TODO(sky): fix lifetime here. - MojoPlatformHandle platform_handle = resource_map["ui_test.pak"]; - // TODO(sky): the first call should be for strings, not images. ui::ResourceBundle::InitSharedInstanceWithPakFileRegion( - base::File(platform_handle).Pass(), + resource_loader.ReleaseFile(kResourceResourcesPak), base::MemoryMappedFile::Region::kWholeFile); ui::ResourceBundle::GetSharedInstance().AddDataPackFromFile( - base::File(platform_handle).Pass(), ui::SCALE_FACTOR_100P); + resource_loader.ReleaseFile(kResourceUIPak), ui::SCALE_FACTOR_100P); } compositor_thread_.Start(); diff --git a/components/html_viewer/html_viewer_resources.grd b/components/html_viewer/html_viewer_resources.grd new file mode 100644 index 0000000..e231218 --- /dev/null +++ b/components/html_viewer/html_viewer_resources.grd @@ -0,0 +1,11 @@ +<?xml version="1.0" encoding="UTF-8"?> +<grit latest_public_release="0" current_release="1"> + <outputs> + <output filename="grit/html_viewer_resources.h" type="rc_header"> + <emit emit_type='prepend'></emit> + </output> + <output filename="html_viewer_resources.pak" type="data_package"/> + <output filename="html_viewer_resources.rc" type="rc_all" /> + </outputs> + <translations /> +</grit> diff --git a/components/resource_provider/public/cpp/resource_loader.cc b/components/resource_provider/public/cpp/resource_loader.cc index 3baf4a9..a387e20 100644 --- a/components/resource_provider/public/cpp/resource_loader.cc +++ b/components/resource_provider/public/cpp/resource_loader.cc @@ -5,6 +5,7 @@ #include "components/resource_provider/public/cpp/resource_loader.h" #include "base/bind.h" +#include "base/files/file.h" #include "mojo/common/common_type_converters.h" #include "mojo/platform_handle/platform_handle_functions.h" #include "third_party/mojo/src/mojo/public/cpp/application/connect.h" @@ -14,7 +15,7 @@ namespace resource_provider { ResourceLoader::ResourceLoader(mojo::Shell* shell, const std::set<std::string>& paths) - : loaded_(false) { + : loaded_(false), did_block_(false) { shell->ConnectToApplication("mojo:resource_provider", GetProxy(&resource_provider_service_provider_), nullptr); @@ -31,11 +32,21 @@ ResourceLoader::~ResourceLoader() { } bool ResourceLoader::BlockUntilLoaded() { - CHECK(!loaded_); + if (did_block_) + return loaded_; + + did_block_ = true; resource_provider_.WaitForIncomingMethodCall(); return loaded_; } +base::File ResourceLoader::ReleaseFile(const std::string& path) { + CHECK(resource_map_.count(path)); + scoped_ptr<base::File> file_wrapper(resource_map_[path].Pass()); + resource_map_.erase(path); + return file_wrapper->Pass(); +} + void ResourceLoader::OnGotResources(const std::vector<std::string>& paths, mojo::Array<mojo::ScopedHandle> resources) { // We no longer need the connection to ResourceProvider. @@ -49,7 +60,7 @@ void ResourceLoader::OnGotResources(const std::vector<std::string>& paths, MojoPlatformHandle platform_handle; CHECK(MojoExtractPlatformHandle(resources[i].release().value(), &platform_handle) == MOJO_RESULT_OK); - resource_map_[paths[i]] = platform_handle; + resource_map_[paths[i]].reset(new base::File(platform_handle)); } loaded_ = true; } diff --git a/components/resource_provider/public/cpp/resource_loader.h b/components/resource_provider/public/cpp/resource_loader.h index d3ef25d..1162180 100644 --- a/components/resource_provider/public/cpp/resource_loader.h +++ b/components/resource_provider/public/cpp/resource_loader.h @@ -11,12 +11,17 @@ #include <vector> #include "base/basictypes.h" +#include "base/memory/scoped_ptr.h" #include "components/resource_provider/public/interfaces/resource_provider.mojom.h" #include "mojo/platform_handle/platform_handle.h" #include "third_party/mojo/src/mojo/public/cpp/bindings/array.h" #include "third_party/mojo/src/mojo/public/cpp/system/handle.h" #include "third_party/mojo/src/mojo/public/interfaces/application/service_provider.mojom.h" +namespace base { +class File; +} + namespace mojo { class Shell; } @@ -29,21 +34,23 @@ namespace resource_provider { // have been obtained. class ResourceLoader { public: - using ResourceMap = std::map<std::string, MojoPlatformHandle>; - ResourceLoader(mojo::Shell* shell, const std::set<std::string>& paths); ~ResourceLoader(); // Uses WaitForIncomingMessage() to block until the results are available, or // an error occurs. Returns true if the resources were obtained, false on - // error. + // error. This immediately returns if the resources have already been + // obtained. bool BlockUntilLoaded(); - const ResourceMap& resource_map() const { return resource_map_; } + // Releases and returns the file wrapping the handle. + base::File ReleaseFile(const std::string& path); bool loaded() const { return loaded_; } private: + using ResourceMap = std::map<std::string, scoped_ptr<base::File>>; + // Callback when resources have loaded. void OnGotResources(const std::vector<std::string>& paths, mojo::Array<mojo::ScopedHandle> resources); @@ -55,6 +62,7 @@ class ResourceLoader { ResourceMap resource_map_; bool loaded_; + bool did_block_; DISALLOW_COPY_AND_ASSIGN(ResourceLoader); }; diff --git a/components/resource_provider/resource_provider_apptest.cc b/components/resource_provider/resource_provider_apptest.cc index a4e57e2..7853e32 100644 --- a/components/resource_provider/resource_provider_apptest.cc +++ b/components/resource_provider/resource_provider_apptest.cc @@ -58,14 +58,12 @@ class ResourceProviderApplicationTest : public mojo::test::ApplicationTestBase { ResourceContentsMap GetResources(const std::set<std::string>& paths) { ResourceLoader loader(application_impl()->shell(), paths); loader.BlockUntilLoaded(); - const ResourceLoader::ResourceMap& resource_loader_results( - loader.resource_map()); // Load the contents of each of the handles. ResourceContentsMap results; - for (auto& pair : resource_loader_results) { - base::File file(pair.second); - results[pair.first] = ReadFile(&file); + for (auto& path : paths) { + base::File file(loader.ReleaseFile(path)); + results[path] = ReadFile(&file); } return results; } |