tabs: make fields private

The Tabs object exposes an array of Tab objects and the current selected
index in that array. The these two fields are sometimes modified in
goroutines, which can lead to data races causing fatal out of bounds
accesses on the tab array.

Hide these fields as private API. Expose only what needs to be seen from
the outside. This will prepare for protecting concurrent access with
a lock in the next commit.

Signed-off-by: Robin Jarry <robin@jarry.cc>
Acked-by: Koni Marti <koni.marti@gmail.com>

6 files changed, 145 insertions, 126 deletions

diff --git a/commands/compose/postpone.go b/commands/compose/postpone.go
index e1c0568..4b1e441 100644
--- a/commands/compose/postpone.go
+++ b/commands/compose/postpone.go
@@ -35,9 +35,13 @@ func (Postpone) Execute(aerc *widgets.Aerc, args []string) error {
 	if acct == nil {
 		return errors.New("No account selected")
 	}
-	composer, _ := aerc.SelectedTabContent().(*widgets.Composer)
+	tab := aerc.SelectedTab()
+	if tab == nil {
+		return errors.New("No tab selected")
+	}
+	composer, _ := tab.Content.(*widgets.Composer)
 	config := composer.Config()
-	tabName := aerc.TabNames()[aerc.SelectedTabIndex()]
+	tabName := tab.Name
 
 	if config.Postpone == "" {
 		return errors.New("No Postpone location configured")
diff --git a/commands/compose/send.go b/commands/compose/send.go
index 776779e..cca1540 100644
--- a/commands/compose/send.go
+++ b/commands/compose/send.go
@@ -43,8 +43,12 @@ func (Send) Execute(aerc *widgets.Aerc, args []string) error {
 	if len(args) > 1 {
 		return errors.New("Usage: send")
 	}
-	composer, _ := aerc.SelectedTabContent().(*widgets.Composer)
-	tabName := aerc.TabNames()[aerc.SelectedTabIndex()]
+	tab := aerc.SelectedTab()
+	if tab == nil {
+		return errors.New("No selected tab")
+	}
+	composer, _ := tab.Content.(*widgets.Composer)
+	tabName := tab.Name
 	config := composer.Config()
 
 	if config.Outgoing == "" {
diff --git a/commands/move-tab.go b/commands/move-tab.go
index 4151bd7..8648ed3 100644
--- a/commands/move-tab.go
+++ b/commands/move-tab.go
@@ -34,18 +34,11 @@ func (MoveTab) Execute(aerc *widgets.Aerc, args []string) error {
 		return fmt.Errorf("failed to parse index argument: %v", err)
 	}
 
-	i := aerc.SelectedTabIndex()
-	l := aerc.NumTabs()
-
-	if strings.HasPrefix(joinedArgs, "+") {
-		i = (i + n) % l
-	} else if strings.HasPrefix(joinedArgs, "-") {
-		i = (((i + n) % l) + l) % l
-	} else {
-		i = n
+	var relative bool
+	if strings.HasPrefix(joinedArgs, "+") || strings.HasPrefix(joinedArgs, "-") {
+		relative = true
 	}
-
-	aerc.MoveTab(i)
+	aerc.MoveTab(n, relative)
 
 	return nil
 }
diff --git a/lib/ui/tab.go b/lib/ui/tab.go
index a67bdab..153acca 100644
--- a/lib/ui/tab.go
+++ b/lib/ui/tab.go
@@ -10,10 +10,10 @@ import (
 )
 
 type Tabs struct {
-	Tabs       []*Tab
+	tabs       []*Tab
 	TabStrip   *TabStrip
 	TabContent *TabContent
-	Selected   int
+	curIndex   int
 	history    []int
 
 	uiConfig *config.UIConfig
@@ -54,18 +54,26 @@ func (tabs *Tabs) Add(content Drawable, name string, uiConf *config.UIConfig) *T
 		Name:    name,
 		uiConf:  uiConf,
 	}
-	tabs.Tabs = append(tabs.Tabs, tab)
-	tabs.TabStrip.Invalidate()
+	tabs.tabs = append(tabs.tabs, tab)
+	tabs.Select(len(tabs.tabs) - 1)
 	content.OnInvalidate(tabs.invalidateChild)
 	return tab
 }
 
+func (tabs *Tabs) Names() []string {
+	var names []string
+	for _, tab := range tabs.tabs {
+		names = append(names, tab.Name)
+	}
+	return names
+}
+
 func (tabs *Tabs) invalidateChild(d Drawable) {
-	if tabs.Selected >= len(tabs.Tabs) {
+	if tabs.curIndex >= len(tabs.tabs) {
 		return
 	}
 
-	if tabs.Tabs[tabs.Selected].Content == d {
+	if tabs.tabs[tabs.curIndex].Content == d {
 		if tabs.onInvalidateContent != nil {
 			tabs.onInvalidateContent(tabs.TabContent)
 		}
@@ -74,9 +82,9 @@ func (tabs *Tabs) invalidateChild(d Drawable) {
 
 func (tabs *Tabs) Remove(content Drawable) {
 	indexToRemove := -1
-	for i, tab := range tabs.Tabs {
+	for i, tab := range tabs.tabs {
 		if tab.Content == content {
-			tabs.Tabs = append(tabs.Tabs[:i], tabs.Tabs[i+1:]...)
+			tabs.tabs = append(tabs.tabs[:i], tabs.tabs[i+1:]...)
 			tabs.removeHistory(i)
 			indexToRemove = i
 			break
@@ -86,20 +94,19 @@ func (tabs *Tabs) Remove(content Drawable) {
 		return
 	}
 	// only pop the tab history if the closing tab is selected
-	if indexToRemove == tabs.Selected {
+	if indexToRemove == tabs.curIndex {
 		index, ok := tabs.popHistory()
 		if ok {
-			tabs.Select(index)
-			interactive, ok := tabs.Tabs[tabs.Selected].Content.(Interactive)
-			if ok {
-				interactive.Focus(true)
-			}
+			tabs.selectPriv(index)
 		}
-	} else if indexToRemove < tabs.Selected {
+	} else if indexToRemove < tabs.curIndex {
 		// selected tab is now one to the left of where it was
-		tabs.Selected--
+		tabs.Select(tabs.curIndex - 1)
+	}
+	interactive, ok := tabs.tabs[tabs.curIndex].Content.(Interactive)
+	if ok {
+		interactive.Focus(true)
 	}
-	tabs.TabStrip.Invalidate()
 }
 
 func (tabs *Tabs) Replace(contentSrc Drawable, contentTarget Drawable, name string) {
@@ -107,9 +114,9 @@ func (tabs *Tabs) Replace(contentSrc Drawable, contentTarget Drawable, name stri
 		Content: contentTarget,
 		Name:    name,
 	}
-	for i, tab := range tabs.Tabs {
+	for i, tab := range tabs.tabs {
 		if tab.Content == contentSrc {
-			tabs.Tabs[i] = replaceTab
+			tabs.tabs[i] = replaceTab
 			tabs.Select(i)
 			if c, ok := contentSrc.(io.Closer); ok {
 				c.Close()
@@ -121,20 +128,44 @@ func (tabs *Tabs) Replace(contentSrc Drawable, contentTarget Drawable, name stri
 	contentTarget.OnInvalidate(tabs.invalidateChild)
 }
 
-func (tabs *Tabs) Select(index int) {
-	if index >= len(tabs.Tabs) {
-		index = len(tabs.Tabs) - 1
+func (tabs *Tabs) Get(index int) *Tab {
+	if index < 0 || index >= len(tabs.tabs) {
+		return nil
+	}
+	return tabs.tabs[index]
+}
+
+func (tabs *Tabs) Selected() *Tab {
+	if tabs.curIndex < 0 || tabs.curIndex >= len(tabs.tabs) {
+		return nil
 	}
+	return tabs.tabs[tabs.curIndex]
+}
 
-	if tabs.Selected != index {
+func (tabs *Tabs) Select(index int) bool {
+	if index < 0 || index >= len(tabs.tabs) {
+		return false
+	}
+
+	if tabs.curIndex != index {
 		// only push valid tabs onto the history
-		if tabs.Selected < len(tabs.Tabs) {
-			tabs.pushHistory(tabs.Selected)
+		if tabs.curIndex < len(tabs.tabs) {
+			tabs.pushHistory(tabs.curIndex)
 		}
-		tabs.Selected = index
+		tabs.curIndex = index
 		tabs.TabStrip.Invalidate()
 		tabs.TabContent.Invalidate()
 	}
+	return true
+}
+
+func (tabs *Tabs) SelectName(name string) bool {
+	for i, tab := range tabs.tabs {
+		if tab.Name == name {
+			return tabs.Select(i)
+		}
+	}
+	return false
 }
 
 func (tabs *Tabs) SelectPrevious() bool {
@@ -142,24 +173,25 @@ func (tabs *Tabs) SelectPrevious() bool {
 	if !ok {
 		return false
 	}
-	tabs.Select(index)
-	return true
+	return tabs.Select(index)
 }
 
-func (tabs *Tabs) MoveTab(to int) {
-	from := tabs.Selected
+func (tabs *Tabs) MoveTab(to int, relative bool) {
+	from := tabs.curIndex
 
+	if relative {
+		to = from + to
+	}
 	if to < 0 {
 		to = 0
 	}
-
-	if to >= len(tabs.Tabs) {
-		to = len(tabs.Tabs) - 1
+	if to >= len(tabs.tabs) {
+		to = len(tabs.tabs) - 1
 	}
 
-	tab := tabs.Tabs[from]
+	tab := tabs.tabs[from]
 	if to > from {
-		copy(tabs.Tabs[from:to], tabs.Tabs[from+1:to+1])
+		copy(tabs.tabs[from:to], tabs.tabs[from+1:to+1])
 		for i, h := range tabs.history {
 			if h == from {
 				tabs.history[i] = to
@@ -169,7 +201,7 @@ func (tabs *Tabs) MoveTab(to int) {
 			}
 		}
 	} else if from > to {
-		copy(tabs.Tabs[to+1:from+1], tabs.Tabs[to:from])
+		copy(tabs.tabs[to+1:from+1], tabs.tabs[to:from])
 		for i, h := range tabs.history {
 			if h == from {
 				tabs.history[i] = to
@@ -182,44 +214,44 @@ func (tabs *Tabs) MoveTab(to int) {
 		return
 	}
 
-	tabs.Tabs[to] = tab
-	tabs.Selected = to
+	tabs.tabs[to] = tab
+	tabs.curIndex = to
 	tabs.TabStrip.Invalidate()
 }
 
 func (tabs *Tabs) PinTab() {
-	if tabs.Tabs[tabs.Selected].pinned {
+	if tabs.tabs[tabs.curIndex].pinned {
 		return
 	}
 
-	pinEnd := len(tabs.Tabs)
-	for i, t := range tabs.Tabs {
+	pinEnd := len(tabs.tabs)
+	for i, t := range tabs.tabs {
 		if !t.pinned {
 			pinEnd = i
 			break
 		}
 	}
 
-	for _, t := range tabs.Tabs {
-		if t.pinned && t.indexBeforePin > tabs.Selected-pinEnd {
+	for _, t := range tabs.tabs {
+		if t.pinned && t.indexBeforePin > tabs.curIndex-pinEnd {
 			t.indexBeforePin -= 1
 		}
 	}
 
-	tabs.Tabs[tabs.Selected].pinned = true
-	tabs.Tabs[tabs.Selected].indexBeforePin = tabs.Selected - pinEnd
+	tabs.tabs[tabs.curIndex].pinned = true
+	tabs.tabs[tabs.curIndex].indexBeforePin = tabs.curIndex - pinEnd
 
-	tabs.MoveTab(pinEnd)
+	tabs.MoveTab(pinEnd, false)
 }
 
 func (tabs *Tabs) UnpinTab() {
-	if !tabs.Tabs[tabs.Selected].pinned {
+	if !tabs.tabs[tabs.curIndex].pinned {
 		return
 	}
 
-	pinEnd := len(tabs.Tabs)
-	for i, t := range tabs.Tabs {
-		if i != tabs.Selected && t.pinned && t.indexBeforePin > tabs.Tabs[tabs.Selected].indexBeforePin {
+	pinEnd := len(tabs.tabs)
+	for i, t := range tabs.tabs {
+		if i != tabs.curIndex && t.pinned && t.indexBeforePin > tabs.tabs[tabs.curIndex].indexBeforePin {
 			t.indexBeforePin += 1
 		}
 		if !t.pinned {
@@ -228,23 +260,23 @@ func (tabs *Tabs) UnpinTab() {
 		}
 	}
 
-	tabs.Tabs[tabs.Selected].pinned = false
+	tabs.tabs[tabs.curIndex].pinned = false
 
-	tabs.MoveTab(tabs.Tabs[tabs.Selected].indexBeforePin + pinEnd - 1)
+	tabs.MoveTab(tabs.tabs[tabs.curIndex].indexBeforePin+pinEnd-1, false)
 }
 
 func (tabs *Tabs) NextTab() {
-	next := tabs.Selected + 1
-	if next >= len(tabs.Tabs) {
+	next := tabs.curIndex + 1
+	if next >= len(tabs.tabs) {
 		next = 0
 	}
 	tabs.Select(next)
 }
 
 func (tabs *Tabs) PrevTab() {
-	next := tabs.Selected - 1
+	next := tabs.curIndex - 1
 	if next < 0 {
-		next = len(tabs.Tabs) - 1
+		next = len(tabs.tabs) - 1
 	}
 	tabs.Select(next)
 }
@@ -284,13 +316,13 @@ func (tabs *Tabs) removeHistory(index int) {
 // TODO: Color repository
 func (strip *TabStrip) Draw(ctx *Context) {
 	x := 0
-	for i, tab := range strip.Tabs {
+	for i, tab := range strip.tabs {
 		uiConfig := strip.uiConfig
 		if tab.uiConf != nil {
 			uiConfig = tab.uiConf
 		}
 		style := uiConfig.GetStyle(config.STYLE_TAB)
-		if strip.Selected == i {
+		if strip.curIndex == i {
 			style = uiConfig.GetStyleSelected(config.STYLE_TAB)
 		}
 		tabWidth := 32
@@ -319,7 +351,7 @@ func (strip *TabStrip) Invalidate() {
 
 func (strip *TabStrip) MouseEvent(localX int, localY int, event tcell.Event) {
 	changeFocus := func(focus bool) {
-		interactive, ok := strip.parent.Tabs[strip.parent.Selected].Content.(Interactive)
+		interactive, ok := strip.parent.tabs[strip.parent.curIndex].Content.(Interactive)
 		if ok {
 			interactive.Focus(focus)
 		}
@@ -330,8 +362,8 @@ func (strip *TabStrip) MouseEvent(localX int, localY int, event tcell.Event) {
 	case *tcell.EventMouse:
 		switch event.Buttons() {
 		case tcell.Button1:
-			selectedTab, ok := strip.Clicked(localX, localY)
-			if !ok || selectedTab == strip.parent.Selected {
+			selectedTab, ok := strip.clicked(localX, localY)
+			if !ok || selectedTab == strip.parent.curIndex {
 				return
 			}
 			unfocus()
@@ -346,18 +378,12 @@ func (strip *TabStrip) MouseEvent(localX int, localY int, event tcell.Event) {
 			strip.parent.PrevTab()
 			refocus()
 		case tcell.Button3:
-			selectedTab, ok := strip.Clicked(localX, localY)
+			selectedTab, ok := strip.clicked(localX, localY)
 			if !ok {
 				return
 			}
 			unfocus()
-			if selectedTab == strip.parent.Selected {
-				strip.parent.CloseTab(selectedTab)
-			} else {
-				current := strip.parent.Selected
-				strip.parent.CloseTab(selectedTab)
-				strip.parent.Select(current)
-			}
+			strip.parent.CloseTab(selectedTab)
 			refocus()
 		}
 	}
@@ -367,9 +393,9 @@ func (strip *TabStrip) OnInvalidate(onInvalidate func(d Drawable)) {
 	strip.onInvalidateStrip = onInvalidate
 }
 
-func (strip *TabStrip) Clicked(mouseX int, mouseY int) (int, bool) {
+func (strip *TabStrip) clicked(mouseX int, mouseY int) (int, bool) {
 	x := 0
-	for i, tab := range strip.Tabs {
+	for i, tab := range strip.tabs {
 		trunc := runewidth.Truncate(tab.Name, 32, "…")
 		length := len(trunc) + 2
 		if x <= mouseX && mouseX < x+length {
@@ -381,27 +407,27 @@ func (strip *TabStrip) Clicked(mouseX int, mouseY int) (int, bool) {
 }
 
 func (content *TabContent) Children() []Drawable {
-	children := make([]Drawable, len(content.Tabs))
-	for i, tab := range content.Tabs {
+	children := make([]Drawable, len(content.tabs))
+	for i, tab := range content.tabs {
 		children[i] = tab.Content
 	}
 	return children
 }
 
 func (content *TabContent) Draw(ctx *Context) {
-	if content.Selected >= len(content.Tabs) {
+	if content.curIndex >= len(content.tabs) {
 		width := ctx.Width()
 		height := ctx.Height()
 		ctx.Fill(0, 0, width, height, ' ',
 			content.uiConfig.GetStyle(config.STYLE_TAB))
 	}
 
-	tab := content.Tabs[content.Selected]
+	tab := content.tabs[content.curIndex]
 	tab.Content.Draw(ctx)
 }
 
 func (content *TabContent) MouseEvent(localX int, localY int, event tcell.Event) {
-	tab := content.Tabs[content.Selected]
+	tab := content.tabs[content.curIndex]
 	switch tabContent := tab.Content.(type) {
 	case Mouseable:
 		tabContent.MouseEvent(localX, localY, event)
@@ -412,7 +438,7 @@ func (content *TabContent) Invalidate() {
 	if content.onInvalidateContent != nil {
 		content.onInvalidateContent(content)
 	}
-	tab := content.Tabs[content.Selected]
+	tab := content.tabs[content.curIndex]
 	tab.Content.Invalidate()
 }
 
diff --git a/widgets/account.go b/widgets/account.go
index 6e03160..c76b6fc 100644
--- a/widgets/account.go
+++ b/widgets/account.go
@@ -234,7 +234,7 @@ func (acct *AccountView) SelectedMessagePart() *PartInfo {
 }
 
 func (acct *AccountView) isSelected() bool {
-	return acct.aerc.NumTabs() > 0 && acct == acct.aerc.SelectedAccount()
+	return acct == acct.aerc.SelectedAccount()
 }
 
 func (acct *AccountView) onMessage(msg types.WorkerMessage) {
diff --git a/widgets/aerc.go b/widgets/aerc.go
index f480b34..c56c4df 100644
--- a/widgets/aerc.go
+++ b/widgets/aerc.go
@@ -105,8 +105,14 @@ func NewAerc(conf *config.AercConfig, logger *log.Logger,
 		aerc.NewTab(wizard, "New account")
 	}
 
+	tabs.Select(0)
+
 	tabs.CloseTab = func(index int) {
-		switch content := aerc.tabs.Tabs[index].Content.(type) {
+		tab := aerc.tabs.Get(index)
+		if tab == nil {
+			return
+		}
+		switch content := tab.Content.(type) {
 		case *AccountView:
 			return
 		case *AccountWizard:
@@ -284,7 +290,7 @@ func (aerc *Aerc) Event(event tcell.Event) bool {
 				aerc.BeginExCommand("")
 				return true
 			}
-			interactive, ok := aerc.tabs.Tabs[aerc.tabs.Selected].Content.(ui.Interactive)
+			interactive, ok := aerc.SelectedTabContent().(ui.Interactive)
 			if ok {
 				return interactive.Event(event)
 			}
@@ -334,18 +340,15 @@ func (aerc *Aerc) SelectedAccountUiConfig() *config.UIConfig {
 }
 
 func (aerc *Aerc) SelectedTabContent() ui.Drawable {
-	if aerc.NumTabs() == 0 || aerc.SelectedTabIndex() >= aerc.NumTabs() {
+	tab := aerc.tabs.Selected()
+	if tab == nil {
 		return nil
 	}
-	return aerc.tabs.Tabs[aerc.SelectedTabIndex()].Content
+	return tab.Content
 }
 
-func (aerc *Aerc) SelectedTabIndex() int {
-	return aerc.tabs.Selected
-}
-
-func (aerc *Aerc) NumTabs() int {
-	return len(aerc.tabs.Tabs)
+func (aerc *Aerc) SelectedTab() *ui.Tab {
+	return aerc.tabs.Selected()
 }
 
 func (aerc *Aerc) NewTab(clickable ui.Drawable, name string) *ui.Tab {
@@ -355,7 +358,6 @@ func (aerc *Aerc) NewTab(clickable ui.Drawable, name string) *ui.Tab {
 		uiConf = conf
 	}
 	tab := aerc.tabs.Add(clickable, name, uiConf)
-	aerc.tabs.Select(len(aerc.tabs.Tabs) - 1)
 	aerc.UpdateStatus()
 	return tab
 }
@@ -369,8 +371,8 @@ func (aerc *Aerc) ReplaceTab(tabSrc ui.Drawable, tabTarget ui.Drawable, name str
 	aerc.tabs.Replace(tabSrc, tabTarget, name)
 }
 
-func (aerc *Aerc) MoveTab(i int) {
-	aerc.tabs.MoveTab(i)
+func (aerc *Aerc) MoveTab(i int, relative bool) {
+	aerc.tabs.MoveTab(i, relative)
 }
 
 func (aerc *Aerc) PinTab() {
@@ -390,33 +392,23 @@ func (aerc *Aerc) PrevTab() {
 }
 
 func (aerc *Aerc) SelectTab(name string) bool {
-	for i, tab := range aerc.tabs.Tabs {
-		if tab.Name == name {
-			aerc.tabs.Select(i)
-			aerc.UpdateStatus()
-			return true
-		}
+	ok := aerc.tabs.SelectName(name)
+	if ok {
+		aerc.UpdateStatus()
 	}
-	return false
+	return ok
 }
 
 func (aerc *Aerc) SelectTabIndex(index int) bool {
-	for i := range aerc.tabs.Tabs {
-		if i == index {
-			aerc.tabs.Select(i)
-			aerc.UpdateStatus()
-			return true
-		}
+	ok := aerc.tabs.Select(index)
+	if ok {
+		aerc.UpdateStatus()
 	}
-	return false
+	return ok
 }
 
 func (aerc *Aerc) TabNames() []string {
-	var names []string
-	for _, tab := range aerc.tabs.Tabs {
-		names = append(names, tab.Name)
-	}
-	return names
+	return aerc.tabs.Names()
 }
 
 func (aerc *Aerc) SelectPreviousTab() bool {
@@ -463,7 +455,7 @@ func (aerc *Aerc) focus(item ui.Interactive) {
 		aerc.focused.Focus(false)
 	}
 	aerc.focused = item
-	interactive, ok := aerc.tabs.Tabs[aerc.tabs.Selected].Content.(ui.Interactive)
+	interactive, ok := aerc.SelectedTabContent().(ui.Interactive)
 	if item != nil {
 		item.Focus(true)
 		if ok {