Merged FrameView and LayoutBox scrolling in EventHandler.

Merged FrameView and LayoutBox scrolling in EventHandler.

Scrolling a FrameView (window and iframes) has some small differences from
scrolling a normal LayoutBox. Because of this, we've generally forked all our
scrolling code to take two different paths based on whether a LayoutBox is a
FrameView/LayoutView. This has caused pain in that much logic has been
duplicated between the two.

This patch removes the duplication in the calling code and makes the decision
based on the virtual LayoutBox::scroll function.

BUG=591124
Committed: https://crrev.com/fd7ede807eb0e64820c805e8d5d0caf7ba9c5516
Cr-Commit-Position: refs/heads/master@{#380403}

[bokan, 2016-03-01 19:49:24]

bokan@chromium.org changed reviewers:
+ tdresser@chromium.org

[bokan, 2016-03-01 19:49:25]

In case you're not sick of these yet :P

https://codereview.chromium.org/1752043002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/input/EventHandler.cpp (right):

https://codereview.chromium.org/1752043002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/input/EventHandler.cpp:1821: ASSERT((granularity
== ScrollByPixel || granularity == ScrollByPrecisePixel)
Alternatively, we can just remove this ASSERT and wait for any bugs that might
be lurking.

[tdresser, 2016-03-01 21:57:40]

https://codereview.chromium.org/1752043002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/input/EventHandler.cpp (left):

https://codereview.chromium.org/1752043002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/input/EventHandler.cpp:1785: deltaY = deltaY > 0
? 1 : -1;
I think this logic is so that we never coalesce page wheel scrolls into
scrolling multiple pages. I think this may make sense, though it only applies
when you have page based scrolling on in Windows.

https://codereview.chromium.org/1752043002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/input/EventHandler.cpp (right):

https://codereview.chromium.org/1752043002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/input/EventHandler.cpp:603: ScrollResult
EventHandler::physicalScroll(ScrollGranularity granularity, const FloatSize&
delta, Node* startNode, Node** stopNode, bool& consumed)
Would it be better to include this as part of the ScrollResult?

(Question, not suggestion)

https://codereview.chromium.org/1752043002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/input/EventHandler.cpp:1821: ASSERT((granularity
== ScrollByPixel || granularity == ScrollByPrecisePixel)
On 2016/03/01 19:49:25, bokan wrote:
> Alternatively, we can just remove this ASSERT and wait for any bugs that might
> be lurking.

ASSERT seems reasonable to me.

https://codereview.chromium.org/1752043002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/input/EventHandler.cpp:1821: ASSERT((granularity
== ScrollByPixel || granularity == ScrollByPrecisePixel)
This should check if the granularity is page based, I think.

[bokan, 2016-03-01 22:09:39]

https://codereview.chromium.org/1752043002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/input/EventHandler.cpp (left):

https://codereview.chromium.org/1752043002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/input/EventHandler.cpp:1785: deltaY = deltaY > 0
? 1 : -1;
On 2016/03/01 21:57:40, tdresser wrote:
> I think this logic is so that we never coalesce page wheel scrolls into
> scrolling multiple pages. I think this may make sense, though it only applies
> when you have page based scrolling on in Windows.

Ah, could we ensure this is enforced at coalescing instead or do we honour a
non-(1|0) delta in other places?

https://codereview.chromium.org/1752043002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/input/EventHandler.cpp (right):

https://codereview.chromium.org/1752043002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/input/EventHandler.cpp:603: ScrollResult
EventHandler::physicalScroll(ScrollGranularity granularity, const FloatSize&
delta, Node* startNode, Node** stopNode, bool& consumed)
On 2016/03/01 21:57:40, tdresser wrote:
> Would it be better to include this as part of the ScrollResult?
> 
> (Question, not suggestion)

I tried that at first but it got ambiguous in a number of places. As far as I
can tell, this is the only place that makes the distinction between scrolling
and consumption so I think it makes sense to leave it separate for now. We can
reevaluate later if it makes sense in other places. At the least, it would take
a separate patch where we adjust all the userScroll call sites to check for
consumed instead.

