Closed
Bug 1192043
Opened 9 years ago
Closed 9 years ago
Add mechanism to proxy native calls to Gecko thread
Categories
(Core Graveyard :: Widget: Android, defect)
Tracking
(firefox43 fixed)
RESOLVED
FIXED
mozilla43
Tracking | Status | |
---|---|---|
firefox43 | --- | fixed |
People
(Reporter: jchen, Assigned: jchen)
Details
Attachments
(4 files)
8.04 KB,
patch
|
snorp
:
review+
|
Details | Diff | Splinter Review |
5.08 KB,
patch
|
snorp
:
review+
|
Details | Diff | Splinter Review |
3.86 KB,
patch
|
snorp
:
review+
|
Details | Diff | Splinter Review |
17.33 KB,
patch
|
snorp
:
review+
|
Details | Diff | Splinter Review |
Currently GeckoEvents are all handled on the Gecko thread. When we switch them to native calls, we are going to want some kind of automatic mechanism to forward the native call to the Gecko thread.
Assignee | ||
Comment 1•9 years ago
|
||
Add more constructors in LocalRef and GlobalRef to accommodate use cases where a JNIEnv is already available for performance reasons. Also add more null-checks when creating references for performance reasons.
Attachment #8650077 -
Flags: review?(snorp)
Assignee | ||
Comment 2•9 years ago
|
||
The mangled name of a NativeStubImpl instantiation is longer than necessary because of the ReturnType arg. This patch turns it into a bool parameter. It also reorders the parameters for cleanness.
Attachment #8650078 -
Flags: review?(snorp)
Assignee | ||
Comment 3•9 years ago
|
||
This patch fixes a compile error when using WeakPtr, where Impl* was
expected but SupportsWeakPtr<Impl>* was given.
This patch also fixes a compile error when using the DisposeNative
implementation provided by the autogenerated Natives class. e.g.,
> struct Foo : Bar::Natives<Foo> {
> using Bar::Natives<Foo>::DisposeNative;
> };
This uses a default implementation of DisposeNative instead of a custom
implementation, and resulted in a compile error that this patch fixes.
Attachment #8650079 -
Flags: review?(snorp)
Assignee | ||
Comment 4•9 years ago
|
||
If a C++ class implements native calls that all return void, it can choose to have those calls go through a custom proxy function by inheriting from mozilla::jni::UsesNativeCallProxy and override the ProxyNativeCall member. ProxyNativeCall accepts a rvalue reference to a functor object specific to each call, and can alter the calling environment (e.g. dispatch the call to another thread) before issuing the actual native call through the functor's operator(). Any JNI refs contained in the call are automatically turned into global refs so that the call can continue to work outside of the original JNI call. Native call proxy will be used to implement automatic dispatching of native calls from the main thread to run on the Gecko thread.
Attachment #8650081 -
Flags: review?(snorp)
Comment on attachment 8650077 [details] [diff] [review] Add more JNIEnv support and null-check refs in JNI ref classs (v1) Review of attachment 8650077 [details] [diff] [review]: ----------------------------------------------------------------- ::: widget/android/jni/Refs.h @@ +506,5 @@ > + } > + > + // Get the raw JNI reference that can be used as a return value. > + // Returns the same JNI type (jobject, jstring, etc.) as the underlying Ref. > + auto Forget() -> decltype(Ref<Cls>(nullptr).Get()) I seriously cannot parse this. Can you explain?
Attachment #8650078 -
Flags: review?(snorp) → review+
Attachment #8650079 -
Flags: review?(snorp) → review+
Comment on attachment 8650081 [details] [diff] [review] Add support for proxying native calls (v1) Review of attachment 8650081 [details] [diff] [review]: ----------------------------------------------------------------- I don't think I really understand this wizardry, but alright. ::: tools/profiler/core/GeckoSampler.cpp @@ +548,5 @@ > #if defined(SPS_OS_android) && !defined(MOZ_WIDGET_GONK) > if (ProfileJava()) { > mozilla::widget::GeckoJavaSampler::PauseJavaProfiling(); > > + aWriter.Start(); Was this left in by accident?
Attachment #8650081 -
Flags: review?(snorp) → review+
Comment on attachment 8650077 [details] [diff] [review] Add more JNIEnv support and null-check refs in JNI ref classs (v1) Review of attachment 8650077 [details] [diff] [review]: ----------------------------------------------------------------- We discussed on irc, so I get it now.
Attachment #8650077 -
Flags: review?(snorp) → review+
https://hg.mozilla.org/integration/mozilla-inbound/rev/e25900da1968 https://hg.mozilla.org/integration/mozilla-inbound/rev/bf759946aa72 https://hg.mozilla.org/integration/mozilla-inbound/rev/8cd372463964 https://hg.mozilla.org/integration/mozilla-inbound/rev/450d1f83b001
Comment 9•9 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/e25900da1968 https://hg.mozilla.org/mozilla-central/rev/bf759946aa72 https://hg.mozilla.org/mozilla-central/rev/8cd372463964 https://hg.mozilla.org/mozilla-central/rev/450d1f83b001
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
status-firefox43:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla43
Updated•3 years ago
|
Product: Core → Core Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•