~eliasnaur/gio

23 2

Android PendingIntents

Gregory Pomerantz
Details
Message ID
<b9fd1e4e-d429-ee68-b9d4-e46427f694e4@wow.st>
DKIM signature
pass
Download raw message
Hi, I'm working with the openpgp-api from OpenKeychain on Android. I'm 
able to build and include a jar file of the API's java code into my apk 
file using gogio. I can connect to a keychain installed on an Android 
device, and I can encrypt and decrypt messages from Go using JNI calls 
and a custom connector class, all via the JVM and Context returned by 
app.PlatformHandle.

So far so good. However, there are circumstances in the API where user 
interaction is required. When this happens, the openpgp API returns a 
PendingIntent which, when sent, causes the Open Keychain app to ask the 
user to do something like grant key access or enter a passphrase. 
Everything works except that there is no way for the Gio application to 
find out when the user interaction completes or what its result was -- 
you can confirm that the PendingIntent was sent, but cannot get a result 
back. As far as I can tell, the only way to get results back from a 
PendingIntent is to send it from an Activity using 
Activity.startIntentSenderForResult() and waiting for a callback on an 
overridden onActivityResult() method.

I'm not sure where else this might come up on Android. Any thoughts on 
how (or whether) to make this work? There should be a few possibilities:

1. let the application send the PendingIntent through GioActivity via 
app/internal/window.SendPendingIntent() or something like that -- this 
function can return a channel on which the result code returned to 
onActivityResult will be sent.

2. give the application a reference to the GioActivity object (via 
window.Handle) and add java methods to send PendingIntents and receive 
results.

2. allow imported Go packages to run their own activities by including 
Activity subclasses in their .jar files -- these will need to be written 
into the AndroidManifest.xml file.
Details
Message ID
<BYFTDZIC5TBS.1EM79NWJOK762@testmac>
In-Reply-To
<b9fd1e4e-d429-ee68-b9d4-e46427f694e4@wow.st> (view parent)
DKIM signature
pass
Download raw message
On Thu Nov 14, 2019 at 11:14 AM Gregory Pomerantz wrote:
> 
> 2. give the application a reference to the GioActivity object (via 
> window.Handle) and add java methods to send PendingIntents and receive 
> results.
> 

The context stuff is rather gross because no other platform needs it.
How about an Android-specific Run method on Handle:

// Run a function that needs access to the window handle. The handle
// is invalidated when f returns.
func (h *Handle) Run(w *Window, f func(h Handle))

Run takes a callback users can't use the WindowHandle outside its
lifecycle.

> 2. allow imported Go packages to run their own activities by including 
> Activity subclasses in their .jar files -- these will need to be written 
> into the AndroidManifest.xml file.
> 

I'd like to embed Gio UIs as Views into custom Activities at some point,
but that is a much larger task than your suggestion above.

-- elias
Gregory Pomerantz
Details
Message ID
<07da364c-2a30-7059-fcbd-23b926e25372@wow.st>
In-Reply-To
<BYFTDZIC5TBS.1EM79NWJOK762@testmac> (view parent)
DKIM signature
pass
Download raw message
On 11/14/19 1:00 PM, Elias Naur wrote:

> On Thu Nov 14, 2019 at 11:14 AM Gregory Pomerantz wrote:
>> 2. give the application a reference to the GioActivity object (via
>> window.Handle) and add java methods to send PendingIntents and receive
>> results.
> The context stuff is rather gross because no other platform needs it.
> How about an Android-specific Run method on Handle:
>
> // Run a function that needs access to the window handle. The handle
> // is invalidated when f returns.
> func (h *Handle) Run(w *Window, f func(h Handle))
>
> Run takes a callback users can't use the WindowHandle outside its
> lifecycle.
If the callback receives a Handle, why does there need to be a *Handle 
receiver? Also, in many cases the functions using the Handle will need a 
JVM too -- perhaps we can build in that functionality and give the 
callback the Handle and a JVM.
Details
Message ID
<BYG0DM5WY4Q8.LJS9QGJXHJU0@testmac>
In-Reply-To
<07da364c-2a30-7059-fcbd-23b926e25372@wow.st> (view parent)
DKIM signature
pass
Download raw message
On Thu Nov 14, 2019 at 1:20 PM Gregory Pomerantz wrote:
> On 11/14/19 1:00 PM, Elias Naur wrote:
> 
> > On Thu Nov 14, 2019 at 11:14 AM Gregory Pomerantz wrote:
> >> 2. give the application a reference to the GioActivity object (via
> >> window.Handle) and add java methods to send PendingIntents and receive
> >> results.
> > The context stuff is rather gross because no other platform needs it.
> > How about an Android-specific Run method on Handle:
> >
> > // Run a function that needs access to the window handle. The handle
> > // is invalidated when f returns.
> > func (h *Handle) Run(w *Window, f func(h Handle))
> >
> > Run takes a callback users can't use the WindowHandle outside its
> > lifecycle.
> If the callback receives a Handle, why does there need to be a *Handle 
> receiver?

The method receiver is an app handle, the function argument is a window
handle. I hope to avoid adding types just for the sake of Android.

> Also, in many cases the functions using the Handle will need a 
> JVM too -- perhaps we can build in that functionality and give the 
> callback the Handle and a JVM.

The JVM is in the method receiver if needed.
Gregory Pomerantz
Details
Message ID
<2dfbfa5c-99c8-0036-2ac5-d6b7dd96efd9@wow.st>
In-Reply-To
<BYG0DM5WY4Q8.LJS9QGJXHJU0@testmac> (view parent)
DKIM signature
pass
Download raw message
On 11/14/19 6:29 PM, Elias Naur wrote:

> The method receiver is an app handle, the function argument is a window
> handle. I hope to avoid adding types just for the sake of Android.
Right makes sense.
>> Also, in many cases the functions using the Handle will need a
>> JVM too -- perhaps we can build in that functionality and give the
>> callback the Handle and a JVM.
> The JVM is in the method receiver if needed.

I think I meant to say a JNIEnv, but either way it is straightforward to 
get one from the JVM if needed.

On some more experimentation the cleanest approach may be to allow app 
developers to load android Fragments into the main GioActivity. 
Something like app.LoadFragment(uintptr) (with a pointer to a Fragment 
instance) or app.LoadFragment(string) (taking a fully qualified class 
name and returning a uintptr to the Fragment instance) could work if the 
app package has a global pointing to the GioActivity instance. This way 
there does not need to be anything in the manifest as you would need for 
additional Activities. Fragments are able to send Intents and 
PendingIntents and receive responses by overriding their own 
onActivityResult() method. Packages could do this specifically for 
Android by importing gioui.org/app and would not need any other help 
from the users. This will solve the problem I'm having with openpgp-api 
at least, and any other Android APIs that need to work in a similar way.
Details
Message ID
<BYGD5ZDHEZ5C.1XWDIGWQHJDPK@testmac>
In-Reply-To
<2dfbfa5c-99c8-0036-2ac5-d6b7dd96efd9@wow.st> (view parent)
DKIM signature
pass
Download raw message
On Thu Nov 14, 2019 at 11:19 PM Gregory Pomerantz wrote:
> On 11/14/19 6:29 PM, Elias Naur wrote:
> 
> > The method receiver is an app handle, the function argument is a window
> > handle. I hope to avoid adding types just for the sake of Android.
> Right makes sense.
> >> Also, in many cases the functions using the Handle will need a
> >> JVM too -- perhaps we can build in that functionality and give the
> >> callback the Handle and a JVM.
> > The JVM is in the method receiver if needed.
> 
> I think I meant to say a JNIEnv, but either way it is straightforward to 
> get one from the JVM if needed.
> 

