[webkit-reviews] review granted: [Bug 107424] Make page scale shrink FrameView in applyPageScaleInCompositor mode : [Attachment 185064] Patch

[bugzilla-daemon at webkit.org]

Levi Weintraub <leviw at chromium.org> has granted Alexandre Elias
<aelias at chromium.org>'s request for review:
Bug 107424: Make page scale shrink FrameView in applyPageScaleInCompositor mode
https://bugs.webkit.org/show_bug.cgi?id=107424

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

------- Additional Comments from Levi Weintraub <leviw at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=185064&action=review


Okay. This looks good to me.

> Source/WebCore/ChangeLog:31
> +	   (WebCore::HistoryController::saveScrollPositionAndViewStateToItem):
Use pageScaleFactor here because frameScaleFactor always returns 1 with our
setting.

Nit: consistent line wrapping.

> Source/WebCore/ChangeLog:53
> +	   (WebCore::RenderLayerCompositor::updateRootLayerPosition): Clip
layer should use unscaled size.
> +	   * rendering/TextAutosizer.cpp:
> +	   (WebCore::TextAutosizer::processSubtree): Text autosizer should use
unscaled size.

Saying "why" would be nice, though I understand these cases.

> Source/WebKit/chromium/ChangeLog:40
> +	   (WebKit::WebViewImpl::scaledSize): Returns the post page-scale size
similar to what visibleContentRect() now returns, except that it may be at a
different scale than the current one.
> +	   (WebKit::WebViewImpl::size): Back to returning density-independent
size without any tricks, not the "layoutSize()" fake viewport.
> +	   (WebKit::WebViewImpl::resize):
> +	   (WebKit::WebViewImpl::handleInputEvent): No need to apply
implTransform anymore as WebKit knows the true scroll offset; just divide event
coords by pageScaleFactor.
> +	   (WebKit::WebViewImpl::clampOffsetAtScale): Make this method support
applyPageScaleFactorInCompositor.  This is used to pre-clamp scroll offsets at
a given viewport size.
> +	   (WebKit::WebViewImpl::setPageScaleFactorPreservingScrollOffset):
Make this method support applyPageScaleFactorInCompositor (don't scale scroll
offsets as they are now scale-independent).
> +	   (WebKit::WebViewImpl::setPageScaleFactor): Make this method always
use clampOffsetAtScale instead of bypassing it, since it's now supported.  Also
notify the compositor to update its state.
> +	   (WebKit::WebViewImpl::contentsSize): Convenience method, removed
difference between scaled and unscaled.
> +	   (WebKit::WebViewImpl::layoutSize): This method returned the "fake"
size we used to give FrameView.  Now no longer used for much.
> +	   (WebKit::WebViewImpl::computePageScaleFactorLimits):
> +	   (WebKit::WebViewImpl::didChangeContentsSize): Remove unnecessary
resize() now that we can give the true size to FrameView.
> +	   (WebKit::WebViewImpl::updateLayerTreeViewport): Use layoutSize()
directly now that FrameView no longer uses it.

Thanks! This makes it a lot easier to follow. nit: super long lines.

> Source/Platform/chromium/public/WebLayerTreeView.h:106
> +    // FIXME: remove this after WebKit roll
> +    virtual WebFloatPoint adjustEventPointForPinchZoom(const WebFloatPoint&
p) const { return p; }

Make sure we get this :)