Hook up window.scrollTo and friends.

Hook up window.scrollTo and friends.

BUG=417782, 450279
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=189296

[skobes, 2015-01-28 04:57:24]

Patchset #1 (id:1) has been deleted

[skobes, 2015-01-28 05:45:05]

Patchset #1 (id:20001) has been deleted

[skobes, 2015-01-29 03:16:37]

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

[skobes, 2015-01-29 03:16:38]

(depends on http://crrev.com/879123002)

[skobes, 2015-01-31 00:30:58]

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

[skobes, 2015-01-31 00:31:24]

Hi ajuma, would you mind taking a look at this?

[ajuma, 2015-01-31 00:53:18]

https://codereview.chromium.org/879163002/diff/60001/Source/core/frame/PinchV...
File Source/core/frame/PinchViewport.cpp (right):

https://codereview.chromium.org/879163002/diff/60001/Source/core/frame/PinchV...
Source/core/frame/PinchViewport.cpp:162: view->setScrollPosition(targetOffset);
Does this (and the other places in this file where we access or set the
FrameView's scroll position) also need to be changed to go through the
scrollableArea() accessor? Or are these left to be fixed later since they aren't
called during scrollTo?

[skobes, 2015-01-31 00:59:13]

https://codereview.chromium.org/879163002/diff/60001/Source/core/frame/PinchV...
File Source/core/frame/PinchViewport.cpp (right):

https://codereview.chromium.org/879163002/diff/60001/Source/core/frame/PinchV...
Source/core/frame/PinchViewport.cpp:162: view->setScrollPosition(targetOffset);
On 2015/01/31 00:53:17, ajuma wrote:
> Does this (and the other places in this file where we access or set the
> FrameView's scroll position) also need to be changed to go through the
> scrollableArea() accessor? Or are these left to be fixed later since they
aren't
> called during scrollTo?

Yes, everything that touches the FrameView's scroll position will eventually
need to be updated.  I'm trying to keep the changes small though.

[ajuma, 2015-01-31 01:00:23]

On 2015/01/31 00:59:13, skobes wrote:
>
https://codereview.chromium.org/879163002/diff/60001/Source/core/frame/PinchV...
> File Source/core/frame/PinchViewport.cpp (right):
> 
>
https://codereview.chromium.org/879163002/diff/60001/Source/core/frame/PinchV...
> Source/core/frame/PinchViewport.cpp:162:
view->setScrollPosition(targetOffset);
> On 2015/01/31 00:53:17, ajuma wrote:
> > Does this (and the other places in this file where we access or set the
> > FrameView's scroll position) also need to be changed to go through the
> > scrollableArea() accessor? Or are these left to be fixed later since they
> aren't
> > called during scrollTo?
> 
> Yes, everything that touches the FrameView's scroll position will eventually
> need to be updated.  I'm trying to keep the changes small though.

Sounds good, lgtm.

[skobes, 2015-01-31 01:05:29]

The CQ bit was checked by skobes@chromium.org

[commit-bot: I haz the power, 2015-01-31 01:05:50]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/879163002/60001

[commit-bot: I haz the power, 2015-01-31 01:12:18]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-01-31 01:12:19]

Try jobs failed on following builders:
  blink_presubmit on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/blink_presubmit/builds/2...)

[skobes, 2015-01-31 01:19:48]

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

[skobes, 2015-01-31 01:19:49]

+jchaffraix for OWNERS

[skobes, 2015-01-31 01:21:32]

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

[skobes, 2015-01-31 01:21:33]

or maybe eae@ is in a better time zone

[eae, 2015-01-31 03:39:31]

On 2015/01/31 01:21:33, skobes wrote:
> or maybe eae@ is in a better time zone

LGTM

[skobes, 2015-01-31 03:43:35]

skobes@chromium.org changed reviewers:
- jchaffraix@chromium.org, vollick@chromium.org

[skobes, 2015-01-31 03:43:45]

The CQ bit was checked by skobes@chromium.org

[commit-bot: I haz the power, 2015-01-31 03:43:51]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/879163002/60001

[commit-bot: I haz the power, 2015-01-31 03:47:46]

Committed patchset #2 (id:60001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=189296