[webkit-reviews] review denied: [Bug 103090] [WK2] TiledBackingStore: page contents is scaled wrongly : [Attachment 175710] patch

[bugzilla-daemon at webkit.org]

Kenneth Rohde Christiansen <kenneth at webkit.org> has denied Mikhail Pozdnyakov
<mikhail.pozdnyakov at intel.com>'s request for review:
Bug 103090: [WK2] TiledBackingStore: page contents is scaled wrongly
https://bugs.webkit.org/show_bug.cgi?id=103090

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

------- Additional Comments from Kenneth Rohde Christiansen
<kenneth at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=175710&action=review


> Source/WebKit2/ChangeLog:16
> +
> +	   Before this change the page contents scaling in
PageViewportController was defined all the times
> +	   by m_rawAttributes.initialScale. If initial scale had not been
specified in the viewport meta tag
> +	   it was set to m_minimumScaleToFit inside
PageViewportController::didChangeViewportAttributes().
> +	   The problem was that m_minimumScaleToFit could have wrong value as
contents size might have not be updated
> +	   at the time PageViewportController::didChangeViewportAttributes()
was invoked.
> +	   The solution is to use m_minimumScaleToFit for contents scaling if
initial scale is not specified in the viewport meta tag,
> +	   as it is updated all the time. Also a flag m_initiallyFitToViewport
is added to PageViewportController
> +	   to detect whether m_minimumScaleToFit should be used for scaling.
> +

Could you use shorter lines, some newlines etc to make this easier to read?
Also doesn't this affect css @viewport as well?

Why don't we add a test?

> Source/WebKit2/UIProcess/PageViewportController.cpp:111
> -{
> +{	

That seems wrong