Don't force layout for scrollTo(0,0).

Don't force layout for scrollTo(0,0).

The ICB origin will always be a valid scroll position, so there is no
need to force a synchronous layout before setting the scroll position to
(0,0).

R=rbyers@chromium.org
BUG=465574
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=193490

[rune, 2015-04-09 11:55:11]

PTAL

[Rick Byers, 2015-04-09 17:46:12]

Nice.  LGTM with nit.

https://codereview.chromium.org/1071983002/diff/1/Source/core/frame/LocalDOMW...
File Source/core/frame/LocalDOMWindow.cpp (right):

https://codereview.chromium.org/1071983002/diff/1/Source/core/frame/LocalDOMW...
Source/core/frame/LocalDOMWindow.cpp:1213: if (x || y)
can you add a brief comment saying why?  I.e. 'Force a layout only when it's
possible the specified scroll position isn't valid'

[Rick Byers, 2015-04-09 17:46:13]

Nice.  LGTM with nit.

[esprehn, 2015-04-09 18:32:17]

esprehn@chromium.org changed reviewers:
+ esprehn@chromium.org

[esprehn, 2015-04-09 18:32:18]

Can we fix setting scrollTop = 0 and scrollLeft = 0 also?

[rune, 2015-04-09 20:51:37]

On 2015/04/09 18:32:18, esprehn wrote:
> Can we fix setting scrollTop = 0 and scrollLeft = 0 also?

You mean on HTML and BODY? Or Element in general?

Could we do that in a separate CL?

[esprehn, 2015-04-09 21:13:29]

On 2015/04/09 at 20:51:37, rune wrote:
> On 2015/04/09 18:32:18, esprehn wrote:
> > Can we fix setting scrollTop = 0 and scrollLeft = 0 also?
> 
> You mean on HTML and BODY? Or Element in general?
> 
> Could we do that in a separate CL?

In general? I'm not sure if scrolling to the top ever needs a layout. Maybe in
some writing modes?

Yeah it's fine to do a separate CL, it's just nice not to have perf cliffs
between similar APIs.

lgtm

[rune, 2015-04-09 21:45:05]

https://codereview.chromium.org/1071983002/diff/1/Source/core/frame/LocalDOMW...
File Source/core/frame/LocalDOMWindow.cpp (right):

https://codereview.chromium.org/1071983002/diff/1/Source/core/frame/LocalDOMW...
Source/core/frame/LocalDOMWindow.cpp:1213: if (x || y)
On 2015/04/09 17:46:11, Rick Byers wrote:
> can you add a brief comment saying why?  I.e. 'Force a layout only when it's
> possible the specified scroll position isn't valid'

Done.

[rune, 2015-04-09 21:45:09]

The CQ bit was checked by rune@opera.com

[rune, 2015-04-09 21:45:10]

The patchset sent to the CQ was uploaded after l-g-t-m from
esprehn@chromium.org, rbyers@chromium.org
Link to the patchset: https://codereview.chromium.org/1071983002/#ps20001
(title: "Added comments")

[commit-bot: I haz the power, 2015-04-09 21:45:54]

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

[commit-bot: I haz the power, 2015-04-09 23:41:34]

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