[webkit-reviews] review granted: [Bug 79252] [WK2][GTK] WebProcess SIGSEVs due to incorrect clipboard handling : [Attachment 128268] Patch

[bugzilla-daemon at webkit.org]

Martin Robinson <mrobinson at webkit.org> has granted Sergio Villar Senin
<svillar at igalia.com>'s request for review:
Bug 79252: [WK2][GTK] WebProcess SIGSEVs due to incorrect clipboard handling
https://bugs.webkit.org/show_bug.cgi?id=79252

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

------- Additional Comments from Martin Robinson <mrobinson at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=128268&action=review


Great use of FrameDestructionObserver.

> Source/WebKit2/WebProcess/WebCoreSupport/gtk/WebEditorClientGtk.cpp:136
> +    explicit EditorClientFrameDestructionObserver(Frame* frame, GClosure*
closure)

I think you can omit "explicit" here.

> Source/WebKit2/WebProcess/WebCoreSupport/gtk/WebEditorClientGtk.cpp:143
> +    void frameDestroyed() { g_closure_invalidate(m_closure);
FrameDestructionObserver::frameDestroyed(); }

I'd prefer this to be two spread over multiple lines.

> Source/WebKit2/WebProcess/WebCoreSupport/gtk/WebEditorClientGtk.cpp:150
> +	   EditorClientFrameDestructionObserver* observer =
static_cast<EditorClientFrameDestructionObserver*>(data);
> +	   delete observer;

You can just do:

delete data;