[webkit-reviews] review granted: [Bug 30354] [Qt] Plugins : QGVLauncher crashes on exit : [Attachment 41374] Various fixes

[bugzilla-daemon at webkit.org]

Holger Freyther <zecke at selfish.org> has granted Girish Ramakrishnan
<girish at forwardbias.in>'s request for review:
Bug 30354: [Qt] Plugins : QGVLauncher crashes on exit
https://bugs.webkit.org/show_bug.cgi?id=30354

Attachment 41374: Various fixes
https://bugs.webkit.org/attachment.cgi?id=41374&action=review

------- Additional Comments from Holger Freyther <zecke at selfish.org>

>  QWebView::~QWebView()
>  {
> -    if (d->page)
> +    if (d->page) {
>	   d->page->d->view = 0;
> +	   d->page->d->client = 0;
> +    }

As discussed on IRC this might have a bad side effect if someone is doing.

view1->setPage(page)
view2->setPage(page)
delete view1;

but we want to solve it by adding a warning to QWebView::setPage.


> +    if (html.contains("</embed>")) {
> +	   QTest::qWait(2000); // some reasonable time for the PluginStream to
feed test.swf to flash
> +    }

I fear our coding style guidelines wants you to omit the curly braces here.
This needs to be fixed when landing. In general it would be nice if we could
wait for things like QEvent::UpdateRequest, QEvent::ShowEvent instead of the
qWait but in practive we can not. It is making the test a bit fragile but I see
it is the best we can do right now.

I'm leaving the commit-queue to "?" as this test wants a resource introduced by
another patch and the patch needs a tweak.