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

Issue 181833003: [Android] Out with the Android GR, in with the new unified C++ GR (Closed)

Created:
6 years, 10 months ago by jdduke (slow)
Modified:
6 years, 9 months ago
Reviewers:
Ted C, tdresser, Jói, Sami
CC:
chromium-reviews, craigdh+watch_chromium.org, jam, joi+watch-content_chromium.org, bulach+watch_chromium.org, yfriedman+watch_chromium.org, ilevy-cc_chromium.org, darin-cc_chromium.org, klundberg+watch_chromium.org, frankf+watch_chromium.org, Rick Byers, Dominik Grewe
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Visibility:
Public.

Description

[Android] Out with the Android GR, in with the new unified C++ GR Wire up the new gesture detection code from ui/events/gesture_detection in ContentViewCoreImpl. Also remove the old gesture detection code in ContentViewGestureHandler and SnapScrollController. BUG=332418, 320428, 341613 Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=254356

Patch Set 1 #

Total comments: 2

Patch Set 2 : Cleanup and rebase #

Total comments: 17

Patch Set 3 : Code review #

Patch Set 4 : Move TouchDispositionGestureFilter and GestureEventPacket to ui #

Patch Set 5 : Rebase #

Patch Set 6 : Fix type count #

Total comments: 11

Patch Set 7 : More code review #

Patch Set 8 : More cleanup #

Patch Set 9 : Fix win builds #

Patch Set 10 : Fix win linkage #

Patch Set 11 : Win builds are simply depressing #

Patch Set 12 : Rebase #

Unified diffs Side-by-side diffs Delta from patch set Stats (+1652 lines, -5620 lines) Patch
M content/browser/android/content_view_core_impl.h View 1 2 3 4 5 6 7 10 chunks +23 lines, -38 lines 0 comments Download
M content/browser/android/content_view_core_impl.cc View 1 2 3 4 5 6 7 14 chunks +62 lines, -208 lines 0 comments Download
M content/browser/android/gesture_event_type_list.h View 1 chunk +2 lines, -3 lines 0 comments Download
A content/browser/renderer_host/input/content_gesture_provider.h View 1 2 3 1 chunk +73 lines, -0 lines 0 comments Download
A content/browser/renderer_host/input/content_gesture_provider.cc View 1 2 3 4 5 6 1 chunk +217 lines, -0 lines 0 comments Download
D content/browser/renderer_host/input/gesture_event_packet.h View 1 2 3 4 1 chunk +0 lines, -55 lines 0 comments Download
D content/browser/renderer_host/input/gesture_event_packet.cc View 1 2 3 4 1 chunk +0 lines, -88 lines 0 comments Download
M content/browser/renderer_host/input/motion_event_android.h View 1 chunk +48 lines, -46 lines 0 comments Download
M content/browser/renderer_host/input/motion_event_android.cc View 1 4 chunks +116 lines, -56 lines 0 comments Download
A + content/browser/renderer_host/input/motion_event_web.h View 1 2 3 4 5 6 2 chunks +30 lines, -30 lines 0 comments Download
A content/browser/renderer_host/input/motion_event_web.cc View 1 2 1 chunk +149 lines, -0 lines 0 comments Download
M content/browser/renderer_host/input/touch_disposition_gesture_filter.h View 1 2 3 1 chunk +0 lines, -123 lines 0 comments Download
D content/browser/renderer_host/input/touch_disposition_gesture_filter.cc View 1 2 3 4 1 chunk +0 lines, -332 lines 0 comments Download
M content/browser/renderer_host/input/touch_disposition_gesture_filter_unittest.cc View 1 2 3 1 chunk +0 lines, -765 lines 0 comments Download
M content/browser/renderer_host/input/web_input_event_builders_android.h View 2 chunks +4 lines, -1 line 0 comments Download
M content/browser/renderer_host/input/web_input_event_builders_android.cc View 3 chunks +14 lines, -14 lines 0 comments Download
M content/content_browser.gypi View 1 2 3 4 5 6 4 chunks +5 lines, -4 lines 0 comments Download
M content/content_tests.gypi View 1 2 3 4 5 6 7 8 9 10 11 1 chunk +0 lines, -1 line 0 comments Download
M content/public/android/java/src/org/chromium/content/browser/ContentViewCore.java View 1 2 3 4 5 6 7 25 chunks +61 lines, -229 lines 0 comments Download
D content/public/android/java/src/org/chromium/content/browser/ContentViewGestureHandler.java View 1 chunk +0 lines, -993 lines 0 comments Download
D content/public/android/java/src/org/chromium/content/browser/SnapScrollController.java View 1 chunk +0 lines, -138 lines 0 comments Download
M content/public/android/java/src/org/chromium/content/common/ContentSwitches.java View 1 2 3 4 5 6 1 chunk +0 lines, -3 lines 0 comments Download
D content/public/android/javatests/src/org/chromium/content/browser/ContentViewGestureHandlerTest.java View 1 chunk +0 lines, -1731 lines 0 comments Download
M content/public/common/content_switches.h View 1 2 3 4 5 6 1 chunk +1 line, -0 lines 0 comments Download
M content/public/common/content_switches.cc View 1 2 3 4 5 6 1 chunk +3 lines, -0 lines 0 comments Download
M ui/events/events.gyp View 1 2 3 3 chunks +7 lines, -2 lines 0 comments Download
M ui/events/gesture_detection/gesture_config_helper_aura.cc View 1 2 3 4 5 6 7 8 9 10 1 chunk +1 line, -1 line 0 comments Download
A + ui/events/gesture_detection/gesture_event_data.h View 1 2 3 4 5 6 7 8 9 5 chunks +32 lines, -23 lines 0 comments Download
A ui/events/gesture_detection/gesture_event_data.cc View 1 2 3 1 chunk +25 lines, -0 lines 0 comments Download
A + ui/events/gesture_detection/gesture_event_data_packet.h View 1 2 3 4 2 chunks +22 lines, -20 lines 0 comments Download
A ui/events/gesture_detection/gesture_event_data_packet.cc View 1 2 3 4 1 chunk +78 lines, -0 lines 0 comments Download
D ui/events/gesture_detection/gesture_event_params.h View 1 2 3 1 chunk +0 lines, -104 lines 0 comments Download
D ui/events/gesture_detection/gesture_event_params.cc View 1 2 3 1 chunk +0 lines, -18 lines 0 comments Download
M ui/events/gesture_detection/gesture_provider.h View 1 2 3 5 chunks +13 lines, -10 lines 0 comments Download
M ui/events/gesture_detection/gesture_provider.cc View 1 2 3 23 chunks +99 lines, -96 lines 0 comments Download
M ui/events/gesture_detection/gesture_provider_unittest.cc View 1 2 3 22 chunks +43 lines, -43 lines 0 comments Download
M ui/events/gesture_detection/mock_motion_event.h View 1 2 3 3 chunks +12 lines, -4 lines 0 comments Download
M ui/events/gesture_detection/mock_motion_event.cc View 1 2 3 4 5 6 7 8 3 chunks +42 lines, -0 lines 0 comments Download
M ui/events/gesture_detection/motion_event.h View 1 chunk +3 lines, -3 lines 0 comments Download
A + ui/events/gesture_detection/touch_disposition_gesture_filter.h View 1 2 3 4 5 6 4 chunks +33 lines, -27 lines 0 comments Download
A + ui/events/gesture_detection/touch_disposition_gesture_filter.cc View 1 2 3 4 13 chunks +112 lines, -104 lines 0 comments Download
A + ui/events/gesture_detection/touch_disposition_gesture_filter_unittest.cc View 1 2 3 4 5 10 chunks +322 lines, -307 lines 0 comments Download

