[mobile] app, cmd/gomobile: subclass NativeActivity

84 views
Skip to first unread message

David Crawshaw (Gerrit)

unread,
Jul 8, 2015, 1:55:14 PM7/8/15
to Hyang-Ah Hana Kim, Ian Lance Taylor, golang-co...@googlegroups.com
Reviewers: Hyang-Ah Hana Kim

David Crawshaw uploaded a change:
https://go-review.googlesource.com/11980

app, cmd/gomobile: subclass NativeActivity

Subclassing NativeActivity makes two things possible. Firstly, we can
implement an InputConnection to offer good support for IMEs,
necessary for good keyboard support. Secondly, we can use it to
overlay WebViews onto the NativeActivity.

But to sublcass NativeActivity, we need to compile Java. To keep the
toolchain go gettable, this is done with go generate.

While here, check the exception after FindClass. Apparently it can
throw an exception.

Updates golang/go#9361.
Updates golang/go#10247.

Change-Id: I672545997f0c9a7580f06988a273c03404772247
---
A app/GoNativeActivity.java
M app/android.c
M app/android.go
M cmd/gomobile/build.go
A cmd/gomobile/dex.go
M cmd/gomobile/doc.go
A cmd/gomobile/gendex.go
M cmd/gomobile/manifest.go
M cmd/gomobile/writer.go
9 files changed, 290 insertions(+), 79 deletions(-)



diff --git a/app/GoNativeActivity.java b/app/GoNativeActivity.java
new file mode 100644
index 0000000..4a1234b
--- /dev/null
+++ b/app/GoNativeActivity.java
@@ -0,0 +1,51 @@
+package org.golang.app;
+
+import android.app.Activity;
+import android.app.NativeActivity;
+import android.content.Context;
+import android.content.pm.ActivityInfo;
+import android.content.pm.PackageManager;
+import android.os.Bundle;
+import android.util.Log;
+
+public class GoNativeActivity extends NativeActivity {
+ private static GoNativeActivity goNativeActivity;
+
+ public GoNativeActivity() {
+ super();
+ goNativeActivity = this;
+ }
+
+ String getTmpdir() {
+ return getCacheDir().getAbsolutePath();
+ }
+
+ private void load() {
+ // Interestingly, NativeActivity uses a different method
+ // to find native code to execute, avoiding
+ // System.loadLibrary. The result is Java methods
+ // implemented in C with JNIEXPORT (and JNI_OnLoad) are not
+ // available unless an explicit call to System.loadLibrary
+ // is done. So we do it here, borrowing the name of the
+ // library from the same AndroidManifest.xml metadata used
+ // by NativeActivity.
+ try {
+ ActivityInfo ai = getPackageManager().getActivityInfo(
+ getIntent().getComponent(), PackageManager.GET_META_DATA);
+ if (ai.metaData == null) {
+ Log.e("Go", "loadLibrary: no manifest metadata found");
+ return;
+ }
+ String libName = ai.metaData.getString("android.app.lib_name");
+ System.loadLibrary(libName);
+ } catch (Exception e) {
+ Log.e("Go", "loadLibrary failed", e);
+ }
+ }
+
+ @Override
+ public void onCreate(Bundle savedInstanceState) {
+ load();
+ super.onCreate(savedInstanceState);
+ }
+}
diff --git a/app/android.c b/app/android.c
index 6f7b6e2..fa21127 100644
--- a/app/android.c
+++ b/app/android.c
@@ -20,7 +20,8 @@

