fix(web): wait for the saved analytics preference (#1656)

pkg/download/downloader.go

@@ -2405,9 +2405,7 @@ func (d *Downloader) doStart(task *Task) (err error) {
 				return err
 			}
 		}
-		if err := d.triggerOnStart(task); err != nil {
-			return err
-		}
+		d.triggerOnStart(task)
 		if !d.taskIsRunningGeneration(task, generation) {
 			return nil
 		}

pkg/download/engine/cancellation_test.go

@@ -1,47 +0,0 @@
-package engine
-
-import (
-	"context"
-	"errors"
-	"github.com/dop251/goja"
-	"testing"
-	"time"
-)
-
-func TestCloseUnblocksPendingPromise(t *testing.T) {
-	e := NewEngine(nil)
-	entered := make(chan struct{})
-	_, err := e.runOnLoop(func(r *goja.Runtime) (goja.Value, error) {
-		return goja.Undefined(), r.Set("entered", func() { close(entered) })
-	})
-	if err != nil {
-		t.Fatal(err)
-	}
-	done := make(chan error, 1)
-	go func() { _, err := e.RunString(`entered(); new Promise(()=>{});`); done <- err }()
-	select {
-	case <-entered:
-	case <-time.After(time.Second):
-		t.Fatal("script did not enter")
-	}
-	e.Close()
-	select {
-	case err := <-done:
-		if err == nil {
-			t.Fatal("closed execution returned success")
-		}
-	case <-time.After(300 * time.Millisecond):
-		t.Fatal("Engine.Close left the Promise waiter blocked")
-	}
-}
-
-func TestExecutionDeadlineInterruptsSynchronousScript(t *testing.T) {
-	e := NewEngine(nil)
-	defer e.Close()
-	ctx, cancel := context.WithTimeout(context.Background(), 30*time.Millisecond)
-	defer cancel()
-	_, err := e.RunStringContext(ctx, `while (true) {}`)
-	if !errors.Is(err, context.DeadlineExceeded) {
-		t.Fatalf("got %v, want deadline exceeded", err)
-	}
-}

pkg/download/engine/engine.go

