Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(911)

Issue 2774443002: chrome/android: Push EventFilters into the Layout implementations. (Closed)

Created:
3 years, 9 months ago by Khushal
Modified:
3 years, 8 months ago
CC:
chromium-reviews, agrieve+watch_chromium.org
Target Ref:
refs/heads/master
Project:
chromium
Visibility:
Public.

Description

chrome/android: Push EventFilters into the Layout implementations. EventFilters are currently built by the LayoutManager and all gestures are routed through it to the active layout. This indirection is unnecessary since only the StackLayout requires gesture handling. This also ends up exposing public APIs on the Layouts making it difficult to follow the path for events from different sources (CompositorView vs the Toolbar). This change pushes EventFilters deeper into the Layout implementations, so the Layouts directly consume touch events and generate gestures if necessary. Layout still has an API for swipe gestures coming from the toolbar. BUG= Review-Url: https://codereview.chromium.org/2774443002 Cr-Original-Commit-Position: refs/heads/master@{#459980} Committed: https://chromium.googlesource.com/chromium/src/+/ef25f0df9d626028752de7d945d89abe15d39dfb Review-Url: https://codereview.chromium.org/2774443002 Cr-Commit-Position: refs/heads/master@{#460525} Committed: https://chromium.googlesource.com/chromium/src/+/2d3c8ab9800e2eebc6a379a78f5afba38bd4f640

Patch Set 1 #

Total comments: 6

Patch Set 2 : rebase #

Patch Set 3 : real rebase only #

Patch Set 4 : addressed comments #

Patch Set 5 : rebase #

Patch Set 6 : rebase #

Patch Set 7 : fix for re-landing #

Unified diffs Side-by-side diffs Delta from patch set Stats (+291 lines, -441 lines) Patch
D chrome/android/java/src/org/chromium/chrome/browser/compositor/layouts/GestureHandlerLayoutDelegate.java View 1 chunk +0 lines, -65 lines 0 comments Download
M chrome/android/java/src/org/chromium/chrome/browser/compositor/layouts/Layout.java View 1 2 3 6 chunks +11 lines, -84 lines 0 comments Download
M chrome/android/java/src/org/chromium/chrome/browser/compositor/layouts/LayoutManagerChrome.java View 1 2 3 4 5 chunks +8 lines, -22 lines 0 comments Download
M chrome/android/java/src/org/chromium/chrome/browser/compositor/layouts/LayoutManagerChromePhone.java View 1 chunk +1 line, -2 lines 0 comments Download
M chrome/android/java/src/org/chromium/chrome/browser/compositor/layouts/LayoutManagerChromeTablet.java View 2 4 chunks +2 lines, -66 lines 0 comments Download
M chrome/android/java/src/org/chromium/chrome/browser/compositor/layouts/LayoutManagerDocument.java View 2 3 chunks +0 lines, -6 lines 0 comments Download
M chrome/android/java/src/org/chromium/chrome/browser/compositor/layouts/StaticLayout.java View 1 2 3 2 chunks +6 lines, -2 lines 0 comments Download
M chrome/android/java/src/org/chromium/chrome/browser/compositor/layouts/ToolbarSwipeLayout.java View 1 2 3 4 chunks +11 lines, -3 lines 0 comments Download
M chrome/android/java/src/org/chromium/chrome/browser/compositor/layouts/phone/SimpleAnimationLayout.java View 1 2 3 4 chunks +11 lines, -3 lines 0 comments Download
M chrome/android/java/src/org/chromium/chrome/browser/compositor/layouts/phone/StackLayout.java View 1 2 3 9 chunks +147 lines, -89 lines 0 comments Download
M chrome/android/java/src/org/chromium/chrome/browser/compositor/overlays/strip/StripLayoutHelperManager.java View 1 2 3 4 5 6 5 chunks +80 lines, -92 lines 0 comments Download
M chrome/android/java/src/org/chromium/chrome/browser/widget/OverviewListLayout.java View 1 2 3 3 chunks +11 lines, -3 lines 0 comments Download
M chrome/android/java_sources.gni View 1 2 3 4 5 1 chunk +0 lines, -1 line 0 comments Download
M chrome/test/android/javatests/src/org/chromium/chrome/test/util/TabStripUtils.java View 2 3 chunks +3 lines, -3 lines 0 comments Download

Messages

Total messages: 27 (15 generated)
Khushal
This depends on a couple of other changes but wanted to get your thoughts about ...
3 years, 9 months ago (2017-03-22 21:20:09 UTC) #3
mdjones
Thanks for the clean up! This mostly looks good, just wanted to discuss a few ...
3 years, 9 months ago (2017-03-23 17:18:55 UTC) #4
David Trainor- moved to gerrit
lgtm, but just mmake sure the expected behavior still works for cross-layout interaction and events. ...
3 years, 9 months ago (2017-03-24 17:06:43 UTC) #5
Khushal
https://codereview.chromium.org/2774443002/diff/1/chrome/android/java/src/org/chromium/chrome/browser/compositor/layouts/Layout.java File chrome/android/java/src/org/chromium/chrome/browser/compositor/layouts/Layout.java (right): https://codereview.chromium.org/2774443002/diff/1/chrome/android/java/src/org/chromium/chrome/browser/compositor/layouts/Layout.java#newcode102 chrome/android/java/src/org/chromium/chrome/browser/compositor/layouts/Layout.java:102: protected EventFilter mEventFilter; On 2017/03/24 17:06:43, David Trainor-ping if ...
3 years, 9 months ago (2017-03-24 22:33:09 UTC) #6
mdjones
lgtm
3 years, 9 months ago (2017-03-28 00:46:28 UTC) #9
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2774443002/80001
3 years, 9 months ago (2017-03-28 01:33:05 UTC) #13
commit-bot: I haz the power
Committed patchset #5 (id:80001) as https://chromium.googlesource.com/chromium/src/+/ef25f0df9d626028752de7d945d89abe15d39dfb
3 years, 9 months ago (2017-03-28 02:00:41 UTC) #17
Matt Giuca
A revert of this CL (patchset #5 id:80001) has been created in https://codereview.chromium.org/2784463002/ by mgiuca@chromium.org. ...
3 years, 9 months ago (2017-03-28 05:53:01 UTC) #18
Khushal
I had missed passing the setting so the AreaGestureEventFilter for the tab strip wouldn't auto-offset ...
3 years, 8 months ago (2017-03-29 03:15:12 UTC) #20
mdjones
lgtm
3 years, 8 months ago (2017-03-29 17:18:37 UTC) #21
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2774443002/120001
3 years, 8 months ago (2017-03-29 18:44:51 UTC) #24
commit-bot: I haz the power
3 years, 8 months ago (2017-03-29 20:40:45 UTC) #27
Message was sent while issue was closed.
Committed patchset #7 (id:120001) as
https://chromium.googlesource.com/chromium/src/+/2d3c8ab9800e2eebc6a379a78f5a...

Powered by Google App Engine
This is Rietveld 408576698