Yeah, and you could say that Run executes in the main thread to make it
easier for the user.

> On some more experimentation the cleanest approach may be to allow app 
> developers to load android Fragments into the main GioActivity. 
> Something like app.LoadFragment(uintptr) (with a pointer to a Fragment 
> instance) or app.LoadFragment(string) (taking a fully qualified class 
> name and returning a uintptr to the Fragment instance) could work if the 
> app package has a global pointing to the GioActivity instance. This way 
> there does not need to be anything in the manifest as you would need for 
> additional Activities. Fragments are able to send Intents and 
> PendingIntents and receive responses by overriding their own 
> onActivityResult() method. Packages could do this specifically for 
> Android by importing gioui.org/app and would not need any other help 
> from the users. This will solve the problem I'm having with openpgp-api 
> at least, and any other Android APIs that need to work in a similar way.
> 

I'd rather not have a global GioActivity, because it's very likely
multiple GioActivities (or GioViews) will be supported in the future.

Moreover, Fragments are again an Android-only concept I'd rather not put
into package app. Note, Fragments are deprecated from API 28:

	https://developer.android.com/reference/android/app/Fragment

How about adding package gioui.org/app/android with all the special
types, functions etc. for Android? Move Handle and PlatformHandle from
package app to package android:

	func Run(w *app.Window, f func(h *Activity))

	type Activity struct {
		Context uintptr
	}

	// Moved/copied from app/internal/window/handle_android.go.
	type Handle struct {
		// JVM is the JNI *JVM pointer.
		JVM uintptr
		// Context is a global reference to the
		// application's
		// android.content.Context instance.
		Context uintptr
	}

	// Moved from package app.
	func PlatformHandle() *Handle

	type ActivityResultEvent struct {
		...
	}

	type ActivityResultOp struct {
		Key event.Key
	}

ActivityResultEvents would simply be sent from os_android.go on
GioActivity.onActivityResult, and an Android-aware widget can receive
them by declaring an ActivityResultOp.

If it works, we won't have to pollute package app with any
Android-specific API that has to be bent for portability, and you can
have a much nicer Android API in package android.

WDYT?
Details
Message ID
<CAMAFT9W5RfFA0M-MBi-KbyLhSEq__=8SX8P6ouE_2R-GHo+uxg@mail.gmail.com>
In-Reply-To
<BYGD5ZDHEZ5C.1XWDIGWQHJDPK@testmac> (view parent)
DKIM signature
pass
Download raw message
On Fri, Nov 15, 2019 at 10:30 AM Elias Naur <mail@eliasnaur.com> wrote:
>
> On Thu Nov 14, 2019 at 11:19 PM Gregory Pomerantz wrote:
> > On 11/14/19 6:29 PM, Elias Naur wrote:
> >
> > > The method receiver is an app handle, the function argument is a window
> > > handle. I hope to avoid adding types just for the sake of Android.
> > Right makes sense.
> > >> Also, in many cases the functions using the Handle will need a
> > >> JVM too -- perhaps we can build in that functionality and give the
> > >> callback the Handle and a JVM.
> > > The JVM is in the method receiver if needed.
> >
> > I think I meant to say a JNIEnv, but either way it is straightforward to
> > get one from the JVM if needed.
> >
>
> Yeah, and you could say that Run executes in the main thread to make it
> easier for the user.
>
> > On some more experimentation the cleanest approach may be to allow app
> > developers to load android Fragments into the main GioActivity.
> > Something like app.LoadFragment(uintptr) (with a pointer to a Fragment
> > instance) or app.LoadFragment(string) (taking a fully qualified class
> > name and returning a uintptr to the Fragment instance) could work if the
> > app package has a global pointing to the GioActivity instance. This way
> > there does not need to be anything in the manifest as you would need for
> > additional Activities. Fragments are able to send Intents and
> > PendingIntents and receive responses by overriding their own
> > onActivityResult() method. Packages could do this specifically for
> > Android by importing gioui.org/app and would not need any other help
> > from the users. This will solve the problem I'm having with openpgp-api
> > at least, and any other Android APIs that need to work in a similar way.
> >
>
> I'd rather not have a global GioActivity, because it's very likely
> multiple GioActivities (or GioViews) will be supported in the future.
>
> Moreover, Fragments are again an Android-only concept I'd rather not put
> into package app. Note, Fragments are deprecated from API 28:
>
>         https://developer.android.com/reference/android/app/Fragment
>
> How about adding package gioui.org/app/android with all the special
> types, functions etc. for Android? Move Handle and PlatformHandle from
> package app to package android:
>
>         func Run(w *app.Window, f func(h *Activity))
>
>         type Activity struct {
>                 Context uintptr
>         }
>
>         // Moved/copied from app/internal/window/handle_android.go.
>         type Handle struct {
>                 // JVM is the JNI *JVM pointer.
>                 JVM uintptr
>                 // Context is a global reference to the
>                 // application's
>                 // android.content.Context instance.
>                 Context uintptr
>         }
>
>         // Moved from package app.
>         func PlatformHandle() *Handle
>
>         type ActivityResultEvent struct {
>                 ...
>         }
>
>         type ActivityResultOp struct {
>                 Key event.Key
>         }
>
> ActivityResultEvents would simply be sent from os_android.go on
> GioActivity.onActivityResult, and an Android-aware widget can receive
> them by declaring an ActivityResultOp.
>
> If it works, we won't have to pollute package app with any
> Android-specific API that has to be bent for portability, and you can
> have a much nicer Android API in package android.
>
> WDYT?

Thinking more about it, if you run into circular import problems or difficulties
handing internal state across the package app and package android, I think
we could simply do all the above in package app itself, in _android.* files.
In other words, let's drop making PlatformHandle and so on defined in a
cross-platform way and just have GOOS-specific API in package app.
We still want it neat and minimal, but don't have to worry about squeezing
the API to fit other platforms.

