PDF printing support in CEF

[GoogleCodeExporter]

There is a topic on CEF forum
http://www.magpcss.org/ceforum/viewtopic.php?f=8&t=12403

Attached patch adds method PrintToPDF to CefBrowserHost class:

CefBrowserHost::PrintToPDF(const CefString& output_path,
                           const CefPdfPrintSettings& print_settings,
                           CefRefPtr<CefPdfPrintCallback> finish_callback)

This method saves current document into PDF file. CefPdfPrintSettings structure 
provides print settings which are similar to CefPrintSettings. 
CefPdfPrintCallback is called in UI thread when the task has finished. Tasks 
can not be run in parallel. There is no UI.

In general the code is working but there are some issues:
- header & footer printing is not working - I have no idea why
- I have no UI so I have no Preview UI ID - I just set it to 12345678
- because of base::Unretained(this) application should not be closed before 
finish_callback is called

Original issue reported on code.google.com by alexe...@gmail.com on 18 Dec 2014 at 11:57

Attachments:

• cef_pdf_printing.zip

[GoogleCodeExporter]

Tried applying the patches to 2171 but had this build error:
FAILED: ninja -t msvc -e environment.x86 -- "C:\Program Files (x86)\Microsoft Vi
sual Studio 12.0\VC\bin\amd64_x86\cl.exe" /nologo /showIncludes /FC @obj\cef\lib
cef\browser\libcef_static.browser_host_impl.obj.rsp /c ..\..\cef\libcef\browser\
browser_host_impl.cc /Foobj\cef\libcef\browser\libcef_static.browser_host_impl.o
bj /Fdobj\cef\libcef_static.cc.pdb
f:\cef-stuff\download\chromium\src\cef\libcef\browser\browser_host_impl.cc(478)
: error C2259: 'CefBrowserHostImpl' : cannot instantiate abstract class
        due to following members:
        'void CefBrowserHost::PrintToPDF(const CefString &,const CefPdfPrintSett
ings &,CefRefPtr<CefPdfPrintCallback>)' : is abstract
        f:\cef-stuff\download\chromium\src\cef\include\cef_browser.h(404) : see
declaration of 'CefBrowserHost::PrintToPDF'
f:\cef-stuff\download\chromium\src\cef\libcef\browser\browser_host_impl.cc(835)
: error C2509: 'PrintToPDF' : member function not declared in 'CefBrowserHostImp
l'
        f:\cef-stuff\download\chromium\src\cef\libcef\browser\browser_host_impl.
h(93) : see declaration of 'CefBrowserHostImpl'
f:\cef-stuff\download\chromium\src\cef\libcef\browser\browser_host_impl.cc(835)

Original comment by alexandr...@gmail.com on 9 Jan 2015 at 9:07

[GoogleCodeExporter]

The patch here was made for CEF trunk, not 2171 branch. You can use original 
patch from http://www.magpcss.org/ceforum/viewtopic.php?f=8&t=12403
It was for 2171 branch

Original comment by alexe...@gmail.com on 9 Jan 2015 at 11:10

[GoogleCodeExporter]

My bad. Gonna try the other one. Does the 2171 patch implement the cefclient 
test or just the interface? In the trunk version there are two separate 
patches. Thanks for publishing this, I was looking for a way to tune some 
printing options *and* export to pdf and your patch seems to do both.

Original comment by alexandr...@gmail.com on 10 Jan 2015 at 11:37

[GoogleCodeExporter]

I found a a compiler error in 
0002-Run-PDF-printing-test-from-CefClient-menu.patch
I created new patch for 2171 and fixed the trunk patch

Original comment by alexe...@gmail.com on 10 Jan 2015 at 3:22

Attachments:

• patches.zip

[GoogleCodeExporter]

@#4: Attaching the patches for trunk as separate files to make reviewing easier.

Original comment by magreenb...@gmail.com on 12 Jan 2015 at 11:12

Attachments:

• 0001-PDF-printing-support.patch
• 0002-Run-PDF-printing-test-from-CefClient-menu.patch

[GoogleCodeExporter]

@#4: Thanks for the patch, some comments:

1. Generate patch files with `git diff --no-prefix` and don't include the 
auto-generated translation files.

2. Wrap all lines to 80 characters (in browser_host_impl.cc, etc).

