Closed Bug 1359713 Opened 3 years ago Closed 3 years ago

[Mortar] [Windows] Implement LoadDocument by path in PDFiumEngineShim

Categories

(Core :: Printing: Output, enhancement)

enhancement
Not set

Tracking

()

RESOLVED FIXED
mozilla55
Tracking Status
firefox55 --- fixed

People

(Reporter: fatseng, Assigned: fatseng)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

No description provided.
Assignee: nobody → fatseng
Status: NEW → ASSIGNED
Comment on attachment 8867084 [details]
Bug 1359713 - Stop loading the contents of PDF files into memory in PDFViaEMFPrintHelper

In PDFViaEMFPrintHelper, we maintain a buffer to store PDF context beacuse
using LoadMemDocument(). Calling to LoadDocument() could avoid the effort to
manage the buffer.
Attachment #8867084 - Flags: review?(jwatt)
Comment on attachment 8867084 [details]
Bug 1359713 - Stop loading the contents of PDF files into memory in PDFViaEMFPrintHelper

https://reviewboard.mozilla.org/r/138692/#review146260

A good improvement, thanks.

::: commit-message-d0bc8:1
(Diff revision 3)
> +Bug 1359713 - Implement loading PDF by path in PDFiumEngineShim

Not only are you implementing the functionality, you're also switching from reading the contents of PDFs into memory to just passing a path. The latter part is even more important, so I think the summary line should be something like "Stop loading the contents of PDF files into memory in PDFViaEMFPrintHelper".

::: commit-message-d0bc8:5
(Diff revision 3)
> +Bug 1359713 - Implement loading PDF by path in PDFiumEngineShim
> +
> +In PDFViaEMFPrintHelper, we maintain a buffer to store PDF context beacuse
> +using LoadMemDocument(). Calling to LoadDocument() could avoid the effort to
> +manage the buffer.

Make this:

This change changes PDFViaEMFPrintHelper so that instead of loading the entire contents of a PDF document into memory to pass to PDFium, it now simply passes PDFium a file system path to load the PDF from. This should help us reduce memory use (and potentially avoid running out of memory) when larger PDFs are printed.
Attachment #8867084 - Flags: review?(jwatt) → review+
Pushed by fatseng@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/d6217ec9ce3b
Stop loading the contents of PDF files into memory in PDFViaEMFPrintHelper r=jwatt
Depends on: 1345710
https://hg.mozilla.org/mozilla-central/rev/d6217ec9ce3b
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
Blocks: pdf-printing
You need to log in before you can comment on or make changes to this bug.