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

Issue 671503005: Plumb composition character bounds for Android 5.0 (Closed)

Created:
6 years, 2 months ago by yukawa
Modified:
6 years, 1 month ago
CC:
chromium-reviews, creis+watch_chromium.org, yusukes+watch_chromium.org, yukishiino+watch_chromium.org, nasko+codewatch_chromium.org, jam, penghuang+watch_chromium.org, mkwst+moarreviews-ipc_chromium.org, nona+watch_chromium.org, darin-cc_chromium.org, mkwst+moarreviews-renderer_chromium.org, James Su
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Project:
chromium
Visibility:
Public.

Description

Plumb composition character bounds for Android 5.0 This is a groundwork to support CursorAnchorInfo API for Android 5.0. No user visible change is intended with this CL. With this CL, ImeCompositionRangeChanged event will be routed from the renderer to the browser in Android 5.0 and above. The underlying functionality has been widely used in desktop OSes for years. There should be no performance impact for existing in Android 4.4 and prior version. Performance impact on Android 5.0 devices will be tracked as Issue 427090. Actual plumbing from native to Java layer will be handled in subsequent CLs. BUG=424866, 427090 TEST=Manually done on Nexus 7 Build/LPX13D Committed: https://crrev.com/5f21c6afc5ae1a50834dc4b228807a266b2ee81f Cr-Commit-Position: refs/heads/master@{#301384}

Patch Set 1 #

Total comments: 2

Patch Set 2 : Rebase #

Total comments: 4
Unified diffs Side-by-side diffs Delta from patch set Stats (+37 lines, -18 lines) Patch
M content/browser/frame_host/render_widget_host_view_child_frame.h View 1 1 chunk +1 line, -1 line 0 comments Download
M content/browser/frame_host/render_widget_host_view_child_frame.cc View 1 1 chunk +1 line, -1 line 0 comments Download
M content/browser/renderer_host/render_widget_host_impl.h View 1 1 chunk +1 line, -1 line 0 comments Download
M content/browser/renderer_host/render_widget_host_impl.cc View 1 2 chunks +2 lines, -2 lines 0 comments Download
M content/browser/renderer_host/render_widget_host_view_android.h View 1 1 chunk +3 lines, -0 lines 0 comments Download
M content/browser/renderer_host/render_widget_host_view_android.cc View 1 1 chunk +6 lines, -0 lines 4 comments Download
M content/browser/renderer_host/render_widget_host_view_base.h View 1 1 chunk +1 line, -1 line 0 comments Download
M content/common/input_messages.h View 1 chunk +2 lines, -1 line 0 comments Download
M content/renderer/render_view_impl.h View 1 1 chunk +1 line, -1 line 0 comments Download
M content/renderer/render_view_impl.cc View 1 1 chunk +1 line, -1 line 0 comments Download
M content/renderer/render_widget.h View 1 2 chunks +4 lines, -3 lines 0 comments Download
M content/renderer/render_widget.cc View 1 6 chunks +13 lines, -5 lines 0 comments Download
M content/test/test_render_view_host.h View 1 1 chunk +1 line, -1 line 0 comments Download

Messages

