[webkit-reviews] review denied: [Bug 31591] [Qt] Crashing tests after updating to Qt-4.6.0 : [Attachment 44230] patch v3

[bugzilla-daemon at webkit.org]

Kenneth Rohde Christiansen <kenneth at webkit.org> has denied Andras Becsi
<abecsi at inf.u-szeged.hu>'s request for review:
Bug 31591: [Qt] Crashing tests after updating to Qt-4.6.0
https://bugs.webkit.org/show_bug.cgi?id=31591

Attachment 44230: patch v3
https://bugs.webkit.org/attachment.cgi?id=44230&action=review

------- Additional Comments from Kenneth Rohde Christiansen
<kenneth at webkit.org>

> +    /*
> +    * Create a dummy container object to track the page in DRT.
> +    * QObject is used instead of QWidget to prevent DRT from
> +    * showing the main view when deleting the container.
> +    */

Please use // comment style inside code. That is our style

> +    QObject* container = new QObject(m_mainView);
> +    //create a QWebPage we want to return

Missing space between // and create

> +    QWebPage* page =static_cast<QWebPage*>(new WebPage(container, this));

Missing space after =

> +    //gets cleaned up in closeRemainingWindows()
>      windows.append(container);

same thing

> +
> +    //connect the needed signals to the page

same thing

> +    connect(page, SIGNAL(frameCreated(QWebFrame*)),
> +	       this, SLOT(connectFrame(QWebFrame*)));

Make this a one liner


> +    connect(page, SIGNAL(loadFinished(bool)),
> +	       m_controller, SLOT(maybeDump(bool)));

same as above

> +// Also count the main view

change to // Include the main view in the count

> +    return windows.count() + 1;
>  }