[webkit-reviews] review denied: [Bug 60722] [Qt] fix http/tests/plugins/plugin-document-has-focus.html : [Attachment 93557] Patch

[bugzilla-daemon at webkit.org]

Benjamin Poulain <benjamin at webkit.org> has denied Robert Hogan
<robert at webkit.org>'s request for review:
Bug 60722: [Qt] fix http/tests/plugins/plugin-document-has-focus.html
https://bugs.webkit.org/show_bug.cgi?id=60722

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

------- Additional Comments from Benjamin Poulain <benjamin at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=93557&action=review

> Source/WebKit/qt/WebCoreSupport/DumpRenderTreeSupportQt.h:110
> +    static void setView(QWebPage*, QWidget* view);

What would you think about:
-we get rid of this
-the WebPage allocate its own view in the constructor.

> Tools/DumpRenderTree/qt/DumpRenderTreeQt.cpp:179
> +    if (!m_eventSender)
> +	   m_eventSender = new EventSender(this);
> +    return m_eventSender;

Can we skip the lazy allocation?

> Tools/DumpRenderTree/qt/DumpRenderTreeQt.h:219
> +    EventSender* m_eventSender;

Why not allocate the event sender with the object:
EventSender* m_eventSender; -> EventSender m_eventSender;
This way we don't have to care about the memory.

If that can't be done for some reason, please use a smart pointer so that there
is no need to delete m_eventSender in the destructor.