static jclass find_class(JNIEnv *env, const char *class_name) {
jclass clazz = (*env)->FindClass(env, class_name);
- if (clazz == NULL) {
+ if (clazz == NULL || (*env)->ExceptionCheck(env)) {
+ (*env)->ExceptionClear(env);
LOG_FATAL("cannot find %s", class_name);
return NULL;
}
@@ -29,7 +30,8 @@

static jmethodID find_method(JNIEnv *env, jclass clazz, const char *name,
const char *sig) {
jmethodID m = (*env)->GetMethodID(env, clazz, name, sig);
- if (m == 0) {
+ if (m == 0 || (*env)->ExceptionCheck(env)) {
+ (*env)->ExceptionClear(env);
LOG_FATAL("cannot find method %s %s", name, sig);
return 0;
}
@@ -42,92 +44,41 @@
return -1;
}

+ // Load classes here, which uses the correct ClassLoader.
current_vm = vm;
current_ctx = NULL;
+ current_ctx_clazz = find_class(env, "org/golang/app/GoNativeActivity");
+ current_ctx_clazz = (jclass)(*env)->NewGlobalRef(env, current_ctx_clazz);

return JNI_VERSION_1_6;
}

-static void init_from_context() {
- if (current_ctx == NULL) {
- return;
+// Entry point from our subclassed NativeActivity.
+//
+// By here, the Go runtime has been initialized (as we are running in
+// -buildmode=c-shared) but main.main hasn't been called yet.
+void ANativeActivity_onCreate(ANativeActivity *activity, void* savedState,
size_t savedStateSize) {
+ JNIEnv* env = activity->env;
+
+ // Note that activity->clazz is mis-named.
+ current_vm = activity->vm;
+ current_ctx = (*env)->NewGlobalRef(env, activity->clazz);
+
+ // Set TMPDIR.
+ jmethodID gettmpdir = find_method(env,
current_ctx_clazz, "getTmpdir", "()Ljava/lang/String;");
+ jstring jpath = (jstring)(*env)->CallObjectMethod(env, current_ctx,
gettmpdir, NULL);
+ const char* tmpdir = (*env)->GetStringUTFChars(env, jpath, NULL);
+ if (setenv("TMPDIR", tmpdir, 1) != 0) {
+ LOG_INFO("setenv(\"TMPDIR\", \"%s\", 1) failed: %d", tmpdir, errno);
}
+ (*env)->ReleaseStringUTFChars(env, jpath, tmpdir);

- int attached = 0;
- JNIEnv* env;
- switch ((*current_vm)->GetEnv(current_vm, (void**)&env, JNI_VERSION_1_6))
{
- case JNI_OK:
- break;
- case JNI_EDETACHED:
- if ((*current_vm)->AttachCurrentThread(current_vm, &env, 0) != 0) {
- LOG_FATAL("cannot attach JVM");
- }
- attached = 1;
- break;
- case JNI_EVERSION:
- LOG_FATAL("bad JNI version");
- }
-
- // String path = context.getCacheDir().getAbsolutePath();
- jclass context_clazz = find_class(env, "android/content/Context");
- jmethodID getcachedir = find_method(env,
context_clazz, "getCacheDir", "()Ljava/io/File;");
- jobject file = (*env)->CallObjectMethod(env, current_ctx, getcachedir,
NULL);
- jclass file_clazz = find_class(env, "java/io/File");
- jmethodID getabsolutepath = find_method(env,
file_clazz, "getAbsolutePath", "()Ljava/lang/String;");
- jstring jpath = (jstring)(*env)->CallObjectMethod(env, file,
getabsolutepath, NULL);
- const char* path = (*env)->GetStringUTFChars(env, jpath, NULL);
- if (setenv("TMPDIR", path, 1) != 0) {
- LOG_INFO("setenv(\"TMPDIR\", \"%s\", 1) failed: %d", path, errno);
- }
- (*env)->ReleaseStringUTFChars(env, jpath, path);
-
- if (attached) {
- (*current_vm)->DetachCurrentThread(current_vm);
- }
-}
-
-// has_prefix_key returns 1 if s starts with prefix.
-static int has_prefix(const char *s, const char* prefix) {
- while (*prefix) {
- if (*prefix++ != *s++)
- return 0;
- }
- return 1;
-}
-
-// getenv_raw searches environ for name prefix and returns the string pair.
-// For example, getenv_raw("PATH=") returns "PATH=/bin".
-// If no entry is found, the name prefix is returned. For example "PATH=".
-static const char* getenv_raw(const char *name) {
- extern char** environ;
- char** env = environ;
-
- for (env = environ; *env; env++) {
- if (has_prefix(*env, name)) {
- return *env;
- }
- }
- return name;
-}
-
-static void* call_main_and_wait() {
- init_from_context();
+ // Call the Go main.main.
uintptr_t mainPC = (uintptr_t)dlsym(RTLD_DEFAULT, "main.main");
if (!mainPC) {
LOG_FATAL("missing main.main");
}
callMain(mainPC);
-}
-
-// Entry point from NativeActivity.
-//
-// By here, the Go runtime has been initialized (as we are running in
-// -buildmode=c-shared) but main.main hasn't been called yet.
-void ANativeActivity_onCreate(ANativeActivity *activity, void* savedState,
size_t savedStateSize) {
- // Note that activity->clazz is mis-named.
- current_vm = activity->vm;
- current_ctx = (*activity->env)->NewGlobalRef(activity->env,
activity->clazz);
- call_main_and_wait();

// These functions match the methods on Activity, described at
// http://developer.android.com/reference/android/app/Activity.html
diff --git a/app/android.go b/app/android.go
index 6290c88..8e18c49 100644
--- a/app/android.go
+++ b/app/android.go
@@ -45,6 +45,8 @@
// current_ctx is Android's android.context.Context. May be NULL.
jobject current_ctx;

+jclass current_ctx_clazz;
+
jclass app_find_class(JNIEnv* env, const char* name);
*/
import "C"
diff --git a/cmd/gomobile/build.go b/cmd/gomobile/build.go
index e2f53de..f3c6e54 100644
--- a/cmd/gomobile/build.go
+++ b/cmd/gomobile/build.go
@@ -2,17 +2,21 @@
// Use of this source code is governed by a BSD-style
// license that can be found in the LICENSE file.

+//go:generate go run gendex.go -o dex.go
+
package main

import (
"bytes"
"crypto/x509"
+ "encoding/base64"
"encoding/pem"
"errors"
"fmt"
"go/build"
"io"
"io/ioutil"
+ "log"
"os"
"os/exec"
"path"
@@ -207,6 +211,18 @@
return err
}

+ w, err = apkwcreate("classes.dex")
+ if err != nil {
+ return err
+ }
+ dexData, err := base64.StdEncoding.DecodeString(dexStr)
+ if err != nil {
+ log.Fatal("internal error: bad dexStr")
+ }
+ if _, err := w.Write(dexData); err != nil {
+ return err
+ }
+
w, err = apkwcreate("lib/armeabi/lib" + libName + ".so")
if err != nil {
return err
diff --git a/cmd/gomobile/dex.go b/cmd/gomobile/dex.go
new file mode 100644
index 0000000..b1a191a
--- /dev/null
+++ b/cmd/gomobile/dex.go
@@ -0,0 +1,44 @@
+// Copyright 2015 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+// File is automatically generated by gendex.go. DO NOT EDIT.
+
+package main
+
+var dexStr =
`ZGV4CjAzNQBgMPcbh5xnMiJMhJmv7LSwWLsd8n8QqVIUBwAAcAAAAHhWNBIAAAAAAAAAAH` +
+ `QGAAApAAAAcAAAAA8AAAAUAQAADAAAAFABAAACAAAA4AEAABAAAADwAQAAAQAAAHACAACE` +
+ `BAAAkAIAAJ4DAACmAwAAqgMAAMEDAADEAwAAyQMAAM8DAADSAwAA1gMAANsDAAD5AwAAGg` +
+ `QAADQEAABXBAAAfAQAAJEEAAClBAAAtQQAAMwEAADgBAAA9AQAAAsFAAAuBQAAMQUAADUF` +
+ `AABLBQAATgUAAF8FAABwBQAAfQUAAIsFAACWBQAAqQUAALQFAAC/BQAA0QUAANcFAADkBQ` +
+ `AA+AUAACEGAAArBgAAAwAAAAkAAAAKAAAACwAAAAwAAAANAAAADgAAAA8AAAAQAAAAEQAA` +
+ `ABIAAAATAAAAFAAAABUAAAAWAAAABAAAAAAAAAB0AwAABQAAAAAAAAB8AwAABgAAAAIAAA` +
+ `AAAAAABgAAAAMAAAAAAAAACAAAAAQAAACIAwAABgAAAAUAAAAAAAAABgAAAAgAAAAAAAAA` +
+ `BgAAAAoAAAAAAAAABwAAAAoAAACQAwAAFgAAAA4AAAAAAAAAFwAAAA4AAACYAwAAFwAAAA` +
+ `4AAACQAwAABAAGACcAAAANAA0AIgAAAAEACQAAAAAAAQAKACgAAAADAAIAHQAAAAUABAAb` +
+ `AAAABgAIACAAAAAHAAAAGQAAAAcAAQAZAAAACAAHABoAAAALAAsAJAAAAA0ACQAAAAAADQ` +
+ `AGABwAAAANAAMAHgAAAA0ABQAfAAAADQAHACEAAAANAAkAIwAAAA0ACgAoAAAADQAAAAEA` +
+ `AAABAAAAAAAAAAIAAAAAAAAAWQYAAAAAAAABAAEAAQAAADUGAAAGAAAAcBAAAAAAaQABAA` +
+ `4ABAABAAMAAQA8BgAAMwAAAG4QDAADAAwAbhALAAMADAFuEAIAAQAMARMCgABuMAMAEAIM` +
+ `AFQBAAA5AQoAGgABABoBJgBxIAUAEAAOAFQAAAAaARgAbiAEABAADABxEAgAAAAo9A0AGg` +
+ `EBABoCJQBxMAYAIQAo6wAAAAAAACkAAQABAQkqAgABAAEAAABMBgAACQAAAG4QCgABAAwA` +
+ `bhAHAAAADAARAAAAAgACAAIAAABRBgAABwAAAHAQDgAAAG8gAQAQAA4AAAACAAAACgAKAA` +
+ `MAAAAKAAoADAAAAAIAAAACAAAAAQAAAAoAAAABAAAABgAGPGluaXQ+AAJHbwAVR29OYXRp` +
+ `dmVBY3Rpdml0eS5qYXZhAAFJAANJTEwABElMTEwAAUwAAkxMAANMTEkAHExhbmRyb2lkL2` +
+ `FwcC9OYXRpdmVBY3Rpdml0eTsAH0xhbmRyb2lkL2NvbnRlbnQvQ29tcG9uZW50TmFtZTsA` +
+ `GExhbmRyb2lkL2NvbnRlbnQvSW50ZW50OwAhTGFuZHJvaWQvY29udGVudC9wbS9BY3Rpdm` +
+ `l0eUluZm87ACNMYW5kcm9pZC9jb250ZW50L3BtL1BhY2thZ2VNYW5hZ2VyOwATTGFuZHJv` +
+ `aWQvb3MvQnVuZGxlOwASTGFuZHJvaWQvdXRpbC9Mb2c7AA5MamF2YS9pby9GaWxlOwAVTG` +
+ `phdmEvbGFuZy9FeGNlcHRpb247ABJMamF2YS9sYW5nL1N0cmluZzsAEkxqYXZhL2xhbmcv` +
+ `U3lzdGVtOwAVTGphdmEvbGFuZy9UaHJvd2FibGU7ACFMb3JnL2dvbGFuZy9hcHAvR29OYX` +
+ `RpdmVBY3Rpdml0eTsAAVYAAlZMABRhbmRyb2lkLmFwcC5saWJfbmFtZQABZQAPZ2V0QWJz` +
+ `b2x1dGVQYXRoAA9nZXRBY3Rpdml0eUluZm8AC2dldENhY2hlRGlyAAxnZXRDb21wb25lbn` +
+ `QACWdldEludGVudAARZ2V0UGFja2FnZU1hbmFnZXIACWdldFN0cmluZwAJZ2V0VG1wZGly` +
+ `ABBnb05hdGl2ZUFjdGl2aXR5AARsb2FkAAtsb2FkTGlicmFyeQASbG9hZExpYnJhcnkgZm` +
+ `FpbGVkACdsb2FkTGlicmFyeTogbm8gbWFuaWZlc3QgbWV0YWRhdGEgZm91bmQACG1ldGFE` +
+ `YXRhAAhvbkNyZWF0ZQAPAAcOPC0AIQAHDgESEEt/Ansdh0seABQABw4AMAEABw48PAABAA` +
+ `ICAQoJgYAEkAUFAqwFDQCwBgIB1AYAAAANAAAAAAAAAAEAAAAAAAAAAQAAACkAAABwAAAA` +
+ `AgAAAA8AAAAUAQAAAwAAAAwAAABQAQAABAAAAAIAAADgAQAABQAAABAAAADwAQAABgAAAA` +
+ `EAAABwAgAAASAAAAQAAACQAgAAARAAAAUAAAB0AwAAAiAAACkAAACeAwAAAyAAAAQAAAA1` +
+ `BgAAACAAAAEAAABZBgAAABAAAAEAAAB0BgAA` +
+ ``
diff --git a/cmd/gomobile/doc.go b/cmd/gomobile/doc.go
index f1aae0e..fd9cead 100644
--- a/cmd/gomobile/doc.go
+++ b/cmd/gomobile/doc.go
@@ -2,7 +2,7 @@
// Use of this source code is governed by a BSD-style
// license that can be found in the LICENSE file.

-// DO NOT EDIT. GENERATED BY 'gomobile help documentation'.
+// DO NOT EDIT. GENERATED BY 'gomobile help documentation doc.go'.

/*
Gomobile is a tool for building and running mobile apps written in Go.
diff --git a/cmd/gomobile/gendex.go b/cmd/gomobile/gendex.go
new file mode 100644
index 0000000..3d1dd49
--- /dev/null
+++ b/cmd/gomobile/gendex.go
@@ -0,0 +1,129 @@
+// Copyright 2015 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+// +build ignore
+
+package main
+
+import (
+ "bytes"
+ "encoding/base64"
+ "flag"
+ "fmt"
+ "go/format"
+ "io/ioutil"
+ "log"
+ "os"
+ "os/exec"
+ "path/filepath"
+)
+
+var outfile = flag.String("o", "", "result will be written file")
+
+func main() {
+ flag.Parse()
+
+ androidHome := os.Getenv("ANDROID_HOME")
+ if androidHome == "" {
+ log.Fatal("ANDROID_HOME not set")
+ }
+
+ tmpdir, err := ioutil.TempDir("", "gendex-")
+ if err != nil {
+ log.Fatal(err)
+ }
+ defer os.RemoveAll(tmpdir)
+ if err := os.MkdirAll(tmpdir+"/work/org/golang/app", 0775); err != nil {
+ log.Fatal(err)
+ }
+ javaFiles, err := filepath.Glob("../../app/*.java")
+ if err != nil {
+ log.Fatal(err)
+ }
+ if len(javaFiles) == 0 {
+ log.Fatal("could not find ../../app/*.java files")
+ }
+ platform := findLast(androidHome + "/platforms")
+ cmd := exec.Command(
+ "javac",
+ "-source", "1.7",
+ "-target", "1.7",
+ "-bootclasspath", platform+"/android.jar",
+ "-d", tmpdir+"/work",
+ )
+ cmd.Args = append(cmd.Args, javaFiles...)
+ if out, err := cmd.CombinedOutput(); err != nil {
+ fmt.Println(cmd.Args)
+ os.Stderr.Write(out)
+ log.Fatal(err)
+ }
+ buildTools := findLast(androidHome + "/build-tools")
+ cmd = exec.Command(
+ buildTools+"/dx",
+ "--dex",
+ "--output="+tmpdir+"/classes.dex",
+ tmpdir+"/work",
+ )
+ if out, err := cmd.CombinedOutput(); err != nil {
+ os.Stderr.Write(out)
+ log.Fatal(err)
+ }
+ src, err := ioutil.ReadFile(tmpdir + "/classes.dex")
+ if err != nil {
+ log.Fatal(err)
+ }
+ data := base64.StdEncoding.EncodeToString(src)
+
+ buf := new(bytes.Buffer)
+ fmt.Fprint(buf, header)
+
+ var piece string
+ for len(data) > 0 {
+ l := 70
+ if l > len(data) {
+ l = len(data)
+ }
+ piece, data = data[:l], data[l:]
+ fmt.Fprintf(buf, "\t`%s` + \n", piece)
+ }
+ fmt.Fprintf(buf, "\t``")
+ out, err := format.Source(buf.Bytes())
+ if err != nil {
+ buf.WriteTo(os.Stderr)
+ log.Fatal(err)
+ }
+
+ w, err := os.Create(*outfile)
+ if err != nil {
+ log.Fatal(err)
+ }
+ if _, err := w.Write(out); err != nil {
+ log.Fatal(err)
+ }
+ if err := w.Close(); err != nil {
+ log.Fatal(err)
+ }
+}
+
+func findLast(path string) string {
+ dir, err := os.Open(path)
+ if err != nil {
+ log.Fatal(err)
+ }
+ children, err := dir.Readdirnames(-1)
+ if err != nil {
+ log.Fatal(err)
+ }
+ return path + "/" + children[len(children)-1]
+}
+
+var header = `// Copyright 2015 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+// File is automatically generated by gendex.go. DO NOT EDIT.
+
+package main
+
+var dexStr = `
diff --git a/cmd/gomobile/manifest.go b/cmd/gomobile/manifest.go
index 04384e2..5ab308b 100644
--- a/cmd/gomobile/manifest.go
+++ b/cmd/gomobile/manifest.go
@@ -62,8 +62,8 @@
android:versionName="1.0">

<uses-sdk android:minSdkVersion="9" />
- <application android:label="{{.Name}}" android:hasCode="false"
android:debuggable="true">
- <activity android:name="android.app.NativeActivity"
+ <application android:label="{{.Name}}" android:debuggable="true">
+ <activity android:name="org.golang.app.GoNativeActivity"
android:label="{{.Name}}"
android:configChanges="orientation|keyboardHidden">
<meta-data android:name="android.app.lib_name"
android:value="{{.LibName}}" />
diff --git a/cmd/gomobile/writer.go b/cmd/gomobile/writer.go
index 3da75c8..6224f27 100644
--- a/cmd/gomobile/writer.go
+++ b/cmd/gomobile/writer.go
@@ -149,8 +149,20 @@
return fmt.Errorf("apk: %v", err)
}

+ hasDex := false
+ for _, entry := range w.manifest {
+ if entry.name == "classes.dex" {
+ hasDex = true
+ break
+ }
+ }
+
manifest := new(bytes.Buffer)
- fmt.Fprint(manifest, manifestHeader)
+ if hasDex {
+ fmt.Fprint(manifest, manifestDexHeader)
+ } else {
+ fmt.Fprint(manifest, manifestHeader)
+ }
certBody := new(bytes.Buffer)

for _, entry := range w.manifest {
@@ -206,6 +218,12 @@

`

+const manifestDexHeader = `Manifest-Version: 1.0
+Dex-Location: classes.dex
+Created-By: 1.0 (Go)
+
+`
+
const certHeader = `Signature-Version: 1.0
Created-By: 1.0 (Go)
`

--
https://go-review.googlesource.com/11980
Gerrit-Reviewer: Hyang-Ah Hana Kim <hya...@gmail.com>

Hyang-Ah Hana Kim (Gerrit)

unread,
Jul 8, 2015, 4:58:04 PM7/8/15
to David Crawshaw, Hyang-Ah Hana Kim, golang-co...@googlegroups.com
Hyang-Ah Hana Kim has posted comments on this change.

app, cmd/gomobile: subclass NativeActivity

Patch Set 1:

(6 comments)

https://go-review.googlesource.com/#/c/11980/1/app/android.c
File app/android.c:

PS1, Line 24: );
i am not familiar with ExceptionCheck/ExceptionClear. do we need
ExceptionClear before ExceptionCheck?


Line 51: current_ctx_clazz = (jclass)(*env)->NewGlobalRef(env,
current_ctx_clazz);
why should this be a a global var and GlobalRef'd? I may be wrong - gerrit
doesn't allow me to search easily - and it seems this is being used only in
ANativeActivity_onCreate. Can't this be computed locally in the function?


Line 73: }
Does this mean that users must avoid init function that depends on TMPDIR
or other env vars set here?


https://go-review.googlesource.com/#/c/11980/1/cmd/gomobile/build.go
File cmd/gomobile/build.go:

Line 220: log.Fatal("internal error: bad dexStr")
doesn't err carry any interesting info to help users? (I guess not)


https://go-review.googlesource.com/#/c/11980/1/cmd/gomobile/gendex.go
File cmd/gomobile/gendex.go:

Line 6:
package doc for future code maintainers :-) - about what this is about and
why it's needed and what's needed (ANDROID_HOME, javac version, ... and
SDK version if it matters.)


PS1, Line 38: )
I found log.Fatal is not a good choice - it os.Exit so the deferred clean
up functions don't run. We need to panic or have some atexit facility to
delete tmpdir.


--
https://go-review.googlesource.com/11980
Gerrit-Reviewer: Hyang-Ah Hana Kim <hya...@gmail.com>
Gerrit-HasComments: Yes

David Crawshaw (Gerrit)

unread,
Jul 11, 2015, 8:56:54 AM7/11/15
to Hyang-Ah Hana Kim, golang-co...@googlegroups.com
David Crawshaw uploaded a new patch set:
https://go-review.googlesource.com/11980

app, cmd/gomobile: subclass NativeActivity

Subclassing NativeActivity makes two things possible. Firstly, we can
implement an InputConnection to offer good support for IMEs,
necessary for good keyboard support. Secondly, we can use it to
overlay WebViews onto the NativeActivity.

But to sublcass NativeActivity, we need to compile Java. To keep the
toolchain go gettable, this is done with go generate.

While here, check the exception after FindClass. Apparently it can
throw an exception.

Updates golang/go#9361.
Updates golang/go#10247.

Change-Id: I672545997f0c9a7580f06988a273c03404772247
---
A app/GoNativeActivity.java
M app/android.c
M app/android.go
M cmd/gomobile/build.go
A cmd/gomobile/dex.go
M cmd/gomobile/doc.go
A cmd/gomobile/gendex.go
M cmd/gomobile/manifest.go
M cmd/gomobile/writer.go
9 files changed, 316 insertions(+), 77 deletions(-)

David Crawshaw (Gerrit)

unread,
Jul 11, 2015, 8:59:37 AM7/11/15
to Hyang-Ah Hana Kim, golang-co...@googlegroups.com
David Crawshaw has posted comments on this change.

app, cmd/gomobile: subclass NativeActivity

Patch Set 1:

(6 comments)

https://go-review.googlesource.com/#/c/11980/1/app/android.c
File app/android.c:

PS1, Line 24: );
> i am not familiar with ExceptionCheck/ExceptionClear. do we need
> ExceptionC
Done.

I believe ExceptionClear is safe to call even if there's no exception
pending, so I removed ExceptionCheck.


Line 51: current_ctx_clazz = (jclass)(*env)->NewGlobalRef(env,
current_ctx_clazz);
> why should this be a a global var and GlobalRef'd? I may be wrong - gerrit
In future CLs, I intend to add several helper methods to GoNativeActivity
that get called from various parts of android.c. Having this around saves a
few lines of code.


Line 73: }
> Does this mean that users must avoid init function that depends on TMPDIR
> o
Yes. This has been true since the runtime initialization moved into a
global constructor. There's no easy way around it. Even with our own custom
Java code, I am not aware of a setenv equivalent in the Java API we can use.

One possibility would be to package a second .so that does nothing but
expose setenv to GoNativeActivity. It could System.loadLibrary that, set
TMPDIR, then System.loadLibrary the Go c-shared library. Sounds like a lot
of machinery just for TMPDIR in init functions.


https://go-review.googlesource.com/#/c/11980/1/cmd/gomobile/build.go
File cmd/gomobile/build.go:

Line 220: log.Fatal("internal error: bad dexStr")
> doesn't err carry any interesting info to help users? (I guess not)
Done.


https://go-review.googlesource.com/#/c/11980/1/cmd/gomobile/gendex.go
File cmd/gomobile/gendex.go:

Line 6:
> package doc for future code maintainers :-) - about what this is about and
Done


PS1, Line 38: )
> I found log.Fatal is not a good choice - it os.Exit so the deferred clean
> u
Done


--
https://go-review.googlesource.com/11980
Gerrit-Reviewer: David Crawshaw <craw...@golang.org>
Gerrit-Reviewer: Hyang-Ah Hana Kim <hya...@gmail.com>
Gerrit-HasComments: Yes

Hyang-Ah Hana Kim (Gerrit)

unread,
Jul 11, 2015, 9:29:34 AM7/11/15
to David Crawshaw, Hyang-Ah Hana Kim, golang-co...@googlegroups.com
Hyang-Ah Hana Kim has posted comments on this change.

app, cmd/gomobile: subclass NativeActivity

Patch Set 2: Code-Review+2

--
https://go-review.googlesource.com/11980
Gerrit-Reviewer: David Crawshaw <craw...@golang.org>
Gerrit-Reviewer: Hyang-Ah Hana Kim <hya...@gmail.com>
Gerrit-HasComments: No

David Crawshaw (Gerrit)

unread,
Jul 13, 2015, 2:57:22 PM7/13/15
to Hyang-Ah Hana Kim, golang-co...@googlegroups.com
Reviewers: Hyang-Ah Hana Kim

David Crawshaw uploaded a new patch set:
https://go-review.googlesource.com/11980

app, cmd/gomobile: subclass NativeActivity

Subclassing NativeActivity makes two things possible. Firstly, we can
implement an InputConnection to offer good support for IMEs,
necessary for good keyboard support. Secondly, we can use it to
overlay WebViews onto the NativeActivity.

But to sublcass NativeActivity, we need to compile Java. To keep the
toolchain go gettable, this is done with go generate.

While here, check the exception after FindClass. Apparently it can
throw an exception.

Updates golang/go#9361.
Updates golang/go#10247.

Change-Id: I672545997f0c9a7580f06988a273c03404772247
---
A app/GoNativeActivity.java
M app/android.c
M app/android.go
M cmd/gomobile/build.go
M cmd/gomobile/build_androidapp.go
A cmd/gomobile/dex.go
A cmd/gomobile/gendex.go
M cmd/gomobile/manifest.go
M cmd/gomobile/writer.go
9 files changed, 315 insertions(+), 76 deletions(-)


--
https://go-review.googlesource.com/11980
Gerrit-Reviewer: David Crawshaw <craw...@golang.org>

David Crawshaw (Gerrit)

unread,
Jul 13, 2015, 3:05:30 PM7/13/15
to golang-...@googlegroups.com, Hyang-Ah Hana Kim, golang-co...@googlegroups.com
David Crawshaw has submitted this change and it was merged.

app, cmd/gomobile: subclass NativeActivity

Subclassing NativeActivity makes two things possible. Firstly, we can
implement an InputConnection to offer good support for IMEs,
necessary for good keyboard support. Secondly, we can use it to
overlay WebViews onto the NativeActivity.

But to sublcass NativeActivity, we need to compile Java. To keep the
toolchain go gettable, this is done with go generate.

While here, check the exception after FindClass. Apparently it can
throw an exception.

Updates golang/go#9361.
Updates golang/go#10247.

Change-Id: I672545997f0c9a7580f06988a273c03404772247
Reviewed-on: https://go-review.googlesource.com/11980
Reviewed-by: Hyang-Ah Hana Kim <hya...@gmail.com>
---
A app/GoNativeActivity.java
M app/android.c
M app/android.go
M cmd/gomobile/build.go
M cmd/gomobile/build_androidapp.go
A cmd/gomobile/dex.go
A cmd/gomobile/gendex.go
M cmd/gomobile/manifest.go
M cmd/gomobile/writer.go
9 files changed, 315 insertions(+), 76 deletions(-)

Approvals:
Hyang-Ah Hana Kim: Looks good to me, approved


--
https://go-review.googlesource.com/11980
Gerrit-Reviewer: David Crawshaw <craw...@golang.org>
Reply all
Reply to author
Forward
0 new messages