Test framework and tests for CSSOM View smooth scroll

Test harness and tests for CSSOM View smooth scroll

This adds a ScrollBehaviorTest class and tests using this class.

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

[ajuma, 2014-07-08 21:44:19]

PTAL. If the general approach looks good, I'll add more tests (for subframe and
overflow scrolls).

[Ian Vollick, 2014-07-09 14:55:43]

I really like this direction! I just have one aesthetic nit.

(Also, I assume you'll need to add entries to TestExpectations for these new
tests?)

https://codereview.chromium.org/378953002/diff/1/LayoutTests/fast/scroll-beha...
File LayoutTests/fast/scroll-behavior/main-frame-scrollBy.html (right):

https://codereview.chromium.org/378953002/diff/1/LayoutTests/fast/scroll-beha...
LayoutTests/fast/scroll-behavior/main-frame-scrollBy.html:27: }
nit: I think the OO here is OK, but I wonder if it'd be easier to read and
extend if you got your customization by passing callbacks to
ScrollBehaviorTest's ctor rather than by subclassing the test.

In particular if you gave it a cssScroll, jsScroll, and getStartPosition
callback, I think you could get away without touching prototypes in the tests
that consume scroll-behavior-test.js.

[ajuma, 2014-07-09 20:27:55]

PTAL. I've added tests for overflow and subframe scrolls.

On 2014/07/09 14:55:43, Ian Vollick wrote:
> (Also, I assume you'll need to add entries to TestExpectations for these new
> tests?)

No, see the *expected.txt files, which have "FAIL" lines for the individual test
cases that fail (note that all the test cases where the expected result is an
instant scroll already pass).

https://codereview.chromium.org/378953002/diff/1/LayoutTests/fast/scroll-beha...
File LayoutTests/fast/scroll-behavior/main-frame-scrollBy.html (right):

https://codereview.chromium.org/378953002/diff/1/LayoutTests/fast/scroll-beha...
LayoutTests/fast/scroll-behavior/main-frame-scrollBy.html:27: }
On 2014/07/09 14:55:43, Ian Vollick wrote:
> nit: I think the OO here is OK, but I wonder if it'd be easier to read and
> extend if you got your customization by passing callbacks to
> ScrollBehaviorTest's ctor rather than by subclassing the test.
> 
> In particular if you gave it a cssScroll, jsScroll, and getStartPosition
> callback, I think you could get away without touching prototypes in the tests
> that consume scroll-behavior-test.js.

Added jsScroll and getEndPosition callbacks. (We don't need a cssScroll
callback, since the code for setting the scroll-behavior CSS property is the
same in all tests.)

[Ian Vollick, 2014-07-10 03:32:42]

lgtm.

> > (Also, I assume you'll need to add entries to TestExpectations for these new
> > tests?)

> No, see the *expected.txt files, which have "FAIL" lines for the individual
test
> cases that fail (note that all the test cases where the expected result is an
> instant scroll already pass).

Whoops, of course. Sorry for the noise.

https://codereview.chromium.org/378953002/diff/1/LayoutTests/fast/scroll-beha...
File LayoutTests/fast/scroll-behavior/main-frame-scrollBy.html (right):

https://codereview.chromium.org/378953002/diff/1/LayoutTests/fast/scroll-beha...
LayoutTests/fast/scroll-behavior/main-frame-scrollBy.html:27: }
On 2014/07/09 20:27:55, ajuma wrote:
> On 2014/07/09 14:55:43, Ian Vollick wrote:
> > nit: I think the OO here is OK, but I wonder if it'd be easier to read and
> > extend if you got your customization by passing callbacks to
> > ScrollBehaviorTest's ctor rather than by subclassing the test.
> > 
> > In particular if you gave it a cssScroll, jsScroll, and getStartPosition
> > callback, I think you could get away without touching prototypes in the
tests
> > that consume scroll-behavior-test.js.
> 
> Added jsScroll and getEndPosition callbacks. (We don't need a cssScroll
> callback, since the code for setting the scroll-behavior CSS property is the
> same in all tests.)
Thanks for doing that. I think it's a little prettier.

[Ian Vollick, 2014-07-10 03:33:02]

The CQ bit was checked by vollick@chromium.org

[commit-bot: I haz the power, 2014-07-10 03:33:36]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/ajuma@chromium.org/378953002/80001

[commit-bot: I haz the power, 2014-07-10 04:22:43]

Change committed as 177797