Issues noticed with wheel event handling code when Smooth Scroll enabled

[kphanee@chromium]

Hi,

During debugging input_handler_proxy.cc today for some issue, I noticed couple of issues in the smooth scroll code path. As I am not much aware of smooth scroll design, not sure whether these are genuine issues or code impl is still under progress and these will be fixed in future changes.

Issue# 1)

I accidentally came across the code inside HandleMouseWheel which I feel is a simply overlooked coding error [1] but might be wrong.

  } else if (smooth_scroll_enabled_) {

cc::InputHandler::ScrollStatus scroll_status = input_handler_->ScrollAnimated(gfx::Point(wheel_event.x, wheel_event.y), scroll_delta);

switch (scroll_status) {

      case cc::InputHandler::SCROLL_STARTED:

        result = DID_HANDLE;

        break;

      case cc::InputHandler::SCROLL_IGNORED:

        result = DROP_EVENT; (break keyword missing for this switch case)

      default:

        result = DID_NOT_HANDLE;

        break;

    }

  } else {

Simple code walk through indicates that the call to InputScrollElasticityController::ObserveWheelEventAndResult() [2] might get blocked for SCROLL_IGNORED status as missing break statement changes the result value changes from DROP_EVENT to DID_NOT_HANDLE. I couldn't debug further as InputScrollElasticityController is defined only for Mac platforms currently. I couldn't find any unit tests also to check scenario. So I am not sure how to test this scenario and find whether this is an issue or not. Any idea whether this is an issue or not? If yes, how can we reproduce it (atleast through unit tests) on Linux platform?

Issue# 2)

When enabled smooth scrolling feature from chrome://flags, wheel events are not handled in simple (mostly text) pages. [3]

For ex: on chrome://flags page, if we perform mouse wheel move events over the page content, page doesn't scroll at all. If we do the same action over the scroll bars of the page, page does scroll. However, if we perform same action on some other regular web pages (for ex. news.google.com) page does scroll properly.

Not sure whether this is a bug in smooth scroll impl or the feature is still under development and in due course this will be handled properly. A quick check pointed me to crbug.com/133097 under which concerned code was added to input_handler_proxy.cc. The bug is marked as Fixed.

If the smooth scroll impl is completed and this is indeed an issue, I will raise a bug accordingly.

[1] https://code.google.com/p/chromium/codesearch#chromium/src/content/renderer/input/input_handler_proxy.cc&l=374

[2] https://code.google.com/p/chromium/codesearch#chromium/src/content/renderer/input/input_handler_proxy.cc&l=416

[3] https://www.ietf.org/rfc/rfc3261.txt

[Tim Dresser]

+Steve Kobes 

Issue 2 should be fixed by https://codereview.chromium.org/1418493003/.

[kphanee@chromium]

Thanks Tim for pointing it. I will sync to ToT.
Any idea about Issue 1?

[Tim Dresser]

I suspect that skobes@ or +Yash Malik would be the right person to address #1.

[Yash Malik]

Without too much background here, I think that issue 1 is in fact a coding error. Other input handlers in input_handler_proxy also return DROP_EVENT in the case where the input handler returns SCROLL_IGNORED. 

skobes@ WDYT?

[Steve Kobes]

Yes, it looks like an error introduced by http://crrev.com/713413002.  I'll send a patch for it.

[Steve Kobes]

This is fixed at http://crrev.com/355496.

[kphanee@chromium]

Cool. Thanks :)