-- elias
Gregory Pomerantz
Details
Message ID
<ed8d38c3-5782-0ee5-4e82-5f553efc9c94@wow.st>
In-Reply-To
<CAMAFT9W5RfFA0M-MBi-KbyLhSEq__=8SX8P6ouE_2R-GHo+uxg@mail.gmail.com> (view parent)
DKIM signature
pass
Download raw message
On 11/15/19 8:53 AM, Elias Naur wrote:
> Thinking more about it, if you run into circular import problems or difficulties
> handing internal state across the package app and package android, I think
> we could simply do all the above in package app itself, in _android.* files.
> In other words, let's drop making PlatformHandle and so on defined in a
> cross-platform way and just have GOOS-specific API in package app.
> We still want it neat and minimal, but don't have to worry about squeezing
> the API to fit other platforms.
Ok, this makes sense. We will have *_android.go files under 
gioui.org/app with GOOS-specific code and APIs.

FYI Fragments are available in the android support library (now being 
phased out) and the new AndroidX library. We can move to that though it 
would require compiling GioActivity with reference to AndroidX -- 
GioActivity will need to inherit from 
androidx.fragment.app.FragmentActivity. This will make it quite 
challenging to compile using javac as we will have to manually download 
all dependencies and figure out where the required source files are. 
This would be a good incentive to take Java compilation out of cmd/gogio 
and move everything to a go:generate that produces a jar file, which 
could be packaged with gogio as a binary so everybody does not need to 
have a full, up-to-date version of the Android SDK.

I do think Fragments will ultimately prove to be the least invasive for 
gogio/app as the library developers will be able to do pretty much 
everything they need to do in their own custom fragments. There may be 
other capabilities needed in the future other than receiving 
ActivityResults. onRequestPermissionsResult() comes to mind though that 
probably gets built into Gio in a cross-platform manner eventually. 
There may be lifecycle-related issues that need to be addressed, for 
example APIs that have to re-connect in onStart() or save state in 
onSaveInstanceState(). Again these might be things you are planning on 
integrating into gogio but in the mean time they may be needed by 
packages outside of gio.

With respect to the API, I'm not convinced that an ActivityResultOp is 
the best approach -- wouldn't that require the Gio app developer to 
maintain a widget (which doesn't do anything except on Android) and 
remember to call its Layout() method every frame even if it is not 
associated with any visible UI? That could get confusing and lead to 
situations where something works everywhere except Android, making 
testing difficult and platform-specific for app developers.
Details
Message ID
<BYGK98C20QQG.2JMVJT3933XPV@testmac>
In-Reply-To
<ed8d38c3-5782-0ee5-4e82-5f553efc9c94@wow.st> (view parent)
DKIM signature
pass
Download raw message
On Fri Nov 15, 2019 at 9:46 AM Gregory Pomerantz wrote:
> On 11/15/19 8:53 AM, Elias Naur wrote:
> > Thinking more about it, if you run into circular import problems or difficulties
> > handing internal state across the package app and package android, I think
> > we could simply do all the above in package app itself, in _android.* files.
> > In other words, let's drop making PlatformHandle and so on defined in a
> > cross-platform way and just have GOOS-specific API in package app.
> > We still want it neat and minimal, but don't have to worry about squeezing
> > the API to fit other platforms.
> Ok, this makes sense. We will have *_android.go files under 
> gioui.org/app with GOOS-specific code and APIs.
> 
> FYI Fragments are available in the android support library (now being 
> phased out) and the new AndroidX library. We can move to that though it 
> would require compiling GioActivity with reference to AndroidX -- 
> GioActivity will need to inherit from 
> androidx.fragment.app.FragmentActivity. This will make it quite 
> challenging to compile using javac as we will have to manually download 
> all dependencies and figure out where the required source files are. 
> This would be a good incentive to take Java compilation out of cmd/gogio 
> and move everything to a go:generate that produces a jar file, which 
> could be packaged with gogio as a binary so everybody does not need to 
> have a full, up-to-date version of the Android SDK.
> 

We can use the standard Fragment even if it's deprecated. The point I
was trying to make is that Fragments is a fairly ugly API for what we're
trying to achieve. Even for Android :)

However, Fragments may be the best we got. See below.

