New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 615948 link

Starred by 7 users

Issue metadata

Status: Started
Owner:
Last visit > 30 days ago
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Mac
Pri: 2
Type: Bug

Blocked on: View detail
issue 476553
issue 637521

Blocking:
issue 594907
issue 605131


Participants' hotlists:
MacViews-Task-Queue


Sign in to add a comment

[Mac]Views: Elastic scrolling for views::ScrollView (also: smooth scrolling, etc.)

Project Member Reported by tapted@chromium.org, May 31 2016

Issue description

Chrome Version       : 52.0.2739.2
OS Version: OS X 10.11.5

Scroll views on Mac should have "elastic" behaviour. This involves:

 - Hooking up views/ui::FooEvents into the pipeline that LayerTreeHost uses
 - Using cc::LayerTreeHost to do scrolling with layers/layer animations (e.g. for momentum, bounce-back, etc.)
 - Tweaking views::ScrollView to forward events appropriately, and host appropriate layers to do the scrolling
 - Tweaking rendering of the scrollbars to shrink when overscrolling

There's a working prototype in https://codereview.chromium.org/1680613002/

Design doc: https://go/views-elastic-scrolling
 

Comment 1 by tapted@chromium.org, May 31 2016

Blocking: 594907
Labels: Proj-MacViews
Project Member

Comment 3 by bugdroid1@chromium.org, Jun 27 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/16a0ce331a936f7c70425f4b53dedc14b840f43f

commit 16a0ce331a936f7c70425f4b53dedc14b840f43f
Author: tapted <tapted@chromium.org>
Date: Mon Jun 27 01:19:43 2016

Cleanup: decouple web_input_event_aura and ui_events_helper

MakeWebGestureEventFromUIEvent() is only called from
web_input_event_aura.cc. Move it to an anonymous namespace so that
web_input_event_aura.cc is easier to move to a component without having
to drag ui_events_helper.h along with it.

BUG=615948