Messages

Total messages: 31 (0 generated)
jdduke (slow)
I think this is ready for review. All tests are passing (see https://codereview.chromium.org/128613003/), though I ...
6 years, 10 months ago (2014-02-26 20:57:01 UTC) #1
Sami
content/browser/android lgtm, good job! > Note that I've ripped out double-tap UMA reporting completely, but ...
6 years, 9 months ago (2014-02-27 14:10:24 UTC) #2
tdresser
https://codereview.chromium.org/181833003/diff/10001/content/browser/renderer_host/input/content_gesture_provider.cc File content/browser/renderer_host/input/content_gesture_provider.cc (right): https://codereview.chromium.org/181833003/diff/10001/content/browser/renderer_host/input/content_gesture_provider.cc#newcode19 content/browser/renderer_host/input/content_gesture_provider.cc:19: WebGestureEvent CreateGesture(const ui::GestureEventParams& params, This seems to be the ...
6 years, 9 months ago (2014-02-27 15:28:29 UTC) #3
jdduke (slow)
On 2014/02/27 14:10:24, Sami wrote: > content/browser/android lgtm, good job! > > > Note that ...
6 years, 9 months ago (2014-02-27 17:40:28 UTC) #4
jdduke (slow)
Per tdresser@'s request, I've moved TouchDispositionGestureFilter and GestureEventPacket to ui/, and also renamed GestureEventParams to ...
6 years, 9 months ago (2014-02-28 02:09:25 UTC) #5
jdduke (slow)
joi@: Review for content/public/common? Thanks. Just a note, the switch addition is really a move ...
6 years, 9 months ago (2014-02-28 02:13:05 UTC) #6
Jói
//content/public/common LGTM On Fri, Feb 28, 2014 at 2:13 AM, <jdduke@chromium.org> wrote: > joi@: Review ...
6 years, 9 months ago (2014-02-28 09:59:10 UTC) #7
tdresser
Thanks Jared! content/browser/renderer_host/input/* and ui/events/gesture_detection/* LGTM, modulo the cros compilation failure. https://codereview.chromium.org/181833003/diff/110001/content/browser/renderer_host/input/content_gesture_provider.cc File content/browser/renderer_host/input/content_gesture_provider.cc (right): ...
6 years, 9 months ago (2014-02-28 15:11:27 UTC) #8
jdduke (slow)
https://codereview.chromium.org/181833003/diff/110001/content/browser/renderer_host/input/content_gesture_provider.cc File content/browser/renderer_host/input/content_gesture_provider.cc (right): https://codereview.chromium.org/181833003/diff/110001/content/browser/renderer_host/input/content_gesture_provider.cc#newcode118 content/browser/renderer_host/input/content_gesture_provider.cc:118: ToTouchDispositionGestureFilterAck(InputEventAckState ack_state) { On 2014/02/28 15:11:27, tdresser wrote: > ...
6 years, 9 months ago (2014-02-28 19:44:54 UTC) #9
Ted C
lgtm /w a nit and a potential code cleanup https://codereview.chromium.org/181833003/diff/110001/content/browser/android/content_view_core_impl.cc File content/browser/android/content_view_core_impl.cc (right): https://codereview.chromium.org/181833003/diff/110001/content/browser/android/content_view_core_impl.cc#newcode1134 content/browser/android/content_view_core_impl.cc:1134: ...
6 years, 9 months ago (2014-02-28 19:52:32 UTC) #10
jdduke (slow)
Thanks everybody for review! https://codereview.chromium.org/181833003/diff/110001/content/browser/android/content_view_core_impl.cc File content/browser/android/content_view_core_impl.cc (right): https://codereview.chromium.org/181833003/diff/110001/content/browser/android/content_view_core_impl.cc#newcode1134 content/browser/android/content_view_core_impl.cc:1134: // generating touches were never ...
6 years, 9 months ago (2014-02-28 20:22:03 UTC) #11
jdduke (slow)
The CQ bit was checked by jdduke@chromium.org
6 years, 9 months ago (2014-02-28 20:22:11 UTC) #12
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/jdduke@chromium.org/181833003/140001
6 years, 9 months ago (2014-02-28 20:23:09 UTC) #13
commit-bot: I haz the power
The CQ bit was unchecked by commit-bot@chromium.org
6 years, 9 months ago (2014-02-28 21:10:20 UTC) #14
commit-bot: I haz the power
Retried try job too often on win_rel for step(s) base_unittests, browser_tests, interactive_ui_tests, net_unittests, unit_tests http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_rel&number=272275
6 years, 9 months ago (2014-02-28 21:10:21 UTC) #15
jdduke (slow)
The CQ bit was checked by jdduke@chromium.org
6 years, 9 months ago (2014-02-28 21:17:57 UTC) #16
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/jdduke@chromium.org/181833003/160001
6 years, 9 months ago (2014-02-28 21:18:35 UTC) #17
jdduke (slow)
The CQ bit was unchecked by jdduke@chromium.org
6 years, 9 months ago (2014-02-28 21:57:46 UTC) #18
jdduke (slow)
The CQ bit was checked by jdduke@chromium.org
6 years, 9 months ago (2014-02-28 21:59:22 UTC) #19
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/jdduke@chromium.org/181833003/180001
6 years, 9 months ago (2014-02-28 22:00:22 UTC) #20
jdduke (slow)
The CQ bit was checked by jdduke@chromium.org
6 years, 9 months ago (2014-03-01 00:11:19 UTC) #21
jdduke (slow)
The CQ bit was unchecked by jdduke@chromium.org
6 years, 9 months ago (2014-03-01 00:25:18 UTC) #22
jdduke (slow)
The CQ bit was checked by jdduke@chromium.org
6 years, 9 months ago (2014-03-01 00:27:13 UTC) #23
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/jdduke@chromium.org/181833003/220001
6 years, 9 months ago (2014-03-01 00:30:25 UTC) #24
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/jdduke@chromium.org/181833003/220001
6 years, 9 months ago (2014-03-01 01:11:40 UTC) #25
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/jdduke@chromium.org/181833003/220001
6 years, 9 months ago (2014-03-01 01:32:08 UTC) #26
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/jdduke@chromium.org/181833003/220001
6 years, 9 months ago (2014-03-01 04:06:20 UTC) #27
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/jdduke@chromium.org/181833003/220001
6 years, 9 months ago (2014-03-01 04:13:37 UTC) #28
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/jdduke@chromium.org/181833003/220001
6 years, 9 months ago (2014-03-01 04:24:36 UTC) #29
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/jdduke@chromium.org/181833003/220001
6 years, 9 months ago (2014-03-01 04:40:25 UTC) #30
commit-bot: I haz the power
6 years, 9 months ago (2014-03-01 05:15:12 UTC) #31
Message was sent while issue was closed.
Change committed as 254356

Powered by Google App Engine
This is Rietveld 408576698