Closed Bug 1344892 Opened 3 years ago Closed 3 years ago

Let native calls dispatch to XPCOM event queue

Categories

(Core :: Widget: Android, enhancement)

All
Android
enhancement
Not set

Tracking

()

RESOLVED FIXED
mozilla55
Tracking Status
firefox55 --- fixed

People

(Reporter: jchen, Assigned: jchen)

References

Details

Attachments

(4 files, 4 obsolete files)

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.
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.
Attachment #8844193 - Flags: review?(snorp)
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.
Attachment #8844194 - Flags: review?(snorp)
Updated to have "gecko_priority" option in addition to old "gecko" option.
Attachment #8844529 - Flags: review?(snorp)
Attachment #8844193 - Attachment is obsolete: true
Attachment #8844193 - Flags: review?(snorp)
Attachment #8844194 - Attachment is obsolete: true
Attachment #8844194 - Flags: review?(snorp)
Attachment #8844195 - Attachment is obsolete: true
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.
Attachment #8844529 - Flags: review?(snorp) → review+
Attachment #8844530 - Flags: review?(snorp) → review+
Pushed by nchen@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/ffeffef36f34
Let native calls dispatch to XPCOM event queue; r=snorp
Attached patch Updated patch (v1) (obsolete) — Splinter Review
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.
Attachment #8847290 - Flags: review+
Pushed by nchen@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/4f5a93b89b2b
Let native calls dispatch to XPCOM event queue; r=snorp
Updated patch that makes GeckoThread.waitOnGecko wait for both queues to flush.
Attachment #8847290 - Attachment is obsolete: true
Pushed by nchen@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/caf3e7f1f230
Let native calls dispatch to XPCOM event queue; r=snorp
Flags: needinfo?(nchen)
https://hg.mozilla.org/mozilla-central/rev/caf3e7f1f230
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
Depends on: 1356563
You need to log in before you can comment on or make changes to this bug.