Review-Url: https://codereview.chromium.org/2097743002
Cr-Commit-Position: refs/heads/master@{#402094}

[modify] https://crrev.com/16a0ce331a936f7c70425f4b53dedc14b840f43f/content/browser/renderer_host/ui_events_helper.cc
[modify] https://crrev.com/16a0ce331a936f7c70425f4b53dedc14b840f43f/content/browser/renderer_host/ui_events_helper.h
[modify] https://crrev.com/16a0ce331a936f7c70425f4b53dedc14b840f43f/content/browser/renderer_host/web_input_event_aura.cc

Project Member

Comment 4 by bugdroid1@chromium.org, Jul 1 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/85282366fed27efe2e19a686b1376ad9eca37d84

commit 85282366fed27efe2e19a686b1376ad9eca37d84
Author: tapted <tapted@chromium.org>
Date: Fri Jul 01 03:41:43 2016

Decouple EventWithLatencyInfo from WebInputEventTraits

Only EventWithLatencyInfo uses [Can]Coalesce() from WebInputEventTraits.
WebInputEventTraits gets used for IPC stuff so is kinda sensitive, but
EventWithLatencyInfo isn't involved with IPC. So, coupling these
together also makes refactoring unnecessarily difficult.

E.g. the coupling means that if we want to move EventWithLatencyInfo to
a component, then WebInputEventTraits and a bunch of dependencies that
EventWithLatencyInfo doesn't care about need to come as well.

To decouple, the guts of [Can]Coalesce() moves to a new .cc file for
EventWithLatencyInfo. The types are known, so we get some added type
safety versus WebInputEventTraits which isn't templatized in the .h. The
instantiations in the .cc become a bit simpler and the runtime type
checks and static casts behind in web_input_event_trait.cc's Apply(..)
are not required.

BUG=615948
TBR=jochen@chromium.org (iwyu)

Review-Url: https://codereview.chromium.org/2111593003
Cr-Commit-Position: refs/heads/master@{#403402}

[modify] https://crrev.com/85282366fed27efe2e19a686b1376ad9eca37d84/content/browser/renderer_host/input/gesture_event_queue.cc
[modify] https://crrev.com/85282366fed27efe2e19a686b1376ad9eca37d84/content/browser/renderer_host/input/input_router_impl.cc
[modify] https://crrev.com/85282366fed27efe2e19a686b1376ad9eca37d84/content/browser/renderer_host/input/mouse_wheel_event_queue.cc
[modify] https://crrev.com/85282366fed27efe2e19a686b1376ad9eca37d84/content/browser/renderer_host/input/render_widget_host_latency_tracker.cc
[modify] https://crrev.com/85282366fed27efe2e19a686b1376ad9eca37d84/content/browser/renderer_host/input/synthetic_gesture_target_base.cc
[modify] https://crrev.com/85282366fed27efe2e19a686b1376ad9eca37d84/content/browser/renderer_host/render_widget_host_unittest.cc
[modify] https://crrev.com/85282366fed27efe2e19a686b1376ad9eca37d84/content/browser/renderer_host/render_widget_host_view_aura_unittest.cc
[modify] https://crrev.com/85282366fed27efe2e19a686b1376ad9eca37d84/content/browser/renderer_host/render_widget_host_view_mac_unittest.mm
[modify] https://crrev.com/85282366fed27efe2e19a686b1376ad9eca37d84/content/browser/web_contents/web_contents_impl.cc
[add] https://crrev.com/85282366fed27efe2e19a686b1376ad9eca37d84/content/common/input/event_with_latency_info.cc
[modify] https://crrev.com/85282366fed27efe2e19a686b1376ad9eca37d84/content/common/input/event_with_latency_info.h
[modify] https://crrev.com/85282366fed27efe2e19a686b1376ad9eca37d84/content/common/input/web_input_event_traits.cc
[modify] https://crrev.com/85282366fed27efe2e19a686b1376ad9eca37d84/content/common/input/web_input_event_traits.h
[modify] https://crrev.com/85282366fed27efe2e19a686b1376ad9eca37d84/content/common/input/web_input_event_traits_unittest.cc
[modify] https://crrev.com/85282366fed27efe2e19a686b1376ad9eca37d84/content/content_common.gypi
[modify] https://crrev.com/85282366fed27efe2e19a686b1376ad9eca37d84/content/renderer/input/input_event_filter_unittest.cc
[modify] https://crrev.com/85282366fed27efe2e19a686b1376ad9eca37d84/content/renderer/input/main_thread_event_queue.h

Project Member

Comment 5 by sheriffbot@chromium.org, Jul 7 2016

Labels: -M-53 M-54 MovedFrom-53
Moving this nonessential bug to the next milestone.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Project Member

Comment 6 by bugdroid1@chromium.org, Aug 4 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/033117f328bb004ae5f466351734f175d07ab5c7

commit 033117f328bb004ae5f466351734f175d07ab5c7
Author: tapted <tapted@chromium.org>
Date: Thu Aug 04 04:35:43 2016

Remove the swizzling of -[NSEvent window] and the middle button in EventsMacTest

The NSEvent initializers for creating test events are limited, so we
currently swizzle some properties. In a follow-up, we also want test
events that include phase information for scrolling. The swizzling for that
was becoming complex, but we can replace the swizzling by making more
use of CGEventSetIntegerValueField(..) and adding a tiny bit of reflection.

BUG=615948
TEST=events_unittests still passing

Review-Url: https://codereview.chromium.org/2209003002
Cr-Commit-Position: refs/heads/master@{#409718}

[modify] https://crrev.com/033117f328bb004ae5f466351734f175d07ab5c7/ui/events/cocoa/events_mac_unittest.mm

Project Member

Comment 7 by bugdroid1@chromium.org, Aug 11 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/315a111b6cdd928cbd0862dd97f5a98482dd0261

commit 315a111b6cdd928cbd0862dd97f5a98482dd0261
Author: tapted <tapted@chromium.org>
Date: Thu Aug 11 01:34:04 2016

Mac: Share kScrollbarPixelsPerCocoaTick between ui:: and blink:: events

Currently scrolling in MacViews with a "real" mouse wheel is too fast. A
multiplier is applied that maps the values used for
kCGScrollEventUnitLine to (effectively) kCGScrollEventUnitPixel, and the
one used for MacViews was too big. Share the constant that content::
uses by moving it to ui/events/cocoa/cocoa_event_utils.h

Add a test to ensure the event types agree. Problem: the initializers on
NSEvent for creating test events are too limited. We also want to beef
up testing of scroll event streams for elastic scrolling and whatnot, so
move the stuff in events_mac_unittest.mm to cocoa_test_event_utils:: for
creating test scroll events.

BUG=615948

Review-Url: https://codereview.chromium.org/2226933004
Cr-Commit-Position: refs/heads/master@{#411216}

[modify] https://crrev.com/315a111b6cdd928cbd0862dd97f5a98482dd0261/content/browser/renderer_host/input/web_input_event_builders_mac.mm
[modify] https://crrev.com/315a111b6cdd928cbd0862dd97f5a98482dd0261/content/browser/renderer_host/input/web_input_event_builders_mac_unittest.mm
[modify] https://crrev.com/315a111b6cdd928cbd0862dd97f5a98482dd0261/ui/events/cocoa/cocoa_event_utils.h
[modify] https://crrev.com/315a111b6cdd928cbd0862dd97f5a98482dd0261/ui/events/cocoa/events_mac.mm
[modify] https://crrev.com/315a111b6cdd928cbd0862dd97f5a98482dd0261/ui/events/cocoa/events_mac_unittest.mm
[modify] https://crrev.com/315a111b6cdd928cbd0862dd97f5a98482dd0261/ui/events/test/cocoa_test_event_utils.h
[modify] https://crrev.com/315a111b6cdd928cbd0862dd97f5a98482dd0261/ui/events/test/cocoa_test_event_utils.mm

Project Member

Comment 8 by bugdroid1@chromium.org, Aug 11 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/941118827f5240dedb40082cffb1ead6c6d621cc

commit 941118827f5240dedb40082cffb1ead6c6d621cc
Author: tapted <tapted@chromium.org>
Date: Thu Aug 11 01:56:00 2016

Revert of Mac: Share kScrollbarPixelsPerCocoaTick between ui:: and blink:: events (patchset #5 id:140001 of https://codereview.chromium.org/2226933004/ )

Reason for revert:
Somehow... this slipped through the trybots -
web_input_event_builders_mac_unittest.mm:586:49:error: too many arguments to function call, expected 2, have 4

Original issue's description:
> Mac: Share kScrollbarPixelsPerCocoaTick between ui:: and blink:: events
>
> Currently scrolling in MacViews with a "real" mouse wheel is too fast. A
> multiplier is applied that maps the values used for
> kCGScrollEventUnitLine to (effectively) kCGScrollEventUnitPixel, and the
> one used for MacViews was too big. Share the constant that content::
> uses by moving it to ui/events/cocoa/cocoa_event_utils.h
>
> Add a test to ensure the event types agree. Problem: the initializers on
> NSEvent for creating test events are too limited. We also want to beef
> up testing of scroll event streams for elastic scrolling and whatnot, so
> move the stuff in events_mac_unittest.mm to cocoa_test_event_utils:: for
> creating test scroll events.
>
> BUG=615948
>
> Committed: https://crrev.com/315a111b6cdd928cbd0862dd97f5a98482dd0261
> Cr-Commit-Position: refs/heads/master@{#411216}

TBR=avi@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=615948

Review-Url: https://codereview.chromium.org/2234143003
Cr-Commit-Position: refs/heads/master@{#411223}

[modify] https://crrev.com/941118827f5240dedb40082cffb1ead6c6d621cc/content/browser/renderer_host/input/web_input_event_builders_mac.mm
[modify] https://crrev.com/941118827f5240dedb40082cffb1ead6c6d621cc/content/browser/renderer_host/input/web_input_event_builders_mac_unittest.mm
[modify] https://crrev.com/941118827f5240dedb40082cffb1ead6c6d621cc/ui/events/cocoa/cocoa_event_utils.h
[modify] https://crrev.com/941118827f5240dedb40082cffb1ead6c6d621cc/ui/events/cocoa/events_mac.mm
[modify] https://crrev.com/941118827f5240dedb40082cffb1ead6c6d621cc/ui/events/cocoa/events_mac_unittest.mm
[modify] https://crrev.com/941118827f5240dedb40082cffb1ead6c6d621cc/ui/events/test/cocoa_test_event_utils.h
[modify] https://crrev.com/941118827f5240dedb40082cffb1ead6c6d621cc/ui/events/test/cocoa_test_event_utils.mm

Project Member

Comment 9 by bugdroid1@chromium.org, Aug 11 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/a3d3da13da743ee2b63e5e0fd45d899e45ce2979

commit a3d3da13da743ee2b63e5e0fd45d899e45ce2979
Author: tapted <tapted@chromium.org>
Date: Thu Aug 11 06:04:40 2016

Mac: Share kScrollbarPixelsPerCocoaTick between ui:: and blink:: events

Currently scrolling in MacViews with a "real" mouse wheel is too fast. A
multiplier is applied that maps the values used for
kCGScrollEventUnitLine to (effectively) kCGScrollEventUnitPixel, and the
one used for MacViews was too big. Share the constant that content::
uses by moving it to ui/events/cocoa/cocoa_event_utils.h

Add a test to ensure the event types agree. Problem: the initializers on
NSEvent for creating test events are too limited. We also want to beef
up testing of scroll event streams for elastic scrolling and whatnot, so
move the stuff in events_mac_unittest.mm to cocoa_test_event_utils:: for
creating test scroll events.

BUG=615948

Committed: https://crrev.com/315a111b6cdd928cbd0862dd97f5a98482dd0261
Review-Url: https://codereview.chromium.org/2226933004
Cr-Original-Commit-Position: refs/heads/master@{#411216}
Cr-Commit-Position: refs/heads/master@{#411275}

[modify] https://crrev.com/a3d3da13da743ee2b63e5e0fd45d899e45ce2979/content/browser/renderer_host/input/web_input_event_builders_mac.mm
[modify] https://crrev.com/a3d3da13da743ee2b63e5e0fd45d899e45ce2979/content/browser/renderer_host/input/web_input_event_builders_mac_unittest.mm
[modify] https://crrev.com/a3d3da13da743ee2b63e5e0fd45d899e45ce2979/ui/events/cocoa/cocoa_event_utils.h
[modify] https://crrev.com/a3d3da13da743ee2b63e5e0fd45d899e45ce2979/ui/events/cocoa/events_mac.mm
[modify] https://crrev.com/a3d3da13da743ee2b63e5e0fd45d899e45ce2979/ui/events/cocoa/events_mac_unittest.mm
[modify] https://crrev.com/a3d3da13da743ee2b63e5e0fd45d899e45ce2979/ui/events/test/cocoa_test_event_utils.h
[modify] https://crrev.com/a3d3da13da743ee2b63e5e0fd45d899e45ce2979/ui/events/test/cocoa_test_event_utils.mm

Blockedon: 637521
Project Member

Comment 11 by bugdroid1@chromium.org, Aug 13 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/4b70c528d7a6652e534bc5d9efd19d2d57d3fe4e

commit 4b70c528d7a6652e534bc5d9efd19d2d57d3fe4e
Author: tapted <tapted@chromium.org>
Date: Sat Aug 13 09:09:32 2016

Scroll with Layers in views::ScrollView

Initially, do this only on Mac, or with
--enable-features=ToolkitViewsScrollWithLayers.

This is a first step for bringing up elastic scrolling of a
views::ScrollView on Mac. Still, by itself, it results in significant
performance improvements for scrolling in toolkit-views and allows the
UI thread to keep pace with the high rate of touchpad scroll events
produced by Cocoa on Mac.

To take advantage of existing scroll handling for smooth-scrolling and
elasticity, later CLs will route scroll events through the
cc::InputHandler (i.e. LayerTreeHostImpl). Since the ui::Compositor is
single-threaded on the UI thread, there is no distinct impl side. For a
consistent event flow, perform all scrolling on the impl side. This
requires plumbing through a way for a views::ScrollView to set the
scroll offset on the impl side directly, in response to an input event.

views::ScrollView now makes its contents layer-backed and clips
it to the layer added to the existing ScrollView viewport.

BUG=615948,  594907 ,  605131 
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel

Review-Url: https://codereview.chromium.org/2188133002
Cr-Commit-Position: refs/heads/master@{#411889}

[modify] https://crrev.com/4b70c528d7a6652e534bc5d9efd19d2d57d3fe4e/cc/input/input_handler.h
[modify] https://crrev.com/4b70c528d7a6652e534bc5d9efd19d2d57d3fe4e/cc/trees/layer_tree_host_impl.cc
[modify] https://crrev.com/4b70c528d7a6652e534bc5d9efd19d2d57d3fe4e/cc/trees/layer_tree_host_impl.h
[modify] https://crrev.com/4b70c528d7a6652e534bc5d9efd19d2d57d3fe4e/cc/trees/single_thread_proxy.cc
[modify] https://crrev.com/4b70c528d7a6652e534bc5d9efd19d2d57d3fe4e/chrome/browser/ui/views/extensions/extension_install_dialog_view.cc
[modify] https://crrev.com/4b70c528d7a6652e534bc5d9efd19d2d57d3fe4e/chrome/browser/ui/views/extensions/extension_install_dialog_view.h
[modify] https://crrev.com/4b70c528d7a6652e534bc5d9efd19d2d57d3fe4e/ui/compositor/compositor.cc
[modify] https://crrev.com/4b70c528d7a6652e534bc5d9efd19d2d57d3fe4e/ui/compositor/compositor.h
[modify] https://crrev.com/4b70c528d7a6652e534bc5d9efd19d2d57d3fe4e/ui/compositor/layer.cc
[modify] https://crrev.com/4b70c528d7a6652e534bc5d9efd19d2d57d3fe4e/ui/compositor/layer.h
[modify] https://crrev.com/4b70c528d7a6652e534bc5d9efd19d2d57d3fe4e/ui/events/blink/input_handler_proxy_unittest.cc
[modify] https://crrev.com/4b70c528d7a6652e534bc5d9efd19d2d57d3fe4e/ui/views/controls/scroll_view.cc
[modify] https://crrev.com/4b70c528d7a6652e534bc5d9efd19d2d57d3fe4e/ui/views/controls/scroll_view.h
[modify] https://crrev.com/4b70c528d7a6652e534bc5d9efd19d2d57d3fe4e/ui/views/controls/scroll_view_unittest.cc
[modify] https://crrev.com/4b70c528d7a6652e534bc5d9efd19d2d57d3fe4e/ui/views/view.cc

Project Member

Comment 12 by bugdroid1@chromium.org, Sep 27 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/b94b06c07b4dada802959a10f8753d3b1a2dcc03

commit b94b06c07b4dada802959a10f8753d3b1a2dcc03
Author: tapted <tapted@chromium.org>
Date: Tue Sep 27 02:16:15 2016

MacViews: Send Mac scrollWheel NSEvents as ui::ET_SCROLL (ui::ScrollEvent).

On Mac, mouse wheel ticks and two-finger trackpad scroll events both
arrive via -[NSView scrollWheel:]. These are currently converted to a
ui::MouseWheel. However, ui::MouseWheel doesn't have the necessary event
phase information that the cc::InputHandler needs to properly calculate
scroll elasticity.

ui::ScrollEvent is a closer fit, but Mac generates a continuous event
"stream" through the momentum portion of a scroll "flick". To support
this, add an EventMomentumPhase enum, and populate the ScrollEvent with
it.  EventMomentumPhase is a simplified representation of the phase
information on the native NSEvent: it hides states that don't matter to
the scrolling machinery for the cc::InputHandler (i.e.
cc::LayerTreeHostImpl).

Elastic scrolling overview CL: http://crrev.com/2189583004

Add test coverage by fleshing out cocoa_test_event_utils::
TestScrollEvent(..). Fixes possible flakiness in tests using that method
since it didn't previously set event flags explicitly to zero. The
result could be that NSEvent uses the current global keyboard state to
populate its event flags, which could be influenced by tests running in
parallel.

BUG= 355659 , 615948,  640457 

Review-Url: https://codereview.chromium.org/2193153002
Cr-Commit-Position: refs/heads/master@{#421082}

[modify] https://crrev.com/b94b06c07b4dada802959a10f8753d3b1a2dcc03/base/mac/sdk_forward_declarations.h
[modify] https://crrev.com/b94b06c07b4dada802959a10f8753d3b1a2dcc03/content/browser/renderer_host/input/web_input_event_builders_mac_unittest.mm
[modify] https://crrev.com/b94b06c07b4dada802959a10f8753d3b1a2dcc03/ui/events/cocoa/events_mac.mm
[modify] https://crrev.com/b94b06c07b4dada802959a10f8753d3b1a2dcc03/ui/events/cocoa/events_mac_unittest.mm
[modify] https://crrev.com/b94b06c07b4dada802959a10f8753d3b1a2dcc03/ui/events/event.cc
[modify] https://crrev.com/b94b06c07b4dada802959a10f8753d3b1a2dcc03/ui/events/event.h
[modify] https://crrev.com/b94b06c07b4dada802959a10f8753d3b1a2dcc03/ui/events/event_constants.h
[modify] https://crrev.com/b94b06c07b4dada802959a10f8753d3b1a2dcc03/ui/events/event_utils.cc
[modify] https://crrev.com/b94b06c07b4dada802959a10f8753d3b1a2dcc03/ui/events/event_utils.h
[modify] https://crrev.com/b94b06c07b4dada802959a10f8753d3b1a2dcc03/ui/events/events_default.cc
[modify] https://crrev.com/b94b06c07b4dada802959a10f8753d3b1a2dcc03/ui/events/events_stub.cc
[modify] https://crrev.com/b94b06c07b4dada802959a10f8753d3b1a2dcc03/ui/events/test/cocoa_test_event_utils.h
[modify] https://crrev.com/b94b06c07b4dada802959a10f8753d3b1a2dcc03/ui/events/test/cocoa_test_event_utils.mm
[modify] https://crrev.com/b94b06c07b4dada802959a10f8753d3b1a2dcc03/ui/events/win/events_win.cc
[modify] https://crrev.com/b94b06c07b4dada802959a10f8753d3b1a2dcc03/ui/events/x/events_x.cc
[modify] https://crrev.com/b94b06c07b4dada802959a10f8753d3b1a2dcc03/ui/views/cocoa/bridged_content_view.mm
[modify] https://crrev.com/b94b06c07b4dada802959a10f8753d3b1a2dcc03/ui/views/controls/scroll_view.cc
[modify] https://crrev.com/b94b06c07b4dada802959a10f8753d3b1a2dcc03/ui/views/controls/scroll_view.h

Project Member

Comment 13 by bugdroid1@chromium.org, Sep 27 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/b94b06c07b4dada802959a10f8753d3b1a2dcc03

commit b94b06c07b4dada802959a10f8753d3b1a2dcc03
Author: tapted <tapted@chromium.org>
Date: Tue Sep 27 02:16:15 2016

MacViews: Send Mac scrollWheel NSEvents as ui::ET_SCROLL (ui::ScrollEvent).

On Mac, mouse wheel ticks and two-finger trackpad scroll events both
arrive via -[NSView scrollWheel:]. These are currently converted to a
ui::MouseWheel. However, ui::MouseWheel doesn't have the necessary event
phase information that the cc::InputHandler needs to properly calculate
scroll elasticity.

ui::ScrollEvent is a closer fit, but Mac generates a continuous event
"stream" through the momentum portion of a scroll "flick". To support
this, add an EventMomentumPhase enum, and populate the ScrollEvent with
it.  EventMomentumPhase is a simplified representation of the phase
information on the native NSEvent: it hides states that don't matter to
the scrolling machinery for the cc::InputHandler (i.e.
cc::LayerTreeHostImpl).

Elastic scrolling overview CL: http://crrev.com/2189583004

Add test coverage by fleshing out cocoa_test_event_utils::
TestScrollEvent(..). Fixes possible flakiness in tests using that method
since it didn't previously set event flags explicitly to zero. The
result could be that NSEvent uses the current global keyboard state to
populate its event flags, which could be influenced by tests running in
parallel.

BUG= 355659 , 615948,  640457 

Review-Url: https://codereview.chromium.org/2193153002
Cr-Commit-Position: refs/heads/master@{#421082}

[modify] https://crrev.com/b94b06c07b4dada802959a10f8753d3b1a2dcc03/base/mac/sdk_forward_declarations.h
[modify] https://crrev.com/b94b06c07b4dada802959a10f8753d3b1a2dcc03/content/browser/renderer_host/input/web_input_event_builders_mac_unittest.mm
[modify] https://crrev.com/b94b06c07b4dada802959a10f8753d3b1a2dcc03/ui/events/cocoa/events_mac.mm
[modify] https://crrev.com/b94b06c07b4dada802959a10f8753d3b1a2dcc03/ui/events/cocoa/events_mac_unittest.mm
[modify] https://crrev.com/b94b06c07b4dada802959a10f8753d3b1a2dcc03/ui/events/event.cc
[modify] https://crrev.com/b94b06c07b4dada802959a10f8753d3b1a2dcc03/ui/events/event.h
[modify] https://crrev.com/b94b06c07b4dada802959a10f8753d3b1a2dcc03/ui/events/event_constants.h
[modify] https://crrev.com/b94b06c07b4dada802959a10f8753d3b1a2dcc03/ui/events/event_utils.cc
[modify] https://crrev.com/b94b06c07b4dada802959a10f8753d3b1a2dcc03/ui/events/event_utils.h
[modify] https://crrev.com/b94b06c07b4dada802959a10f8753d3b1a2dcc03/ui/events/events_default.cc
[modify] https://crrev.com/b94b06c07b4dada802959a10f8753d3b1a2dcc03/ui/events/events_stub.cc
[modify] https://crrev.com/b94b06c07b4dada802959a10f8753d3b1a2dcc03/ui/events/test/cocoa_test_event_utils.h
[modify] https://crrev.com/b94b06c07b4dada802959a10f8753d3b1a2dcc03/ui/events/test/cocoa_test_event_utils.mm
[modify] https://crrev.com/b94b06c07b4dada802959a10f8753d3b1a2dcc03/ui/events/win/events_win.cc
[modify] https://crrev.com/b94b06c07b4dada802959a10f8753d3b1a2dcc03/ui/events/x/events_x.cc
[modify] https://crrev.com/b94b06c07b4dada802959a10f8753d3b1a2dcc03/ui/views/cocoa/bridged_content_view.mm
[modify] https://crrev.com/b94b06c07b4dada802959a10f8753d3b1a2dcc03/ui/views/controls/scroll_view.cc
[modify] https://crrev.com/b94b06c07b4dada802959a10f8753d3b1a2dcc03/ui/views/controls/scroll_view.h

Project Member

Comment 14 by bugdroid1@chromium.org, Oct 25 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/5a1cee4f8acd9731b7af2c616b42aa892fdfdd2e

commit 5a1cee4f8acd9731b7af2c616b42aa892fdfdd2e
Author: tapted <tapted@chromium.org>
Date: Tue Oct 25 02:49:15 2016

Views: Give ScrollViewTest a test harness.

Currently the tests are all TEST rather than TEST_F.

Add a test harness (TEST_F) to reduce some boilerplate and encapsulate
some subtle setup code required for overlay scrollers on Mac.

Part of this is to do with ScopedPreferredScrollerStyle: Interleaving
swizzlers like `std::unique_ptr x(new X1); x.reset(new X2);` causes
incorrect behaviour and leakage of the swizzled methods across tests run
in the same process. Specifically, X1's destructor restores the original
methods, but then X2's destructor will restore X1's swizzled methods,
since they were in effect when X2 was constructed. Make it harder for
devs to shoot themselves in the foot with this.

BUG=615948

Review-Url: https://codereview.chromium.org/2442223002
Cr-Commit-Position: refs/heads/master@{#427253}

[modify] https://crrev.com/5a1cee4f8acd9731b7af2c616b42aa892fdfdd2e/ui/base/test/scoped_preferred_scroller_style_mac.mm
[modify] https://crrev.com/5a1cee4f8acd9731b7af2c616b42aa892fdfdd2e/ui/views/controls/scroll_view_unittest.cc

WebKit recently overhauled their scrolling elasticity to use a bunch of private APIs from El Capitan https://github.com/WebKit/webkit/commit/785dbda55347273e8c1709df8d89470ffac6077b

The bug: https://bugs.webkit.org/show_bug.cgi?id=147261

says "Now that AppKit has wheel event filters and a momentum scrolling calculator, the current implementation of snap scroll animation on Mac should be refactored to use AppKit."  More stuff to investigate..
Components: Internals>Views
Blockedon: 476553
Labels: -M-54 M-66
Ideally this happens after Issue 476553, so adding that to track.

The next steps to fix the issue here are quite similar to the steps necessary to fix Issue 546520 (elastic scrolling for <div>s), which is also blocked on 476553.

There is/was a working prototype in #c0, so it _could_ happen sooner. With some tweaks, that prototype could also fix <div>s. However, landing the fix without care could mean more work for Issue 476553.
Description: Show this description
Cc: tapted@chromium.org
Owner: spqc...@chromium.org
Summary: [Mac]Views: Elastic scrolling for views::ScrollView (also: smooth scrolling, etc.) (was: [Mac]VIews: Elastic scrolling for views::ScrollView (also: smooth scrolling, etc.))
over to spqchan@ since she's working on this
Labels: -MovedFrom-53 -M-66 M-68 MacViews-Dialogs Target-68
spqchan: let's aim this at M68.
** Bulk Edit **

FYI: Starting 04/13 M68 will be in canary, M68 Dev promotion will be on 04/26.

Project Member

Comment 22 by bugdroid1@chromium.org, Apr 20 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/f582ff2a8dd2e14e5f9140bfd5b3bfc1b68a9e7a

commit f582ff2a8dd2e14e5f9140bfd5b3bfc1b68a9e7a
Author: spqchan <spqchan@chromium.org>
Date: Fri Apr 20 06:29:58 2018

[MacViews] Route Scroll Events to InputHandler and Add Momentum

Introduced a "UiCompositorScrollWithLayers" feature flag that's enabled
by default on MacOS, and disabled on all platforms.

Testing: cc_unittests LayerTreeHostImplTest.ScrollByScrollingNode

Overview CL:
https://chromium-review.googlesource.com/c/chromium/src/+/953622

Bug: 615948
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;master.tryserver.blink:linux_trusty_blink_rel;luci.chromium.try:android_optional_gpu_tests_rel
Change-Id: I2cef584b4f2a43107d89cf10c643f58b493cc35c
Reviewed-on: https://chromium-review.googlesource.com/954230
Commit-Queue: Sarah Chan <spqchan@chromium.org>
Reviewed-by: ccameron <ccameron@chromium.org>
Reviewed-by: Sadrul Chowdhury <sadrul@chromium.org>
Reviewed-by: Trent Apted <tapted@chromium.org>
Cr-Commit-Position: refs/heads/master@{#552275}
[modify] https://crrev.com/f582ff2a8dd2e14e5f9140bfd5b3bfc1b68a9e7a/cc/trees/layer_tree_host_impl.cc
[modify] https://crrev.com/f582ff2a8dd2e14e5f9140bfd5b3bfc1b68a9e7a/cc/trees/layer_tree_host_impl_unittest.cc
[modify] https://crrev.com/f582ff2a8dd2e14e5f9140bfd5b3bfc1b68a9e7a/ui/base/ui_base_features.cc
[modify] https://crrev.com/f582ff2a8dd2e14e5f9140bfd5b3bfc1b68a9e7a/ui/base/ui_base_features.h
[modify] https://crrev.com/f582ff2a8dd2e14e5f9140bfd5b3bfc1b68a9e7a/ui/compositor/BUILD.gn
[modify] https://crrev.com/f582ff2a8dd2e14e5f9140bfd5b3bfc1b68a9e7a/ui/compositor/compositor.cc
[modify] https://crrev.com/f582ff2a8dd2e14e5f9140bfd5b3bfc1b68a9e7a/ui/compositor/compositor.h
[add] https://crrev.com/f582ff2a8dd2e14e5f9140bfd5b3bfc1b68a9e7a/ui/compositor/overscroll/DEPS
[add] https://crrev.com/f582ff2a8dd2e14e5f9140bfd5b3bfc1b68a9e7a/ui/compositor/overscroll/scroll_input_handler.cc
[add] https://crrev.com/f582ff2a8dd2e14e5f9140bfd5b3bfc1b68a9e7a/ui/compositor/overscroll/scroll_input_handler.h
[modify] https://crrev.com/f582ff2a8dd2e14e5f9140bfd5b3bfc1b68a9e7a/ui/views/controls/scroll_view.cc
[modify] https://crrev.com/f582ff2a8dd2e14e5f9140bfd5b3bfc1b68a9e7a/ui/views/controls/scroll_view_unittest.cc

Pls mark the bug as fixed if CL is landed in trunk and nothing else is pending. Thank you.
M68 branch is coming soon on this Thursday, 05/24 and M68 Beta promotion is on 06/07. 
This bug is marked as P1 for M68. Pls land the fix to trunk ASAP (if possible before 4:00 PM PT this Thursday in order to make it to M68 branch build cut. Thank you.
Labels: -Target-68 Target-69
Labels: -M-68 M-69
Moving to "M-69" per comment #25.
Project Member

Comment 27 by bugdroid1@chromium.org, Jun 22 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/c25f27a222ff1140a7457e8006199c2726ef291f

commit c25f27a222ff1140a7457e8006199c2726ef291f
Author: spqchan <spqchan@chromium.org>
Date: Fri Jun 22 00:55:14 2018

Refactor InputScrollElasticityController

Refactor ObserveGestureEventAndResult() in
InputScrollElasticityController by moving the
logic into separate methods. This is to decouple
the momentum update logic from the event type
i.e. so that elasticity can be updated by a
ui::ScrollEvent as well as a blink::WebGestureEvent.
As a result, the methods are public so that they can be
called to handle ui::ScrollEvents.

This refactor is a part of elastic scrolling:
https://chromium-review.googlesource.com/c/chromium/src/+/1094097

Bug: 615948
Change-Id: I15dc619bee0577c1323b1e88a3c8924d7f31b47a
Reviewed-on: https://chromium-review.googlesource.com/1103841
Commit-Queue: Sarah Chan <spqchan@chromium.org>
Reviewed-by: Sadrul Chowdhury <sadrul@chromium.org>
Reviewed-by: Trent Apted <tapted@chromium.org>
Cr-Commit-Position: refs/heads/master@{#569478}
[modify] https://crrev.com/c25f27a222ff1140a7457e8006199c2726ef291f/ui/events/blink/input_scroll_elasticity_controller.cc
[modify] https://crrev.com/c25f27a222ff1140a7457e8006199c2726ef291f/ui/events/blink/input_scroll_elasticity_controller.h

Labels: MacViews-Release
Labels: -MacViews-Release
Labels: -M-69 Group-MacOS_Platform_Integration_and_Participation
Labels: M-69
Labels: ReleaseBlock-Stable
Labels: -Pri-1 -ReleaseBlock-Stable Pri-2
Triage: Reassessed priority.
Labels: -M-69 -Target-69 M-70 Target-70
Labels: Hotlist-DesktopUIChecked Hotlist-DesktopUIValid
*** UI Mass Triage***

Seems like WIP and bug is valid, hence tagging with appropriate label.

Sign in to add a comment