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

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

Can't Edit
Can't Publish+Mail
Start Review
Created:
8 months, 3 weeks ago by EhsanK
Modified:
7 months, 4 weeks 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
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. ...
8 months, 3 weeks 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 ...
8 months, 3 weeks 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 ...
8 months, 3 weeks 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 ...
8 months, 3 weeks 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 ...
8 months, 3 weeks 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 ...
8 months, 3 weeks 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. > > ...
8 months, 3 weeks 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 ...
8 months, 3 weeks 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 ...
8 months, 3 weeks 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 ...
8 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* ...
8 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 ...
8 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 ...
8 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
8 months ago (2017-02-22 02:04:56 UTC) #52
commit-bot: I haz the power
Exceeded global retry quota
8 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
7 months, 4 weeks ago (2017-02-22 19:46:41 UTC) #56
commit-bot: I haz the power
7 months, 4 weeks 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 81bcdb8aa