Update visual viewport API to take browser zoom into account.

Update visual viewport API to take browser zoom into account.

BUG=604932
Committed: https://crrev.com/67d8ccb45177b214dbdb51375f1ed1ba31ffa07b
Cr-Commit-Position: refs/heads/master@{#389398}

[ymalik, 2016-04-20 20:15:21]

ymalik@chromium.org changed reviewers:
+ bokan@chromium.org

[bokan, 2016-04-21 13:03:32]

lgtm once the changes below are made

https://codereview.chromium.org/1905693003/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/frame/VisualViewport.cpp (right):

https://codereview.chromium.org/1905693003/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/frame/VisualViewport.cpp:219: return
adjustScrollForAbsoluteZoom(visibleRect().x(), mainFrame()->pageZoomFactor());
ComputedStyle.h is a strange place for this function to live. Please move this
function into a new core/page/scrolling/ScrollUtils.h file (this seems like
overkill but we're overdue, other stuff belongs there too).

At the same time, the overload in ComputedStyle that takes (double, const
ComputedStyle&) is unused so remove it. Also rename to
adjustScrollForZoomFactor.

https://codereview.chromium.org/1905693003/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/frame/VisualViewport.cpp:239:
setLocation(FloatPoint(x * mainFrame()->pageZoomFactor(), visibleRect().y()));
use location().y() rather than visibleRect.

https://codereview.chromium.org/1905693003/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/frame/VisualViewport.cpp:259: return
adjustScrollForAbsoluteZoom(visibleRect().width(),
mainFrame()->pageZoomFactor());
visibleSize().width() rather than visibleRect

[ymalik, 2016-04-24 02:25:30]

https://codereview.chromium.org/1905693003/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/frame/VisualViewport.cpp (right):

https://codereview.chromium.org/1905693003/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/frame/VisualViewport.cpp:219: return
adjustScrollForAbsoluteZoom(visibleRect().x(), mainFrame()->pageZoomFactor());
On 2016/04/21 13:03:32, bokan wrote:
> ComputedStyle.h is a strange place for this function to live. Please move this
> function into a new core/page/scrolling/ScrollUtils.h file (this seems like
> overkill but we're overdue, other stuff belongs there too).
> 
> At the same time, the overload in ComputedStyle that takes (double, const
> ComputedStyle&) is unused so remove it. Also rename to
> adjustScrollForZoomFactor.

I'll make those changes in a follow up CL if you don't mind. Don't want to
clutter this bug fix with cleanup as the functions are used from a bunch of
places.

https://codereview.chromium.org/1905693003/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/frame/VisualViewport.cpp:239:
setLocation(FloatPoint(x * mainFrame()->pageZoomFactor(), visibleRect().y()));
On 2016/04/21 13:03:32, bokan wrote:
> use location().y() rather than visibleRect.

Done.

https://codereview.chromium.org/1905693003/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/frame/VisualViewport.cpp:259: return
adjustScrollForAbsoluteZoom(visibleRect().width(),
mainFrame()->pageZoomFactor());
On 2016/04/21 13:03:32, bokan wrote:
> visibleSize().width() rather than visibleRect

Done.

[ymalik, 2016-04-24 02:25:41]

The CQ bit was checked by ymalik@chromium.org

[ymalik, 2016-04-24 02:25:42]

The patchset sent to the CQ was uploaded after l-g-t-m from bokan@chromium.org
Link to the patchset: https://codereview.chromium.org/1905693003/#ps20001
(title: "address review comments % cleanup")

[commit-bot: I haz the power, 2016-04-24 02:25:52]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1905693003/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1905693003/20001

[ymalik, 2016-04-24 02:42:34]

The CQ bit was unchecked by ymalik@chromium.org

[ymalik, 2016-04-24 03:04:58]

The CQ bit was checked by ymalik@chromium.org

[commit-bot: I haz the power, 2016-04-24 03:05:06]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1905693003/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1905693003/20001

[commit-bot: I haz the power, 2016-04-24 03:43:04]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-04-24 03:44:19]

Description was changed from

==========
Update visual viewport API to take browser zoom into account.

BUG=604932
==========

to

==========
Update visual viewport API to take browser zoom into account.

BUG=604932

Committed: https://crrev.com/67d8ccb45177b214dbdb51375f1ed1ba31ffa07b
Cr-Commit-Position: refs/heads/master@{#389398}
==========

[commit-bot: I haz the power, 2016-04-24 03:44:20]

Patchset 2 (id:??) landed as
https://crrev.com/67d8ccb45177b214dbdb51375f1ed1ba31ffa07b
Cr-Commit-Position: refs/heads/master@{#389398}