https://codereview.chromium.org/1752043002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/input/EventHandler.cpp:1821: ASSERT((granularity
== ScrollByPixel || granularity == ScrollByPrecisePixel)
On 2016/03/01 21:57:40, tdresser wrote:
> On 2016/03/01 19:49:25, bokan wrote:
> > Alternatively, we can just remove this ASSERT and wait for any bugs that
might
> > be lurking.
> 
> ASSERT seems reasonable to me.

Acknowledged.

https://codereview.chromium.org/1752043002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/input/EventHandler.cpp:1821: ASSERT((granularity
== ScrollByPixel || granularity == ScrollByPrecisePixel)
On 2016/03/01 21:57:40, tdresser wrote:
> This should check if the granularity is page based, I think.

Right now, this check is making sure that only Pixel events have a delta that's
not 1 or 0. Do we want ScrollByLine to behave the same way, and if so, why treat
ScrollByLine and ScrollByPage differently?

My personal preference would be to honour whatever delta/granularity combo makes
it into EventHandler and ensure any kind of rules like this are enforced when we
generate the event. WDYT?

[tdresser, 2016-03-02 14:27:21]

https://codereview.chromium.org/1752043002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/input/EventHandler.cpp (left):

https://codereview.chromium.org/1752043002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/input/EventHandler.cpp:1785: deltaY = deltaY > 0
? 1 : -1;
On 2016/03/01 22:09:39, bokan wrote:
> On 2016/03/01 21:57:40, tdresser wrote:
> > I think this logic is so that we never coalesce page wheel scrolls into
> > scrolling multiple pages. I think this may make sense, though it only
applies
> > when you have page based scrolling on in Windows.
> 
> Ah, could we ensure this is enforced at coalescing instead or do we honour a
> non-(1|0) delta in other places? 

Moving this to
https://code.google.com/p/chromium/codesearch#chromium/src/content/common/inp...
makes sense to me.

https://codereview.chromium.org/1752043002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/input/EventHandler.cpp (right):

https://codereview.chromium.org/1752043002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/input/EventHandler.cpp:603: ScrollResult
EventHandler::physicalScroll(ScrollGranularity granularity, const FloatSize&
delta, Node* startNode, Node** stopNode, bool& consumed)
On 2016/03/01 22:09:39, bokan wrote:
> On 2016/03/01 21:57:40, tdresser wrote:
> > Would it be better to include this as part of the ScrollResult?
> > 
> > (Question, not suggestion)
> 
> I tried that at first but it got ambiguous in a number of places. As far as I
> can tell, this is the only place that makes the distinction between scrolling
> and consumption so I think it makes sense to leave it separate for now. We can
> reevaluate later if it makes sense in other places. At the least, it would
take
> a separate patch where we adjust all the userScroll call sites to check for
> consumed instead.

Acknowledged.

https://codereview.chromium.org/1752043002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/input/EventHandler.cpp:1821: ASSERT((granularity
== ScrollByPixel || granularity == ScrollByPrecisePixel)
On 2016/03/01 22:09:39, bokan wrote:
> On 2016/03/01 21:57:40, tdresser wrote:
> > This should check if the granularity is page based, I think.
> 
> Right now, this check is making sure that only Pixel events have a delta
that's
> not 1 or 0. Do we want ScrollByLine to behave the same way, and if so, why
treat
> ScrollByLine and ScrollByPage differently?
> 
> My personal preference would be to honour whatever delta/granularity combo
makes
> it into EventHandler and ensure any kind of rules like this are enforced when
we
> generate the event. WDYT?

Oops, the assert should fire if we coalesce page scrolls.

I think we've just treated scrollByPage differently historically.

You moved this from ScrollAnimator here:
https://codereview.chromium.org/1298973004/diff/100001/Source/core/input/Even...

Before that it was moved by apple folks in 2011, so I think the original reasons
for this are lost now...

