Create new grit output format that describes resource compression |
|||||
Issue descriptiongrit is a way of packing together resources for use in Chrome: https://www.chromium.org/developers/tools-we-use-in-chromium/grit WebUI uses .grd and .grdp files to pull in files from disk and pile them on top of eachother in a big file. They're pulled out a .pak file via a generated .h file formatted like this: #define IDR_<name> <byte_offset_in_pak_file> Which is abstracted away a bit by ui::{DataPack,ResourceBundle}: https://cs.chromium.org/chromium/src/ui/base/resource/data_pack.cc?dr=CSs&l=261 agrieve@, dschuyler@, smaier@, and I added a way to compress source files before placing them in the bundle and decompress them after extracting (ex: compress="gzip" in .grd files). However, the files are manually marked as compressed by humans (which is error prone) via: data_source->UseGzip(std::unordered_set<std::string>({"not_compressed.js"})); https://chromium.googlesource.com/chromium/src/+/master/docs/optimizing_web_uis.md#Gzip-compression-of-web-resources If we created a new output format similar to rc_header but describing any compression applied to the resource, we could remove a bit of code from WebUI (where we manually mark resources as compressed at runtime with specific paths :/). Relevant links: An example use of compress="gzip" and an output of rc_header: https://cs.chromium.org/chromium/src/ui/webui/resources/webui_resources.grd?type=cs&q=type%3D+compress%3D%22gzip%22&l=6,11,12,21 Where some of the grit output formatting logic lives: https://cs.chromium.org/chromium/src/tools/grit/grit/format/resource_map.py?type=cs&q=rc_header+resource_map_header&sq=package:chromium&l=74 https://cs.chromium.org/chromium/src/tools/grit/grit/format/rc_header.py?type=cs&sq=package:chromium Code we could remove: https://cs.chromium.org/chromium/src/content/public/browser/web_ui_data_source.h?type=cs&l=112-113 and implementation and all [error-prone] calls.
,
Dec 12 2017
,
Dec 13 2017
https://chromium-review.googlesource.com/c/chromium/src/+/823271 is how i'm proposing to do this
,
Dec 21 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/78cd41e37b74854855b45102083572a8eb18f051 commit 78cd41e37b74854855b45102083572a8eb18f051 Author: Dan Beam <dbeam@chromium.org> Date: Thu Dec 21 04:23:02 2017 Add a new grit output format that knows whether resources are gzipped This is to reduce the amount of custom code written to deal with cases where a .grd entry may or may not be gzipped. The new output formats (both .h and .cc generators) can be invoked via: <output filename="gzip_map.h" type="gzipped_resource_map_header"> <output filename="gzip_map.cc" type="gzipped_resource_file_map_source"> This generates a .h with this definition: struct GzippedGritResourceMap { int value; const char* name; bool gzipped; // only true if compress="gzip" in .grd file }; and a .cc with this kind of data: const GzippedGritResourceMap kWebuiResources[] = { {"blah.js", IDR_BLAH_JS, true}, // is_gzipped }; I also edited the data source corresponding to chrome://resources/ requests to use the generated .h data and updated some aliasing code (and moved the mojo bindings for chrome://bluetooth-internals onto it). The eventual goal is to remove manual gzip exclusion of resources (i.e. WebUIDataSource::UseGzip(), WebUIDataSourceImpl::excluded_paths_). Bug: 738243 Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation;master.tryserver.chromium.mac:ios-simulator-cronet;master.tryserver.chromium.mac:ios-simulator-full-configs Change-Id: Id31c84cfef517fb9cc46bd7bbd90c297674ed2a2 Reviewed-on: https://chromium-review.googlesource.com/823271 Commit-Queue: Dan Beam (no longer on Chrome) <dbeam@chromium.org> Reviewed-by: Kurt Horimoto <kkhorimoto@chromium.org> Reviewed-by: Demetrios Papadopoulos <dpapad@chromium.org> Reviewed-by: Lei Zhang <thestig@chromium.org> Reviewed-by: John Abd-El-Malek <jam@chromium.org> Cr-Commit-Position: refs/heads/master@{#525601} [modify] https://crrev.com/78cd41e37b74854855b45102083572a8eb18f051/content/browser/webui/shared_resources_data_source.cc [modify] https://crrev.com/78cd41e37b74854855b45102083572a8eb18f051/ios/web/webui/shared_resources_data_source_ios.mm [modify] https://crrev.com/78cd41e37b74854855b45102083572a8eb18f051/mojo/public/js/BUILD.gn [modify] https://crrev.com/78cd41e37b74854855b45102083572a8eb18f051/mojo/public/js/mojo_bindings_resources.grd [modify] https://crrev.com/78cd41e37b74854855b45102083572a8eb18f051/tools/grit/grit/format/resource_map.py [modify] https://crrev.com/78cd41e37b74854855b45102083572a8eb18f051/tools/grit/grit/format/resource_map_unittest.py [modify] https://crrev.com/78cd41e37b74854855b45102083572a8eb18f051/tools/grit/grit/tool/build.py [modify] https://crrev.com/78cd41e37b74854855b45102083572a8eb18f051/ui/base/webui/web_ui_util.cc [modify] https://crrev.com/78cd41e37b74854855b45102083572a8eb18f051/ui/base/webui/web_ui_util.h [modify] https://crrev.com/78cd41e37b74854855b45102083572a8eb18f051/ui/webui/resources/webui_resources.grd
,
Jan 5 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/99afc077fe37586a0d3ec0c4536515f1610f98b8 commit 99afc077fe37586a0d3ec0c4536515f1610f98b8 Author: Dan Beam <dbeam@chromium.org> Date: Fri Jan 05 04:33:27 2018 Make unpack_pak.py handle gzipped resource maps R=dpapad@chromium.org BUG=738243 Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation Change-Id: I9b4581f5948f0572ef78da0974a4ba7b35e0d41f Reviewed-on: https://chromium-review.googlesource.com/850826 Reviewed-by: Demetrios Papadopoulos <dpapad@chromium.org> Commit-Queue: Dan Beam (no longer on Chrome) <dbeam@chromium.org> Cr-Commit-Position: refs/heads/master@{#527210} [modify] https://crrev.com/99afc077fe37586a0d3ec0c4536515f1610f98b8/chrome/browser/resources/PRESUBMIT.py [modify] https://crrev.com/99afc077fe37586a0d3ec0c4536515f1610f98b8/chrome/browser/resources/unpack_pak.py [add] https://crrev.com/99afc077fe37586a0d3ec0c4536515f1610f98b8/chrome/browser/resources/unpack_pak_test.py
,
May 8 2018
,
May 8 2018
note: I started both these CLs before I ran out of 20% interest: https://chromium-review.googlesource.com/c/chromium/src/+/849753 https://chromium-review.googlesource.com/c/chromium/src/+/861957 the one that adds a bit of whether a resource is gzipped into pak files seems like a better idea; it's just not fully working yet (don't remember why).
,
Jun 27 2018
> the one that adds a bit of whether a resource is gzipped into pak files seems like a better idea; it's just not fully working yet (don't remember why). After playing with that CL for quite a few days, I think I figured out why it was not working. See the diff between that CL and the working version* at [1]. Specifically there were 3 fixes needed. 1) On the Python side, aliased files were not properly skipped when generating the bit array, throwing an out of range error during building. 2) On the Python side, gzipped_ids sets were not properly combined when RePack was called. 3) On the C++ side, pointer arithmetic to get the bit index was wrong. It was assuming that it operates an byte count (therefore divided by 8 to get the bit index), which is not true. C++ knows the type of the pointer and already takes into account how many bytes its entry occupies. [1] https://chromium-review.googlesource.com/c/chromium/src/+/1111463/1..9 * I've only tried a few files so far, not a full test suite, so it's kind of working for now.
,
Jun 27 2018
|
|||||
►
Sign in to add a comment |
|||||
Comment 1 by dbeam@chromium.org
, Nov 28 2017