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

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

Created:
3 years, 11 months ago by EhsanK
Modified:
3 years, 10 months ago
Reviewers:
kenrb, Charlie Reis
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

Messages

Total messages: 59 (42 generated)
EhsanK
Hello Charlie, Hopefully, the last browser side CL for android IME. Please take a look. ...
3 years, 11 months ago (2017-01-25 19:27:47 UTC) #4
Charlie Reis
Hmm, I have some concerns (or at least confusion) about these methods below. Maybe you ...
3 years, 11 months 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 ...
3 years, 11 months ago (2017-01-26 19:16:30 UTC) #16
Charlie Reis
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 ...
3 years, 11 months 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 ...
3 years, 10 months 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 ...
3 years, 10 months ago (2017-01-27 16:21:15 UTC) #20
Charlie Reis
On 2017/01/27 16:21:15, EhsanK wrote: > Adding kenrb@ regarding the question for tests. > > ...
3 years, 10 months 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 years, 10 months ago (2017-01-31 15:53:15 UTC) #30
Charlie Reis
On 2017/01/31 15:53:15, EhsanK wrote: > Thanks Charlie. I removed yet another method which seemed ...
3 years, 10 months 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 ...
3 years, 10 months ago (2017-02-16 19:08:14 UTC) #37
Charlie Reis
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* ...
3 years, 10 months 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 ...
3 years, 10 months 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 ...
3 years, 10 months 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
3 years, 10 months ago (2017-02-22 02:04:56 UTC) #52
commit-bot: I haz the power
Exceeded global retry quota
3 years, 10 months 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
3 years, 10 months ago (2017-02-22 19:46:41 UTC) #56
commit-bot: I haz the power
3 years, 10 months 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...

Powered by Google App Engine
This is Rietveld 408576698