1211704 - Convert IME events to native calls

Status: RESOLVED FIXED

(Core Graveyard :: Widget: Android, defect)

Comment 1

[(inactive) Jim Chen [:jchen] [:darchons]]

Attachment: Add GeckoEditable native methods (v1)

These native methods will replace the IME events used by GeckoEditable.

Comment 2

[(inactive) Jim Chen [:jchen] [:darchons]]

Attachment: Convert IME events to native calls (v1)

Remove IME events from GeckoEvent and use the newly added native calls
in GeckoEditable. The changes are mostly straightforward find & replace.

Comment 3

[(inactive) Jim Chen [:jchen] [:darchons]]

Attachment: Convert IME event handler in nsWindow to native calls (v1)

Move the IME event handler implementation in nsWindow::OnIMEEvent to
individual native calls in nsWindow::Natives. This patch also moves most
member variables and helper functions related to IME to inside
nsWindow::Natives. This has the benefit of better organization and saves
some memory because only the top-level nsWindow now keeps IME states.

GetIMEComposition and RemoveIMEComposition are kept inside nsWindow
because they are not strictly related to IME events, and they are used
by some other event handlers in nsWindow.

Comment 4

[(inactive) Jim Chen [:jchen] [:darchons]]

Attachment: Count key and IME events as UI events (v1)

GeckoEvent.KEY_EVENT and GeckoEvent.IME_EVENT used to count as UI events
for the hang monitor. We should count the new native calls as UI
events also, through this patch.

Comment 5

[(inactive) Jim Chen [:jchen] [:darchons]]

Attachment: Remove obsolete GeckoEvent definitions (v1)

Remove obsolete GeckoEvent definitions from GeckoEvent.java and
AndroidJavaWrappers.cpp/h.

Comment 6

[Eugen Sawin [:esawin]]

Comment on attachment 8671151 [details] [diff] [review]
Convert IME event handler in nsWindow to native calls (v1)

Review of attachment 8671151 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good, but it's becoming difficult to follow the subtle changes in semantics, do we have any tests for this?

::: widget/android/nsWindow.cpp
@@ +223,5 @@
> +        * Java controls the composition, and Gecko shadows the Java
> +           composition through update composition events
> +    */
> +
> +    struct IMEChange final {

Maybe IMETextChange would be better?

Comment 7

[Eugen Sawin [:esawin]]

Comment on attachment 8671154 [details] [diff] [review]
Remove obsolete GeckoEvent definitions (v1)

Review of attachment 8671154 [details] [diff] [review]:
-----------------------------------------------------------------

LGTM (assuming it builds).

Comment 8

[(inactive) Jim Chen [:jchen] [:darchons]]

Attachment: Let GeckoEditable call disposeNative (v1)

Right now we call disposeNative on GeckoEditable in the
nsWindow::Natives destructor. However, we may still have pending native
calls in the event queue at that point, and these events will cause
exceptions when handled. This patch makes GeckoEditable call
disposeNative, after ensuring there's no pending calls.

Comment 9

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/8b395733b46e
https://hg.mozilla.org/integration/mozilla-inbound/rev/e1a8d414c6ea
https://hg.mozilla.org/integration/mozilla-inbound/rev/0855074944cd
https://hg.mozilla.org/integration/mozilla-inbound/rev/5200b0f4db3f
https://hg.mozilla.org/integration/mozilla-inbound/rev/f375aacdbc7c
https://hg.mozilla.org/integration/mozilla-inbound/rev/d9882266e0b3

Comment 10

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/8b395733b46e
https://hg.mozilla.org/mozilla-central/rev/e1a8d414c6ea
https://hg.mozilla.org/mozilla-central/rev/0855074944cd
https://hg.mozilla.org/mozilla-central/rev/5200b0f4db3f
https://hg.mozilla.org/mozilla-central/rev/f375aacdbc7c
https://hg.mozilla.org/mozilla-central/rev/d9882266e0b3