I can imagine that a user rapidly wheel scrolling by line wants to scroll main
lines, but a user rapidly wheel scrolling by page would just want to advance to
the next page?

Hard to say...

I'd be fine switching this to scroll one page per unit of delta, but if we're
going to do that, let's land it in a separate patch.

[szager1, 2016-03-02 19:14:18]

Description was changed from

==========
Merged FrameView and LayoutBox scrolling in EventHandler.

Scrolling a FrameView (window and iframes) has some small differences from
scrolling a normal LayoutBox. Because of this, we've generally forked all our
scrolling code to take two different paths based on whether a LayoutBox is a
FrameView/LayoutView. This has caused pain in that much logic has been
duplicated between the two.

This patch removes the duplication in the calling code and makes the decision
based on the virtual LayoutBox::scroll function.

BUG=591124
==========

to

==========
Merged FrameView and LayoutBox scrolling in EventHandler.

Scrolling a FrameView (window and iframes) has some small differences from
scrolling a normal LayoutBox. Because of this, we've generally forked all our
scrolling code to take two different paths based on whether a LayoutBox is a
FrameView/LayoutView. This has caused pain in that much logic has been
duplicated between the two.

This patch removes the duplication in the calling code and makes the decision
based on the virtual LayoutBox::scroll function.

BUG=591124
==========

[szager1, 2016-03-02 19:14:34]

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

[szager1, 2016-03-02 19:14:35]

Minor drive-by...

https://codereview.chromium.org/1752043002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/input/EventHandler.cpp (right):

https://codereview.chromium.org/1752043002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/input/EventHandler.cpp:633: curBox =
curBox->isLayoutView() ? nullptr : curBox->containingBlock();
Is is necessary to special-case LayoutView here?

https://codereview.chromium.org/1752043002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/input/EventHandler.cpp:666: curBox =
curBox->isLayoutView() ? nullptr : curBox->containingBlock();
ditto

https://codereview.chromium.org/1752043002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/layout/LayoutView.cpp (right):

https://codereview.chromium.org/1752043002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/layout/LayoutView.cpp:1026: {
I think you should probably add this at the top of this method (skobes@ should
confirm):

if (document().settings() && document().settings()->rootLayerScrolls())
    return LayoutBlockFlow::scroll(granularity, delta);

[bokan, 2016-03-02 20:37:13]

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

[bokan, 2016-03-02 20:37:14]

Had to fix up how I do wheel scroll chaining after:
https://codereview.chromium.org/1731353003/

+skobes@ to take a look at suggestion in LayoutView

https://codereview.chromium.org/1752043002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/input/EventHandler.cpp (right):

https://codereview.chromium.org/1752043002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/input/EventHandler.cpp:633: curBox =
curBox->isLayoutView() ? nullptr : curBox->containingBlock();
On 2016/03/02 19:14:34, szager1 wrote:
> Is is necessary to special-case LayoutView here?

Ah, I thought it was since I assumed containingBlock on an iframe's LayoutView
would return the owner LayoutObject in the parent document but I just checked
this and that's false. Thanks!

https://codereview.chromium.org/1752043002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/input/EventHandler.cpp:666: curBox =
curBox->isLayoutView() ? nullptr : curBox->containingBlock();
On 2016/03/02 19:14:34, szager1 wrote:
> ditto

Acknowledged.

