a6b10e0 Merge pull request #118 from bnema/fix/review-issues-117 · dumber

M internal/infrastructure/webkit/download_handler.go +16 -9


@@ -33,6 +33,13 @@ func NewDownloadHandler(downloadPath string, handler port.DownloadEventHandler)
33	33		}
34	34		}
35	35
	36	+	// downloadState tracks the state of a single download to prevent duplicate notifications.
	37	+	// This is allocated per-download to ensure proper isolation between concurrent downloads.
	38	+	type downloadState struct {
	39	+	failed bool
	40	+	mu sync.Mutex
	41	+	}
	42	+
36	43		// HandleDownload sets up signal handlers for a new download.
37	44		func (h DownloadHandler) HandleDownload(ctx context.Context, download webkit.Download) {
38	45		log := logging.FromContext(ctx)

@@ -42,9 +49,9 @@ func (h DownloadHandler) HandleDownload(ctx context.Context, download webkit.D
42	49		eventHandler := h.eventHandler
43	50		h.mu.RUnlock()
44	51
45		-	// Track if download failed (to avoid duplicate finished notifications).
46		-	var downloadFailed bool
47		-	var failedMu sync.Mutex
	52	+	// Track download state in a dedicated struct to ensure proper isolation.
	53	+	// Each download gets its own state to prevent interference between concurrent downloads.
	54	+	state := &downloadState{}
48	55
49	56		// Handle decide-destination signal to set download path.
50	57		//nolint:unparam // bool return is required by WebKit's signal signature (false = handled synchronously)

@@ -89,9 +96,9 @@ func (h DownloadHandler) HandleDownload(ctx context.Context, download webkit.D
89	96
90	97		// Handle failed signal.
91	98		failedCb := func(d webkit.Download, _ uintptr) {
92		-	failedMu.Lock()
93		-	downloadFailed = true
94		-	failedMu.Unlock()
	99	+	state.mu.Lock()
	100	+	state.failed = true
	101	+	state.mu.Unlock()
95	102
96	103		dest := d.GetDestination()
97	104		filename := extractFilename(dest)

@@ -114,9 +121,9 @@ func (h DownloadHandler) HandleDownload(ctx context.Context, download webkit.D
114	121
115	122		// Handle finished signal.
116	123		finishedCb := func(d webkit.Download) {
117		-	failedMu.Lock()
118		-	failed := downloadFailed
119		-	failedMu.Unlock()
	124	+	state.mu.Lock()
	125	+	failed := state.failed
	126	+	state.mu.Unlock()
120	127
121	128		// Don't send finished event if we already sent a failed event.
122	129		if failed {

 	}
 }
+// downloadState tracks the state of a single download to prevent duplicate notifications.
+// This is allocated per-download to ensure proper isolation between concurrent downloads.
+type downloadState struct {
+	failed bool
+	mu     sync.Mutex
+}
 // HandleDownload sets up signal handlers for a new download.
 func (h *DownloadHandler) HandleDownload(ctx context.Context, download *webkit.Download) {
 	log := logging.FromContext(ctx)
 	eventHandler := h.eventHandler
 	h.mu.RUnlock()
-	// Track if download failed (to avoid duplicate finished notifications).
+	// Track download state in a dedicated struct to ensure proper isolation.
-	var downloadFailed bool
+	// Each download gets its own state to prevent interference between concurrent downloads.
-	var failedMu sync.Mutex
+	state := &downloadState{}
 	// Handle decide-destination signal to set download path.
 	//nolint:unparam // bool return is required by WebKit's signal signature (false = handled synchronously)
 	// Handle failed signal.
 	failedCb := func(d webkit.Download, _ uintptr) {
-		failedMu.Lock()
+		state.mu.Lock()
-		downloadFailed = true
+		state.failed = true
-		failedMu.Unlock()
+		state.mu.Unlock()
 		dest := d.GetDestination()
 		filename := extractFilename(dest)
 	// Handle finished signal.
 	finishedCb := func(d webkit.Download) {
-		failedMu.Lock()
+		state.mu.Lock()
-		failed := downloadFailed
+		failed := state.failed
-		failedMu.Unlock()
+		state.mu.Unlock()
 		// Don't send finished event if we already sent a failed event.
 		if failed {

M internal/infrastructure/webkit/webview.go +2 -2


@@ -778,7 +778,7 @@ func (wv *WebView) GoBack(ctx context.Context) error {
778	778		// Fall back to JavaScript history.back() for SPA navigation
779	779		// This handles pushState/replaceState history that WebKit's BackForwardList doesn't track
780	780		wv.RunJavaScript(ctx, "history.back()", "")
781		-	log.Debug().Int("webview_id", int(wv.id)).Msg("webview go back (js)")
	781	+	log.Debug().Int("webview_id", int(wv.id)).Msg("webview go back (js fallback)")
782	782		return nil
783	783		}
784	784

@@ -800,7 +800,7 @@ func (wv *WebView) GoForward(ctx context.Context) error {
800	800		// Fall back to JavaScript history.forward() for SPA navigation
801	801		// This handles pushState/replaceState history that WebKit's BackForwardList doesn't track
802	802		wv.RunJavaScript(ctx, "history.forward()", "")
803		-	log.Debug().Int("webview_id", int(wv.id)).Msg("webview go forward (js)")
	803	+	log.Debug().Int("webview_id", int(wv.id)).Msg("webview go forward (js fallback)")
804	804		return nil
805	805		}
806	806


@@ -778,7 +778,7 @@ func (wv *WebView) GoBack(ctx context.Context) error {
778		// Fall back to JavaScript history.back() for SPA navigation	778		// Fall back to JavaScript history.back() for SPA navigation
779		// This handles pushState/replaceState history that WebKit's BackForwardList doesn't track	779		// This handles pushState/replaceState history that WebKit's BackForwardList doesn't track
780		wv.RunJavaScript(ctx, "history.back()", "")	780		wv.RunJavaScript(ctx, "history.back()", "")
781	-	log.Debug().Int("webview_id", int(wv.id)).Msg("webview go back (js)")	781	+	log.Debug().Int("webview_id", int(wv.id)).Msg("webview go back (js fallback)")
782		return nil	782		return nil
783		}	783		}
784			784

@@ -800,7 +800,7 @@ func (wv *WebView) GoForward(ctx context.Context) error {
800		// Fall back to JavaScript history.forward() for SPA navigation	800		// Fall back to JavaScript history.forward() for SPA navigation
801		// This handles pushState/replaceState history that WebKit's BackForwardList doesn't track	801		// This handles pushState/replaceState history that WebKit's BackForwardList doesn't track
802		wv.RunJavaScript(ctx, "history.forward()", "")	802		wv.RunJavaScript(ctx, "history.forward()", "")
803	-	log.Debug().Int("webview_id", int(wv.id)).Msg("webview go forward (js)")	803	+	log.Debug().Int("webview_id", int(wv.id)).Msg("webview go forward (js fallback)")
804		return nil	804		return nil
805		}	805		}
806			806

M internal/ui/app.go +8 -3


@@ -476,9 +476,14 @@ func (a *App) initDownloadHandler(ctx context.Context) {
476	476		}
477	477		}
478	478		if downloadPath == "" {
479		-	// Final fallback to ~/Downloads.
480		-	home, _ := os.UserHomeDir()
481		-	downloadPath = filepath.Join(home, "Downloads")
	479	+	// Final fallback to ~/Downloads, with /tmp as last resort.
	480	+	home, err := os.UserHomeDir()
	481	+	if err != nil {
	482	+	log.Warn().Err(err).Msg("failed to get home dir, using /tmp for downloads")
	483	+	downloadPath = "/tmp"
	484	+	} else {
	485	+	downloadPath = filepath.Join(home, "Downloads")
	486	+	}
482	487		}
483	488
484	489		// Create download event adapter to show toasts.

 		}
 	}
 	if downloadPath == "" {
-		// Final fallback to ~/Downloads.
+		// Final fallback to ~/Downloads, with /tmp as last resort.
-		home, _ := os.UserHomeDir()
+		home, err := os.UserHomeDir()
-		downloadPath = filepath.Join(home, "Downloads")
+		if err != nil {
+			log.Warn().Err(err).Msg("failed to get home dir, using /tmp for downloads")
+			downloadPath = "/tmp"
+		} else {
+			downloadPath = filepath.Join(home, "Downloads")
+		}
 	}
 	// Create download event adapter to show toasts.

M internal/ui/coordinator/update.go +3 -4


@@ -99,11 +99,10 @@ func (c *UpdateCoordinator) checkAsync(ctx context.Context) {
99	99		// showUpdateNotification displays a toast notification about the update.
100	100		func (c UpdateCoordinator) showUpdateNotification(ctx context.Context, result usecase.CheckUpdateOutput) {
101	101		var msg string
102		-	if result.CanAutoUpdate && c.config.Update.AutoDownload {
	102	+	switch {
	103	+	case result.CanAutoUpdate && c.config.Update.AutoDownload:
103	104		msg = fmt.Sprintf("Downloading update %s...", result.LatestVersion)
104		-	} else if result.CanAutoUpdate {
105		-	msg = fmt.Sprintf("Update %s available", result.LatestVersion)
106		-	} else {
	105	+	default:
107	106		msg = fmt.Sprintf("Update %s available", result.LatestVersion)
108	107		}
109	108

 // showUpdateNotification displays a toast notification about the update.
 func (c *UpdateCoordinator) showUpdateNotification(ctx context.Context, result *usecase.CheckUpdateOutput) {
 	var msg string
-	if result.CanAutoUpdate && c.config.Update.AutoDownload {
+	switch {
+	case result.CanAutoUpdate && c.config.Update.AutoDownload:
 		msg = fmt.Sprintf("Downloading update %s...", result.LatestVersion)
-	} else if result.CanAutoUpdate {
+	default:
-		msg = fmt.Sprintf("Update %s available", result.LatestVersion)
-	} else {
 		msg = fmt.Sprintf("Update %s available", result.LatestVersion)
 	}