Drag'n drop Autoscroll does not work properly on inner frames

Drag'n drop Autoscroll does not work properly on inner frames

RenderBox::calculateAutoscrollDirection does not properly translate
inner frames' coordinates in order to determine its auto-scrollability.
By coincidence, if the inner frame box it placed near to page's 0, 0 position
(upper left corner), it might work.

Patch fixes it by changing ::calculateAutoscrollDirection algorithm to operate
with window coordinates, taking inner frames offset and scroll position into account.
The behavior of auto-scrollable non-frame overflow boxes, including divs, still works
as it is used to.

Test: fast/events/drag-and-drop-autoscroll-inner-frame.html

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

[Julien - ping for review, 2013-09-09 23:13:14]

lgtm

https://codereview.chromium.org/24074002/diff/1/LayoutTests/fast/events/drag-...
File LayoutTests/fast/events/drag-and-drop-autoscroll-inner-frame-expected.txt
(right):

https://codereview.chromium.org/24074002/diff/1/LayoutTests/fast/events/drag-...
LayoutTests/fast/events/drag-and-drop-autoscroll-inner-frame-expected.txt:9: 
The output is missing the TEST COMPLETE message.

https://codereview.chromium.org/24074002/diff/1/LayoutTests/fast/events/drag-...
File LayoutTests/fast/events/drag-and-drop-autoscroll-inner-frame.html (right):

https://codereview.chromium.org/24074002/diff/1/LayoutTests/fast/events/drag-...
LayoutTests/fast/events/drag-and-drop-autoscroll-inner-frame.html:29: if
(!window.eventSender)
We don't get here if !window.eventSender as setTimeout below is never called.

https://codereview.chromium.org/24074002/diff/1/LayoutTests/fast/events/drag-...
LayoutTests/fast/events/drag-and-drop-autoscroll-inner-frame.html:66: if
(iframe.contentDocument.body.scrollTop > 0) {
Nit: I would flip the logic to do the failing case first as it's shorter and
matches the code more closely (early returns).

[yosin_UTC9, 2013-09-10 02:09:37]

ACK
Thanks for fixing bug!

https://codereview.chromium.org/24074002/diff/1/LayoutTests/fast/events/drag-...
File LayoutTests/fast/events/drag-and-drop-autoscroll-inner-frame.html (right):

https://codereview.chromium.org/24074002/diff/1/LayoutTests/fast/events/drag-...
LayoutTests/fast/events/drag-and-drop-autoscroll-inner-frame.html:75:
log("Passed: Autoscroll should have scrolled the iframe downwards, and did.");
We should use testPassed(), e.g. testPassed('Autoscroll should have scrolled the
iframe downwards, and did.');

https://codereview.chromium.org/24074002/diff/1/LayoutTests/fast/events/drag-...
LayoutTests/fast/events/drag-and-drop-autoscroll-inner-frame.html:79:
log("Failed: Autoscroll should have scrolled the iframe downwards, but did
not.");
We should use testFailed(), e.g. testFailed('Autoscroll should have scrolled the
iframe downwards, but did not');

https://codereview.chromium.org/24074002/diff/1/LayoutTests/fast/events/drag-...
LayoutTests/fast/events/drag-and-drop-autoscroll-inner-frame.html:85: if
(iframe.contentDocument.body.scrollTop < middleTermScrollOffset)
We should use shouldBeTrue(), e.g.
shouldBeTrue('iframe.contentDocument.body.scrollTop < middleTermScrollOffset')

https://codereview.chromium.org/24074002/diff/1/LayoutTests/fast/events/drag-...
LayoutTests/fast/events/drag-and-drop-autoscroll-inner-frame.html:101:
setTimeout(testIt, 0);
We should start test window.onload rather than timeout. On heavy load bot,
timout event is fired before load event and makes tests flaky.

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

https://codereview.chromium.org/24074002/diff/1/Source/core/rendering/RenderB...
Source/core/rendering/RenderBox.cpp:781: IntPoint point = windowPoint;
nit: can we introduce new variable windowAutoscrollPoint, or another to denote
we are on window coordinate?

https://codereview.chromium.org/24074002/diff/1/Source/core/rendering/RenderB...
Source/core/rendering/RenderBox.cpp:784: box =
view()->frameView()->contentsToWindow(box);
nit: can we introduce new variable windowBox to denote we are on window
coordinate?

[commit-bot: I haz the power, 2013-09-10 16:16:53]

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

[commit-bot: I haz the power, 2013-09-10 18:21:37]

Change committed as 157535