https://codereview.chromium.org/1752043002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/input/EventHandler.cpp:1821: ASSERT((granularity
== ScrollByPixel || granularity == ScrollByPrecisePixel)
On 2016/03/02 14:27:20, tdresser wrote:
> On 2016/03/01 22:09:39, bokan wrote:
> > On 2016/03/01 21:57:40, tdresser wrote:
> > > This should check if the granularity is page based, I think.
> > 
> > Right now, this check is making sure that only Pixel events have a delta
> that's
> > not 1 or 0. Do we want ScrollByLine to behave the same way, and if so, why
> treat
> > ScrollByLine and ScrollByPage differently?
> > 
> > My personal preference would be to honour whatever delta/granularity combo
> makes
> > it into EventHandler and ensure any kind of rules like this are enforced
when
> we
> > generate the event. WDYT?
> 
> Oops, the assert should fire if we coalesce page scrolls.
> 
> I think we've just treated scrollByPage differently historically.
> 
> You moved this from ScrollAnimator here:
>
https://codereview.chromium.org/1298973004/diff/100001/Source/core/input/Even...
> 
> Before that it was moved by apple folks in 2011, so I think the original
reasons
> for this are lost now...
> 
> I can imagine that a user rapidly wheel scrolling by line wants to scroll main
> lines, but a user rapidly wheel scrolling by page would just want to advance
to
> the next page?
> 
> Hard to say...
> 
> I'd be fine switching this to scroll one page per unit of delta, but if we're
> going to do that, let's land it in a separate patch.

Ok, I've removed the ASSERT here and reinstated the clamp to 1/-1/0. I'll try
moving it up to happen during coalescing in a future patch.

https://codereview.chromium.org/1752043002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/layout/LayoutView.cpp (right):

https://codereview.chromium.org/1752043002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/layout/LayoutView.cpp:1026: {
On 2016/03/02 19:14:35, szager1 wrote:
> I think you should probably add this at the top of this method (skobes@ should
> confirm):
> 
> if (document().settings() && document().settings()->rootLayerScrolls())
>     return LayoutBlockFlow::scroll(granularity, delta);

Thanks for the heads up. This shouldn't change the flow from what we do today -
we're just calling applyScrollDelta from here rather than EventHandler, but I
can believe it's wrong today. I'll let skobes@ comment.

[skobes, 2016-03-02 23:26:30]

https://codereview.chromium.org/1752043002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/layout/LayoutView.cpp (right):

https://codereview.chromium.org/1752043002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/layout/LayoutView.cpp:1026: {
On 2016/03/02 20:37:13, bokan wrote:
> On 2016/03/02 19:14:35, szager1 wrote:
> > I think you should probably add this at the top of this method (skobes@
should
> > confirm):
> > 
> > if (document().settings() && document().settings()->rootLayerScrolls())
> >     return LayoutBlockFlow::scroll(granularity, delta);
> 
> Thanks for the heads up. This shouldn't change the flow from what we do today
-
> we're just calling applyScrollDelta from here rather than EventHandler, but I
> can believe it's wrong today. I'll let skobes@ comment.

LocalFrame::applyScrollDelta calls through FrameView::scrollableArea() so it
will do the right thing in root layer scrolling mode.

That said, perhaps this overload can go away with scroll customization, given
that we already have logic in Element::nativeApplyScroll to call
LocalFrame::applyScrollDelta?

https://codereview.chromium.org/1752043002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/layout/LayoutView.h (right):

https://codereview.chromium.org/1752043002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/layout/LayoutView.h:213: ScrollResult
scroll(ScrollGranularity, const FloatSize&) override;
Maybe worth a clarifying comment here to the effect that the rootLayerScrolls
setting determines which ScrollableArea ultimately gets scrolled by this method.

[bokan, 2016-03-02 23:33:19]

https://codereview.chromium.org/1752043002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/layout/LayoutView.cpp (right):

https://codereview.chromium.org/1752043002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/layout/LayoutView.cpp:1026: {
On 2016/03/02 23:26:30, skobes wrote:
> On 2016/03/02 20:37:13, bokan wrote:
> > On 2016/03/02 19:14:35, szager1 wrote:
> > > I think you should probably add this at the top of this method (skobes@
> should
> > > confirm):
> > > 
> > > if (document().settings() && document().settings()->rootLayerScrolls())
> > >     return LayoutBlockFlow::scroll(granularity, delta);
> > 
> > Thanks for the heads up. This shouldn't change the flow from what we do
today
> -
> > we're just calling applyScrollDelta from here rather than EventHandler, but
I
> > can believe it's wrong today. I'll let skobes@ comment.
> 
> LocalFrame::applyScrollDelta calls through FrameView::scrollableArea() so it
> will do the right thing in root layer scrolling mode.
> 
> That said, perhaps this overload can go away with scroll customization, given
> that we already have logic in Element::nativeApplyScroll to call
> LocalFrame::applyScrollDelta?

