~eliasnaur/gio

2 2

Re: [PATCH 1/2] app: add RegisterDelegate method on *Window for Android

Details
Message ID
<BYLHHF16B1LK.2QNL0NCBO56SE@testmac>
DKIM signature
pass
Download raw message
On Wed Nov 20, 2019 at 6:56 PM Gregory Pomerantz wrote:
> On 11/20/19 6:15 PM, Elias Naur wrote:
> 
> >> +type Handle window.Handle
> >> +
> >> +// PlatformHandle returns the Android platform-specific Handle.
> >> +func PlatformHandle() *Handle {
> >> +	return (*Handle)(window.PlatformHandle)
> >> +}
> > Do you need it? If so, make it RegisterDelegate. If not, leave it out for now,
> > including the app/internal/window support types.
> 
> 
> For JNI-only apps that do not want to register a java class, and so we 
> can call RegisterNatives to install our callback that triggers once the 
> Delegate is installed (we can use the default ugly names convention 
> here, so this is perhaps not requred).
> 
> 

If we can live without it, let's leave it out until we need it. The less
platform-specific code the better.

> 
> 
> Can we get exceptions out of the handler? We can return a string from 
> registerDelegate() with the error (or "" on success) and turn it into a 
> Go error but this will make registerDelegate a blocking call. As I am 
> using this so far, I am depending on the installed delegate to call back 
> to Go to register success -- if it fails to do so, the library can log 
> an error after a timeout. That said, if we know for sure that 
> registerDelegate will fail (e.g. getContext is nil?) we can return an error.
> 

Yes, the ExceptionCheck/ExceptionDescribe/ExceptionClear JNI functions
can check, extract and clear exceptions.

> 
> > I still think the argument type should just be android.content.Context, and let be up to
> > the delegate to cast GioView's getContext to Activity. That way we have more wiggle room
> > when allowing embedding of Gio windows into existing Android apps.
> >
> > If you can somehow guarantee that View.getContext is alway of type Activity, by
> > all means keep the signature but use (Activity)getContext() instead of manually
> > tracking it.
> >
> > Also, make init/onRegister/... a method and create the instance with
> > Class.newInstance. Constructors don't have a name for signifying the meaning of
> > the method.
> 
> 
> Ok. One thought: if we want to explicitly support Fragments, then we can 
> detect if the object extends Fragment and attach it to the view's 
> Context automatically. The onAttach method of Fragment is called with 
> the Context, which will be an instanceof Activity if the Fragment is 
> attached to an Activity. If it is not a Fragment we manually call 
> onRegister() with the Context. I think Fragments will be able to restart 
> themselves automatically this way without any intervention from Gio. I 
> suppose we can leave it up to the devs to manually re-register other 
> classes (and document this behavior)?
> 

Good idea, but how about _only_ supporting Fragments then? That way we
don't have to invent a special constructor signature or init method.
Constructing fragments from class names has prior art: class names is
how you embed fragments in layout xml files.

Gio will have to cast getContext to Activity, but I think that's fine
for now. The exception to error conversion will handle unusual cases
where GioView is embedded into something else.

Re: [PATCH 1/2] app: add RegisterDelegate method on *Window for Android

Gregory Pomerantz
Details
Message ID
<884b36b4-fd9b-3474-d042-71805dfe647a@wow.st>
In-Reply-To
<BYLHHF16B1LK.2QNL0NCBO56SE@testmac> (view parent)
DKIM signature
pass
Download raw message
On 11/21/19 4:56 AM, Elias Naur wrote:

> Yes, the ExceptionCheck/ExceptionDescribe/ExceptionClear JNI functions
> can check, extract and clear exceptions.


If the exception is inside the handler I am concerned it will crash the 
wrong thread, I will give it a try and see if it works. Otherwise we can 
serialize them to strings within the handler and block until we get 
something back.


> Good idea, but how about _only_ supporting Fragments then? That way we
> don't have to invent a special constructor signature or init method.
> Constructing fragments from class names has prior art: class names is
> how you embed fragments in layout xml files.


This does look to be the cleanest approach. I will check to confirm that 
the callbacks work with their default names.

Re: [PATCH 1/2] app: add RegisterDelegate method on *Window for Android

Details
Message ID
<BYLL0N94ISHF.188KNO4QJZ9S0@toolbox>
In-Reply-To
<884b36b4-fd9b-3474-d042-71805dfe647a@wow.st> (view parent)
DKIM signature
pass
Download raw message
On Thu Nov 21, 2019 at 7:20 AM Gregory Pomerantz wrote:
> On 11/21/19 4:56 AM, Elias Naur wrote:
> 
> > Yes, the ExceptionCheck/ExceptionDescribe/ExceptionClear JNI functions
> > can check, extract and clear exceptions.
> 
> 
> If the exception is inside the handler I am concerned it will crash the 
> wrong thread, I will give it a try and see if it works. Otherwise we can 
> serialize them to strings within the handler and block until we get 
> something back.
> 
> 

Good point. That suggests we shouldn't return an error from RegisterDelegate
at all, and define exceptions as fatal.