Chromium Code Reviews
Help | Chromium Project | Sign in
(1)

Issue 2653283002: Route IME Events to Focused RenderWidgets (Android) (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
1 month ago by EhsanK
Modified:
4 days, 22 hours ago
CC:
chromium-reviews, jam, darin-cc_chromium.org, site-isolation-reviews_chromium.org
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Route IME Events to Focused RenderWidgets (Android) ImeAdapterAndroid routes its IME events to the either RenderFrame or RenderWidget. While the IME events for RenderFrame are correctly sent to the focused frame, the ones for RenderWidget are still being sent to the page's RenderWidgetHostViewAndorid. This CL will change the destination by using the focused widget, namely, the RenderWidgetHost underneath the RenderWidgetHostViewAndroid (including itself) which has keyboard focused. This is essential to make IME for android work with OOPIFs. BUG=578168 Review-Url: https://codereview.chromium.org/2653283002 Cr-Commit-Position: refs/heads/master@{#452176} Committed: https://chromium.googlesource.com/chromium/src/+/a06dc6c5060618f557be9310cf37080000b77db3

Patch Set 1 #

Total comments: 24

Patch Set 2 : Addressing creis@'s comments #

Total comments: 1

Patch Set 3 : Fixed an error #

Total comments: 4

Patch Set 4 : Addressing creis@'s comments #

Total comments: 8

Patch Set 5 : Rebased #

Patch Set 6 : Addressed creis@'s comments #

Patch Set 7 : Rebased #

Patch Set 8 : Added a Content Browser Test #

Total comments: 1

Patch Set 9 : Minor fixes #

Total comments: 4

Patch Set 10 : Rebased #

Patch Set 11 : Addressing creis@'s comments #

Unified diffs Side-by-side diffs Delta from patch set Stats (+167 lines, -35 lines) Patch
M content/browser/renderer_host/ime_adapter_android.h View 1 2 3 4 5 6 7 3 chunks +8 lines, -4 lines 0 comments Download
M content/browser/renderer_host/ime_adapter_android.cc View 1 2 3 4 5 8 chunks +18 lines, -31 lines 0 comments Download
M content/browser/renderer_host/render_widget_host_view_android.h View 1 2 3 4 5 6 7 8 9 10 1 chunk +2 lines, -0 lines 0 comments Download
M content/browser/site_per_process_browsertest.cc View 1 2 3 4 5 6 7 8 9 10 2 chunks +139 lines, -0 lines 0 comments Download
Commit queue not available (can’t edit this change).

Messages

Total messages: 59 (42 generated)
EhsanK
Hello Charlie, Hopefully, the last browser side CL for android IME. Please take a look. ...
1 month ago (2017-01-25 19:27:47 UTC) #4
Charlie Reis (slow)
Hmm, I have some concerns (or at least confusion) about these methods below. Maybe you ...
1 month ago (2017-01-25 22:00:22 UTC) #7
EhsanK
Thanks Charlie! PTAL. https://codereview.chromium.org/2653283002/diff/1/content/browser/renderer_host/ime_adapter_android.cc File content/browser/renderer_host/ime_adapter_android.cc (right): https://codereview.chromium.org/2653283002/diff/1/content/browser/renderer_host/ime_adapter_android.cc#newcode318 content/browser/renderer_host/ime_adapter_android.cc:318: RenderWidgetHost* rwh = rwhva_->GetRenderWidgetHost(); On 2017/01/25 ...
1 month ago (2017-01-26 19:16:30 UTC) #16
Charlie Reis (slow)
Thanks-- looks almost ready. What did you think about my earlier question about tests? https://codereview.chromium.org/2653283002/diff/1/content/browser/renderer_host/ime_adapter_android.cc ...
1 month ago (2017-01-27 01:07:22 UTC) #17
EhsanK
Hello Charlie, PTAL. Sorry I forgot to answer your question on the tests. I think ...
1 month ago (2017-01-27 16:16:47 UTC) #18
EhsanK
Adding kenrb@ regarding the question for tests. I just talked to Ken an he was ...
1 month ago (2017-01-27 16:21:15 UTC) #20
Charlie Reis (slow)
On 2017/01/27 16:21:15, EhsanK wrote: > Adding kenrb@ regarding the question for tests. > > ...
4 weeks, 1 day ago (2017-01-29 18:08:14 UTC) #25
EhsanK
Thanks Charlie. I removed yet another method which seemed to have been unused for a ...
3 weeks, 6 days ago (2017-01-31 15:53:15 UTC) #30
Charlie Reis (slow)
On 2017/01/31 15:53:15, EhsanK wrote: > Thanks Charlie. I removed yet another method which seemed ...
3 weeks, 6 days ago (2017-01-31 17:01:28 UTC) #31
EhsanK
Hello Charlie and sorry it took a while to update this. I ended up adding ...
1 week, 3 days ago (2017-02-16 19:08:14 UTC) #37
Charlie Reis (slow)
Thanks. LGTM, assuming the android_n5x_swarming_rel failure isn't related. https://codereview.chromium.org/2653283002/diff/160001/content/browser/renderer_host/render_widget_host_view_android.h File content/browser/renderer_host/render_widget_host_view_android.h (right): https://codereview.chromium.org/2653283002/diff/160001/content/browser/renderer_host/render_widget_host_view_android.h#newcode253 content/browser/renderer_host/render_widget_host_view_android.h:253: ImeAdapterAndroid* ...
5 days, 22 hours ago (2017-02-21 19:30:59 UTC) #40
EhsanK
Thanks Charlie. The failure in the bot seems hardly relevant. I am running dry-run again ...
5 days, 21 hours ago (2017-02-21 20:41:38 UTC) #43
EhsanK
On 2017/02/21 20:41:38, EhsanK wrote: > Thanks Charlie. The failure in the bot seems hardly ...
5 days, 21 hours ago (2017-02-21 20:41:56 UTC) #44
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/2653283002/190001
5 days, 16 hours ago (2017-02-22 02:04:56 UTC) #52
commit-bot: I haz the power
Exceeded global retry quota
5 days, 16 hours ago (2017-02-22 02:09:04 UTC) #54
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/2653283002/190001
4 days, 22 hours ago (2017-02-22 19:46:41 UTC) #56
commit-bot: I haz the power
4 days, 22 hours ago (2017-02-22 20:08:25 UTC) #59
Message was sent while issue was closed.
Committed patchset #11 (id:190001) as
https://chromium.googlesource.com/chromium/src/+/a06dc6c5060618f557be9310cf37...
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f8e48bd