[webkit-reviews] review denied: [Bug 54138] [Qt] QWebPage with WebGL content crashes when rendering if no QWebView parent : [Attachment 86288] Check webPageClient before referencing it

[bugzilla-daemon at webkit.org]

Benjamin Poulain <benjamin at webkit.org> has denied Andrew Wason
<rectalogic at rectalogic.com>'s request for review:
Bug 54138: [Qt] QWebPage with WebGL content crashes when rendering if no
QWebView parent
https://bugs.webkit.org/show_bug.cgi?id=54138

Attachment 86288: Check webPageClient before referencing it
https://bugs.webkit.org/attachment.cgi?id=86288&action=review

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

The change make sense to me, I would just like better testing.

> Source/WebKit/qt/tests/qwebpage/tst_qwebpage.cpp:2482
> +void tst_QWebPage::webGLScreenshotWithoutView()

I would like this test, with and without AcceleratedCompositingEnabled. Just to
have good coverage.

> Source/WebKit/qt/tests/qwebpage/tst_qwebpage.cpp:2484
> +    QWebPage* page = new QWebPage;

You should allocate the QWebPage on the stack to avoid dealing with the memory
manually.

> Source/WebKit/qt/tests/qwebpage/tst_qwebpage.cpp:2492
> +    ::waitForSignal(mainFrame, SIGNAL(loadFinished(bool)), 2000);

Is loadFinished() called for setHtml? Otherwise the test would always spend 2
seconds waiting.