[webkit-reviews] review granted: [Bug 126979] [GTK] Web process sometimes crashes when printing in synchronous mode : [Attachment 221254] Patch

[bugzilla-daemon at webkit.org]

Gustavo Noronha (kov) <gns at gnome.org> has granted Carlos Garcia Campos
<cgarcia at igalia.com>'s request for review:
Bug 126979: [GTK] Web process sometimes crashes when printing in synchronous
mode
https://bugs.webkit.org/show_bug.cgi?id=126979

Attachment 221254: Patch
https://bugs.webkit.org/attachment.cgi?id=221254&action=review

------- Additional Comments from Gustavo Noronha (kov) <gns at gnome.org>
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?