From 20a540e68055f7f40ccf739e7e5ba4b6d33bb6a7 Mon Sep 17 00:00:00 2001 From: Jeremy Apthorp Date: Wed, 7 Nov 2018 07:11:01 -0800 Subject: [PATCH] chore: add explanation to resource_file_conflict.patch (#15612) * chore: add explanation to resource_file_conflict.patch * Update resource_file_conflict.patch --- .../chromium/resource_file_conflict.patch | 44 +++++++++++++++++++ 1 file changed, 44 insertions(+) diff --git a/patches/common/chromium/resource_file_conflict.patch b/patches/common/chromium/resource_file_conflict.patch index 93daeb6f9937..2c7d44a52087 100644 --- a/patches/common/chromium/resource_file_conflict.patch +++ b/patches/common/chromium/resource_file_conflict.patch @@ -8,6 +8,50 @@ that chrome code hardcodes require that we generate resources at these paths, but GN throws errors if there are multiple targets that generate the same files. +This is due to the hardcoded names here: +https://chromium.googlesource.com/chromium/src/+/69.0.3497.106/ui/base/resource/resource_bundle.cc#780 +and here: +https://chromium.googlesource.com/chromium/src/+/69.0.3497.106/ui/base/resource/resource_bundle_mac.mm#50 + +This isn't needed on Mac because resource files are copied into the app bundle, +and are built in `$root_out_dir/electron_repack` (while Chromium's resources +target `$root_out_dir/repack`), but on Windows and Linux, the resource files go +directly in `$root_out_dir`, and so they conflict. + +We don't actually ever generate Chromium's resource paks, but without this +patch, GN refuses to generate the ninja files: + + ERROR at //tools/grit/repack.gni:35:3: Duplicate output file. + action(_repack_target_name) { + ^---------------------------- + Two or more targets generate the same output: + chrome_100_percent.pak + + This is can often be fixed by changing one of the target names, or by + setting an output_name on one of them. + + Collisions: + //chrome:packed_resources_100_percent + //electron:packed_resources_100_percent + + See //tools/grit/repack.gni:35:3: Collision. + action(_repack_target_name) { + ^---------------------------- + + +Some alternatives to this patch: + +1. Refactor upstream in such a way that the "chrome" pak names were + configurable, for instance by adding a method to ResourceBundle::Delegate that + LoadChromeResources would check. +2. Pass a Delegate that overrides `GetPathForResourcePack`, check for the + `chrome_{100,200}_percent.pak` filenames, and rewrite them to + `electron_{100,200}_percent.pak`. +3. Initialize the resource bundle with DO_NOT_LOAD_COMMON_RESOURCES and load + the paks ourselves. + +None of these options seems like a substantial maintainability win over this patch to me (@nornagon). + diff --git a/chrome/BUILD.gn b/chrome/BUILD.gn index a47af42a5a5afc1560f11ee0ccfa5fc177745caa..db8311121e755eb955ff05dee61a9706ae747977 100644 --- a/chrome/BUILD.gn