[webkit-reviews] review granted: [Bug 126981] [GTK] UI process crashes when closing the window right after printing with javascript : [Attachment 221152] 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 126981: [GTK] UI process crashes when closing the window right after
printing with javascript
https://bugs.webkit.org/show_bug.cgi?id=126981

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

------- Additional Comments from Gustavo Noronha (kov) <gns at gnome.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=221152&action=review


> Source/WebKit2/ChangeLog:17
> +	   The UI process crashes because when the page is closed, the web
> +	   view is destroyed before the print operation has actually
> +	   finished. We were connecting to the destroy signal of the view to
> +	   be notified when it's destroyed to unref the print
> +	   operation. That's wrong because it assumes that the print
> +	   operation is destroyed in the finished callback. Use a weak pointer
> +	   instead, to make sure the web view pointer is set to NULL when the
> +	   view is destroyed and emit the finished callback always so that
> +	   the user can clean up the operation even when the web view has
> +	   been closed.

I don't understand the "That's wrong because it assumes that the print
operation is destroyed in the finished callback." bit in this description,
perhaps you meant destroy callback? I think you should mention the unref too,
how about "That's wrong because we can't be sure the print operation is
destroyed by the unref in the callback for destroy, and it may try to use the
now invalid webView pointer."?

I think readability would be improved if you made the lines longer, too.