The problem is that it's still possible to receive and process the EndPrinting message while the print operation is ongoing. For rendering print pages we are using a nested main loop and idle sources with a higher priority than the idle source used by the I/O thread, so that EndPrinting is never processed until the printing has actually finished. The problem is that sometimes the EndPrinting message is received before starting to render the pages, while we are still enumerating the printers to find the required one. When running in sync mode gtk_enumerate_printers() also uses a nested main loop, so that while waiting for the printer backends to reply, the EndPrinting message can be attended by the run loop, deleting the print context and the print operation. When gtk_enumerate_printers() finishes, the print operation has already been deleted unexpectedly.
Created attachment 221254 [details] Patch
Comment on attachment 221254 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=221254&action=review LGTM, only a few questions to ensure I understood the life-cycle of the printer list correctly. > Source/WebKit2/ChangeLog:12 > + UI process like EndPrinting. When the EndPriting message is Readability would be improved by using longer lines and/or splitting in paragraphs, I think. > Source/WebKit2/WebProcess/WebCoreSupport/WebChromeClient.cpp:610 > + RefPtr<PrinterListGtk> printerList = PrinterListGtk::shared(); Do I understand correctly that the printer list is kept alive by this reference, but is destroyed afterwards? I was a bit concerned when I saw PrinterListGtk::s_sharedPrinterList, since that could mean the printer list would only be obtained once, but I think I understand the idea now. > Source/WebKit2/WebProcess/WebPage/gtk/PrinterListGtk.h:53 > + Vector<GRefPtr<GtkPrinter>, 4> m_printerList; Any particular reason for using 4 here?
(In reply to comment #2) > (From update of attachment 221254 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=221254&action=review > > LGTM, only a few questions to ensure I understood the life-cycle of the printer list correctly. Thanks! > > Source/WebKit2/ChangeLog:12 > > + UI process like EndPrinting. When the EndPriting message is > > Readability would be improved by using longer lines and/or splitting in paragraphs, I think. my emacs. . . > > Source/WebKit2/WebProcess/WebCoreSupport/WebChromeClient.cpp:610 > > + RefPtr<PrinterListGtk> printerList = PrinterListGtk::shared(); > > Do I understand correctly that the printer list is kept alive by this reference, but is destroyed afterwards? I was a bit concerned when I saw PrinterListGtk::s_sharedPrinterList, since that could mean the printer list would only be obtained once, but I think I understand the idea now. Yes, it's tricky. The thing is that for sync printing, we need to make sure that the PrinterList object has already been created, for async printing the PrinterList object is created in WebPrintOperationGtk::startPrint(). The shared method returns a RefPtr, but when created the reference is adopted. So what happens is the following: Sync printing: WebChromeClient::print() -> creates the PrinterList WebPrintOperationGtk::startPrint() -> gets a ref of the existing PrinterList WebPrintOperationGtk::startPrint() -> releases its ref WebChromeClient::print() -> PrinterList is destroyed Async printing: WebPrintOperationGtk::startPrint() -> creates the PrinterList WebPrintOperationGtk::startPrint() -> PrinterList is destroyed > > Source/WebKit2/WebProcess/WebPage/gtk/PrinterListGtk.h:53 > > + Vector<GRefPtr<GtkPrinter>, 4> m_printerList; > > Any particular reason for using 4 here? Nope, I assumed there are usually less than 4 printers (print to file, real printer and possibly a cups-pdf virtual printer).
Adding wk2 owner because of the change in WebChromeClient
(In reply to comment #3) > > > Source/WebKit2/ChangeLog:12 > > > + UI process like EndPrinting. When the EndPriting message is > > > > Readability would be improved by using longer lines and/or splitting in paragraphs, I think. > > my emacs. . . Mx- set-fill-column <RET> 92 <RET> \o/ =) > > Do I understand correctly that the printer list is kept alive by this reference, but is destroyed afterwards? I was a bit concerned when I saw PrinterListGtk::s_sharedPrinterList, since that could mean the printer list would only be obtained once, but I think I understand the idea now. > > Yes, it's tricky. The thing is that for sync printing, we need to make sure that the PrinterList object has already … Makes sense, thanks!
Committed r162073: <http://trac.webkit.org/changeset/162073>