[webkit-reviews] review denied: [Bug 53796] [Qt] Fixed background does not work with Tiled Backing Store : [Attachment 179700] Patch

[bugzilla-daemon at webkit.org]

Jocelyn Turcotte <jocelyn.turcotte at digia.com> has denied Allan Sandfeld Jensen
<allan.jensen at digia.com>'s request for review:
Bug 53796: [Qt] Fixed background does not work with Tiled Backing Store
https://bugs.webkit.org/show_bug.cgi?id=53796

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

------- Additional Comments from Jocelyn Turcotte <jocelyn.turcotte at digia.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=179700&action=review


> Source/WebCore/rendering/RenderObject.cpp:1913
> -	   if (view()->frameView()->delegatesScrolling())
> +	   if (view()->frameView()->delegatesScrolling() ||
view()->frameView()->paintsEntireContents())

I think we could replace delegatesScrolling by paintsEntireContents instead of
OR-ing it, they seem to always be enabled together.
This hack is more related to painting than scrolling anyway.

> Source/WebKit/qt/WebCoreSupport/FrameLoaderClientQt.cpp:291
> -
> -	   m_frame->view()->setPaintsEntireContents(resizesToContents);
> +	   bool paintsEntireContents = resizesToContents ||
m_webFrame->pageAdapter->settings->testAttribute(QWebSettings::TiledBackingStor
eEnabled);
> +	   m_frame->view()->setPaintsEntireContents(paintsEntireContents);

This is nasty since this forces parts of the resizesToContents behavior if
QWebSettings::TiledBackingStoreEnabled is set without
QGraphicsWebView::resizesToContents reflecting it. Like stated in comment #2, I
see no advantage of using TiledBackingStoreEnabled without resizesToContents,
so perhaps we should just work around the API design defect by making
TiledBackingStoreEnabled conditional to resizesToContents, or to automatically
enable the later if the former is set.