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

Issue 2826303003: Let selection events bypass ContentViewCore (Closed)

Created:
3 years, 8 months ago by Jinsuk Kim
Modified:
3 years, 8 months ago
Reviewers:
Theresa, boliu
CC:
chromium-reviews, yusukes+watch_chromium.org, shuchen+watch_chromium.org, jam, nona+watch_chromium.org, darin-cc_chromium.org, agrieve+watch_chromium.org, James Su, amaralp
Target Ref:
refs/heads/master
Project:
chromium
Visibility:
Public.

Description

Let selection events bypass ContentViewCore Introduces native SelectionPopupController to route selection events directly to Java layer without going through ContentViewCore. The native class leverages RenderWidgetHostConnector for updating its connection to RWHVA when it gets swapped out or destroyed. BUG=626765 Review-Url: https://codereview.chromium.org/2826303003 Cr-Commit-Position: refs/heads/master@{#466944} Committed: https://chromium.googlesource.com/chromium/src/+/3fbb94e525cb002ae6b5cae23edc036df3daaac6

Patch Set 1 #

Patch Set 2 : fix tests #

Total comments: 4

Patch Set 3 : comments #

Total comments: 6

Patch Set 4 : fix bug #

Patch Set 5 : rebase #

Patch Set 6 : call Initialize() #

Unified diffs Side-by-side diffs Delta from patch set Stats (+174 lines, -62 lines) Patch
M chrome/android/javatests/src/org/chromium/chrome/browser/contextualsearch/ContextualSearchTapEventTest.java View 1 2 chunks +3 lines, -1 line 0 comments Download
M content/browser/BUILD.gn View 1 2 3 4 1 chunk +2 lines, -0 lines 0 comments Download
M content/browser/android/browser_jni_registrar.cc View 2 chunks +3 lines, -0 lines 0 comments Download
M content/browser/android/content_view_core_impl.h View 1 chunk +0 lines, -4 lines 0 comments Download
M content/browser/android/content_view_core_impl.cc View 1 chunk +0 lines, -23 lines 0 comments Download
A content/browser/android/selection_popup_controller.h View 1 2 1 chunk +45 lines, -0 lines 0 comments Download
A content/browser/android/selection_popup_controller.cc View 1 2 3 4 5 1 chunk +76 lines, -0 lines 0 comments Download
M content/browser/renderer_host/render_widget_host_view_android.h View 3 chunks +5 lines, -0 lines 0 comments Download
M content/browser/renderer_host/render_widget_host_view_android.cc View 5 chunks +5 lines, -4 lines 0 comments Download
M content/public/android/BUILD.gn View 1 chunk +1 line, -0 lines 0 comments Download
M content/public/android/java/src/org/chromium/content/browser/ContentViewCore.java View 1 2 3 4 chunks +6 lines, -19 lines 0 comments Download
M content/public/android/java/src/org/chromium/content/browser/SelectionPopupController.java View 1 2 3 9 chunks +22 lines, -6 lines 0 comments Download
M content/public/android/javatests/src/org/chromium/content/browser/PopupZoomerTest.java View 1 2 chunks +6 lines, -5 lines 0 comments Download

Messages

Total messages: 22 (12 generated)
Jinsuk Kim
3 years, 8 months ago (2017-04-20 07:28:53 UTC) #7
boliu
https://codereview.chromium.org/2826303003/diff/40001/content/browser/android/selection_popup_controller.h File content/browser/android/selection_popup_controller.h (right): https://codereview.chromium.org/2826303003/diff/40001/content/browser/android/selection_popup_controller.h#newcode36 content/browser/android/selection_popup_controller.h:36: private: private destructor override (don't want other things deleting ...
3 years, 8 months ago (2017-04-20 18:17:29 UTC) #8
Jinsuk Kim
https://codereview.chromium.org/2826303003/diff/40001/content/browser/android/selection_popup_controller.h File content/browser/android/selection_popup_controller.h (right): https://codereview.chromium.org/2826303003/diff/40001/content/browser/android/selection_popup_controller.h#newcode36 content/browser/android/selection_popup_controller.h:36: private: On 2017/04/20 18:17:28, boliu wrote: > private destructor ...
3 years, 8 months ago (2017-04-20 22:55:54 UTC) #9
boliu
https://codereview.chromium.org/2826303003/diff/60001/content/public/android/java/src/org/chromium/content/browser/ContentViewCore.java File content/public/android/java/src/org/chromium/content/browser/ContentViewCore.java (right): https://codereview.chromium.org/2826303003/diff/60001/content/public/android/java/src/org/chromium/content/browser/ContentViewCore.java#newcode873 content/public/android/java/src/org/chromium/content/browser/ContentViewCore.java:873: mSelectionPopupController.setScrollInProgress(isScrollInProgress()); put this inside setTouchScrollInProgress https://codereview.chromium.org/2826303003/diff/60001/content/public/android/java/src/org/chromium/content/browser/ContentViewCore.java#newcode2357 content/public/android/java/src/org/chromium/content/browser/ContentViewCore.java:2357: mSelectionPopupController.setScrollInProgress(flinging); this ...
3 years, 8 months ago (2017-04-20 23:16:49 UTC) #10
Jinsuk Kim
https://codereview.chromium.org/2826303003/diff/60001/content/public/android/java/src/org/chromium/content/browser/ContentViewCore.java File content/public/android/java/src/org/chromium/content/browser/ContentViewCore.java (right): https://codereview.chromium.org/2826303003/diff/60001/content/public/android/java/src/org/chromium/content/browser/ContentViewCore.java#newcode873 content/public/android/java/src/org/chromium/content/browser/ContentViewCore.java:873: mSelectionPopupController.setScrollInProgress(isScrollInProgress()); On 2017/04/20 23:16:49, boliu wrote: > put this ...
3 years, 8 months ago (2017-04-20 23:49:06 UTC) #11
boliu
lgtm
3 years, 8 months ago (2017-04-21 00:37:36 UTC) #12
Jinsuk Kim
Updated so that the SelectionPopupController calls |Initialize()| that was added in https://crrev.com/2832153002 twellington@chromium.org: Please review ...
3 years, 8 months ago (2017-04-24 05:12:03 UTC) #15
Theresa
ContextualSearchTapEventTest.java lgtm
3 years, 8 months ago (2017-04-24 15:07:24 UTC) #16
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/2826303003/120001
3 years, 8 months ago (2017-04-25 11:05:19 UTC) #19
commit-bot: I haz the power
3 years, 8 months ago (2017-04-25 11:42:56 UTC) #22
Message was sent while issue was closed.
Committed patchset #6 (id:120001) as
https://chromium.googlesource.com/chromium/src/+/3fbb94e525cb002ae6b5cae23edc...

Powered by Google App Engine
This is Rietveld 408576698