GODT-1346: GODT-1340 GODT-1315 QML changes

GODT-1365: Create ComboBox component
GODT-1338: GODT-1343 Help view buttons
GODT-1340: Not crashing, user list updating in main thread.
GODT-1345: adding panic handlers

internal/frontend/qt/frontend.go

@@ -60,7 +60,6 @@ type FrontendQt struct {
 	newVersionInfo updater.VersionInfo
 
 	log                *logrus.Entry
-	usersMtx           sync.Mutex
 	initializing       sync.WaitGroup
 	initializationDone sync.Once
 

internal/frontend/qt/frontend_events.go

@@ -43,6 +43,11 @@ func (f *FrontendQt) watchEvents() {
 	userChangedCh := f.eventListener.ProvideChannel(events.UserRefreshEvent)
 	certIssue := f.eventListener.ProvideChannel(events.TLSCertIssue)
 
+	// This loop is executed outside main Qt application thread. In order
+	// to make sure that all signals are propagated correctly to QML we
+	// must call QMLBackend signals to apply any changes to GUI. The
+	// signals will make sure the changes are executed in main Qt app
+	// thread.
 	for {
 		select {
 		case errorDetails := <-errorCh:
@@ -77,7 +82,7 @@ func (f *FrontendQt) watchEvents() {
 		case <-updateApplicationCh:
 			f.updateForce()
 		case userID := <-userChangedCh:
-			f.userChanged(userID)
+			f.qml.UserChanged(userID)
 		case <-certIssue:
 			f.qml.ApiCertIssue()
 		}

internal/frontend/qt/frontend_init.go

@@ -48,7 +48,7 @@ func (f *FrontendQt) initiateQtApplication() error {
 	// QML Engine and path
 	f.engine = qml.NewQQmlApplicationEngine(f.app)
 
-	f.qml = NewQMLBackend(nil)
+	f.qml = NewQMLBackend(f.engine)
 	f.qml.setup(f)
 	f.engine.RootContext().SetContextProperty("go", f.qml)
 

internal/frontend/qt/frontend_updates.go

@@ -40,7 +40,7 @@ func (f *FrontendQt) checkUpdates() error {
 
 func (f *FrontendQt) checkUpdatesAndNotify(isRequestFromUser bool) {
 	checkingUpdates.Lock()
-	defer checkingUpdates.Lock()
+	defer checkingUpdates.Unlock()
 	defer f.qml.CheckUpdatesFinished()
 
 	if err := f.checkUpdates(); err != nil {
@@ -68,7 +68,7 @@ func (f *FrontendQt) checkUpdatesAndNotify(isRequestFromUser bool) {
 
 func (f *FrontendQt) updateForce() {
 	checkingUpdates.Lock()
-	defer checkingUpdates.Lock()
+	defer checkingUpdates.Unlock()
 
 	version := ""
 	if err := f.checkUpdates(); err == nil {

internal/frontend/qt/frontend_users.go

@@ -22,79 +22,10 @@ package qt
 import (
 	"context"
 	"encoding/base64"
-	"github.com/ProtonMail/proton-bridge/internal/frontend/types"
+	"github.com/ProtonMail/proton-bridge/internal/users"
 	"github.com/ProtonMail/proton-bridge/pkg/pmapi"
 )
 
-func (f *FrontendQt) loadUsers() {
-	f.usersMtx.Lock()
-	defer f.usersMtx.Unlock()
-
-	f.qml.Users().clear()
-
-	for _, user := range f.bridge.GetUsers() {
-		f.qml.Users().addUser(newQMLUserFromBacked(f, user))
-	}
-
-	// If there are no active accounts.
-	if f.qml.Users().Count() == 0 {
-		f.log.Info("No active accounts")
-	}
-}
-
-func (f *FrontendQt) userChanged(userID string) {
-	f.usersMtx.Lock()
-	defer f.usersMtx.Unlock()
-
-	fUsers := f.qml.Users()
-
-	index := fUsers.indexByID(userID)
-	user, err := f.bridge.GetUser(userID)
-
-	if user == nil || err != nil {
-		if index >= 0 { // delete existing user
-			fUsers.removeUser(index)
-		}
-		return
-	}
-
-	if index < 0 { // add non-existing user
-		fUsers.addUser(newQMLUserFromBacked(f, user))
-		return
-	}
-
-	// update exiting user
-	fUsers.users[index].update(user)
-}
-
-func newQMLUserFromBacked(f *FrontendQt, user types.User) *QMLUser {
-	qu := NewQMLUser(nil)
-	qu.ID = user.ID()
-
-	qu.update(user)
-
-	qu.ConnectToggleSplitMode(func(activateSplitMode bool) {
-		go func() {
-			defer qu.ToggleSplitModeFinished()
-			if activateSplitMode == user.IsCombinedAddressMode() {
-				user.SwitchAddressMode()
-			}
-			qu.SetSplitMode(!user.IsCombinedAddressMode())
-		}()
-	})
-
-	qu.ConnectLogout(func() {
-		qu.SetLoggedIn(false)
-		go user.Logout()
-	})
-
-	qu.ConnectConfigureAppleMail(func(address string) {
-		go f.configureAppleMail(qu.ID, address)
-	})
-
-	return qu
-}
-
 func (f *FrontendQt) login(username, password string) {
 	var err error
 	f.password, err = base64.StdEncoding.DecodeString(password)
@@ -107,6 +38,7 @@ func (f *FrontendQt) login(username, password string) {
 
 	f.authClient, f.auth, err = f.bridge.Login(username, f.password)
 	if err != nil {
+		// TODO login free user error
 		f.qml.LoginUsernamePasswordError(err.Error())
 		f.loginClean()
 		return
@@ -185,29 +117,24 @@ func (f *FrontendQt) login2Password(username, mboxPassword string) {
 func (f *FrontendQt) finishLogin() {
 	defer f.loginClean()
 
-	if f.auth == nil || f.authClient == nil {
-		f.log.Errorf("Finish login: Authethication incomplete %p %p", f.auth, f.authClient)
+	if len(f.password) == 0 || f.auth == nil || f.authClient == nil {
+		f.log.
+			WithField("hasPass", len(f.password) != 0).
+			WithField("hasAuth", f.auth != nil).
+			WithField("hasClient", f.authClient != nil).
+			Error("Finish login: authethication incomplete")
 		f.qml.Login2PasswordErrorAbort("Missing authentication, try again.")
 		return
 	}
 
-	user, err := f.bridge.FinishLogin(f.authClient, f.auth, f.password)
-	if err != nil {
-		f.log.Errorf("Authethication incomplete %p %p", f.auth, f.authClient)
-		f.qml.Login2PasswordErrorAbort("Missing authentication, try again.")
+	_, err := f.bridge.FinishLogin(f.authClient, f.auth, f.password)
+	if err != nil && err != users.ErrUserAlreadyConnected {
+		f.log.WithError(err).Errorf("Finish login failed")
+		f.qml.Login2PasswordErrorAbort(err.Error())
 		return
 	}
 
-	index := f.qml.Users().indexByID(user.ID())
-	if index < 0 {
-		qu := newQMLUserFromBacked(f, user)
-		qu.SetSetupGuideSeen(false)
-		f.qml.Users().addUser(qu)
-		return
-	}
-
-	f.qml.Users().users[index].update(user)
-	f.qml.LoginFinished()
+	defer f.qml.LoginFinished()
 }
 
 func (f *FrontendQt) loginAbort(username string) {

internal/frontend/qt/qml_backend.go

@@ -28,6 +28,10 @@ import (
 	"github.com/therecipe/qt/core"
 )
 
+func init() {
+	QMLBackend_QRegisterMetaType()
+}
+
 // QMLBackend connects QML frontend with Go backend.
 type QMLBackend struct {
 	core.QObject
@@ -138,6 +142,8 @@ type QMLBackend struct {
 	_ func(address string)  `signal:addressChangedLogout`
 	_ func(username string) `signal:userDisconnected`
 	_ func()                `signal:apiCertIssue`
+
+	_ func(userID string) `signal:userChanged`
 }
 
 func (q *QMLBackend) setup(f *FrontendQt) {
@@ -150,38 +156,81 @@ func (q *QMLBackend) setup(f *FrontendQt) {
 		return f.showOnStartup
 	})
 
-	q.ConnectIsDockIconVisible(func() bool {
-		return dockIcon.GetDockIconVisibleState()
-	})
-	q.ConnectSetDockIconVisible(func(visible bool) {
-		dockIcon.SetDockIconVisibleState(visible)
-	})
+	q.ConnectIsDockIconVisible(dockIcon.GetDockIconVisibleState)
+	q.ConnectSetDockIconVisible(dockIcon.SetDockIconVisibleState)
 
-	q.SetUsers(NewQMLUserModel(nil))
-	f.loadUsers()
+	um := NewQMLUserModel(q)
+	um.f = f
+	q.SetUsers(um)
+	um.load()
+
+	q.ConnectUserChanged(um.userChanged)
 
 	q.SetGoos(runtime.GOOS)
 
-	q.ConnectLogin(func(u, p string) { go f.login(u, p) })
-	q.ConnectLogin2FA(func(u, p string) { go f.login2FA(u, p) })
-	q.ConnectLogin2Password(func(u, p string) { go f.login2Password(u, p) })
-	q.ConnectLoginAbort(func(u string) { go f.loginAbort(u) })
+	q.ConnectLogin(func(u, p string) {
+		go func() {
+			defer f.panicHandler.HandlePanic()
+			f.login(u, p)
+		}()
+	})
+	q.ConnectLogin2FA(func(u, p string) {
+		go func() {
+			defer f.panicHandler.HandlePanic()
+			f.login2FA(u, p)
+		}()
+	})
+	q.ConnectLogin2Password(func(u, p string) {
+		go func() {
+			defer f.panicHandler.HandlePanic()
+			f.login2Password(u, p)
+		}()
+	})
+	q.ConnectLoginAbort(func(u string) {
+		go func() {
+			defer f.panicHandler.HandlePanic()
+			f.loginAbort(u)
+		}()
+	})
 
-	go f.checkUpdatesAndNotify(false)
-	q.ConnectCheckUpdates(func() { go f.checkUpdatesAndNotify(true) })
+	go func() {
+		defer f.panicHandler.HandlePanic()
+		f.checkUpdatesAndNotify(false)
+	}()
+	q.ConnectCheckUpdates(func() {
+		go func() {
+			defer f.panicHandler.HandlePanic()
+			f.checkUpdatesAndNotify(true)
+		}()
+	})
 
 	f.setIsDiskCacheEnabled()
 	f.setDiskCachePath()
-	q.ConnectChangeLocalCache(func(e bool, d string) { go f.changeLocalCache(e, d) })
+	q.ConnectChangeLocalCache(func(e bool, d string) {
+		go func() {
+			defer f.panicHandler.HandlePanic()
+			f.changeLocalCache(e, d)
+		}()
+	})
 
 	f.setIsAutomaticUpdateOn()
-	q.ConnectToggleAutomaticUpdate(func(m bool) { go f.toggleAutomaticUpdate(m) })
+	q.ConnectToggleAutomaticUpdate(func(m bool) {
+		go func() {
+			defer f.panicHandler.HandlePanic()
+			f.toggleAutomaticUpdate(m)
+		}()
+	})
 
 	f.setIsAutostartOn()
 	q.ConnectToggleAutostart(f.toggleAutostart)
 
 	f.setIsBetaEnabled()
-	q.ConnectToggleBeta(func(m bool) { go f.toggleBeta(m) })
+	q.ConnectToggleBeta(func(m bool) {
+		go func() {
+			defer f.panicHandler.HandlePanic()
+			f.toggleBeta(m)
+		}()
+	})
 
 	q.SetIsDoHEnabled(f.settings.GetBool(settings.AllowProxyKey))
 	q.ConnectToggleDoH(f.toggleDoH)
@@ -195,7 +244,12 @@ func (q *QMLBackend) setup(f *FrontendQt) {
 	q.ConnectChangePorts(f.changePorts)
 	q.ConnectIsPortFree(f.isPortFree)
 
-	q.ConnectTriggerReset(func() { go f.triggerReset() })
+	q.ConnectTriggerReset(func() {
+		go func() {
+			defer f.panicHandler.HandlePanic()
+			f.triggerReset()
+		}()
+	})
 
 	f.setVersion()
 	f.setLogsPath()
@@ -203,9 +257,24 @@ func (q *QMLBackend) setup(f *FrontendQt) {
 	f.setLicensePath()
 
 	f.setCurrentEmailClient()
-	q.ConnectUpdateCurrentMailClient(func() { go f.setCurrentEmailClient() })
-	q.ConnectReportBug(func(d, a, e string, i bool) { go f.reportBug(d, a, e, i) })
+	q.ConnectUpdateCurrentMailClient(func() {
+		go func() {
+			defer f.panicHandler.HandlePanic()
+			f.setCurrentEmailClient()
+		}()
+	})
+	q.ConnectReportBug(func(d, a, e string, i bool) {
+		go func() {
+			defer f.panicHandler.HandlePanic()
+			f.reportBug(d, a, e, i)
+		}()
+	})
 
 	f.setKeychain()
-	q.ConnectSelectKeychain(func(k string) { go f.selectKeychain(k) })
+	q.ConnectSelectKeychain(func(k string) {
+		go func() {
+			defer f.panicHandler.HandlePanic()
+			f.selectKeychain(k)
+		}()
+	})
 }

internal/frontend/qt/qml_users.go

@@ -20,10 +20,17 @@
 package qt
 
 import (
+	"sync"
+
 	"github.com/ProtonMail/proton-bridge/internal/frontend/types"
 	"github.com/therecipe/qt/core"
 )
 
+func init() {
+	QMLUser_QRegisterMetaType()
+	QMLUserModel_QRegisterMetaType()
+}
+
 // QMLUserModel stores list of of users
 type QMLUserModel struct {
 	core.QAbstractListModel
@@ -33,72 +40,168 @@ type QMLUserModel struct {
 	_ func()                       `constructor:"init"`
 	_ func(row int) *core.QVariant `slot:"get"`
 
-	users []*QMLUser
+	userIDs  []string
+	userByID map[string]*QMLUser
+	access   sync.RWMutex
+	f        *FrontendQt
 }
 
 func (um *QMLUserModel) init() {
-	um.SetRoles(map[int]*core.QByteArray{
-		int(core.Qt__UserRole + 1): newQByteArrayFromString("object"),
-	})
+	um.access.Lock()
+	defer um.access.Unlock()
+	um.SetCount(0)
 	um.ConnectRowCount(um.rowCount)
 	um.ConnectData(um.data)
 	um.ConnectGet(um.get)
-	um.users = []*QMLUser{}
-	um.setCount()
+	um.ConnectCount(func() int {
+		um.access.RLock()
+		defer um.access.RUnlock()
+		return len(um.userIDs)
+	})
+	um.userIDs = []string{}
+	um.userByID = map[string]*QMLUser{}
 }
 
 func (um *QMLUserModel) data(index *core.QModelIndex, property int) *core.QVariant {
 	if !index.IsValid() {
+		um.f.log.WithField("size", len(um.userIDs)).Info("Trying to get user by invalid index")
 		return core.NewQVariant()
 	}
 	return um.get(index.Row())
 }
 
 func (um *QMLUserModel) get(index int) *core.QVariant {
-	if index < 0 || index >= um.rowCount(nil) {
+	um.access.Lock()
+	defer um.access.Unlock()
+	if index < 0 || index >= len(um.userIDs) {
+		um.f.log.WithField("index", index).WithField("size", len(um.userIDs)).Info("Trying to get user by wrong index")
 		return core.NewQVariant()
 	}
-	return um.users[index].ToVariant()
+
+	u, err := um.getUserByID(um.userIDs[index])
+	if err != nil {
+		um.f.log.WithError(err).Error("Cannot get user from backend")
+		return core.NewQVariant()
+	}
+	return u.ToVariant()
+}
+
+func (um *QMLUserModel) getUserByID(userID string) (*QMLUser, error) {
+	u, ok := um.userByID[userID]
+	if ok {
+		return u, nil
+	}
+
+	user, err := um.f.bridge.GetUser(userID)
+	if err != nil {
+		return nil, err
+	}
+
+	u = newQMLUserFromBacked(um, user)
+	um.userByID[userID] = u
+	return u, nil
 }
 
 func (um *QMLUserModel) rowCount(*core.QModelIndex) int {
-	return len(um.users)
+	um.access.RLock()
+	defer um.access.RUnlock()
+	return len(um.userIDs)
 }
 
 func (um *QMLUserModel) setCount() {
-	um.SetCount(len(um.users))
+	um.SetCount(len(um.userIDs))
 }
 
-func (um *QMLUserModel) addUser(user *QMLUser) {
-	um.BeginInsertRows(core.NewQModelIndex(), um.rowCount(nil), um.rowCount(nil))
-	um.users = append(um.users, user)
-	um.setCount()
+func (um *QMLUserModel) addUser(userID string) {
+	um.BeginInsertRows(core.NewQModelIndex(), len(um.userIDs), len(um.userIDs))
+	um.access.Lock()
+	if um.indexByIDNotSafe(userID) < 0 {
+		um.userIDs = append(um.userIDs, userID)
+	}
+	um.access.Unlock()
 	um.EndInsertRows()
+	um.setCount()
 }
 
 func (um *QMLUserModel) removeUser(row int) {
 	um.BeginRemoveRows(core.NewQModelIndex(), row, row)
-	um.users = append(um.users[:row], um.users[row+1:]...)
-	um.setCount()
+	um.access.Lock()
+	id := um.userIDs[row]
+	um.userIDs = append(um.userIDs[:row], um.userIDs[row+1:]...)
+	delete(um.userByID, id)
+	um.access.Unlock()
 	um.EndRemoveRows()
+	um.setCount()
 }
 
 func (um *QMLUserModel) clear() {
-	um.BeginRemoveRows(core.NewQModelIndex(), 0, um.rowCount(nil))
-	um.users = []*QMLUser{}
-	um.setCount()
-	um.EndRemoveRows()
+	um.BeginResetModel()
+	um.access.Lock()
+	um.userIDs = []string{}
+	um.userByID = map[string]*QMLUser{}
+	um.SetCount(0)
+	um.access.Unlock()
+	um.EndResetModel()
 }
 
-func (um *QMLUserModel) indexByID(id string) int {
-	for i, qu := range um.users {
-		if id == qu.ID {
+func (um *QMLUserModel) load() {
+	um.clear()
+
+	for _, user := range um.f.bridge.GetUsers() {
+		um.addUser(user.ID())
+
+		// We need mark that all existing users already saw setup
+		// guide. This it is OK to construct QML here because it is in main thread.
+		u, err := um.getUserByID(user.ID())
+		if err != nil {
+			um.f.log.WithError(err).Error("Cannot get QMLUser while loading users")
+		}
+		u.SetSetupGuideSeen(true)
+	}
+
+	// If there are no active accounts.
+	if um.Count() == 0 {
+		um.f.log.Info("No active accounts")
+	}
+}
+
+func (um *QMLUserModel) userChanged(userID string) {
+	index := um.indexByIDNotSafe(userID)
+	user, err := um.f.bridge.GetUser(userID)
+
+	if user == nil || err != nil {
+		if index >= 0 { // delete existing user
+			um.removeUser(index)
+		}
+		// if not exiting do nothing
+		return
+	}
+
+	if index < 0 { // add non-existing user
+		um.addUser(userID)
+		return
+	}
+
+	// update exiting user
+	um.userByID[userID].update(user)
+}
+
+func (um *QMLUserModel) indexByIDNotSafe(wantID string) int {
+	for i, id := range um.userIDs {
+		if id == wantID {
 			return i
 		}
 	}
 	return -1
 }
 
+func (um *QMLUserModel) indexByID(id string) int {
+	um.access.RLock()
+	defer um.access.RUnlock()
+
+	return um.indexByIDNotSafe(id)
+}
+
 // QMLUser holds data, slots and signals and for user.
 type QMLUser struct {
 	core.QObject
@@ -116,18 +219,66 @@ type QMLUser struct {
 	_ func(makeItActive bool) `slot:"toggleSplitMode"`
 	_ func()                  `signal:"toggleSplitModeFinished"`
 	_ func()                  `slot:"logout"`
+	_ func()                  `slot:"remove"`
 	_ func(address string)    `slot:"configureAppleMail"`
 
 	ID string
 }
 
+func newQMLUserFromBacked(um *QMLUserModel, user types.User) *QMLUser {
+	qu := NewQMLUser(um)
+	qu.ID = user.ID()
+
+	qu.update(user)
+
+	qu.ConnectToggleSplitMode(func(activateSplitMode bool) {
+		go func() {
+			defer um.f.panicHandler.HandlePanic()
+			defer qu.ToggleSplitModeFinished()
+			if activateSplitMode == user.IsCombinedAddressMode() {
+				user.SwitchAddressMode()
+			}
+			qu.SetSplitMode(!user.IsCombinedAddressMode())
+		}()
+	})
+
+	qu.ConnectLogout(func() {
+		qu.SetLoggedIn(false)
+		go func() {
+			defer um.f.panicHandler.HandlePanic()
+			user.Logout()
+		}()
+	})
+
+	qu.ConnectRemove(func() {
+		go func() {
+			defer um.f.panicHandler.HandlePanic()
+
+			// TODO: remove preferences
+			if err := um.f.bridge.DeleteUser(qu.ID, false); err != nil {
+				um.f.log.WithError(err).Error("Failed to remove user")
+				// TODO: notification
+			}
+		}()
+	})
+
+	qu.ConnectConfigureAppleMail(func(address string) {
+		go func() {
+			defer um.f.panicHandler.HandlePanic()
+			um.f.configureAppleMail(qu.ID, address)
+		}()
+	})
+
+	return qu
+}
+
 func (qu *QMLUser) update(user types.User) {
 	username := user.Username()
 	qu.SetAvatarText(getInitials(username))
 	qu.SetUsername(username)
 	qu.SetLoggedIn(user.IsConnected())
 	qu.SetSplitMode(!user.IsCombinedAddressMode())
-	qu.SetSetupGuideSeen(true)
+	qu.SetSetupGuideSeen(false)
 	qu.SetUsedBytes(1.0)      // TODO
 	qu.SetTotalBytes(10000.0) // TODO
 	qu.SetPassword(user.GetBridgePassword())