@@ -1,7 +1,6 @@
 package engine
 
 import (
-	"context"
 	_ "embed"
 	"errors"
 	"fmt"
@@ -27,7 +26,6 @@ var polyfillScript string
 
 type Engine struct {
 	loop *eventloop.EventLoop
-	done chan struct{}
 
 	closeOnce sync.Once
 	cleanupMu sync.Mutex
@@ -41,13 +39,7 @@ type JSFunction func(goja.FunctionCall) goja.Value
 // RunString executes the script and returns the go type value
 // if script result is promise, it will be resolved
 func (e *Engine) RunString(script string) (value any, err error) {
-	return e.RunStringContext(context.Background(), script)
-}
-
-// RunStringContext bounds an execution, including awaited promises. Cancelling
-// an execution closes its engine so outstanding operations cannot outlive it.
-func (e *Engine) RunStringContext(ctx context.Context, script string) (any, error) {
-	return e.runOnLoopContext(ctx, func(runtime *goja.Runtime) (goja.Value, error) {
+	return e.runOnLoop(func(runtime *goja.Runtime) (goja.Value, error) {
 		return runtime.RunString(script)
 	})
 }
@@ -55,11 +47,7 @@ func (e *Engine) RunStringContext(ctx context.Context, script string) (any, erro
 // CallFunction calls the function and returns the go type value
 // if function result is promise, it will be resolved
 func (e *Engine) CallFunction(fn any, args ...any) (value any, err error) {
-	return e.CallFunctionContext(context.Background(), fn, args...)
-}
-
-func (e *Engine) CallFunctionContext(ctx context.Context, fn any, args ...any) (value any, err error) {
-	return e.runOnLoopContext(ctx, func(runtime *goja.Runtime) (goja.Value, error) {
+	return e.runOnLoop(func(runtime *goja.Runtime) (goja.Value, error) {
 		var jsArgs []goja.Value
 		for _, arg := range args {
 			jsArgs = append(jsArgs, runtime.ToValue(arg))
@@ -81,20 +69,6 @@ func (e *Engine) CallFunctionContext(ctx context.Context, fn any, args ...any) (
 }
 
 func (e *Engine) runOnLoop(fn func(runtime *goja.Runtime) (goja.Value, error)) (any, error) {
-	return e.runOnLoopContext(context.Background(), fn)
-}
-
-var ErrClosed = errors.New("engine closed")
-
-func (e *Engine) runOnLoopContext(ctx context.Context, fn func(runtime *goja.Runtime) (goja.Value, error)) (any, error) {
-	if err := ctx.Err(); err != nil {
-		return nil, err
-	}
-	select {
-	case <-e.done:
-		return nil, ErrClosed
-	default:
-	}
 	type result struct {
 		value any
 		err   error
@@ -158,31 +132,15 @@ func (e *Engine) runOnLoopContext(ctx context.Context, fn func(runtime *goja.Run
 	if !ok {
 		return nil, errors.New("engine loop terminated")
 	}
-	select {
-	case res := <-ch:
-		return res.value, res.err
-	case <-ctx.Done():
-		// Interrupt synchronous JS too; a select alone only stops Promise waiting.
-		e.Close()
-		return nil, ctx.Err()
-	case <-e.done:
-		return nil, ErrClosed
+	res := <-ch
+	if res.err != nil {
+		return nil, res.err
 	}
+	return res.value, nil
 }
 
 func (e *Engine) Close() {
 	e.closeOnce.Do(func() {
-		close(e.done)
-		e.Runtime.Interrupt(ErrClosed)
-		// Wait for interrupted JS to leave the loop, then allow cleanup hooks
-		// to execute their JS cancellation callbacks before terminating it.
-		cleared := make(chan struct{})
-		if e.loop.RunOnLoop(func(runtime *goja.Runtime) {
-			runtime.ClearInterrupt()
-			close(cleared)
-		}) {
-			<-cleared
-		}
 		e.cleanupMu.Lock()
 		cleanups := e.cleanups
 		e.cleanups = nil
@@ -219,7 +177,6 @@ func NewEngine(cfg *Config) *Engine {
 	loop := eventloop.NewEventLoop()
 	engine := &Engine{
 		loop: loop,
-		done: make(chan struct{}),
 	}
 	loop.Start()
 	done := make(chan struct{})

pkg/download/extension.go

@@ -3,7 +3,6 @@ package download
 import (
 	"context"
 	"encoding/json"
-	"errors"
 	"fmt"
 	"io"
 	"net/url"
@@ -328,8 +327,8 @@ func (d *Downloader) triggerOnResolve(req *base.Request) (res *base.Resource, er
 	return
 }
 
-func (d *Downloader) triggerOnStart(task *Task) error {
-	return doTrigger(d,
+func (d *Downloader) triggerOnStart(task *Task) {
+	doTrigger(d,
 		EventOnStart,
 		task.Meta.Req,
 		&OnStartContext{
@@ -414,13 +413,6 @@ func doTrigger[T any](d *Downloader, event ActivationEvent, req *base.Request, c
 					}
 					engine, session := d.newExtensionEngine()
 					defer session.CloseIfIdle()
-					eventCtx, cancelEvent := context.WithTimeout(d.backgroundCtx, d.cfg.ExtensionEventTimeout)
-					defer cancelEvent()
-					defer func() {
-						if eventCtx.Err() != nil {
-							session.Close()
-						}
-					}()
 					gopeed.Runtime = &InstanceRuntime{
 						WebView: d.newExtensionWebViewRuntime(session, ext.buildIdentity()),
 					}
@@ -429,13 +421,13 @@ func doTrigger[T any](d *Downloader, event ActivationEvent, req *base.Request, c
 						gopeed.Logger.logger.Error().Err(err).Msgf("[%s] engine inject failed", ext.buildIdentity())
 						return
 					}
-					_, err = engine.RunStringContext(eventCtx, string(scriptBuf))
+					_, err = engine.RunString(string(scriptBuf))
 					if err != nil {
 						gopeed.Logger.logger.Error().Err(err).Msgf("[%s] run script failed", ext.buildIdentity())
 						return
 					}
 					if fn, ok := gopeed.Events[event]; ok {
-						_, err = engine.CallFunctionContext(eventCtx, fn, ctx)
+						_, err = engine.CallFunction(fn, ctx)
 						if err != nil {
 							gopeed.Logger.logger.Error().Err(err).Msgf("[%s] call function failed: %s", ext.buildIdentity(), event)
 							return
@@ -445,9 +437,6 @@ func doTrigger[T any](d *Downloader, event ActivationEvent, req *base.Request, c
 						}
 					}
 				}()
-				if errors.Is(err, context.Canceled) || errors.Is(err, context.DeadlineExceeded) {
-					return err
-				}
 			}
 		}
 	}

pkg/download/extension_cancellation_test.go

@@ -1,149 +0,0 @@
-package download
-
-import (
-	"context"
-	"errors"
-	"fmt"
-	"net/http"
-	"net/http/httptest"
-	"os"
-	"path/filepath"
-	"sync"
-	"testing"
-	"time"
-
-	"github.com/GopeedLab/gopeed/internal/fetcher"
-	"github.com/GopeedLab/gopeed/pkg/base"
-)
-
-func installCancellationExtension(t *testing.T, d *Downloader, event, script string) {
-	t.Helper()
-	dir := t.TempDir()
-	manifest := fmt.Sprintf(`{"name":"cancel-%s","author":"test","title":"Cancel","version":"1.0.0","scripts":[{"event":%q,"match":{"urls":["*://example.com/*"]},"entry":"index.js"}]}`, event, event)
-	for name, body := range map[string]string{"manifest.json": manifest, "index.js": script} {
-		if err := os.WriteFile(filepath.Join(dir, name), []byte(body), 0600); err != nil {
-			t.Fatal(err)
-		}
-	}
-	if _, err := d.InstallExtensionByFolder(dir, true); err != nil {
-		t.Fatal(err)
-	}
-}
-func newCancellationDownloader(t *testing.T) *Downloader {
-	t.Helper()
-	d := NewDownloader(&DownloaderConfig{Storage: NewMemStorage(), StorageDir: t.TempDir(), TempDir: t.TempDir(), FetchManagers: []fetcher.FetcherManager{&generationTestManager{holdOpen: true}}})
-	if err := d.Setup(); err != nil {
-		t.Fatal(err)
-	}
-	return d
-}
-func TestResolveExtensionStopsOnShutdown(t *testing.T) {
-	entered, release := make(chan struct{}), make(chan struct{})
-	var once sync.Once
-	defer once.Do(func() { close(release) })
-	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
-		close(entered)
-		select {
-		case <-release:
-		case <-r.Context().Done():
-		}
-		w.Write([]byte("ok"))
-	}))
-	defer server.Close()
-	d := newCancellationDownloader(t)
-	defer d.Close()
-	installCancellationExtension(t, d, "onResolve", fmt.Sprintf(`gopeed.events.onResolve(async ctx=>{await fetch(%q);});`, server.URL))
-	done := make(chan error, 1)
-	go func() { _, err := d.Resolve(&base.Request{URL: "https://example.com/cancel"}); done <- err }()
-	select {
-	case <-entered:
-	case <-time.After(2 * time.Second):
-		t.Fatal("handler did not enter")
-	}
-	if err := d.Close(); err != nil {
-		t.Fatal(err)
-	}
-	select {
-	case err := <-done:
-		if !errors.Is(err, context.Canceled) {
-			t.Errorf("got %v, want cancellation", err)
-		}
-	case <-time.After(300 * time.Millisecond):
-		once.Do(func() { close(release) })
-		t.Fatal("shutdown did not cancel extension Resolve")
-	}
-}
-func TestOnErrorCancellationReleasesTaskAPIs(t *testing.T) {
-	entered, release := make(chan struct{}), make(chan struct{})
-	var once sync.Once
-	defer once.Do(func() { close(release) })
-	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
-		close(entered)
-		select {
-		case <-release:
-		case <-r.Context().Done():
-		}
-		w.Write([]byte("ok"))
-	}))
-	defer server.Close()
-	d := newCancellationDownloader(t)
-	defer d.Close()
-	parent, cancel := context.WithCancel(d.backgroundCtx)
-	d.backgroundCtx = parent
-	defer cancel()
-	installCancellationExtension(t, d, "onError", fmt.Sprintf(`gopeed.events.onError(async ctx=>{await fetch(%q);});`, server.URL))
-	d.cfg.MaxRunning = 0
-	id, err := d.CreateDirect(&base.Request{URL: "generation://example.com/task"}, &base.Options{Path: t.TempDir()})
-	if err != nil {
-		t.Fatal(err)
-	}
-	task := d.GetTask(id)
-	task.statusLock.Lock()
-	task.Status = base.DownloadStatusRunning
-	task.statusLock.Unlock()
-	handled := make(chan struct{})
-	go func() { d.doOnError(task, errors.New("source failed")); close(handled) }()
-	select {
-	case <-entered:
-	case <-time.After(2 * time.Second):
-		t.Fatal("onError did not enter")
-	}
-	status, stats := make(chan struct{}), make(chan struct{})
-	go func() { d.RuntimeStatus(id); close(status) }()
-	go func() { d.Stats(id); close(stats) }()
-	cancel()
-	for name, ch := range map[string]<-chan struct{}{"onError": handled, "RuntimeStatus": status, "Stats": stats} {
-		select {
-		case <-ch:
-		case <-time.After(300 * time.Millisecond):
-			once.Do(func() { close(release) })
-			<-handled
-			t.Fatalf("%s stayed blocked after cancellation", name)
-		}
-	}
-}
-
-func TestExtensionEventDeadlineRejectsUnsettledResolve(t *testing.T) {
-	d := newCancellationDownloader(t)
-	defer d.Close()
-	d.cfg.ExtensionEventTimeout = 30 * time.Millisecond
-	installCancellationExtension(t, d, "onResolve", `gopeed.events.onResolve(async ctx=>{await new Promise(()=>{});});`)
-	_, err := d.Resolve(&base.Request{URL: "https://example.com/timeout"})
-	if !errors.Is(err, context.DeadlineExceeded) {
-		t.Fatalf("got %v, want event deadline exceeded", err)
-	}
-}
-func TestExtensionStartDeadlinePreventsFetcherStart(t *testing.T) {
-	d := newCancellationDownloader(t)
-	defer d.Close()
-	d.cfg.ExtensionEventTimeout = 30 * time.Millisecond
-	installCancellationExtension(t, d, "onStart", `gopeed.events.onStart(async ctx=>{await new Promise(()=>{});});`)
-	id, err := d.CreateDirect(&base.Request{URL: "generation://example.com/timeout"}, &base.Options{Path: t.TempDir()})
-	if err != nil {
-		t.Fatal(err)
-	}
-	waitForTaskStatus(t, d, id, base.DownloadStatusError, 2*time.Second)
-	if starts := d.cfg.FetchManagers[0].(*generationTestManager).starts.Load(); starts != 0 {
-		t.Fatalf("timed out onStart still started fetcher %d times", starts)
-	}
-}

pkg/download/extension_engine_session.go

@@ -59,21 +59,6 @@ func (s *engineSession) Release() {
 	}
 }
 
-// Close releases retained resources when the owning event is cancelled.
-func (s *engineSession) Close() {
-	s.mu.Lock()
-	if s.closed {
-		s.mu.Unlock()
-		return
-	}
-	s.closed = true
-	s.mu.Unlock()
-	s.runClosers()
-	if s.engine != nil {
-		s.engine.Close()
-	}
-}
-
 func (s *engineSession) CloseIfIdle() {
 	s.mu.Lock()
 	shouldClose := s.refs == 0 && !s.closed

pkg/download/model.go

@@ -232,15 +232,12 @@ type DownloaderConfig struct {
 	Controller    *controller.Controller
 	FetchManagers []fetcher.FetcherManager
 
-	// ExtensionEventTimeout bounds one script load and its awaited event handler.
-	// Zero uses one minute. Embedders may increase it for long-running handlers.
-	ExtensionEventTimeout time.Duration
-	RefreshInterval       int // RefreshInterval time duration to refresh task progress(ms)
-	Storage               Storage
-	StorageDir            string
-	TempDir               string
-	WhiteDownloadDirs     []string
-	WebViewProvider       enginewebview.Provider
+	RefreshInterval   int // RefreshInterval time duration to refresh task progress(ms)
+	Storage           Storage
+	StorageDir        string
+	TempDir           string
+	WhiteDownloadDirs []string
+	WebViewProvider   enginewebview.Provider
 
 	ProductionMode bool
 
@@ -263,9 +260,6 @@ func (cfg *DownloaderConfig) Init() *DownloaderConfig {
 			new(ed2k.FetcherManager),
 		}
 	}
-	if cfg.ExtensionEventTimeout <= 0 {
-		cfg.ExtensionEventTimeout = time.Minute
-	}
 	if cfg.RefreshInterval == 0 {
 		cfg.RefreshInterval = 350
 	}

ui/flutter/lib/app/application/app_runtime_controller.dart

@@ -53,6 +53,7 @@ class AppRuntimeState {
 class AppRuntimeController extends AsyncNotifier<AppRuntimeState> {
   static StartConfig? _defaultStartConfig;
   Future<void> _preferenceWrites = Future<void>.value();
+  bool _analyticsStarted = false;
 
   @override
   Future<AppRuntimeState> build() async {
@@ -73,6 +74,7 @@ class AppRuntimeController extends AsyncNotifier<AppRuntimeState> {
       startConfig
         ..apiEnable = true
         ..mcpEnable = config.api.mcpEnable;
+      await _initAnalytics(config);
     } else {
       startConfig
         ..apiEnable = config.api.enable
@@ -253,7 +255,8 @@ class AppRuntimeController extends AsyncNotifier<AppRuntimeState> {
   }
 
   Future<void> _initAnalytics(DownloaderConfig config) async {
-    if (!AnalyticsConfig.isConfigured || !config.extra.analyticsEnabled) return;
+    if (_analyticsStarted || !AnalyticsConfig.isConfigured || !config.extra.analyticsEnabled) return;
+    _analyticsStarted = true;
     try {
       final generatedClientId = await Analytics.instance.init(fallbackClientId: config.extra.deviceId);
       if (generatedClientId != null) {
@@ -262,6 +265,7 @@ class AppRuntimeController extends AsyncNotifier<AppRuntimeState> {
       }
       unawaited(Analytics.instance.logAppOpen());
     } catch (error, stackTrace) {
+      _analyticsStarted = false;
       logger.w('GA4 init failed', error, stackTrace);
     }
   }

ui/flutter/lib/app/application/downloader_config_init.dart

@@ -19,7 +19,7 @@ Future<DownloaderConfig> loadDownloaderConfig({
     config = await load();
   } catch (error, stackTrace) {
     onLoadError?.call(error, stackTrace);
-    config = DownloaderConfig();
+    config = DownloaderConfig()..extra.analyticsEnabled = false;
     loaded = false;
   }
   final changed = await initDownloaderConfig(config, defaultDownloadDir: defaultDownloadDir);

ui/flutter/test/app/application/downloader_config_init_test.dart

@@ -32,8 +32,22 @@ void main() {
       // The in-memory fallback still gets usable defaults.
       expect(config.downloadDir, '/downloads');
       expect(config.categories, hasLength(4));
+      expect(config.extra.analyticsEnabled, isFalse);
     });
 
+    for (final analyticsEnabled in [false, true]) {
+      test('preserves the saved analytics preference ($analyticsEnabled)', () async {
+        final stored = configWith()..extra.analyticsEnabled = analyticsEnabled;
+        final config = await loadDownloaderConfig(
+          load: () async => stored,
+          save: (_) async {},
+          defaultDownloadDir: () async => '/downloads',
+        );
+
+        expect(config.extra.analyticsEnabled, analyticsEnabled);
+      });
+    }
+
     test('skips persist when init changed nothing', () async {
       var saved = 0;
       await loadDownloaderConfig(