[webkit-reviews] review granted: [Bug 36436] QGraphicsWebView crash when calling setView on the QWebPage... : [Attachment 51460] Patch

[bugzilla-daemon at webkit.org]

Simon Hausmann <hausmann at webkit.org> has granted Kenneth Rohde Christiansen
<kenneth at webkit.org>'s request for review:
Bug 36436: QGraphicsWebView crash when calling setView on the QWebPage...
https://bugs.webkit.org/show_bug.cgi?id=36436

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

------- Additional Comments from Simon Hausmann <hausmann at webkit.org>

> +    virtual bool isQWidgetClient() { return false; }

This should be const.


> +    d->unsetPageIfExists();
>      d->page = page;

The pattern seems to call this function and then set the page. Why not merge
both into one setPage() function,
that unsets the previous page and sets the new one (even if it's null)?

The rest of the patch looks good to me. I suggest to fix the const before
landing (all three occurences), I leave the setPage vs. unsetPageIfExists() up
to you :)