New issue
Advanced search Search tips

Issue 827332 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Apr 2018
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows , Chrome , Mac
Pri: 3
Type: Bug



Sign in to add a comment

Transport PWG files generated by utility processes using Mojo Shared Buffer

Project Member Reported by thestig@chromium.org, Mar 29 2018

Issue description

In some printing workflows, Chromium does the following to generate a PWG file:

1) The browser process sends a PDF file to a utility process in PwgRasterConverter by:
- Writing the PDF from memory to disk.
- Passing a file handle to the utility process.
2) The utility process renders it to PWG format, and sends it back by:
- Reading the PDF from disk back to memory.
- Converting it to PWG.
- Writing it to disk.
- Passing a file handle to the browser process.
3) The browser process processes the PWG file by:
- Reading the PWG from disk.

So the PDF file goes from memory to disk back to memory, and the PWG file does the same. Performing the writes to disk also involves bouncing between threads, which makes PwgRasterConverter a bit complicated. [1] 

It would be much more straightforward to instead just transport the data to/from the utility process via Mojo Shared Buffers.

The memory consumption of the PDF file is not an issue, since it starts off in memory, and there would be just 1 copy if we plumb it correctly.

The PWG file will consume more memory, since currently the code performs reads/writes in chunks. Whereas this proposal makes it stay in memory the whole time. However, PWG files are typically small (less than 1 MB) so this should be fine. Using a 1310 page PDF spec PDF as a larger than usual test case, when rendered at 600 DPI, the resulting PWG file is 118 MB.

[1] https://chromium.googlesource.com/chromium/src/+/69a7588f/chrome/browser/printing/pwg_raster_converter.cc#102

 
Project Member

Comment 1 by bugdroid1@chromium.org, Apr 27 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/64161ddbd7d3c14d8fe06f965773b720913bf874

commit 64161ddbd7d3c14d8fe06f965773b720913bf874
Author: Lei Zhang <thestig@chromium.org>
Date: Fri Apr 27 00:24:06 2018

Change PrinterProviderPrintJob to only work with data in memory.

When converting to PWG format, read the data into memory from the PWG
file and use the existing code that handles data in memory. This avoids
having to maintain separate code to handle data from files. This is
also to prepare for changing the PWG converter to not write to disk.

BUG= 827332 

Change-Id: Ic36554184190e65cc87988ee2b6235b10b46a0c3
Reviewed-on: https://chromium-review.googlesource.com/977566
Commit-Queue: Lei Zhang <thestig@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Reviewed-by: Rebekah Potter <rbpotter@chromium.org>
Reviewed-by: Toni Barzic <tbarzic@chromium.org>
Cr-Commit-Position: refs/heads/master@{#554214}
[modify] https://crrev.com/64161ddbd7d3c14d8fe06f965773b720913bf874/chrome/browser/ui/webui/print_preview/extension_printer_handler.cc
[modify] https://crrev.com/64161ddbd7d3c14d8fe06f965773b720913bf874/chrome/browser/ui/webui/print_preview/extension_printer_handler_unittest.cc
[modify] https://crrev.com/64161ddbd7d3c14d8fe06f965773b720913bf874/content/browser/blob_storage/chrome_blob_storage_context.cc
[modify] https://crrev.com/64161ddbd7d3c14d8fe06f965773b720913bf874/content/browser/blob_storage/chrome_blob_storage_context.h
[modify] https://crrev.com/64161ddbd7d3c14d8fe06f965773b720913bf874/content/browser/browser_context.cc
[modify] https://crrev.com/64161ddbd7d3c14d8fe06f965773b720913bf874/content/public/browser/browser_context.h
[modify] https://crrev.com/64161ddbd7d3c14d8fe06f965773b720913bf874/extensions/browser/api/printer_provider/printer_provider_apitest.cc
[modify] https://crrev.com/64161ddbd7d3c14d8fe06f965773b720913bf874/extensions/browser/api/printer_provider/printer_provider_print_job.h
[modify] https://crrev.com/64161ddbd7d3c14d8fe06f965773b720913bf874/extensions/browser/api/printer_provider_internal/printer_provider_internal_api.cc

Project Member

Comment 2 by bugdroid1@chromium.org, Apr 27 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ad037d8c61682381f523371243f4e1f1b142bf6c

commit ad037d8c61682381f523371243f4e1f1b142bf6c
Author: Lei Zhang <thestig@chromium.org>
Date: Fri Apr 27 01:42:29 2018

Change PdfToPwgRasterConverter to use shared memory.

Instead of writing PWG data out to disk and reading it back in again in
a different process, write out the data to shared memory and share that
with the receiving process.

BUG= 827332 

Change-Id: Icf43d5efb84b4723ff77997c9d97866bb21d71d0
Reviewed-on: https://chromium-review.googlesource.com/977562
Commit-Queue: Lei Zhang <thestig@chromium.org>
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Reviewed-by: Jay Civelli <jcivelli@chromium.org>
Cr-Commit-Position: refs/heads/master@{#554259}
[modify] https://crrev.com/ad037d8c61682381f523371243f4e1f1b142bf6c/chrome/browser/printing/cloud_print/privet_http_impl.cc
[modify] https://crrev.com/ad037d8c61682381f523371243f4e1f1b142bf6c/chrome/browser/printing/cloud_print/privet_http_impl.h
[modify] https://crrev.com/ad037d8c61682381f523371243f4e1f1b142bf6c/chrome/browser/printing/cloud_print/privet_http_unittest.cc
[modify] https://crrev.com/ad037d8c61682381f523371243f4e1f1b142bf6c/chrome/browser/printing/cloud_print/privet_url_fetcher.cc
[modify] https://crrev.com/ad037d8c61682381f523371243f4e1f1b142bf6c/chrome/browser/printing/cloud_print/privet_url_fetcher.h
[modify] https://crrev.com/ad037d8c61682381f523371243f4e1f1b142bf6c/chrome/browser/printing/pwg_raster_converter.cc
[modify] https://crrev.com/ad037d8c61682381f523371243f4e1f1b142bf6c/chrome/browser/printing/pwg_raster_converter.h
[modify] https://crrev.com/ad037d8c61682381f523371243f4e1f1b142bf6c/chrome/browser/printing/pwg_raster_converter_browsertest.cc
[modify] https://crrev.com/ad037d8c61682381f523371243f4e1f1b142bf6c/chrome/browser/ui/webui/print_preview/extension_printer_handler.cc
[modify] https://crrev.com/ad037d8c61682381f523371243f4e1f1b142bf6c/chrome/browser/ui/webui/print_preview/extension_printer_handler_unittest.cc
[modify] https://crrev.com/ad037d8c61682381f523371243f4e1f1b142bf6c/chrome/services/printing/pdf_to_pwg_raster_converter.cc
[modify] https://crrev.com/ad037d8c61682381f523371243f4e1f1b142bf6c/chrome/services/printing/pdf_to_pwg_raster_converter.h
[modify] https://crrev.com/ad037d8c61682381f523371243f4e1f1b142bf6c/chrome/services/printing/public/mojom/BUILD.gn
[modify] https://crrev.com/ad037d8c61682381f523371243f4e1f1b142bf6c/chrome/services/printing/public/mojom/pdf_to_pwg_raster_converter.mojom

Status: Fixed (was: Assigned)

Sign in to add a comment