1344892 - Let native calls dispatch to XPCOM event queue

Status: RESOLVED FIXED

(Core Graveyard :: Widget: Android, enhancement)

Description

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

Right now all native calls dispatch to the widget event queue in nsAppShell using the | dispatchTo = "gecko" | option. However, for many situations we want to dispatch to the XPCOM event queue in nsThread. Bug 1337910 was one case where we should have dispatched to the XPCOM event queue.

Comment 1

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

Attachment: 1. Add option to dispatch to XPCOM queue (v1)

Rename the original "gecko" option for dispatchTo to "gecko_widget", and
add a new "gecko_xpcom" option that dispatches calls to the XPCOM event
queue.

Comment 2

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

Attachment: 2. Update dispatchTo = "gecko" options (v1)

Update the existing dispatchTo = "gecko" options to either "gecko_widget" or
"gecko_xpcom". Most cases are updated to "gecko_xpcom" because the calls
interface directly with core Gecko code or may invoke JavaScript, which
requires the XPCOM event queue (bug 1337910). Most cases that use
"gecko_widget" involve UI events or JNI management calls like disposeNative.

Comment 3

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

Attachment: 3. Update auto-generated bindings (v1)

Comment 4

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

Attachment: 1. Add option to dispatch to priority queue (v2)

Updated to have "gecko_priority" option in addition to old "gecko" option.

Comment 5

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

Attachment: 2. Update dispatchTo = "gecko" options (v2)

Comment 6

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

Attachment: 3. Update auto-generated bindings (v1.1)

Comment 7

[James Willcox (:snorp) ([e-mail]) (he/him)]

Comment on attachment 8844529 [details] [diff] [review]
1. Add option to dispatch to priority queue (v2)

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

::: mobile/android/geckoview/src/main/java/org/mozilla/gecko/annotation/WrapForJNI.java
@@ +30,5 @@
>  
>      /**
>       * Action to take if member access returns an exception.
> +     * - "abort" will cause a crash if there is a pending exception.
> +     * - "ignore" will not handle any pending exceptions; it is then the caller's

Thanks, I for one was clearly confused about the definition on this one.

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/ffeffef36f34
Let native calls dispatch to XPCOM event queue; r=snorp

Comment 9

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Backed out on jchen's request for unexpected passes and fails with Android reftests:

https://hg.mozilla.org/integration/mozilla-inbound/rev/0a986886241cb0aa08ca8a6b2ef9fd2bca6c0042

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=72479abe01240e9a7a4f49dca9abf2d84b264f74&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-resultStatus=retry&filter-resultStatus=usercancel&filter-resultStatus=runnable
Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=83110685&repo=mozilla-inbound

> REFTEST TEST-UNEXPECTED-FAIL | http://10.0.2.2:8854/tests/image/test/reftest/downscaling/downscale-2a.html?203,52,left != about:blank | image comparison

Comment 10

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

Attachment: Updated patch (v1)

Updated patch that passes reftests. I think the reftest harness relies on some
specific timing between the attachToJava call in LayerView.Compositor and some
other messages.

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/4f5a93b89b2b
Let native calls dispatch to XPCOM event queue; r=snorp

Comment 12

[Iris Hsiao [:ihsiao]]

sorry had to back this out for mochitest rc4 frequency failures like https://treeherder.mozilla.org/logviewer.html#?job_id=83917819&repo=mozilla-inbound&lineNumber=1776

https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&bugfiler&noautoclassify&filter-searchStr=android%204.3%20api15%2B%20tc-m(rc4)&fromchange=141d25bf6372bc0fe2e42d4142d827a0c804ac0d&fromchange=c4b1bf7ab479&tochange=3e129df4d428

https://hg.mozilla.org/integration/mozilla-inbound/rev/6628c9503e36

Comment 13

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

Attachment: Updated patch (v1.1)

Updated patch that makes GeckoThread.waitOnGecko wait for both queues to flush.

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/caf3e7f1f230
Let native calls dispatch to XPCOM event queue; r=snorp

Comment 15

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/caf3e7f1f230