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

Issue 716668 link

Starred by 4 users

Issue metadata

Status: Fixed
Owner:
Closed: Jun 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , All
Pri: 2
Type: Feature

Blocked on:
issue 727839
issue 727848
issue 727853
issue 755594

Blocking:
issue 318381



Sign in to add a comment

Pass TouchActionBits to cc for optimization purposes.

Project Member Reported by wkorman@chromium.org, Apr 28 2017

Issue description

Blink-side work to enable offloading some touch action scrolling cases to compositor thread, re:

https://docs.google.com/document/d/1zxfr8sMCddlUfWp3G4qzf-BSYSjwDfx2Y4rSYQKRdPw/edit?ts=58dc4121#

Rough plan sketch:

Annotate touch action event rects passed to cc with their TouchActionBits. Value must incorporate value from all ancestors.

Revise WebLayer to pass along a struct that includes both rect and enum,rather than just the touch action rect.

ComputedStyle has GetTouchAction(). See also TouchActionUtil ComputeEffectiveTouchAction. But rather than using that, follow smcgruer@ work for inherit-sticky to similarly move TouchAction to rare_inherited_data from non_inherited_rare_data. Perhaps this change: https://codereview.chromium.org/2825343003/

Should be able to delete ComputeEffectiveTouchAction and use the new data instead.

 
Labels: BugSource-Team PaintTeamTriaged-20170501
Cc: sunxd@chromium.org yigu@chromium.org
Owner: xidac...@chromium.org
Sending to xidachen since he's already got half of it in progress.

Comment 4 by rbyers@chromium.org, May 12 2017

