Bug 69865 – Crash in ScrollAnimator.cpp

• Status: RESOLVED
• Resolution: FIXED
• Component: UI Events

[Stephen Chenney]

Line 110 of ScrollAnimator.cpp is crashing - 11 reports so far for Chrome.

Evidence suggests that this assertion:

        if (e.granularity() == ScrollByPageWheelEvent) {
            ASSERT(!e.deltaX());

would fail in the crash situations, because it implicitly verifies that verticalScrollbar is non-null. The root cause of the problem seems to be the specification of ScrollbyPageWheelEvent granularity for a horizontal scrollbar.

[Stephen Chenney]

I have tracked the issue down to this comment and related code in the chromium webkit port:

        // Synthesize mousewheel event from a scroll event.  This is needed to
        // simulate middle mouse scrolling in some laptops.  Use GetAsyncKeyState
        // for key state since we are synthesizing the input event.

The code related to this synthetic event is capable of generating a horizontal page mouse wheel event that violates the existing assumptions in ScrollAnimator.

The devices in question are laptops with TrackPoint interfaces, where there is a mode that enables the pointer to produce whole page horizontal mouse wheel events at the OS level (Windows and Linux).

I believe that the best way to fix this is to add support for horizontal page mouse wheel scrolling in ScrollAnimator.

[Stephen Chenney]

For those mac port folks on the CC list ...

Is there even a way for a mac to generate a paged wheel event? As far as I can tell, CG events can only be per-pixel or per-line wheel scrolling.

On chromium, or windows, this looks to be specific to a small set of devices.

[Stephen Chenney]

Created attachment 112584 [details]
Patch

[Stephen Chenney]

Comment on attachment 112584 [details]
Patch

The new tests are disabled on mac and win (not chromium) because I cannot figure out how to generate a paging wheel event for those platforms. It seems that CoreGraphics does not support such events. Correct me if I'm wrong.

Chromium mac results are different because it uses a different scrollbar theme that sets a min overlap on paging.

[Martin Robinson]

Comment on attachment 112584 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=112584&action=review

> LayoutTests/fast/events/platform-wheelevent-paging-xy-in-scrolling-div.html:58
> +         <!-- This div is 200 pixels high. The content results in scrool bars on
> +          both edges, resulting in an effective content area of 185 x 185 on
> +          linux. The paging context overlap is 24 pixels. So one page of scroll
> +          moves the content by 185 - 24 -= 161 pixels. -->

Does this mean the tests rely on scrollbar width? Scrollbar sizes can vary from platform to platform.

[Stephen Chenney]

Yes, this test depends on scrollbar width. AFAIK you can't suppress the scrollbars while still enabling scrolling, but please let me know if that is not correct. As far as I know, we also try very hard to have scrollbar width match across platforms during layout tests so that both text and pixel expectations match across platforms.

The test also depends on the scrollbar theme and other non-constants such as the percentage of a page scrolled per paging event and the minimum percentage to scroll per page. For example, the chromium mac port produces different scroll behavior. The solution I have for this is to have the expectations set to expect a FAIL for one of the shouldBe tests. This seems sufficient because the expectations still include the value that the test actually produced and hence will catch cases where the behavior changes.

[WebKit Review Bot]

Comment on attachment 112584 [details]
Patch

Rejecting attachment 112584 [details] from commit-queue.

Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 2

Last 500 characters of output:
form-wheelevent-paging-y-in-scrolling-page-expected.txt
patching file LayoutTests/platform/mac/Skipped
Hunk #1 FAILED at 450.
1 out of 1 hunk FAILED -- saving rejects to file LayoutTests/platform/mac/Skipped.rej
patching file LayoutTests/platform/win/Skipped
Hunk #1 FAILED at 1428.
1 out of 1 hunk FAILED -- saving rejects to file LayoutTests/platform/win/Skipped.rej

Failed to run "[u'/mnt/git/webkit-commit-queue/Tools/Scripts/svn-apply', u'--reviewer', u'Anders Carlsson', u'--..." exit_code: 1

Full output: http://queues.webkit.org/results/10333116

[Stephen Chenney]

Created attachment 113731 [details]
Patch

[WebKit Review Bot]

Comment on attachment 113731 [details]
Patch

Clearing flags on attachment: 113731

Committed r99339: <http://trac.webkit.org/changeset/99339>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.

[Tony Chang]

A few of these tests are failing on the Chromium CG bots:

http://test-results.appspot.com/dashboards/flakiness_dashboard.html#tests=fast%2Fevents%2Fplatform-wheelevent-paging-x-in-scrolling-page.html%2Cfast%2Fevents%2Fplatform-wheelevent-paging-y-in-scrolling-page.html%2Cfast%2Fevents%2Fplatform-wheelevent-paging-xy-in-scrolling-page.html

[Stephen Chenney]

The chromium-mac expectations need to be copied into the chromium-cg-mac expectations. Filed https://bugs.webkit.org/show_bug.cgi?id=71606 to track.