3. Line 131:
+  ///
+  // Method that will be executed. |path| is the output path, |ok| is true if
+  // printing was successfull otherwise it is false.
+  ///
+  /*--cef()--*/
+  virtual void OnPdfPrintFinished(const CefString& path, bool ok) =0;

Typo, should be "successful".

4. Line 144:
   ///
+  // Print the current browser contents.
+  ///
+  /*--cef()--*/
+  virtual void PrintToPDF(const CefString& path,
+                          const CefPdfPrintSettings& settings,
+                          CefRefPtr<CefPdfPrintCallback> callback) = 0;

Be more specific in your documentation. For example, "Print the current browser 
contents to the PDF file specified by |path| and execute |callback| on 
completion. The caller is responsible for deleting |path| when done."

5. Line 167:
+typedef struct _cef_pdf_print_settings_t {

Add comments for the members of this structure.

+  cef_string_t header_footer_title;
+  cef_string_t header_footer_url;

Where will these strings be placed on the printed page? Is there a way to 
choose the location (header vs footer)?

+  int page_width;
+  int page_height;
+
+  double margin_top;
+  double margin_right;
+  double margin_bottom;
+  double margin_left;

What is the unit of measurement for these values?

+  int margin_type;

What does this member mean? What values are allowed?

+  int header_footer_enabled;
+  int selection_only;
+  int landscape;
+  int should_print_backgrounds;

Are these boolean values?

6. Line 312:
+void CefBrowserHostImpl::PrintToPdfImpl(const CefString& path,

It shouldn't be necessary to have a separate PrintToPdfImpl method since 
CefPdfPrintSettings should be passable to base::Bind.

7. Line 393:
+struct PdfPrintSettings {

Why not just use the CefPdfPrintSettings type instead of defining another 
equivalent type? Then you also don't need the toPdfPrintSettings function.

8. Line 440:
+void FillInDictionaryFromPdfPrintSettings(const PdfPrintSettings& pdfSettings, 
int requestId, base::DictionaryValue& printSettings)

Where do the values populated in this function come from? Do they need to be 
kept synchronized with values in some other source code file? Please answer 
these questions in source code comments.

9. Line 609:

+void PrintViewManagerBase::PrintToPdfFinished(bool ok) {
+  DCHECK_CURRENTLY_ON(BrowserThread::UI);
+  VLOG(1) << "PrintToPDF: finished, ok = " << ok;
+
+  pdf_print_settings_.reset();
+  pdf_print_callback_.Run(ok);
+  pdf_print_callback_.Reset();
+}

|pdf_print_callback_| should probably be reset before Run() is called (on a 
temporary copy of |pdf_print_callback_|). Otherwise, if the client calls 
PrintToPDF again from inside the callback, bad things might happen. You should 
also probably clear |pdf_output_path_|.

Original comment by magreenb...@gmail.com on 12 Jan 2015 at 11:51

• Changed state: Accepted
• Added labels: Type-Enhancement
• Removed labels: Type-Defect

[GoogleCodeExporter]

@#4: I notice your 0002-Run-PDF-printing-test-from-CefClient-menu.patch file 
only adds tests for Windows. Have you tested the functionality on any other 
platform?

Original comment by magreenb...@gmail.com on 12 Jan 2015 at 11:53

[GoogleCodeExporter]

I agree with your comments and will make corresponding changes in my code.
> Have you tested the functionality on any other platform?
No. Unfortunately I have no experience in writing GUI for OS X or Linux.

Original comment by alexe...@gmail.com on 13 Jan 2015 at 9:16

[GoogleCodeExporter]

New patches without defects described in comment #6

Original comment by alexe...@gmail.com on 17 Jan 2015 at 5:46

Attachments:

• 0001-PDF-printing-support.patch
• 0002-Run-PDF-printing-test-from-CefClient-menu.patch

[GoogleCodeExporter]

[deleted comment]

[GoogleCodeExporter]

The latest patches are against trunk, right?

Original comment by alexandr...@gmail.com on 26 Jan 2015 at 8:41

[GoogleCodeExporter]

> The latest patches are against trunk, right?
Yes

Original comment by alexe...@gmail.com on 27 Jan 2015 at 9:10

[GoogleCodeExporter]

CEF is transitioning from Google Code to Bitbucket project hosting. If you 
would like to continue receiving notifications on this issue please add 
yourself as a Watcher at the new location: 
https://bitbucket.org/chromiumembedded/cef/issue/1478

Original comment by magreenb...@gmail.com on 14 Mar 2015 at 3:36