Total messages: 32 (6 generated)
yukawa
Hi Ben, Aurimas. I'd like to here your comments before sending review request to OWNERs. ...
6 years, 2 months ago (2014-10-22 00:53:01 UTC) #2
aurimas (slooooooooow)
6 years, 2 months ago (2014-10-22 00:56:49 UTC) #3
aurimas (slooooooooow)
https://codereview.chromium.org/671503005/diff/1/content/renderer/render_widget.cc File content/renderer/render_widget.cc (right): https://codereview.chromium.org/671503005/diff/1/content/renderer/render_widget.cc#newcode1546 content/renderer/render_widget.cc:1546: UpdateCompositionInfo(true); UpdateCompositionInfo is an expensive call. That was the ...
6 years, 2 months ago (2014-10-22 00:59:04 UTC) #4
yukawa
https://codereview.chromium.org/671503005/diff/1/content/renderer/render_widget.cc File content/renderer/render_widget.cc (right): https://codereview.chromium.org/671503005/diff/1/content/renderer/render_widget.cc#newcode1546 content/renderer/render_widget.cc:1546: UpdateCompositionInfo(true); On 2014/10/22 00:59:04, aurimas wrote: > UpdateCompositionInfo is ...
6 years, 2 months ago (2014-10-22 01:11:35 UTC) #5
benm (inactive)
On 2014/10/22 01:11:35, yukawa wrote: > https://codereview.chromium.org/671503005/diff/1/content/renderer/render_widget.cc > File content/renderer/render_widget.cc (right): > > https://codereview.chromium.org/671503005/diff/1/content/renderer/render_widget.cc#newcode1546 > ...
6 years, 2 months ago (2014-10-22 16:03:58 UTC) #6
yukawa
On 2014/10/22 16:03:58, benm wrote: > On 2014/10/22 01:11:35, yukawa wrote: > > > https://codereview.chromium.org/671503005/diff/1/content/renderer/render_widget.cc ...
6 years, 2 months ago (2014-10-23 17:51:57 UTC) #8
yukawa
On 2014/10/23 17:51:57, yukawa wrote: > On 2014/10/22 16:03:58, benm wrote: > > On 2014/10/22 ...
6 years, 2 months ago (2014-10-24 03:46:06 UTC) #9
bcwhite
lgtm
6 years, 2 months ago (2014-10-24 19:20:38 UTC) #10
yukawa
Adding jamesr@, sky@, and mkwst@ for OWNER review. jamesr: content/renderer/* sky: content/browser/*, content/test/test_render_view_host.h mkwst: content/common/input_messages.h
6 years, 1 month ago (2014-10-24 22:38:49 UTC) #12
aurimas (slooooooooow)
lgtm as long as we keep track of the performance of this. I am certain ...
6 years, 1 month ago (2014-10-24 22:39:51 UTC) #13
yukawa
On 2014/10/24 22:39:51, aurimas wrote: > lgtm as long as we keep track of the ...
6 years, 1 month ago (2014-10-24 22:58:20 UTC) #14
jamesr
Can we get a rough idea of the performance impact before landing?
6 years, 1 month ago (2014-10-24 23:05:20 UTC) #15
yukawa
On 2014/10/24 23:05:20, jamesr wrote: > Can we get a rough idea of the performance ...
6 years, 1 month ago (2014-10-24 23:47:32 UTC) #16
jamesr
Can you quantify that in any way? How do you plan to evaluate the impact ...
6 years, 1 month ago (2014-10-24 23:54:34 UTC) #17
yukawa
On 2014/10/24 23:54:34, jamesr wrote: > Can you quantify that in any way? How do ...
6 years, 1 month ago (2014-10-25 00:12:31 UTC) #18
jamesr
I want to know at least a rough idea of what the overall performance impact ...
6 years, 1 month ago (2014-10-25 00:22:43 UTC) #19
yukawa
On 2014/10/25 00:22:43, jamesr wrote: > I want to know at least a rough idea ...
6 years, 1 month ago (2014-10-25 00:38:53 UTC) #20
Mike West
IPC lgtm % comment. https://codereview.chromium.org/671503005/diff/20001/content/browser/renderer_host/render_widget_host_view_android.cc File content/browser/renderer_host/render_widget_host_view_android.cc (right): https://codereview.chromium.org/671503005/diff/20001/content/browser/renderer_host/render_widget_host_view_android.cc#newcode766 content/browser/renderer_host/render_widget_host_view_android.cc:766: // TODO(yukawa): Implement this. It ...
6 years, 1 month ago (2014-10-25 05:59:53 UTC) #21
jamesr
lgtm
6 years, 1 month ago (2014-10-27 05:01:39 UTC) #22
yukawa
On 2014/10/25 05:59:53, Mike West wrote: > IPC lgtm % comment. > > https://codereview.chromium.org/671503005/diff/20001/content/browser/renderer_host/render_widget_host_view_android.cc > ...
6 years, 1 month ago (2014-10-27 05:10:37 UTC) #23
yukawa
sky@: Looks like this CL is ready to be reviewed by you. Can you take ...
6 years, 1 month ago (2014-10-27 05:12:57 UTC) #24
sky
LGTM https://codereview.chromium.org/671503005/diff/20001/content/browser/renderer_host/render_widget_host_view_android.cc File content/browser/renderer_host/render_widget_host_view_android.cc (right): https://codereview.chromium.org/671503005/diff/20001/content/browser/renderer_host/render_widget_host_view_android.cc#newcode766 content/browser/renderer_host/render_widget_host_view_android.cc:766: // TODO(yukawa): Implement this. NOTIMPLEMENTED?
6 years, 1 month ago (2014-10-27 15:28:32 UTC) #25
yukawa
https://codereview.chromium.org/671503005/diff/20001/content/browser/renderer_host/render_widget_host_view_android.cc File content/browser/renderer_host/render_widget_host_view_android.cc (right): https://codereview.chromium.org/671503005/diff/20001/content/browser/renderer_host/render_widget_host_view_android.cc#newcode766 content/browser/renderer_host/render_widget_host_view_android.cc:766: // TODO(yukawa): Implement this. On 2014/10/25 05:59:53, Mike West ...
6 years, 1 month ago (2014-10-27 16:10:30 UTC) #26
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/671503005/20001
6 years, 1 month ago (2014-10-27 16:12:49 UTC) #30
commit-bot: I haz the power
Committed patchset #2 (id:20001)
6 years, 1 month ago (2014-10-27 17:09:42 UTC) #31
commit-bot: I haz the power
6 years, 1 month ago (2014-10-27 17:10:34 UTC) #32
Message was sent while issue was closed.
Patchset 2 (id:??) landed as
https://crrev.com/5f21c6afc5ae1a50834dc4b228807a266b2ee81f
Cr-Commit-Position: refs/heads/master@{#301384}

Powered by Google App Engine
This is Rietveld 408576698