Blocking: 318381
See  issue 263416  for some of the heuristics I had to use to make cc touch hit-testing (https://www.chromium.org/developers/design-documents/compositor-hit-testing) perform well.

 Issue 318381  tracks the larger issue of cc-side touch-action hit-testing.  As discussed offline implementing it correctly (such that the cc-hit-test either does exactly what main would do or reliably falls back to asking main), reliably (eg. >90% of scroll starts avoiding going to main), while minimizing the performance overhead is likely to be pretty tricky.

You'll probably also need to think about how best to reliably test there - there are likely to be lots of fun corner cases.  Perhaps start by figuring out what you would do on CC but just telling the main thread about it and then logging with UMA how often CC and main disagreed.  There may be some races where what either thread decided would be acceptable, but that should be rare.
Project Member

Comment 5 by bugdroid1@chromium.org, May 15 2017

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

commit 28dd0bab32d0c43cba71995b99e2255d36e42b7e
Author: xidachen <xidachen@chromium.org>
Date: Mon May 15 23:35:06 2017

Pass TouchAction bits together with rect in SetTouchEventHandlerRegion

At this moment, the WebLayer::SetTouchEventHandlerRegion only passes
a vector of rects to CC. We would like to pass touch action bits
corresponding to each rect so for optimization purposes.

In this CL, I only changed the API SetTouchEventHandlerRegion, so that
both blink and cc sides can work in parallel.

This CL has no behavior change.

BUG= 716668 
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel

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

[modify] https://crrev.com/28dd0bab32d0c43cba71995b99e2255d36e42b7e/cc/blink/web_layer_impl.cc
[modify] https://crrev.com/28dd0bab32d0c43cba71995b99e2255d36e42b7e/cc/blink/web_layer_impl.h
[modify] https://crrev.com/28dd0bab32d0c43cba71995b99e2255d36e42b7e/third_party/WebKit/Source/core/page/scrolling/ScrollingCoordinator.cpp
[modify] https://crrev.com/28dd0bab32d0c43cba71995b99e2255d36e42b7e/third_party/WebKit/public/platform/WebLayer.h
[add] https://crrev.com/28dd0bab32d0c43cba71995b99e2255d36e42b7e/third_party/WebKit/public/platform/WebTouchInfo.h

Cc: hayleyferr@google.com
Blockedon: 727839
Blockedon: 727848
Blockedon: 727853
Project Member

Comment 10 by bugdroid1@chromium.org, Jul 27 2017

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

commit bb5c0153040f21388c6cf1efd5ad333cf50afb49
Author: Hayley Ferr <hayleyferr@chromium.org>
Date: Thu Jul 27 22:57:19 2017

Changed InputAckHandler to InputDispositionHandler

We would like to be able to use InputAckHandler, |ack_handler_|,
in the case that WhiteListedTouchActions are received.
Changing InputAckHandler to InputDispositionHandler will provide a
better name for OnSetWhiteListedTouchAction logic that will be added
in a later CL. In the later CL, InputDispositionHandler will have an
OnSetWhiteListedTouchAction function that will call into the logic of
determining if a current gesture event is allowed.

TBR=piman@chromium.org

Bug:  716668 
Change-Id: Ib74f5b7062f5521b3a0e158b8ddb4732de4e1c3a
Reviewed-on: https://chromium-review.googlesource.com/589068
Commit-Queue: Hayley Ferr <hayleyferr@chromium.org>
Reviewed-by: Timothy Dresser <tdresser@chromium.org>
Cr-Commit-Position: refs/heads/master@{#490048}
[modify] https://crrev.com/bb5c0153040f21388c6cf1efd5ad333cf50afb49/content/browser/BUILD.gn
[rename] https://crrev.com/bb5c0153040f21388c6cf1efd5ad333cf50afb49/content/browser/renderer_host/input/input_disposition_handler.h
[modify] https://crrev.com/bb5c0153040f21388c6cf1efd5ad333cf50afb49/content/browser/renderer_host/input/input_router_impl.cc
[modify] https://crrev.com/bb5c0153040f21388c6cf1efd5ad333cf50afb49/content/browser/renderer_host/input/input_router_impl.h
[modify] https://crrev.com/bb5c0153040f21388c6cf1efd5ad333cf50afb49/content/browser/renderer_host/input/input_router_impl_unittest.cc
[modify] https://crrev.com/bb5c0153040f21388c6cf1efd5ad333cf50afb49/content/browser/renderer_host/input/legacy_input_router_impl.cc
[modify] https://crrev.com/bb5c0153040f21388c6cf1efd5ad333cf50afb49/content/browser/renderer_host/input/legacy_input_router_impl.h
[modify] https://crrev.com/bb5c0153040f21388c6cf1efd5ad333cf50afb49/content/browser/renderer_host/input/legacy_input_router_impl_perftest.cc
[modify] https://crrev.com/bb5c0153040f21388c6cf1efd5ad333cf50afb49/content/browser/renderer_host/input/legacy_input_router_impl_unittest.cc
[rename] https://crrev.com/bb5c0153040f21388c6cf1efd5ad333cf50afb49/content/browser/renderer_host/input/mock_input_disposition_handler.cc
[rename] https://crrev.com/bb5c0153040f21388c6cf1efd5ad333cf50afb49/content/browser/renderer_host/input/mock_input_disposition_handler.h
[modify] https://crrev.com/bb5c0153040f21388c6cf1efd5ad333cf50afb49/content/browser/renderer_host/render_widget_host_impl.h
[modify] https://crrev.com/bb5c0153040f21388c6cf1efd5ad333cf50afb49/content/test/BUILD.gn

Project Member

Comment 11 by bugdroid1@chromium.org, Jul 28 2017

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

commit b1edc9f4dc2b898f25de3d1608fe108aede00510
Author: Hayley Ferr <hayleyferr@chromium.org>
Date: Fri Jul 28 15:14:21 2017

Added unique_touch_event_id to SetWhiteListedTouchAction IPC

The unique_touch_event_id  in the SetWhiteListedTouchAction IPC
will only be used to verify the whitelisted touch action bit
is associated with the correct touch gesture event.
Logic for determining if a gesture is allowed with the
whitelisted touch action will be added in a later CL.

TBR=piman@chromium.org

Bug:  716668 
Change-Id: I1f69d82a798953ebcd7fcb55227596ab938572bd
Reviewed-on: https://chromium-review.googlesource.com/589608
Commit-Queue: Hayley Ferr <hayleyferr@chromium.org>
Reviewed-by: Timothy Dresser <tdresser@chromium.org>
Reviewed-by: Xida Chen <xidachen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#490393}
[modify] https://crrev.com/b1edc9f4dc2b898f25de3d1608fe108aede00510/content/browser/renderer_host/input/legacy_input_router_impl.cc
[modify] https://crrev.com/b1edc9f4dc2b898f25de3d1608fe108aede00510/content/browser/renderer_host/input/legacy_input_router_impl.h
[modify] https://crrev.com/b1edc9f4dc2b898f25de3d1608fe108aede00510/content/browser/renderer_host/input/legacy_input_router_impl_unittest.cc
[modify] https://crrev.com/b1edc9f4dc2b898f25de3d1608fe108aede00510/content/common/input_messages.h
[modify] https://crrev.com/b1edc9f4dc2b898f25de3d1608fe108aede00510/content/renderer/input/input_event_filter.cc
[modify] https://crrev.com/b1edc9f4dc2b898f25de3d1608fe108aede00510/content/renderer/input/input_event_filter.h
[modify] https://crrev.com/b1edc9f4dc2b898f25de3d1608fe108aede00510/content/renderer/input/input_handler_manager.cc
[modify] https://crrev.com/b1edc9f4dc2b898f25de3d1608fe108aede00510/content/renderer/input/input_handler_manager.h
[modify] https://crrev.com/b1edc9f4dc2b898f25de3d1608fe108aede00510/content/renderer/input/input_handler_manager_client.h
[modify] https://crrev.com/b1edc9f4dc2b898f25de3d1608fe108aede00510/content/renderer/input/input_handler_wrapper.cc
[modify] https://crrev.com/b1edc9f4dc2b898f25de3d1608fe108aede00510/content/renderer/input/input_handler_wrapper.h
[modify] https://crrev.com/b1edc9f4dc2b898f25de3d1608fe108aede00510/content/renderer/input/widget_input_handler_manager.cc
[modify] https://crrev.com/b1edc9f4dc2b898f25de3d1608fe108aede00510/content/renderer/input/widget_input_handler_manager.h
[modify] https://crrev.com/b1edc9f4dc2b898f25de3d1608fe108aede00510/ui/events/blink/input_handler_proxy.cc
[modify] https://crrev.com/b1edc9f4dc2b898f25de3d1608fe108aede00510/ui/events/blink/input_handler_proxy_client.h
[modify] https://crrev.com/b1edc9f4dc2b898f25de3d1608fe108aede00510/ui/events/blink/input_handler_proxy_unittest.cc

Project Member

Comment 12 by bugdroid1@chromium.org, Aug 1 2017

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

commit a976f2ea63383f6a46151ec26e00a5fcf52ccac4
Author: Xida Chen <xidachen@chromium.org>
Date: Tue Aug 01 13:13:49 2017

Pass the actual touch action to CC in ScrollingCoordinator

Right now in ScrollingCoordinator::SetTouchEventTargetRects, we always
send TouchAction::kTouchActionNone when we pass the rect to cc.

This CL change the kTouchActionNone to the real value associated with
that rect.

TBR=flackr@chromium.org

Bug:  716668 
Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel
Change-Id: I92c450fac165c59ba01ae1ef9f037c3001d56403
Reviewed-on: https://chromium-review.googlesource.com/581907
Commit-Queue: Xida Chen <xidachen@chromium.org>
Reviewed-by: Philip Rogers <pdr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#490961}
[modify] https://crrev.com/a976f2ea63383f6a46151ec26e00a5fcf52ccac4/cc/blink/web_layer_impl.cc
[modify] https://crrev.com/a976f2ea63383f6a46151ec26e00a5fcf52ccac4/cc/blink/web_layer_impl.h
[modify] https://crrev.com/a976f2ea63383f6a46151ec26e00a5fcf52ccac4/third_party/WebKit/Source/core/page/scrolling/ScrollingCoordinator.cpp
[modify] https://crrev.com/a976f2ea63383f6a46151ec26e00a5fcf52ccac4/third_party/WebKit/Source/core/page/scrolling/ScrollingCoordinatorTest.cpp
[add] https://crrev.com/a976f2ea63383f6a46151ec26e00a5fcf52ccac4/third_party/WebKit/Source/core/testing/data/touch-action.html
[modify] https://crrev.com/a976f2ea63383f6a46151ec26e00a5fcf52ccac4/third_party/WebKit/public/platform/WebLayer.h

Blockedon: 755594
 crbug.com/772130  captures this.
Status: Fixed (was: Assigned)

Sign in to add a comment