> I do think Fragments will ultimately prove to be the least invasive for 
> gogio/app as the library developers will be able to do pretty much 
> everything they need to do in their own custom fragments. There may be 
> other capabilities needed in the future other than receiving 
> ActivityResults. onRequestPermissionsResult() comes to mind though that 
> probably gets built into Gio in a cross-platform manner eventually. 
> There may be lifecycle-related issues that need to be addressed, for 
> example APIs that have to re-connect in onStart() or save state in 
> onSaveInstanceState(). Again these might be things you are planning on 
> integrating into gogio but in the mean time they may be needed by 
> packages outside of gio.
> 
> With respect to the API, I'm not convinced that an ActivityResultOp is 
> the best approach -- wouldn't that require the Gio app developer to 
> maintain a widget (which doesn't do anything except on Android) and 
> remember to call its Layout() method every frame even if it is not 
> associated with any visible UI? That could get confusing and lead to 
> situations where something works everywhere except Android, making 
> testing difficult and platform-specific for app developers.
> 

Good points. However, do we then need to add anything Fragment-related
to package app at all? Isn't Run(w *Window, func(w *WindowHandle))
enough to get the GioActivity, on which you can then call addFragment
(or whatever it's called)?

-- elias
Gregory Pomerantz
Details
Message ID
<90a6ff64-d003-ed94-4248-e4d7d0ae36c4@wow.st>
In-Reply-To
<BYGK98C20QQG.2JMVJT3933XPV@testmac> (view parent)
DKIM signature
pass
Download raw message
On 11/15/19 10:03 AM, Elias Naur wrote:
> Good points. However, do we then need to add anything Fragment-related
> to package app at all? Isn't Run(w *Window, func(w *WindowHandle))
> enough to get the GioActivity, on which you can then call addFragment
> (or whatever it's called)?

I think so but we will need to add GioActivity to the Window Handle (I 
don't know any way to get the GioActivity from Context). With that we 
can add a new fragment by calling getFragmentManager(), which is a 
public method so we should be good without any modifications to 
GioActivity.java. I think this will be cleaner if we pass the Activity 
directly to the GioView constructor (even though SurfaceView() takes a 
Context, this should still work). Any concerns with that change?

Going forward, if/when the built-in Fragment goes way, we will have to 
migrate to AndroidX but thankfully that is not a task for today.
Details
Message ID
<BYGM0P3CKD9K.92RCB9EV9VBF@testmac>
In-Reply-To
<90a6ff64-d003-ed94-4248-e4d7d0ae36c4@wow.st> (view parent)
DKIM signature
pass
Download raw message
On Fri Nov 15, 2019 at 11:01 AM Gregory Pomerantz wrote:
> On 11/15/19 10:03 AM, Elias Naur wrote:
> > Good points. However, do we then need to add anything Fragment-related
> > to package app at all? Isn't Run(w *Window, func(w *WindowHandle))
> > enough to get the GioActivity, on which you can then call addFragment
> > (or whatever it's called)?
> 
> I think so but we will need to add GioActivity to the Window Handle (I 
> don't know any way to get the GioActivity from Context).

In case of the WindowHandle (or just "Activity" now we're in
_android.go), the GioActivity instance is the Context. In fact, we might
as well name the field such:

	type Activity uintptr

and then

	func Run(w *Window, f func(a Activity))

> With that we can add a new fragment by calling getFragmentManager(), which is a 
> public method so we should be good without any modifications to 
> GioActivity.java. I think this will be cleaner if we pass the Activity 
> directly to the GioView constructor (even though SurfaceView() takes a 
> Context, this should still work). Any concerns with that change?
> 

It is already so in GioActivity.onCreate (Activity is its own Context):

	this.view = new GioView(this);

> Going forward, if/when the built-in Fragment goes way, we will have to 
> migrate to AndroidX but thankfully that is not a task for today.
> 

I wouldn't worry about it. Android will never remove deprecated types,
for the same reason Go won't: backwards compatibility. Deprecation is
just a marker that something better is available.

-- elias
Gregory Pomerantz
Details
Message ID
<1decd4fb-883f-8b0c-e7bd-0cb42fbb2b05@wow.st>
In-Reply-To
<BYGM0P3CKD9K.92RCB9EV9VBF@testmac> (view parent)
DKIM signature
pass
Download raw message
On 11/15/19 11:26 AM, Elias Naur wrote:
> In case of the WindowHandle (or just "Activity" now we're in
> _android.go), the GioActivity instance is the Context. In fact, we might
> as well name the field such:
>
> 	type Activity uintptr
>
> and then
>
> 	func Run(w *Window, f func(a Activity))
Agree.
>> With that we can add a new fragment by calling getFragmentManager(), which is a
>> public method so we should be good without any modifications to
>> GioActivity.java. I think this will be cleaner if we pass the Activity
>> directly to the GioView constructor (even though SurfaceView() takes a
>> Context, this should still work). Any concerns with that change?
> It is already so in GioActivity.onCreate (Activity is its own Context):
>
> 	this.view = new GioView(this);

The GioView constructor takes a Context, and I wasn't sure if that was 
always safe to assume that casting an Activity to a Context preserves 
access to all of the Activity methods.

Plus, runGoMain is called from GioView#initialize, which takes a 
context.getApplicationContext() -- so at least we will have to pass the 
Activity to Initialize and call getApplicationContext() from witihin 
GioView#initialize().

>> Going forward, if/when the built-in Fragment goes way, we will have to
>> migrate to AndroidX but thankfully that is not a task for today.
> I wouldn't worry about it. Android will never remove deprecated types,
> for the same reason Go won't: backwards compatibility. Deprecation is
> just a marker that something better is available.
That's a relief.
Details
Message ID
<BYGMP3HWJXZS.27V53BYGU371H@testmac>
In-Reply-To
<1decd4fb-883f-8b0c-e7bd-0cb42fbb2b05@wow.st> (view parent)
DKIM signature
pass
Download raw message
On Fri Nov 15, 2019 at 11:44 AM Gregory Pomerantz wrote:
> On 11/15/19 11:26 AM, Elias Naur wrote:
> >> With that we can add a new fragment by calling getFragmentManager(), which is a
> >> public method so we should be good without any modifications to
> >> GioActivity.java. I think this will be cleaner if we pass the Activity
> >> directly to the GioView constructor (even though SurfaceView() takes a
> >> Context, this should still work). Any concerns with that change?
> > It is already so in GioActivity.onCreate (Activity is its own Context):
> >
> > 	this.view = new GioView(this);
> 
> The GioView constructor takes a Context, and I wasn't sure if that was 
> always safe to assume that casting an Activity to a Context preserves 
> access to all of the Activity methods.
> 

Now I see what you mean. Yes, changing the GioView constructor to take
a GioActivity is fine if you need it.

> Plus, runGoMain is called from GioView#initialize, which takes a 
> context.getApplicationContext() -- so at least we will have to pass the 
> Activity to Initialize and call getApplicationContext() from witihin 
> GioView#initialize().
> 

GioView#initialize is supposed to run once per app, not per GioView.
Use GioView#onCreateView for passing the Activity to Go.

-- elias
Gregory Pomerantz
Details
Message ID
<24f61990-527e-b674-f232-32fa5c4cf580@wow.st>
In-Reply-To
<BYGMP3HWJXZS.27V53BYGU371H@testmac> (view parent)
DKIM signature
pass
Download raw message
> GioView#initialize is supposed to run once per app, not per GioView.
> Use GioView#onCreateView for passing the Activity to Go.
ok sounds good.
Gregory Pomerantz
Details
Message ID
<2f71eb73-1075-c1c8-6cf4-07c35b5e8a5b@wow.st>
In-Reply-To
<BYGMP3HWJXZS.27V53BYGU371H@testmac> (view parent)
DKIM signature
pass
Download raw message
On 11/15/19 11:58 AM, Elias Naur wrote:

> Now I see what you mean. Yes, changing the GioView constructor to take
> a GioActivity is fine if you need it.
>
>> Plus, runGoMain is called from GioView#initialize, which takes a
>> context.getApplicationContext() -- so at least we will have to pass the
>> Activity to Initialize and call getApplicationContext() from witihin
>> GioView#initialize().
> GioView#initialize is supposed to run once per app, not per GioView.
> Use GioView#onCreateView for passing the Activity to Go.

I replaced Context with Activity in the Android platform handle and now 
have the openpgp-api connector working. If you don't mind I would like 
to push this patch first before restructuring the handle API to make it 
Android-specific.

As far as where to provide the Activity to Gio, perhaps this should be 
done directly from GioActivity as it does not necessarily have any 
relationship to the View? There may be lifecycle issues to work out as 
well, e.g. what happens when the Activity is stopped or runs into a low 
memory situation.

On that last point, should app.Run(...) do anything special when the 
Activity is not available or has been stopped? We can block, or return 
an error instead of running the callback. Let me know your thoughts on 
the best way to deal with that.
Details
Message ID
<BYI39E2E43BC.11FLY1PKA9KIE@testmac>
In-Reply-To
<2f71eb73-1075-c1c8-6cf4-07c35b5e8a5b@wow.st> (view parent)
DKIM signature
pass
Download raw message
On Sat Nov 16, 2019 at 10:38 AM Gregory Pomerantz wrote:
> On 11/15/19 11:58 AM, Elias Naur wrote:
> 
> > Now I see what you mean. Yes, changing the GioView constructor to take
> > a GioActivity is fine if you need it.
> >
> >> Plus, runGoMain is called from GioView#initialize, which takes a
> >> context.getApplicationContext() -- so at least we will have to pass the
> >> Activity to Initialize and call getApplicationContext() from witihin
> >> GioView#initialize().
> > GioView#initialize is supposed to run once per app, not per GioView.
> > Use GioView#onCreateView for passing the Activity to Go.
> 
> I replaced Context with Activity in the Android platform handle and now 
> have the openpgp-api connector working. If you don't mind I would like 
> to push this patch first before restructuring the handle API to make it 
> Android-specific.
> 

I don't think you can just replace the global app context with the
Activity Context. Disregarding multiple windows (activities or
GioViews), there is also the problem of the Activity recreating on
certain configuration changes, resulting in a different Activity handle
that won't be reflected in the result fromPlatformHandle.

In other words, I don't see a way around Run, or something that avoids
the race conditions and memory leaks of just returning a handle to the
Activity.

> As far as where to provide the Activity to Gio, perhaps this should be 
> done directly from GioActivity as it does not necessarily have any 
> relationship to the View? There may be lifecycle issues to work out as 
> well, e.g. what happens when the Activity is stopped or runs into a low 
> memory situation.
> 

I'd like to minimize the smartness in GioActivity because I anticipate
that it will be replaced when embedding GioViews in foreign Activity
instances. For example, the lifecycle callbacks in GioActivity calls into
GioView's start/stop methods for that reason.

> On that last point, should app.Run(...) do anything special when the 
> Activity is not available or has been stopped? We can block, or return 
> an error instead of running the callback. Let me know your thoughts on 
> the best way to deal with that.
>

I think a nil *app.Activity is most appropriate. Additionally, Run
should run on the main (UI) thread, to avoid races with e.g. recreation
of the Activity. Being on the main thread is probably most convenient
for the code needing the Activity handle anyway.

However, to avoid deadlocks where Run is called while your app is
processing a system event, it should *not* block on the completion of
the handle function.

BTW, because of build tags, Run can be a method on Window:

type Activity struct {
	Activity uintptr
}

// Run a function that needs access to the Android Activity for the
// window. The function is run on the UI thread and should
// not block. The function must be prepared to deal with a nil
// activity if none is available.
// Run returns immediately and does not wait for the
// completion of f.
func (w *Window) Run(f func(a *Activity))

-- elias
Gregory Pomerantz
Details
Message ID
<d9713cb8-2675-a508-ddba-1f8770dcb335@wow.st>
In-Reply-To
<BYI39E2E43BC.11FLY1PKA9KIE@testmac> (view parent)
DKIM signature
pass
Download raw message
On 11/17/19 5:10 AM, Elias Naur wrote:

> I don't think you can just replace the global app context with the
> Activity Context. Disregarding multiple windows (activities or
> GioViews), there is also the problem of the Activity recreating on
> certain configuration changes, resulting in a different Activity handle
> that won't be reflected in the result fromPlatformHandle.
>
> In other words, I don't see a way around Run, or something that avoids
> the race conditions and memory leaks of just returning a handle to the
> Activity.
Yes I am starting to see how this needs to work.
> I think a nil *app.Activity is most appropriate. Additionally, Run
> should run on the main (UI) thread, to avoid races with e.g. recreation
> of the Activity. Being on the main thread is probably most convenient
> for the code needing the Activity handle anyway.

Sounds right. An alternative is to queue up pending Run callbacks and 
trigger them at the next onStart(), but this is probably not what most 
people are going to want -- libraries will have to be lifecycle aware 
and restart themselves at the appropriate time.

> However, to avoid deadlocks where Run is called while your app is
> processing a system event, it should *not* block on the completion of
> the handle function.

Sounds right. My thinking now is that we can add a public void Run(long 
ptr) method to GioView, taking a function pointer, which posts a go 
callback to the main UI thread's Handler. This go callback (implemented 
in internal/window/os_android.c) takes a pointer to Activity and calls 
the function pointer with it, along with a *JNIEnv, which we already 
have from the JNI C callback.

GioView will then keep a reference to Activity, which it will save in 
its onStart(Activity) and set to null in onStop() (not sure if these 
should also happen in onPause() and onResume() -- that would prevent 
libraries from calling Run() when the application is in the background 
but otherwise still operational). Is there a race where onStop() is 
called on a different thread or process from the main UI thread (i.e. 
GioView's Activity field is getting set to null while we are trying to 
call back into Go with it).

> BTW, because of build tags, Run can be a method on Window:
Right now window.Driver is an interface, but it will need a Run() method 
to make this work. This will mean having a type-assertion from 
window.Driver to *window in internal/window/os_android.go.
> type Activity struct {
> 	Activity uintptr
> }

not just

type Activity uintptr

?

Also starting to get concerned about circular imports here unless 
Activity is defined in the internal package.

> // Run a function that needs access to the Android Activity for the
> // window. The function is run on the UI thread and should
> // not block. The function must be prepared to deal with a nil
> // activity if none is available.
> // Run returns immediately and does not wait for the
> // completion of f.
> func (w *Window) Run(f func(a *Activity))

I would like to change to:

func (w *Window) Run(f func(a *Activity, env JNIEnv))

We are getting a JNIEnv for free (with the proposal above) and this will 
save the user from having to ask for a new one from a JVM.

Am I on the right track?
Details
Message ID
<BYJ4967EUO20.2ORGIK0BBOQQK@testmac>
In-Reply-To
<d9713cb8-2675-a508-ddba-1f8770dcb335@wow.st> (view parent)
DKIM signature
pass
Download raw message
On Mon Nov 18, 2019 at 9:23 AM Gregory Pomerantz wrote:
> On 11/17/19 5:10 AM, Elias Naur wrote:
> 
> My thinking now is that we can add a public void Run(long 
> ptr) method to GioView, taking a function pointer, which posts a go 
> callback to the main UI thread's Handler. This go callback (implemented 
> in internal/window/os_android.c) takes a pointer to Activity and calls 
> the function pointer with it, along with a *JNIEnv, which we already 
> have from the JNI C callback.
> 

You can't pass function pointers across the language barrier so I can't
see how your proposed solution would work. Rather, I'd expect a single
onUIThread callback from Java to Go, which can be provoked from Go. Then
Run would be:

	var uiThread struct {
		mu sync.Mutext
		funcs []func(...)
	}

	func Run(f func(...)) {
		uiThread.mu.Lock()
		defer uiThread.mu.Unlock()
		uiThread.funcs = append(uiThreads.funcs, f)
		// Posts a Runnable to the UI Thread Handler on the Java side.
		wakeupUIThread()
	}

and onUIThread (called from the wakeup Runnable above).

	// export onUIThread
	func onUIThread(env *C.JNIEnv, ctx C.jobject) {
		uiThread.mu.Lock()
		funcs := uiThread.funcs
		uiThread.funcs = nil
		uiThread.mu.Unlock()
		for _, f := range funcs {
			f(...)
		}
	}

> GioView will then keep a reference to Activity, which it will save in 
> its onStart(Activity) and set to null in onStop() (not sure if these 
> should also happen in onPause() and onResume() -- that would prevent 
> libraries from calling Run() when the application is in the background 
> but otherwise still operational). Is there a race where onStop() is 
> called on a different thread or process from the main UI thread (i.e. 
> GioView's Activity field is getting set to null while we are trying to 
> call back into Go with it).
> 

onStop etc. are always called from the UI thread, so there is no race.

Android's View has a getContext method. Perhaps we should just use that
and rename our app.Activity to app.Context? We're then prepared for when
GioView is embedded in a foreign context in the future.

> > BTW, because of build tags, Run can be a method on Window:
> Right now window.Driver is an interface, but it will need a Run() method 
> to make this work. This will mean having a type-assertion from 
> window.Driver to *window in internal/window/os_android.go.
> > type Activity struct {
> > 	Activity uintptr
> > }
> 
> not just
> 
> type Activity uintptr
> 
> ?

Then its easier to miss the "no current activity" case. It's better to
force the user to dereference Activity/Context.

> 
> Also starting to get concerned about circular imports here unless 
> Activity is defined in the internal package.
> 

That, or you could get away with the internal Run being just

func (w *Window) Run(f func(env uintptr, ctx uintptr))

and then do the wrapping into Context in the app package.

> > // Run a function that needs access to the Android Activity for the
> > // window. The function is run on the UI thread and should
> > // not block. The function must be prepared to deal with a nil
> > // activity if none is available.
> > // Run returns immediately and does not wait for the
> > // completion of f.
> > func (w *Window) Run(f func(a *Activity))
> 
> I would like to change to:
> 
> func (w *Window) Run(f func(a *Activity, env JNIEnv))
> 
> We are getting a JNIEnv for free (with the proposal above) and this will 
> save the user from having to ask for a new one from a JVM.
> 

Sure, add JNIEnv for convenience, but as the first argument like C JNI
functions.
Gregory Pomerantz
Details
Message ID
<accd176f-dbe6-43eb-2f66-efb1cd5fa2bb@wow.st>
In-Reply-To
<BYJ4967EUO20.2ORGIK0BBOQQK@testmac> (view parent)
DKIM signature
pass
Download raw message
On 11/18/19 10:09 AM, Elias Naur wrote:
> You can't pass function pointers across the language barrier so I can't
> see how your proposed solution would work. Rather, I'd expect a single
> onUIThread callback from Java to Go, which can be provoked from Go. Then
We can't use a function pointer from Java, but we can round-trip it 
through Java and back to C. If a C pointer does not always necessarily 
map to a Java long, we can marshal it into a byte array of an 
appropriate length and round trip it back to C that way. This would be 
nicer, I think, than using a mutex?
> onStop etc. are always called from the UI thread, so there is no race.
Perfect, that's what I was hoping.
> Android's View has a getContext method. Perhaps we should just use that
> and rename our app.Activity to app.Context? We're then prepared for when
> GioView is embedded in a foreign context in the future.
This doesn't help for libraries that need Activity -- you can't add a 
fragment to a context. That said, we can have the library "try" to get a 
FragmentManager from the Context, which should work if the Context is 
actually an Activity (as will be the case in the current implementation).
> Then its easier to miss the "no current activity" case. It's better to
> force the user to dereference Activity/Context.

sounds good.
Gregory Pomerantz
Details
Message ID
<25ea8d5c-4834-1af6-c348-d1fb91756d88@wow.st>
In-Reply-To
<accd176f-dbe6-43eb-2f66-efb1cd5fa2bb@wow.st> (view parent)
DKIM signature
pass
Download raw message
So I sent a first draft, this is now working with my openpgp-api 
connector. One downside as compared to making Activity directly 
accessible is that the Gio developer needs to track StageEvents and pass 
along the app.WIndow pointer to the libraries that need it after the 
Android window driver is available. One nice-to-have alternative would 
be a registration mechanism, where a library can call 
app.Register(func(system.Event?)) to be notified of lifecycle changes.
> We can't use a function pointer from Java, but we can round-trip it 
> through Java and back to C. If a C pointer does not always necessarily 
> map to a Java long, we can marshal it into a byte array of an 
> appropriate length and round trip it back to C that way. This would be 
> nicer, I think, than using a mutex?
I am currently using a Java long, which is always 64 bits. Ugly perhaps 
but this should work on any platform where function pointers are 64 bits 
or shorter. Is there any supported platform where that is not the case? 
Does Android run on the AS/400?
>> onStop etc. are always called from the UI thread, so there is no race.
> Perfect, that's what I was hoping.
I am concerned that the Go function calls in the callback path will give 
the Go runtime the opportunity to change threads or processes via the 
scheduler. I'm not confident that we can guarantee the user code is 
still going to be on the main UI thread. Any thoughts?
>> Android's View has a getContext method. Perhaps we should just use that
>> and rename our app.Activity to app.Context? We're then prepared for when
>> GioView is embedded in a foreign context in the future.
> This doesn't help for libraries that need Activity -- you can't add a 
> fragment to a context. That said, we can have the library "try" to get 
> a FragmentManager from the Context, which should work if the Context 
> is actually an Activity (as will be the case in the current 
> implementation).
I have GioView storing the Activity directly, but we can have it store 
the context, and in the Run method, send null to the callback if Context 
is not an instanceof Activity. Let me know if that is preferable.
Details
Message ID
<BYJUJVHDG3J1.1NW7YD7ZPBZHQ@toolbox>
In-Reply-To
<25ea8d5c-4834-1af6-c348-d1fb91756d88@wow.st> (view parent)
DKIM signature
pass
Download raw message
On Mon Nov 18, 2019 at 7:21 PM Gregory Pomerantz wrote:
> So I sent a first draft, this is now working with my openpgp-api 
> connector. One downside as compared to making Activity directly 
> accessible is that the Gio developer needs to track StageEvents and pass 
> along the app.WIndow pointer to the libraries that need it after the 
> Android window driver is available. One nice-to-have alternative would 
> be a registration mechanism, where a library can call 
> app.Register(func(system.Event?)) to be notified of lifecycle changes.


Interesting thought. Let's move all this complexity to Java where it belongs,
simplifying both Gio and your custom Android code.

So: get rid of Run, PlatformHandle etc. and then

// +build android

package app

// RegisterDelegate constructs a Java instance of the specified class and calls
// its public init(java.content.Context) method with the Android application
// context.
func RegisterDelegate(className string) error

// RegisterDelegate constructs a Java instance of the specified class,
// and calls its public initView(java.content.Context) method with the
// window Android context.
// 
// If the delegate class defines a method with the signature
//
//   public boolean onActivityResult(...?)
//
// it is invoked whenever the window's corresponding onActivityResult is
// called with an unrecognized request code.
func (w *Window) RegisterDelegate(className string) error

Both the RegisterDelegate function and method pass the class names to GioView
which then takes care of construction and callbacks.

WDYT?

> > We can't use a function pointer from Java, but we can round-trip it 
> > through Java and back to C. If a C pointer does not always necessarily 
> > map to a Java long, we can marshal it into a byte array of an 
> > appropriate length and round trip it back to C that way. This would be 
> > nicer, I think, than using a mutex?
> I am currently using a Java long, which is always 64 bits. Ugly perhaps 
> but this should work on any platform where function pointers are 64 bits 
> or shorter. Is there any supported platform where that is not the case? 
> Does Android run on the AS/400?

The problem is that it's against Cgo rules to hold on to a Go pointer on the
C side after a Cgo function has returned. That's exactly what Run is doing:
kick off a UI thread callback, then return, not waiting for the callback.

	> C code may not keep a copy of a Go pointer after the call returns.

	(https://golang.org/cmd/cgo/)

Further, you may not pass a Go value containing Go pointers to C. That
includes function pointers (emphasis mine):

	 > Note that values of some Go types, other than the type's zero value,
	 > always include Go pointers. This is true of string, slice,
	 > interface, channel, map, and *function* types. A pointer type may hold
	 > a Go pointer or a C pointer. Array and struct types may or may not
	 > include Go pointers, depending on the element types. All the
	 > discussion below about Go pointers applies not just to pointer
	 > types, but also to other types that include Go pointers.

	> Go code may pass a Go pointer to C provided the Go memory to which it
	> points does not contain any Go pointers.

> >> onStop etc. are always called from the UI thread, so there is no race.
> > Perfect, that's what I was hoping.
> I am concerned that the Go function calls in the callback path will give 
> the Go runtime the opportunity to change threads or processes via the 
> scheduler. I'm not confident that we can guarantee the user code is 
> still going to be on the main UI thread. Any thoughts?

Cgo does an implicit LockOSThread on callbacks from Cgo, so this is not a
concern. And even if Cgo didn't, the Cgo callback from the UI thread would
certainly not be allowed to return before the Go call returned.

> >> Android's View has a getContext method. Perhaps we should just use that
> >> and rename our app.Activity to app.Context? We're then prepared for when
> >> GioView is embedded in a foreign context in the future.
> > This doesn't help for libraries that need Activity -- you can't add a 
> > fragment to a context. That said, we can have the library "try" to get 
> > a FragmentManager from the Context, which should work if the Context 
> > is actually an Activity (as will be the case in the current 
> > implementation).
> I have GioView storing the Activity directly, but we can have it store 
> the context, and in the Run method, send null to the callback if Context 
> is not an instanceof Activity. Let me know if that is preferable.
> 

Why not use the getContext method by itself? It's always set to the current
context, and users can always cast it to Activity if they need to.

-- elias
Gregory Pomerantz
Details
Message ID
<4ebe52b9-ba74-ebb7-e2e5-06a77963af54@wow.st>
In-Reply-To
<BYJUJVHDG3J1.1NW7YD7ZPBZHQ@toolbox> (view parent)
DKIM signature
pass
Download raw message
On 11/19/19 6:45 AM, Elias Naur wrote:

> Interesting thought. Let's move all this complexity to Java where it belongs,
> simplifying both Gio and your custom Android code.
>
> So: get rid of Run, PlatformHandle etc. and then
>
> // +build android
>
> package app
>
> // RegisterDelegate constructs a Java instance of the specified class and calls
> // its public init(java.content.Context) method with the Android application
> // context.
> func RegisterDelegate(className string) error

Does the init() method get the applicationContext (as sent to 
initialize()), or the Context sent to GioView (i.e. the Activity under 
the current structure)?

My thinking was more that we want the library developers to be able to 
find out all they need to know about the application lifecycle without 
requiring the Gio programmer to do anything (like forget to update the 
library when the view is stopped and re-started, which will result in 
Android bugs even though every other platform works fine).

Something like this:

type LifecycleUpdate struct {

     View uintptr

     //this doesn't quite do it because we will want to know about 
onStop()...

     Stage system.Stage

}

func app.SubscribeLifecycle() chan LifecycleUpdate {

     ...

}

We can make a rule that this needs to be called before the first Window 
is instantiated (e.g. in a library's init()) and panic if it is called 
after that.

Given this, and the Run() method (or some other way of getting to the 
Activity that launched the View), we can allow library devs to handle 
everything lifecycle-related and plug in their own Fragments as needed. 
The library user only needs to import "openpgp-api" and call Encrypt() 
and Decrypt() whenever they want (which will return errors if the app is 
not in a state where these calls can be completed successfully).


> // RegisterDelegate constructs a Java instance of the specified class,
> // and calls its public initView(java.content.Context) method with the
> // window Android context.
> //
> // If the delegate class defines a method with the signature
> //
> //   public boolean onActivityResult(...?)
> //
> // it is invoked whenever the window's corresponding onActivityResult is
> // called with an unrecognized request code.
> func (w *Window) RegisterDelegate(className string) error
>
> Both the RegisterDelegate function and method pass the class names to GioView
> which then takes care of construction and callbacks.
>
> WDYT?


Under this approach, the caller doesn't get back an instance of the 
class, so they will not be able to call methods directly on it from Go 
or CGo code. So it might be better to require the user to instantiate 
the object and pass a uintptr, or to return a uintptr pointing to the 
instance. That said the library can work around this by having the 
delegate call back into Go to register a reference to itself with the 
library's Go code.

I think this works, as the library code should be able to call 
Activity#startIntentSenderForResult and have the activity receive the 
results. With this approach, the main Activity needs to route all of the 
ActivityResults back to the delegates. GioActivity will need to send all 
ActivityResults back through GioView, or otherwise maintain references 
to all of the delegates, and figure out whether they have an 
onActivityResult method or not. This also leaves the possibility of two 
different libraries sending intents with the same requestCode (not sure 
how risky this is in practice as it may be unlikely that there is more 
than one live Intent waiting for a result). Fragments allow total 
segregation as far as I can tell, where a Fragment only gets results 
from intents that it sends itself, even if there are multiple Fragments 
on the same Activity. This relieves us of having to track delegates, 
figure out if they have an onActivityResult method, and route results 
back to them.


> The problem is that it's against Cgo rules to hold on to a Go pointer on the
> C side after a Cgo function has returned. That's exactly what Run is doing:
> kick off a UI thread callback, then return, not waiting for the callback.


Aha. Thanks for all of these details. I was focused on whether this 
could be made to work within the unsafe package. So the registry with a 
mutex approach is preferred.


> Why not use the getContext method by itself? It's always set to the current
> context, and users can always cast it to Activity if they need to.

Since Run() is supposed to return an Activity or nil if one is not 
available, I thought it better to return nil when the current Context is 
not an instance of the Activity class (not sure how or when this would 
come up but it is possible if GioView is instantiated by something other 
than an Activity). That said, as GioView is currently always 
instantiated from an Activity, we can just track a reference to that 
instead. We could change it if there is ever going to be another way to 
make a GioView.
Details
Message ID
<BYK682YK3PW0.FFANVA5X416H@testmac>
In-Reply-To
<4ebe52b9-ba74-ebb7-e2e5-06a77963af54@wow.st> (view parent)
DKIM signature
pass
Download raw message
On Tue Nov 19, 2019 at 9:16 AM Gregory Pomerantz wrote:
> On 11/19/19 6:45 AM, Elias Naur wrote:
> 
> > Interesting thought. Let's move all this complexity to Java where it belongs,
> > simplifying both Gio and your custom Android code.
> >
> > So: get rid of Run, PlatformHandle etc. and then
> >
> > // +build android
> >
> > package app
> >
> > // RegisterDelegate constructs a Java instance of the specified class and calls
> > // its public init(java.content.Context) method with the Android application
> > // context.
> > func RegisterDelegate(className string) error
> 
> Does the init() method get the applicationContext (as sent to 
> initialize()), or the Context sent to GioView (i.e. the Activity under 
> the current structure)?
> 

The applicationContext.

But only add RegisterDelegate if you need it. It sounds like you may
only need Window.RegisterDelegate.

> My thinking was more that we want the library developers to be able to 
> find out all they need to know about the application lifecycle without 
> requiring the Gio programmer to do anything (like forget to update the 
> library when the view is stopped and re-started, which will result in 
> Android bugs even though every other platform works fine).
> 

You can add onStop and onStart callbacks to the class passed to
Window.RegisterDelegate. Or the delegate can add a tracking Fragment to
the Activity. Either way, it's all kept at the Java side.

> Something like this:
> 
> type LifecycleUpdate struct {
> 
>      View uintptr
> 
>      //this doesn't quite do it because we will want to know about 
> onStop()...
> 
>      Stage system.Stage
> 
> }
> 
> func app.SubscribeLifecycle() chan LifecycleUpdate {
> 
>      ...
> 
> }
> 

What does this API solve that a window delegate can't do?

> We can make a rule that this needs to be called before the first Window 
> is instantiated (e.g. in a library's init()) and panic if it is called 
> after that.
> 
> Given this, and the Run() method (or some other way of getting to the 
> Activity that launched the View), we can allow library devs to handle 
> everything lifecycle-related and plug in their own Fragments as needed. 
> The library user only needs to import "openpgp-api" and call Encrypt() 
> and Decrypt() whenever they want (which will return errors if the app is 
> not in a state where these calls can be completed successfully).
> 

I don't think it's too onerous to require you to pass an app.Window to
the Encrypt and Decrypt functions. They only work when the window is in
an appropriate state anyway.

> 
> > // RegisterDelegate constructs a Java instance of the specified class,
> > // and calls its public initView(java.content.Context) method with the
> > // window Android context.
> > //
> > // If the delegate class defines a method with the signature
> > //
> > //   public boolean onActivityResult(...?)
> > //
> > // it is invoked whenever the window's corresponding onActivityResult is
> > // called with an unrecognized request code.
> > func (w *Window) RegisterDelegate(className string) error
> >
> > Both the RegisterDelegate function and method pass the class names to GioView
> > which then takes care of construction and callbacks.
> >
> > WDYT?
> 
> 
> Under this approach, the caller doesn't get back an instance of the 
> class, so they will not be able to call methods directly on it from Go 
> or CGo code. So it might be better to require the user to instantiate 
> the object and pass a uintptr, or to return a uintptr pointing to the 
> instance. That said the library can work around this by having the 
> delegate call back into Go to register a reference to itself with the 
> library's Go code.
> 

Yes.

> I think this works, as the library code should be able to call 
> Activity#startIntentSenderForResult and have the activity receive the 
> results. With this approach, the main Activity needs to route all of the 
> ActivityResults back to the delegates. GioActivity will need to send all 
> ActivityResults back through GioView, or otherwise maintain references 
> to all of the delegates, and figure out whether they have an 
> onActivityResult method or not. This also leaves the possibility of two 
> different libraries sending intents with the same requestCode (not sure 
> how risky this is in practice as it may be unlikely that there is more 
> than one live Intent waiting for a result). Fragments allow total 
> segregation as far as I can tell, where a Fragment only gets results 
> from intents that it sends itself, even if there are multiple Fragments 
> on the same Activity. This relieves us of having to track delegates, 
> figure out if they have an onActivityResult method, and route results 
> back to them.
> 

Fragments are a tempting solution. So perhaps Window.RegisterDelegate
should simply call DelegateClass.init(Context activityContext), and
not bother with the lifecycle and onActivityResult callbacks.
Gregory Pomerantz
Details
Message ID
<97b8b4b5-4975-8fab-9cd8-e704f8e0080c@wow.st>
In-Reply-To
<BYK682YK3PW0.FFANVA5X416H@testmac> (view parent)
DKIM signature
pass
Download raw message
On 11/19/19 3:54 PM, Elias Naur wrote:
> On Tue Nov 19, 2019 at 9:16 AM Gregory Pomerantz wrote:
> But only add RegisterDelegate if you need it. It sounds like you may
> only need Window.RegisterDelegate.


Got it, I should be able to do everything with 
Window.RegisterDelegate(). I believe the actual registration will need 
to occur later when the Activity becomes available, in 
internal/window.onCreateView().


> What does this API solve that a window delegate can't do?


This lets libraries get notified if new windows are created and 
destroyed, and they can keep track of the active window. They don't need 
to be manually given a window first. Otherwise the app developer needs 
to manually register the library with every window that it might need to 
get registered with. We don't need it if we enforce manual registration. 
We can go with that for now and see if it becomes a problem.


> I don't think it's too onerous to require you to pass an app.Window to
> the Encrypt and Decrypt functions. They only work when the window is in
> an appropriate state anyway.


I'd like to be able to do this with a single RegisterDelegate() call 
right after we create the window. The window will keep track of what has 
been registered and make sure it gets re-registered if the Activity is 
destroyed and re-created.


>> Under this approach, the caller doesn't get back an instance of the
>> class, so they will not be able to call methods directly on it from Go
>> or CGo code. So it might be better to require the user to instantiate
>> the object and pass a uintptr, or to return a uintptr pointing to the
>> instance. That said the library can work around this by having the
>> delegate call back into Go to register a reference to itself with the
>> library's Go code.
> Yes.

On further thought, the Java class will likely need to have to call back 
into the library's Go code anyway to notify the library that it has been 
installed and has obtained all of the Java resources it needs, so I'm 
happy to have a string passed here. It avoids some JNI calls on the 
library's part.


> Fragments are a tempting solution. So perhaps Window.RegisterDelegate
> should simply call DelegateClass.init(Context activityContext), and
> not bother with the lifecycle and onActivityResult callbacks.


Yes this works for now, why don't we keep it simple? Some libraries will 
only need one-time access to the Activity (or can cache a pointer on the 
Go side via a callback as mentioned above). Others can install a 
Fragment, so there is no need for the lifecycle plumbing within Gio.