Compress Print Preview resources. |
|||||
Issue descriptionCurrently it seems that print preview resources are not compressed, see [1]. Looking at the code, the most likely reason is that the UseGzip mechanism is hard to cooperate with Print preview's needs, which uses some dynamic URL (for example dynamic URLs like chrome://print/<param>=<value>), as well various non-text resources. See an initial (incomplete) attempt at [2], for the new Print preview UI. [1] https://cs.chromium.org/chromium/src/chrome/browser/browser_resources.grd?q=browser_resources.grd+-file:third_party+-file:infra+-file:out/Debug&dr&l=382,383 [2] https://chromium-review.googlesource.com/c/chromium/src/+/988821
,
May 23 2018
dpapad: Do you want to own this?
,
May 23 2018
Will do, although not sure of exactly when I can get back to this. Hopefully M70. This is not a regression anyway, so that should be fine.
,
Jun 27 2018
Marking this as blocked on issue 738243, since resolving that one will make compressing Print Preview resources much easier (no need to manually exclude/kep-track of non-gzipped files).
,
Dec 20
FYI, at https://chromium-review.googlesource.com/c/chromium/src/+/1387158 I am compressing print preview resources producing a 78% size reduction. There is one problem left to solve though, before being able to land these, which I could use some help, summarizing below: 1) Print preview is using SetRequestFilter (see [1]), such that it can dynamically handle requests to URLs like chrome://print/1/0/print.pdf 2) When a WebUIDataSource is configured with UseGzip(), it assumes that all requests, except the ones explicitly excluded are compressed (see [2]) 3) There is currently no way of specifying that a request handled with a request filter is not gzipped, therefore currently the contents of the PDF are unnecessarily "uncompressed" which leads to errors. [1] https://cs.chromium.org/chromium/src/chrome/browser/ui/webui/print_preview/print_preview_ui.cc?l=486 [2] https://cs.chromium.org/chromium/src/content/browser/webui/web_ui_data_source_impl.cc?l=310
,
Jan 11
btw, I think I fixed the issues with chrome://print/<ui_id>/<page_index>/print.pdf dynamic paths here: https://crrev.com/c/1406174 and because this could chop 430K out of a pak file, it seems useful enough to raise to P2
,
Jan 15
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/8a788f9df3a758c45ac181b3804d94929098dd3f commit 8a788f9df3a758c45ac181b3804d94929098dd3f Author: Dan Beam <dbeam@chromium.org> Date: Tue Jan 15 03:01:32 2019 WebUI: remove optional parameter from UseGzip() This is in preparation for making UseGzip() accept a callback instead of an exclusion list. Most callers will be updated to just use something like: source->UseGzip(base::BindRepeating([](const std::string& path) { return path != kForbiddenPath; }); Or, for a more advanced usage: std::vector<std::string> exclude_from_gzip = GatherExcludedPaths(); source->UseGzip(base::BindRepeating([]( const std::vector<std::string>& excluded_paths, const std::string& path) { return !base::ContainsValue(excluded_paths, path) && !DynamicPathDetection(path); }, std::move(exclude_from_gzip))); This is preparation for gzipping print preview resources, which need to dynamically determine exclusions. R=thestig@chromium.org BUG= 840024 Change-Id: I37da4b0546b9079de489571eada2c7041193e8e8 Reviewed-on: https://chromium-review.googlesource.com/c/1407963 Reviewed-by: John Abd-El-Malek <jam@chromium.org> Reviewed-by: Lei Zhang <thestig@chromium.org> Commit-Queue: Dan Beam <dbeam@chromium.org> Cr-Commit-Position: refs/heads/master@{#622700} [modify] https://crrev.com/8a788f9df3a758c45ac181b3804d94929098dd3f/chrome/browser/ui/webui/interventions_internals/interventions_internals_ui.cc [modify] https://crrev.com/8a788f9df3a758c45ac181b3804d94929098dd3f/content/browser/webui/web_ui_data_source_impl.cc [modify] https://crrev.com/8a788f9df3a758c45ac181b3804d94929098dd3f/content/browser/webui/web_ui_data_source_impl.h [modify] https://crrev.com/8a788f9df3a758c45ac181b3804d94929098dd3f/content/browser/webui/web_ui_data_source_unittest.cc [modify] https://crrev.com/8a788f9df3a758c45ac181b3804d94929098dd3f/content/public/browser/web_ui_data_source.h
,
Jan 16
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/a6f790c1ffaeb1f0a9f996bb5d996dcc1cbeb799 commit a6f790c1ffaeb1f0a9f996bb5d996dcc1cbeb799 Author: Dan Beam <dbeam@chromium.org> Date: Wed Jan 16 00:17:49 2019 Change WebUIDataSource::UseGzip() to an exclusion callback (not a list) This is so callers that need to do dynamic exclusion (print preview) can R=thestig@chromium.org BUG= 840024 Change-Id: I27e057a79ee91001e8244ae88a2e267a13236308 Reviewed-on: https://chromium-review.googlesource.com/c/1408032 Reviewed-by: Lei Zhang <thestig@chromium.org> Reviewed-by: John Abd-El-Malek <jam@chromium.org> Commit-Queue: Dan Beam <dbeam@chromium.org> Cr-Commit-Position: refs/heads/master@{#622912} [modify] https://crrev.com/a6f790c1ffaeb1f0a9f996bb5d996dcc1cbeb799/chrome/browser/accessibility/accessibility_ui.cc [modify] https://crrev.com/a6f790c1ffaeb1f0a9f996bb5d996dcc1cbeb799/chrome/browser/ui/webui/md_bookmarks/md_bookmarks_ui.cc [modify] https://crrev.com/a6f790c1ffaeb1f0a9f996bb5d996dcc1cbeb799/chrome/browser/ui/webui/md_downloads/md_downloads_ui.cc [modify] https://crrev.com/a6f790c1ffaeb1f0a9f996bb5d996dcc1cbeb799/chrome/browser/ui/webui/md_history_ui.cc [modify] https://crrev.com/a6f790c1ffaeb1f0a9f996bb5d996dcc1cbeb799/chrome/browser/ui/webui/settings/md_settings_ui.cc [modify] https://crrev.com/a6f790c1ffaeb1f0a9f996bb5d996dcc1cbeb799/content/browser/tracing/tracing_ui.cc [modify] https://crrev.com/a6f790c1ffaeb1f0a9f996bb5d996dcc1cbeb799/content/browser/webui/web_ui_data_source_impl.cc [modify] https://crrev.com/a6f790c1ffaeb1f0a9f996bb5d996dcc1cbeb799/content/browser/webui/web_ui_data_source_impl.h [modify] https://crrev.com/a6f790c1ffaeb1f0a9f996bb5d996dcc1cbeb799/content/browser/webui/web_ui_data_source_unittest.cc [modify] https://crrev.com/a6f790c1ffaeb1f0a9f996bb5d996dcc1cbeb799/content/public/browser/web_ui_data_source.h [modify] https://crrev.com/a6f790c1ffaeb1f0a9f996bb5d996dcc1cbeb799/docs/optimizing_web_uis.md
,
Jan 16
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/b2e1def9a54175d371457e5eaa01582d01f35dea commit b2e1def9a54175d371457e5eaa01582d01f35dea Author: Dan Beam <dbeam@chromium.org> Date: Wed Jan 16 02:23:39 2019 Gzip print preview resources This CL adds a callback to UseGzip() to enable dynamic gzip filtering. Print preview needs this to recognize <ui_id>/<page_index>/print.pdf paths and exclude them from gzipping. $ stat -c %s out/chromium/gen/chrome/print_preview_resources.pak 548597 $ stat -c %s out/chromium/gen/chrome/print_preview_resources.pak 118278 Bytes saved: 430319, 78% of print_preview_resources.pak's total size. Helped along by https://crrev.com/c/1387158 by Demetrios Papadopoulos <dpapad@chromium.org> Bug: 840024 Change-Id: I6ee1da05f8cb01e202d85df64f2636906b5632eb Reviewed-on: https://chromium-review.googlesource.com/c/1406174 Reviewed-by: Lei Zhang <thestig@chromium.org> Commit-Queue: Dan Beam <dbeam@chromium.org> Cr-Commit-Position: refs/heads/master@{#623050} [modify] https://crrev.com/b2e1def9a54175d371457e5eaa01582d01f35dea/chrome/browser/resources/print_preview/print_preview_resources_vulcanized.grd [modify] https://crrev.com/b2e1def9a54175d371457e5eaa01582d01f35dea/chrome/browser/ui/webui/print_preview/print_preview_ui.cc [modify] https://crrev.com/b2e1def9a54175d371457e5eaa01582d01f35dea/chrome/browser/ui/webui/print_preview/print_preview_ui.h [modify] https://crrev.com/b2e1def9a54175d371457e5eaa01582d01f35dea/chrome/browser/ui/webui/print_preview/print_preview_ui_unittest.cc |
|||||
►
Sign in to add a comment |
|||||
Comment 1 by bugdroid1@chromium.org
, May 8 2018