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

[bugzilla-daemon at webkit.org]

Benjamin Poulain <benjamin at webkit.org> has granted 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 93578: Patch
https://bugs.webkit.org/attachment.cgi?id=93578&action=review

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

Looks good, please add the assertion so we make sure we never leak the QWidget.


Let's hope this path will have a better fate than the previous one :) R+

> Tools/DumpRenderTree/qt/DumpRenderTreeQt.cpp:162
> +    DumpRenderTreeSupportQt::setView(this, new
QWidget(qobject_cast<QWidget*>(parent)));

Please add:
ASSERT(qobject_cast<QWidget*>(parent))
before this line.

> Tools/DumpRenderTree/qt/DumpRenderTreeQt.cpp:-467
> -    m_eventSender = new EventSender(m_page);

Do we still need m_eventSender at all now that we have m_page->eventSender()?