[webkit-reviews] review granted: [Bug 131863] [iOS WebKit2] Implement CSS viewport units : [Attachment 229717] patch

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Tim Horton <thorton at apple.com>'s
request for review:
Bug 131863: [iOS WebKit2] Implement CSS viewport units
https://bugs.webkit.org/show_bug.cgi?id=131863

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

------- Additional Comments from Darin Adler <darin at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=229717&action=review


> Source/WebCore/rendering/RenderView.cpp:1050
> +    IntSize viewportSize = frameView().frame().page()->viewportSize();
> +    if (!viewportSize.isEmpty())
> +	   return viewportSize;

Seems a little strange. I understand that we want to distinguish a size that is
set from a size that is not. But it doesn’t seem right that “empty” is now we
track “not set”.

> Source/WebCore/rendering/RenderView.h:202
> +    void setViewportSize(IntSize);

In the old days we would invariably have used "const IntSize&", but I gather
that’s just old fashioned attempt at optimizing that didn’t really work.