Attempt scroll restoration anytime viewport size changes

Attempt scroll restoration anytime viewport size changes

Viewport size determines the scroll clamp boundaries so when it changes,
we should retry any pending scroll restoration in case it is blocked
due to clamping. This can happen when page scale changes.

Removed |FrameView::clampOffsetAtScale| as it was only ever used
with scale 1 and could be replaces with |ScrollableArea::clampScrollPosition|.

FrameView calls the restore method directly making it unnecessary for
ChromeClient to be involved.


BUG=522153
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=201323

[majidvp, 2015-08-24 12:52:45]

majidvp@chromium.org changed reviewers:
+ japhet@chromium.org

[Nate Chapin, 2015-08-24 17:18:18]

japhet@chromium.org changed reviewers:
+ skobes@chromium.org

[Nate Chapin, 2015-08-24 17:18:18]

This seems reasonable to me, but given my limited knowledge of scrolling, I'd
appreciate a second opinion. skobes@?

https://codereview.chromium.org/1300393003/diff/20001/Source/core/frame/Frame...
File Source/core/frame/FrameView.cpp (right):

https://codereview.chromium.org/1300393003/diff/20001/Source/core/frame/Frame...
Source/core/frame/FrameView.cpp:411:
page()->frameHost().visualViewport().mainFrameDidChangeSize();
Nit: this line should be m_frame->host() instead of page()->frameHost(). Would
you mind updating it while you're here?

[skobes, 2015-08-24 18:50:24]

Will this do anything strange to window resizing on desktop?  In general,
resizing the window shouldn't change the scroll position.

https://codereview.chromium.org/1300393003/diff/20001/Source/core/frame/Frame...
File Source/core/frame/FrameView.h (left):

https://codereview.chromium.org/1300393003/diff/20001/Source/core/frame/Frame...
Source/core/frame/FrameView.h:106: IntPoint clampOffsetAtScale(const IntPoint&
offset, float scale) const;
Yay, removing code from FrameView :)

[majidvp, 2015-08-24 19:30:37]

On 2015/08/24 18:50:24, skobes wrote:
> Will this do anything strange to window resizing on desktop?  In general,
> resizing the window shouldn't change the scroll position.
It should not. |restoreScrollPositionAndViewState| is a no-op except when 
frame is loading and then only when the load needs history restoration 
(e.g., history navs, reloads).

See
https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit...

[skobes, 2015-08-24 19:42:39]

lgtm

[majidvp, 2015-08-25 19:12:42]

japhet@: PTAL

https://codereview.chromium.org/1300393003/diff/20001/Source/core/frame/Frame...
File Source/core/frame/FrameView.cpp (right):

https://codereview.chromium.org/1300393003/diff/20001/Source/core/frame/Frame...
Source/core/frame/FrameView.cpp:411:
page()->frameHost().visualViewport().mainFrameDidChangeSize();
On 2015/08/24 17:18:18, Nate Chapin wrote:
> Nit: this line should be m_frame->host() instead of page()->frameHost(). Would
> you mind updating it while you're here?

Done.

[Nate Chapin, 2015-08-27 16:10:38]

lgtm

[majidvp, 2015-08-27 16:51:06]

The CQ bit was checked by majidvp@chromium.org

[majidvp, 2015-08-27 16:51:07]

The patchset sent to the CQ was uploaded after l-g-t-m from skobes@chromium.org
Link to the patchset: https://codereview.chromium.org/1300393003/#ps40001
(title: "s/page()->frameHost()/m_frame->host()/")

[commit-bot: I haz the power, 2015-08-27 16:51:18]

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

[commit-bot: I haz the power, 2015-08-27 17:56:45]

Committed patchset #3 (id:40001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=201323