app: remove error return from Main

Move runtime.LockOSThread from the portable Main to the
platform-specific android main. For darwin, calling LockOSThread in
Main is too late, there has been enough time for the goroutine
calling it to jump off the initial OS thread. For darwin we call
LockOSThread from an init function, which the runtime keeps on the
first thread. For Android we call LockOSThread only to maintain the
thread-local storage for the GL context that is created later, so
it is safe to call it from func main.

Also remove TODOs about starting a gobind app on Android, which
will be simplified in a followup CL.

Tested on darwin/amd64 and android. Not tested on X11.

Change-Id: I34c56abf8b1292959d4d508bfade287d196c0380
Reviewed-on: https://go-review.googlesource.com/11653
Reviewed-by: Nigel Tao <nigeltao@golang.org>

app/android.go

@@ -63,6 +63,7 @@ import (
 	"io"
 	"log"
 	"os"
+	"runtime"
 	"time"
 	"unsafe"
 
@@ -274,40 +275,29 @@ func (a *asset) Close() error {
 	return nil
 }
 
-// TODO(crawshaw): fix up this comment??
-// notifyInitDone informs Java that the program is initialized.
-// A NativeActivity will not create a window until this is called.
-func main(f func(App) error) error {
+func main(f func(App)) {
+	// Preserve this OS thread for the GL context created in windowDraw.
+	runtime.LockOSThread()
+
 	ctag := C.CString("Go")
 	cstr := C.CString("app.Run")
 	C.__android_log_write(C.ANDROID_LOG_INFO, ctag, cstr)
 	C.free(unsafe.Pointer(ctag))
 	C.free(unsafe.Pointer(cstr))
 
-	donec := make(chan error, 1)
+	donec := make(chan struct{})
 	go func() {
-		donec <- f(app{})
+		f(app{})
+		close(donec)
 	}()
 
 	if C.current_native_activity == nil {
-		// TODO: Even though c-shared mode doesn't require main to be called
-		// now, gobind relies on the main being called. In main, app.Run is
-		// called and the start callback initializes Java-Go communication.
-		//
-		// The problem is if the main exits (because app.Run returns), go
-		// runtime exits and kills the app.
-		//
-		// Many things have changed in cgo recently. If we can manage to split
-		// gobind app, native Go app initialization logic, we may able to
-		// consider gobind app not to use main of the go package.
-		//
-		// TODO: do we need to do what used to be stateStart or stateStop?
-		select {}
-	} else {
-		for w := range windowCreated {
-			if done, err := windowDraw(w, queue, donec); done {
-				return err
-			}
+		<-donec
+		return
+	}
+	for w := range windowCreated {
+		if windowDraw(w, queue, donec) {
+			return
 		}
 	}
 	panic("unreachable")

app/app.go

@@ -8,8 +8,6 @@ package app
 
 import (
 	"io"
-	"log"
-	"runtime"
 
 	"golang.org/x/mobile/event"
 )
@@ -18,11 +16,8 @@ import (
 //
 // It calls f on the App, in a separate goroutine, as some OS-specific
 // libraries require being on 'the main thread'.
-func Main(f func(App) error) {
-	runtime.LockOSThread()
-	if err := main(f); err != nil {
-		log.Fatal(err)
-	}
+func Main(f func(App)) {
+	main(f)
 }
 
 // App is how a GUI mobile application interacts with the OS.
@@ -152,7 +147,7 @@ func pump(dst chan interface{}) (src chan interface{}) {
 //
 // Deprecated: call Main directly instead.
 func Run(cb Callbacks) {
-	Main(func(a App) error {
+	Main(func(a App) {
 		var c event.Config
 		for e := range a.Events() {
 			switch e := event.Filter(e).(type) {
@@ -183,7 +178,6 @@ func Run(cb Callbacks) {
 				}
 			}
 		}
-		return nil
 	})
 }
 

app/darwin_amd64.go

@@ -51,19 +51,17 @@ func init() {
 	initThreadID = uint64(C.threadID())
 }
 
-func main(f func(App) error) error {
+func main(f func(App)) {
 	if tid := uint64(C.threadID()); tid != initThreadID {
 		log.Fatalf("app.Main called on thread %d, but app.init ran on %d", tid, initThreadID)
 	}
 
-	var err error
 	go func() {
-		err = f(app{})
+		f(app{})
 		// TODO(crawshaw): trigger runApp to return
 	}()
 
 	C.runApp()
-	return err
 }
 
 var windowHeight geom.Pt

app/loop_android.go

@@ -92,7 +92,7 @@ import (
 
 var firstWindowDraw = true
 
-func windowDraw(w *C.ANativeWindow, queue *C.AInputQueue, donec chan error) (done bool, err error) {
+func windowDraw(w *C.ANativeWindow, queue *C.AInputQueue, donec chan struct{}) (done bool) {
 	C.createEGLWindow(w)
 
 	// TODO: is the library or the app responsible for clearing the buffers?
@@ -136,8 +136,8 @@ func windowDraw(w *C.ANativeWindow, queue *C.AInputQueue, donec chan error) (don
 	for {
 		processEvents(queue)
 		select {
-		case err := <-donec:
-			return true, err
+		case <-donec:
+			return true
 		case <-windowRedrawNeeded:
 			// Re-query the width and height.
 			C.querySurfaceWidthAndHeight()
@@ -170,7 +170,7 @@ func windowDraw(w *C.ANativeWindow, queue *C.AInputQueue, donec chan error) (don
 			}
 		case <-windowDestroyed:
 			sendLifecycle(event.LifecycleStageAlive)
-			return false, nil
+			return false
 		case <-gl.WorkAvailable:
 			gl.DoWork()
 		case <-endDraw:

app/x11.go

@@ -24,6 +24,7 @@ void swapBuffers(void);
 */
 import "C"
 import (
+	"runtime"
 	"time"
 
 	"golang.org/x/mobile/event"
@@ -31,15 +32,17 @@ import (
 	"golang.org/x/mobile/gl"
 )
 
-func main(f func(App) error) error {
+func main(f func(App)) {
+	runtime.LockOSThread()
 	C.createWindow()
 
 	// TODO: send lifecycle events when e.g. the X11 window is iconified or moved off-screen.
 	sendLifecycle(event.LifecycleStageFocused)
 
-	donec := make(chan error, 1)
+	donec := make(chan struct{})
 	go func() {
-		donec <- f(app{})
+		f(app{})
+		close(donec)
 	}()
 
 	// TODO: can we get the actual vsync signal?
@@ -49,8 +52,8 @@ func main(f func(App) error) error {
 
 	for {
 		select {
-		case err := <-donec:
-			return err
+		case <-donec:
+			return
 		case <-gl.WorkAvailable:
 			gl.DoWork()
 		case <-endDraw: