[webkit-reviews] review denied: [Bug 43852] [Qt] resizeToContent seems to trigger infinite resize on some pages : [Attachment 101385] patch

[bugzilla-daemon at webkit.org]

Benjamin Poulain <benjamin at webkit.org> has denied Luiz Agostini
<luiz at webkit.org>'s request for review:
Bug 43852: [Qt] resizeToContent seems to trigger infinite resize on some pages
https://bugs.webkit.org/show_bug.cgi?id=43852

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

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


Eh Kenneth, what's up? Mind having a look and guiding this toward landing? :)

Luiz, could you also integrate the original test (and the alternate tests
people proposed)?

> Source/WebCore/page/DOMWindow.cpp:1094
> -    
> +

Unrelated change, should be in a separate patch .... just kidding ;)

> Source/WebCore/page/DOMWindow.cpp:1099
> +#if PLATFORM(QT)
> +    return static_cast<int>(view->visibleHeight() /
m_frame->pageZoomFactor());
> +#else
>      return static_cast<int>(view->height() / m_frame->pageZoomFactor());
> +#endif

This seems very wrong in my opinion, it means we have a problem of architecture
in WebKit.
A class like DOMWindow should not have platform specific changes like this.

> Source/WebCore/page/DOMWindow.cpp:1115
> +#if PLATFORM(QT)
> +    return static_cast<int>(view->visibleWidth() /
m_frame->pageZoomFactor());
> +#else
>      return static_cast<int>(view->width() / m_frame->pageZoomFactor());
> +#endif

Same problem.

> Source/WebKit/qt/tests/qgraphicswebview/tst_qgraphicswebview.cpp:632
> +    QGraphicsWebView* webView = new QGraphicsWebView();

Why do you allocate the view on the heap? It looks like it is leaked in this
function.