[refactor] several simplifications after review

- simplify notification routine (we dont need no rejected action). we
  just check every hour, as in the original code.
- open links directly from Qt
- rename some global variables to make them less cryptic
- move cleanup function to the same module that created them

6 files changed, 58 insertions, 58 deletions

diff --git a/pkg/backend/api.go b/pkg/backend/api.go
index 74220ed..cf9ec5c 100644
--- a/pkg/backend/api.go
+++ b/pkg/backend/api.go
@@ -6,7 +6,6 @@ import (
 	"C"
 	"fmt"
 	"log"
-	"time"
 	"unsafe"
 
 	"0xacab.org/leap/bitmask-vpn/pkg/bitmask"
@@ -34,17 +33,13 @@ func Quit() {
 		ctx.cfg.SetUserStoppedVPN(true)
 		stopVPN()
 	}
-	cleanupTempDirs()
+	cleanup()
 }
 
 func DonateAccepted() {
 	donateAccepted()
 }
 
-func DonateRejected() {
-	donateRejected()
-}
-
 func SubscribeToEvent(event string, f unsafe.Pointer) {
 	subscribe(event, f)
 }
@@ -53,17 +48,8 @@ func InitializeBitmaskContext() {
 	p := bitmask.GetConfiguredProvider()
 
 	initOnce.Do(func() { initializeContext(p.Provider, p.AppName) })
+	runDonationReminder()
 	go ctx.updateStatus()
-
-	go func() {
-		if needsDonationReminder() {
-			// wait a bit before launching reminder
-			timer := time.NewTimer(time.Minute * 5)
-			<-timer.C
-			showDonate()
-		}
-
-	}()
 }
 
 func RefreshContext() *C.char {
diff --git a/pkg/backend/callbacks.go b/pkg/backend/callbacks.go
index f3bb39f..5fb8642 100644
--- a/pkg/backend/callbacks.go
+++ b/pkg/backend/callbacks.go
@@ -19,11 +19,11 @@ import (
 // }
 import "C"
 
-/* callbacks into C-land */
+/* callbacks into C-land. We keep a registry, and protect its updates with a mutex. */
+
+var callbacks = make(map[string](*[0]byte))
+var callbackMutex sync.Mutex
 
-var mut sync.Mutex
-var stmut sync.Mutex
-var cbs = make(map[string](*[0]byte))
 var initOnce sync.Once
 
 // Events are just a enumeration of all the posible events that C functions can
@@ -38,23 +38,23 @@ const OnStatusChanged string = "OnStatusChanged"
 // subscribe registers a callback from C-land.
 // This callback needs to be passed as a void* C function pointer.
 func subscribe(event string, fp unsafe.Pointer) {
-	mut.Lock()
-	defer mut.Unlock()
+	callbackMutex.Lock()
+	defer callbackMutex.Unlock()
 	e := &Events{}
 	v := reflect.Indirect(reflect.ValueOf(&e))
 	hf := v.Elem().FieldByName(event)
 	if reflect.ValueOf(hf).IsZero() {
 		fmt.Println("ERROR: not a valid event:", event)
 	} else {
-		cbs[event] = (*[0]byte)(fp)
+		callbacks[event] = (*[0]byte)(fp)
 	}
 }
 
 // trigger fires a callback from C-land.
 func trigger(event string) {
-	mut.Lock()
-	defer mut.Unlock()
-	cb := cbs[event]
+	callbackMutex.Lock()
+	defer callbackMutex.Unlock()
+	cb := callbacks[event]
 	if cb != nil {
 		C._do_callback(cb)
 	} else {
diff --git a/pkg/backend/cleanup.go b/pkg/backend/cleanup.go
index 77c55e6..16f36e4 100644
--- a/pkg/backend/cleanup.go
+++ b/pkg/backend/cleanup.go
@@ -1,16 +1,9 @@
 package backend
 
 import (
-	"log"
-	"os"
-	"path"
-	"path/filepath"
+	"0xacab.org/leap/bitmask-vpn/pkg/vpn"
 )
 
-func cleanupTempDirs() {
-	dirs, _ := filepath.Glob(path.Join(os.TempDir(), "leap-*"))
-	for _, d := range dirs {
-		log.Println("removing temp dir:", d)
-		os.RemoveAll(d)
-	}
+func cleanup() {
+	vpn.Cleanup()
 }
diff --git a/pkg/backend/donate.go b/pkg/backend/donate.go
index 608128f..20d5613 100644
--- a/pkg/backend/donate.go
+++ b/pkg/backend/donate.go
@@ -1,12 +1,24 @@
 package backend
 
 import (
-	"log"
 	"time"
 
 	"0xacab.org/leap/bitmask-vpn/pkg/config"
 )
 
+// runDonationReminder checks every hour if we need to show the reminder,
+// and trigger the launching of the dialog if needed.
+func runDonationReminder() {
+	go func() {
+		for {
+			time.Sleep(time.Hour)
+			if needsDonationReminder() {
+				showDonate()
+			}
+		}
+	}()
+}
+
 func wantDonations() bool {
 	if config.AskForDonations == "true" {
 		return true
@@ -19,25 +31,16 @@ func needsDonationReminder() bool {
 }
 
 func donateAccepted() {
-	stmut.Lock()
-	defer stmut.Unlock()
+	statusMutex.Lock()
+	defer statusMutex.Unlock()
 	ctx.DonateDialog = false
-	log.Println("marking as donated")
 	ctx.cfg.SetDonated()
 	go trigger(OnStatusChanged)
 }
 
-func donateRejected() {
-	timer := time.NewTimer(time.Hour)
-	go func() {
-		<-timer.C
-		showDonate()
-	}()
-}
-
 func showDonate() {
-	stmut.Lock()
-	defer stmut.Unlock()
+	statusMutex.Lock()
+	defer statusMutex.Unlock()
 	ctx.DonateDialog = true
 	ctx.cfg.SetLastReminded()
 	go trigger(OnStatusChanged)
diff --git a/pkg/backend/status.go b/pkg/backend/status.go
index 2e9c282..2bfb52d 100644
--- a/pkg/backend/status.go
+++ b/pkg/backend/status.go
@@ -4,6 +4,7 @@ import (
 	"bytes"
 	"encoding/json"
 	"log"
+	"sync"
 
 	"0xacab.org/leap/bitmask-vpn/pkg/bitmask"
 	"0xacab.org/leap/bitmask-vpn/pkg/config"
@@ -17,6 +18,8 @@ const (
 	failedStr   = "failed"
 )
 
+var statusMutex sync.Mutex
+
 // ctx will be our glorious global object.
 // if we ever switch again to a provider-agnostic app, we should keep a map here.
 var ctx *connectionCtx
@@ -42,8 +45,8 @@ type connectionCtx struct {
 }
 
 func (c connectionCtx) toJson() ([]byte, error) {
-	stmut.Lock()
-	defer stmut.Unlock()
+	statusMutex.Lock()
+	defer statusMutex.Unlock()
 	b, err := json.Marshal(c)
 	if err != nil {
 		log.Println(err)
@@ -69,8 +72,8 @@ func (c connectionCtx) updateStatus() {
 }
 
 func setStatus(st status) {
-	stmut.Lock()
-	defer stmut.Unlock()
+	statusMutex.Lock()
+	defer statusMutex.Unlock()
 	ctx.Status = st
 	go trigger(OnStatusChanged)
 }
diff --git a/pkg/vpn/main.go b/pkg/vpn/main.go
index 9d59131..f3c5b83 100644
--- a/pkg/vpn/main.go
+++ b/pkg/vpn/main.go
@@ -19,6 +19,8 @@ import (
 	"io/ioutil"
 	"log"
 	"os"
+	"path"
+	"path/filepath"
 
 	"0xacab.org/leap/bitmask-vpn/pkg/config"
 	"0xacab.org/leap/bitmask-vpn/pkg/vpn/bonafide"
@@ -52,10 +54,15 @@ func Init() (*Bitmask, error) {
 	b := Bitmask{tempdir, statusCh, nil, bonafide, launch, "", nil}
 
 	/*
-		err = b.StopVPN()
-		if err != nil {
-			return nil, err
-		}
+		TODO -- we still want to do this, since it resets the fw/vpn if running
+		from a previous one, but first we need to complete all the
+		system/helper checks that we can do. otherwise this times out with an
+		error that's captured badly as of today.
+
+			err = b.StopVPN()
+			if err != nil {
+				return nil, err
+			}
 	*/
 
 	err = ioutil.WriteFile(b.getCaCertPath(), config.CaCert, 0600)
@@ -87,3 +94,11 @@ func (b *Bitmask) Close() {
 func (b *Bitmask) Version() (string, error) {
 	return "", nil
 }
+
+func Cleanup() {
+	dirs, _ := filepath.Glob(path.Join(os.TempDir(), "leap-*"))
+	for _, d := range dirs {
+		log.Println("removing temp dir:", d)
+		os.RemoveAll(d)
+	}
+}