I think you're right. Right now, I'm just trying to remove some of the crazy
duplication we've got going on in EventHandler. Though I like the symmetry of
calling code not having to make a distinction between LayoutBoxes.

https://codereview.chromium.org/1752043002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/layout/LayoutView.h (right):

https://codereview.chromium.org/1752043002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/layout/LayoutView.h:213: ScrollResult
scroll(ScrollGranularity, const FloatSize&) override;
On 2016/03/02 23:26:30, skobes wrote:
> Maybe worth a clarifying comment here to the effect that the rootLayerScrolls
> setting determines which ScrollableArea ultimately gets scrolled by this
method.

Done.

[skobes, 2016-03-02 23:36:17]

lgtm

[bokan, 2016-03-02 23:37:19]

Tim, ptal, I'll await your l-g-t-m as well.

[tdresser, 2016-03-03 15:28:27]

LGTM with nit.

https://codereview.chromium.org/1752043002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/input/EventHandler.h (right):

https://codereview.chromium.org/1752043002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/input/EventHandler.h:282: ScrollResult
physicalScroll(ScrollGranularity, const FloatSize& delta, Node* startNode,
Node** stopNode, bool& consumed);
Output parameters must be pointers, according to the style guide.

[bokan, 2016-03-03 19:53:26]

https://codereview.chromium.org/1752043002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/input/EventHandler.h (right):

https://codereview.chromium.org/1752043002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/input/EventHandler.h:282: ScrollResult
physicalScroll(ScrollGranularity, const FloatSize& delta, Node* startNode,
Node** stopNode, bool& consumed);
On 2016/03/03 15:28:27, tdresser wrote:
> Output parameters must be pointers, according to the style guide.

Done.

[bokan, 2016-03-03 19:53:59]

The CQ bit was checked by bokan@chromium.org

[bokan, 2016-03-03 19:54:00]

The patchset sent to the CQ was uploaded after l-g-t-m from skobes@chromium.org,
tdresser@chromium.org
Link to the patchset: https://codereview.chromium.org/1752043002/#ps80001
(title: "Made |consumed| a pointer")

[commit-bot: I haz the power, 2016-03-03 19:54:41]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1752043002/80001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1752043002/80001

[commit-bot: I haz the power, 2016-03-03 21:36:43]

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

[commit-bot: I haz the power, 2016-03-03 21:36:44]

Try jobs failed on following builders:
  mac_chromium_rel_ng on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[bokan, 2016-03-09 22:32:09]

The CQ bit was checked by bokan@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-03-09 22:32:38]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1752043002/120001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1752043002/120001

[commit-bot: I haz the power, 2016-03-10 02:41:46]

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

[commit-bot: I haz the power, 2016-03-10 02:41:52]

Dry run: Try jobs failed on following builders:
  win_chromium_rel_ng on tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[bokan, 2016-03-10 13:45:13]

I had to update some adjustments to and because of layout tests for this change.

The platform-wheelevent-paging tests change because they were assuming that page
granularity events on DIVs would scroll deltaX * page but Frames would ignore
deltaX.

The touch-gesture tests were sending out-of-bounds touches which used to just
fallback to the FrameView. We now actually generate a target Node even for the
FrameView so I adjusted the touch coords.

Logical scrolls should fall back to scrolling the Frame. That is, if the user
hasn't clicked anything, nothing's focused, and the user hits the "End" key, we
should scroll the Frame.

I had a typo in the scroll customization path.

tdresser@: still lgtm?

[tdresser, 2016-03-10 13:53:07]

Still LGTM.

https://codereview.chromium.org/1752043002/diff/120001/third_party/WebKit/Lay...
File
third_party/WebKit/LayoutTests/fast/events/platform-wheelevent-paging-y-in-scrolling-div.html
(right):

https://codereview.chromium.org/1752043002/diff/120001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/fast/events/platform-wheelevent-paging-y-in-scrolling-div.html:6:
var givenScrollTop = 2; // When paging, this is ignored. Every event is one
page.
The comment below is still relevant - justifying the 161.

[bokan, 2016-03-10 13:54:12]

https://codereview.chromium.org/1752043002/diff/120001/third_party/WebKit/Lay...
File
third_party/WebKit/LayoutTests/fast/events/platform-wheelevent-paging-y-in-scrolling-div.html
(right):

https://codereview.chromium.org/1752043002/diff/120001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/fast/events/platform-wheelevent-paging-y-in-scrolling-div.html:6:
var givenScrollTop = 2; // When paging, this is ignored. Every event is one
page.
On 2016/03/10 13:53:07, tdresser wrote:
> The comment below is still relevant - justifying the 161.

Acknowledged.

[bokan, 2016-03-10 13:54:20]

The CQ bit was checked by bokan@chromium.org

[bokan, 2016-03-10 13:54:20]

The patchset sent to the CQ was uploaded after l-g-t-m from skobes@chromium.org
Link to the patchset: https://codereview.chromium.org/1752043002/#ps120001
(title: "Fixes for layout tests breaks")

[commit-bot: I haz the power, 2016-03-10 13:54:35]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1752043002/120001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1752043002/120001

[commit-bot: I haz the power, 2016-03-10 14:54:44]

Description was changed from

==========
Merged FrameView and LayoutBox scrolling in EventHandler.

Scrolling a FrameView (window and iframes) has some small differences from
scrolling a normal LayoutBox. Because of this, we've generally forked all our
scrolling code to take two different paths based on whether a LayoutBox is a
FrameView/LayoutView. This has caused pain in that much logic has been
duplicated between the two.

This patch removes the duplication in the calling code and makes the decision
based on the virtual LayoutBox::scroll function.

BUG=591124
==========

to

==========
Merged FrameView and LayoutBox scrolling in EventHandler.

Scrolling a FrameView (window and iframes) has some small differences from
scrolling a normal LayoutBox. Because of this, we've generally forked all our
scrolling code to take two different paths based on whether a LayoutBox is a
FrameView/LayoutView. This has caused pain in that much logic has been
duplicated between the two.

This patch removes the duplication in the calling code and makes the decision
based on the virtual LayoutBox::scroll function.

BUG=591124
==========

[commit-bot: I haz the power, 2016-03-10 14:54:45]

Committed patchset #7 (id:120001)

[commit-bot: I haz the power, 2016-03-10 14:55:53]

Description was changed from

==========
Merged FrameView and LayoutBox scrolling in EventHandler.

Scrolling a FrameView (window and iframes) has some small differences from
scrolling a normal LayoutBox. Because of this, we've generally forked all our
scrolling code to take two different paths based on whether a LayoutBox is a
FrameView/LayoutView. This has caused pain in that much logic has been
duplicated between the two.

This patch removes the duplication in the calling code and makes the decision
based on the virtual LayoutBox::scroll function.

BUG=591124
==========

to

==========
Merged FrameView and LayoutBox scrolling in EventHandler.

Scrolling a FrameView (window and iframes) has some small differences from
scrolling a normal LayoutBox. Because of this, we've generally forked all our
scrolling code to take two different paths based on whether a LayoutBox is a
FrameView/LayoutView. This has caused pain in that much logic has been
duplicated between the two.

This patch removes the duplication in the calling code and makes the decision
based on the virtual LayoutBox::scroll function.

BUG=591124

Committed: https://crrev.com/fd7ede807eb0e64820c805e8d5d0caf7ba9c5516
Cr-Commit-Position: refs/heads/master@{#380403}
==========

[commit-bot: I haz the power, 2016-03-10 14:55:54]

Patchset 7 (id:??) landed as
https://crrev.com/fd7ede807eb0e64820c805e8d5d0caf7ba9c5516
Cr-Commit-Position: refs/heads/master@{#380403}