Scrolling allowed when overflow:hidden (seen on Acid2)

Scrolling allowed when overflow:hidden (seen on Acid2)

Autoscroll, as well as other user-driven scroll actions,
has to respect the scrollability styled into the web page.
More specifically, if a html or body tags are styled with
overflow:hidden, autoscroll should not scroll the containing document.

In order to fix this, patch hardens RenderBox::canAutoscroll as
following: previously, ::canAutoscroll was relying only on
::canBeScrolledAndHasScrollableArea to determine the scrollability
of #document node, which was unconditionally returned as 'true'.
Patch extends ::canAutoscroll to handle the #document case for
main and inner frames, and now it asks through ::isScrollable if
the corresponding document's FrameView is actually user-scrollable.

Note, that the patch changes ::canAutoscroll to cover the non-mainFrame
case now.

Autoscroll'ing the mainframe's document is hard with the current
EventSender machinary. Because of that, patch adds an iframe's document test case.
Test added in fast/events/autoscroll-in-overflow-hidden-html.html

Corresponding WebKit commit: <http://trac.webkit.org/changeset/154722>;

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

[Julien - ping for review, 2013-09-04 20:50:36]

The code change is fine, just concerned about the test.

https://codereview.chromium.org/23450017/diff/1/LayoutTests/fast/events/autos...
File LayoutTests/fast/events/autoscroll-in-overflow-hidden-html-expected.txt
(right):

https://codereview.chromium.org/23450017/diff/1/LayoutTests/fast/events/autos...
LayoutTests/fast/events/autoscroll-in-overflow-hidden-html-expected.txt:1: 
Description of what is tested?

https://codereview.chromium.org/23450017/diff/1/LayoutTests/fast/events/autos...
File LayoutTests/fast/events/autoscroll-in-overflow-hidden-html.html (right):

https://codereview.chromium.org/23450017/diff/1/LayoutTests/fast/events/autos...
LayoutTests/fast/events/autoscroll-in-overflow-hidden-html.html:54:
setTimeout(autoscrollTestPart2, 100);
100ms is a lot of time to wait in a test :(

Couldn't EventSender.leapForward(100) would do what you need?

As a rule setTimeout *should* be avoided in tests unless there is a definite
need for them.

https://codereview.chromium.org/23450017/diff/1/LayoutTests/fast/events/resou...
File
LayoutTests/fast/events/resources/big-page-with-overflow-hidden-and-anchor-scroll.html
(right):

https://codereview.chromium.org/23450017/diff/1/LayoutTests/fast/events/resou...
LayoutTests/fast/events/resources/big-page-with-overflow-hidden-and-anchor-scroll.html:12:
<span> click and hold and drag mouse up to see if it scroll in Safari.</div>
"scroll in Safari"? :)

https://codereview.chromium.org/23450017/diff/1/Source/core/rendering/RenderB...
File Source/core/rendering/RenderBox.cpp (right):

https://codereview.chromium.org/23450017/diff/1/Source/core/rendering/RenderB...
Source/core/rendering/RenderBox.cpp:763: return false;
How about just

return canBeScrolledAndHasScrollableArea();

instead of the convoluted logic?

[Julien - ping for review, 2013-09-04 22:06:59]

lgtm, we should investigate how to remove those stupid 100ms sleep into the
auto-scroll tests.

https://codereview.chromium.org/23450017/diff/1/LayoutTests/fast/events/autos...
File LayoutTests/fast/events/autoscroll-in-overflow-hidden-html.html (right):

https://codereview.chromium.org/23450017/diff/1/LayoutTests/fast/events/autos...
LayoutTests/fast/events/autoscroll-in-overflow-hidden-html.html:54:
setTimeout(autoscrollTestPart2, 100);
On 2013/09/04 20:50:36, Julien Chaffraix wrote:
> 100ms is a lot of time to wait in a test :(
> 
> Couldn't EventSender.leapForward(100) would do what you need?
> 
> As a rule setTimeout *should* be avoided in tests unless there is a definite
> need for them.

We talked about this with Antonio and it seems that this is an underlying issue
with auto-scrolling code firing off a timer. leapForward won't help with that.

[commit-bot: I haz the power, 2013-09-06 14:09:03]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/a1.gomes@sisa.samsung.com/23450017/9001

[commit-bot: I haz the power, 2013-09-06 15:30:26]

Change committed as 157371