feat: implement wave1 A4-A6 crash oauth and filter hardening
- commit
- 4bb25831b515904bce509a19d5b7c46cdabccb4d
- author
- brice · · 2026-02-07 05:06 UTC
- parent
- 9b64c67
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
8 files · +844 -55
M internal/infrastructure/filtering/manager.go +125 -2
| @@ -17,6 +17,7 @@ import ( | |||
| 17 | 17 | const ( | |
| 18 | 18 | storeDirPerm = 0o755 | |
| 19 | 19 | jsonDirPerm = 0o755 | |
| 20 | + | quarantinePerm = 0o755 | |
| 20 | 21 | mergedFilePerm = 0o644 | |
| 21 | 22 | ||
| 22 | 23 | // CacheMaxAge is the maximum age of the filter cache before it's considered stale. | |
| @@ -190,6 +191,12 @@ func (m *Manager) LoadAsync(ctx context.Context) { | |||
| 190 | 191 | return | |
| 191 | 192 | } | |
| 192 | 193 | ||
| 194 | + | if err := m.validateDownloadedFiles(ctx, paths); err != nil { | |
| 195 | + | log.Error().Err(err).Msg("downloaded filter validation failed") | |
| 196 | + | m.setStatus(FilterStatus{State: StateError, Message: "Invalid downloaded filters"}) | |
| 197 | + | return | |
| 198 | + | } | |
| 199 | + | ||
| 193 | 200 | m.setStatus(FilterStatus{State: StateLoading, Message: "Compiling filters..."}) | |
| 194 | 201 | ||
| 195 | 202 | // Merge JSON files into one for compilation | |
| @@ -204,6 +211,29 @@ func (m *Manager) LoadAsync(ctx context.Context) { | |||
| 204 | 211 | filter, err := m.store.Compile(ctx, FilterIdentifier, mergedPath) | |
| 205 | 212 | if err != nil { | |
| 206 | 213 | log.Error().Err(err).Msg("failed to compile filters") | |
| 214 | + | if m.hasActiveFilter() { | |
| 215 | + | version := m.getCachedVersion() | |
| 216 | + | m.setStatus(FilterStatus{ | |
| 217 | + | State: StateActive, | |
| 218 | + | Message: "Filters active (update skipped)", | |
| 219 | + | Version: version, | |
| 220 | + | }) | |
| 221 | + | return | |
| 222 | + | } | |
| 223 | + | m.setStatus(FilterStatus{State: StateError, Message: "Compilation failed"}) | |
| 224 | + | return | |
| 225 | + | } | |
| 226 | + | if filter == nil { | |
| 227 | + | log.Error().Msg("filter compilation returned nil filter") | |
| 228 | + | if m.hasActiveFilter() { | |
| 229 | + | version := m.getCachedVersion() | |
| 230 | + | m.setStatus(FilterStatus{ | |
| 231 | + | State: StateActive, | |
| 232 | + | Message: "Filters active (update skipped)", | |
| 233 | + | Version: version, | |
| 234 | + | }) | |
| 235 | + | return | |
| 236 | + | } | |
| 207 | 237 | m.setStatus(FilterStatus{State: StateError, Message: "Compilation failed"}) | |
| 208 | 238 | return | |
| 209 | 239 | } | |
| @@ -345,16 +375,26 @@ func (m *Manager) CheckForUpdates(ctx context.Context) error { | |||
| 345 | 375 | if err != nil { | |
| 346 | 376 | return fmt.Errorf("failed to download filter update: %w", err) | |
| 347 | 377 | } | |
| 378 | + | if err := m.validateDownloadedFiles(ctx, paths); err != nil { | |
| 379 | + | log.Warn().Err(err).Msg("invalid downloaded filters, keeping existing active filter") | |
| 380 | + | return nil | |
| 381 | + | } | |
| 348 | 382 | ||
| 349 | 383 | // Merge and compile | |
| 350 | 384 | mergedPath, err := m.mergeJSONFiles(ctx, paths) | |
| 351 | 385 | if err != nil { | |
| 352 | - | return fmt.Errorf("failed to merge updated filters: %w", err) | |
| 386 | + | log.Warn().Err(err).Msg("failed to merge updated filters, keeping existing active filter") | |
| 387 | + | return nil | |
| 353 | 388 | } | |
| 354 | 389 | ||
| 355 | 390 | filter, err := m.store.Compile(ctx, FilterIdentifier, mergedPath) | |
| 356 | 391 | if err != nil { | |
| 357 | - | return fmt.Errorf("failed to compile updated filters: %w", err) | |
| 392 | + | log.Warn().Err(err).Msg("failed to compile updated filters, keeping existing active filter") | |
| 393 | + | return nil | |
| 394 | + | } | |
| 395 | + | if filter == nil { | |
| 396 | + | log.Warn().Msg("updated filter compilation returned nil filter, keeping existing active filter") | |
| 397 | + | return nil | |
| 358 | 398 | } | |
| 359 | 399 | ||
| 360 | 400 | m.filterMu.Lock() | |
| @@ -372,6 +412,89 @@ func (m *Manager) CheckForUpdates(ctx context.Context) error { | |||
| 372 | 412 | return nil | |
| 373 | 413 | } | |
| 374 | 414 | ||
| 415 | + | func (m *Manager) hasActiveFilter() bool { | |
| 416 | + | m.filterMu.RLock() | |
| 417 | + | defer m.filterMu.RUnlock() | |
| 418 | + | return m.filter != nil | |
| 419 | + | } | |
| 420 | + | ||
| 421 | + | func (m *Manager) validateDownloadedFiles(ctx context.Context, paths []string) error { | |
| 422 | + | for _, path := range paths { | |
| 423 | + | if err := validateRuleFile(path); err != nil { | |
| 424 | + | if quarantineErr := m.quarantineInvalidFile(ctx, path, "rule_validation_failed"); quarantineErr != nil { | |
| 425 | + | return fmt.Errorf("invalid filter file %s (%v), quarantine failed: %w", path, err, quarantineErr) | |
| 426 | + | } | |
| 427 | + | return fmt.Errorf("invalid filter file %s: %w", path, err) | |
| 428 | + | } | |
| 429 | + | } | |
| 430 | + | return nil | |
| 431 | + | } | |
| 432 | + | ||
| 433 | + | func validateRuleFile(path string) error { | |
| 434 | + | data, err := os.ReadFile(path) | |
| 435 | + | if err != nil { | |
| 436 | + | return fmt.Errorf("read failed: %w", err) | |
| 437 | + | } | |
| 438 | + | ||
| 439 | + | var rules []map[string]json.RawMessage | |
| 440 | + | if err := json.Unmarshal(data, &rules); err != nil { | |
| 441 | + | return fmt.Errorf("json parse failed: %w", err) | |
| 442 | + | } | |
| 443 | + | if len(rules) == 0 { | |
| 444 | + | return fmt.Errorf("empty rule set") | |
| 445 | + | } | |
| 446 | + | ||
| 447 | + | for i, rule := range rules { | |
| 448 | + | trigger, ok := rule["trigger"] | |
| 449 | + | if !ok || len(trigger) == 0 || string(trigger) == "null" { | |
| 450 | + | return fmt.Errorf("rule[%d] missing trigger", i) | |
| 451 | + | } | |
| 452 | + | action, ok := rule["action"] | |
| 453 | + | if !ok || len(action) == 0 || string(action) == "null" { | |
| 454 | + | return fmt.Errorf("rule[%d] missing action", i) | |
| 455 | + | } | |
| 456 | + | var actionObj struct { | |
| 457 | + | Type string `json:"type"` | |
| 458 | + | } | |
| 459 | + | if err := json.Unmarshal(action, &actionObj); err != nil { | |
| 460 | + | return fmt.Errorf("rule[%d] action parse failed: %w", i, err) | |
| 461 | + | } | |
| 462 | + | if actionObj.Type == "" { | |
| 463 | + | return fmt.Errorf("rule[%d] action.type missing", i) | |
| 464 | + | } | |
| 465 | + | } | |
| 466 | + | ||
| 467 | + | return nil | |
| 468 | + | } | |
| 469 | + | ||
| 470 | + | func (m *Manager) quarantineInvalidFile(ctx context.Context, srcPath string, cause string) error { | |
| 471 | + | log := logging.FromContext(ctx).With(). | |
| 472 | + | Str("component", "filter-manager"). | |
| 473 | + | Logger() | |
| 474 | + | ||
| 475 | + | quarantineDir := filepath.Join(m.jsonDir, "quarantine") | |
| 476 | + | if err := os.MkdirAll(quarantineDir, quarantinePerm); err != nil { | |
| 477 | + | return fmt.Errorf("create quarantine dir failed: %w", err) | |
| 478 | + | } | |
| 479 | + | ||
| 480 | + | baseName := filepath.Base(srcPath) | |
| 481 | + | quarantinePath := filepath.Join( | |
| 482 | + | quarantineDir, | |
| 483 | + | fmt.Sprintf("%d-%s", time.Now().UnixNano(), baseName), | |
| 484 | + | ) | |
| 485 | + | if err := os.Rename(srcPath, quarantinePath); err != nil { | |
| 486 | + | return fmt.Errorf("move to quarantine failed: %w", err) | |
| 487 | + | } | |
| 488 | + | ||
| 489 | + | log.Error(). | |
| 490 | + | Str("path", srcPath). | |
| 491 | + | Str("quarantine_path", quarantinePath). | |
| 492 | + | Str("cause", cause). | |
| 493 | + | Msg("quarantined invalid filter file") | |
| 494 | + | ||
| 495 | + | return nil | |
| 496 | + | } | |
| 497 | + | ||
| 375 | 498 | // Enable enables content filtering. | |
| 376 | 499 | func (m *Manager) Enable(ctx context.Context) { | |
| 377 | 500 | log := logging.FromContext(ctx).With(). | |
| @@ -17,6 +17,7 @@ import ( | |||||
| 17 | const ( | 17 | const ( | ||
| 18 | storeDirPerm = 0o755 | 18 | storeDirPerm = 0o755 | ||
| 19 | jsonDirPerm = 0o755 | 19 | jsonDirPerm = 0o755 | ||
| 20 | + | quarantinePerm = 0o755 | |||
| 20 | mergedFilePerm = 0o644 | 21 | mergedFilePerm = 0o644 | ||
| 21 | 22 | ||||
| 22 | // CacheMaxAge is the maximum age of the filter cache before it's considered stale. | 23 | // CacheMaxAge is the maximum age of the filter cache before it's considered stale. | ||
| @@ -190,6 +191,12 @@ func (m *Manager) LoadAsync(ctx context.Context) { | |||||
| 190 | return | 191 | return | ||
| 191 | } | 192 | } | ||
| 192 | 193 | ||||
| 194 | + | if err := m.validateDownloadedFiles(ctx, paths); err != nil { | |||
| 195 | + | log.Error().Err(err).Msg("downloaded filter validation failed") | |||
| 196 | + | m.setStatus(FilterStatus{State: StateError, Message: "Invalid downloaded filters"}) | |||
| 197 | + | return | |||
| 198 | + | } | |||
| 199 | + | ||||
| 193 | m.setStatus(FilterStatus{State: StateLoading, Message: "Compiling filters..."}) | 200 | m.setStatus(FilterStatus{State: StateLoading, Message: "Compiling filters..."}) | ||
| 194 | 201 | ||||
| 195 | // Merge JSON files into one for compilation | 202 | // Merge JSON files into one for compilation | ||
| @@ -204,6 +211,29 @@ func (m *Manager) LoadAsync(ctx context.Context) { | |||||
| 204 | filter, err := m.store.Compile(ctx, FilterIdentifier, mergedPath) | 211 | filter, err := m.store.Compile(ctx, FilterIdentifier, mergedPath) | ||
| 205 | if err != nil { | 212 | if err != nil { | ||
| 206 | log.Error().Err(err).Msg("failed to compile filters") | 213 | log.Error().Err(err).Msg("failed to compile filters") | ||
| 214 | + | if m.hasActiveFilter() { | |||
| 215 | + | version := m.getCachedVersion() | |||
| 216 | + | m.setStatus(FilterStatus{ | |||
| 217 | + | State: StateActive, | |||
| 218 | + | Message: "Filters active (update skipped)", | |||
| 219 | + | Version: version, | |||
| 220 | + | }) | |||
| 221 | + | return | |||
| 222 | + | } | |||
| 223 | + | m.setStatus(FilterStatus{State: StateError, Message: "Compilation failed"}) | |||
| 224 | + | return | |||
| 225 | + | } | |||
| 226 | + | if filter == nil { | |||
| 227 | + | log.Error().Msg("filter compilation returned nil filter") | |||
| 228 | + | if m.hasActiveFilter() { | |||
| 229 | + | version := m.getCachedVersion() | |||
| 230 | + | m.setStatus(FilterStatus{ | |||
| 231 | + | State: StateActive, | |||
| 232 | + | Message: "Filters active (update skipped)", | |||
| 233 | + | Version: version, | |||
| 234 | + | }) | |||
| 235 | + | return | |||
| 236 | + | } | |||
| 207 | m.setStatus(FilterStatus{State: StateError, Message: "Compilation failed"}) | 237 | m.setStatus(FilterStatus{State: StateError, Message: "Compilation failed"}) | ||
| 208 | return | 238 | return | ||
| 209 | } | 239 | } | ||
| @@ -345,16 +375,26 @@ func (m *Manager) CheckForUpdates(ctx context.Context) error { | |||||
| 345 | if err != nil { | 375 | if err != nil { | ||
| 346 | return fmt.Errorf("failed to download filter update: %w", err) | 376 | return fmt.Errorf("failed to download filter update: %w", err) | ||
| 347 | } | 377 | } | ||
| 378 | + | if err := m.validateDownloadedFiles(ctx, paths); err != nil { | |||
| 379 | + | log.Warn().Err(err).Msg("invalid downloaded filters, keeping existing active filter") | |||
| 380 | + | return nil | |||
| 381 | + | } | |||
| 348 | 382 | ||||
| 349 | // Merge and compile | 383 | // Merge and compile | ||
| 350 | mergedPath, err := m.mergeJSONFiles(ctx, paths) | 384 | mergedPath, err := m.mergeJSONFiles(ctx, paths) | ||
| 351 | if err != nil { | 385 | if err != nil { | ||
| 352 | - | return fmt.Errorf("failed to merge updated filters: %w", err) | 386 | + | log.Warn().Err(err).Msg("failed to merge updated filters, keeping existing active filter") |
| 387 | + | return nil | |||
| 353 | } | 388 | } | ||
| 354 | 389 | ||||
| 355 | filter, err := m.store.Compile(ctx, FilterIdentifier, mergedPath) | 390 | filter, err := m.store.Compile(ctx, FilterIdentifier, mergedPath) | ||
| 356 | if err != nil { | 391 | if err != nil { | ||
| 357 | - | return fmt.Errorf("failed to compile updated filters: %w", err) | 392 | + | log.Warn().Err(err).Msg("failed to compile updated filters, keeping existing active filter") |
| 393 | + | return nil | |||
| 394 | + | } | |||
| 395 | + | if filter == nil { | |||
| 396 | + | log.Warn().Msg("updated filter compilation returned nil filter, keeping existing active filter") | |||
| 397 | + | return nil | |||
| 358 | } | 398 | } | ||
| 359 | 399 | ||||
| 360 | m.filterMu.Lock() | 400 | m.filterMu.Lock() | ||
| @@ -372,6 +412,89 @@ func (m *Manager) CheckForUpdates(ctx context.Context) error { | |||||
| 372 | return nil | 412 | return nil | ||
| 373 | } | 413 | } | ||
| 374 | 414 | ||||
| 415 | + | func (m *Manager) hasActiveFilter() bool { | |||
| 416 | + | m.filterMu.RLock() | |||
| 417 | + | defer m.filterMu.RUnlock() | |||
| 418 | + | return m.filter != nil | |||
| 419 | + | } | |||
| 420 | + | ||||
| 421 | + | func (m *Manager) validateDownloadedFiles(ctx context.Context, paths []string) error { | |||
| 422 | + | for _, path := range paths { | |||
| 423 | + | if err := validateRuleFile(path); err != nil { | |||
| 424 | + | if quarantineErr := m.quarantineInvalidFile(ctx, path, "rule_validation_failed"); quarantineErr != nil { | |||
| 425 | + | return fmt.Errorf("invalid filter file %s (%v), quarantine failed: %w", path, err, quarantineErr) | |||
| 426 | + | } | |||
| 427 | + | return fmt.Errorf("invalid filter file %s: %w", path, err) | |||
| 428 | + | } | |||
| 429 | + | } | |||
| 430 | + | return nil | |||
| 431 | + | } | |||
| 432 | + | ||||
| 433 | + | func validateRuleFile(path string) error { | |||
| 434 | + | data, err := os.ReadFile(path) | |||
| 435 | + | if err != nil { | |||
| 436 | + | return fmt.Errorf("read failed: %w", err) | |||
| 437 | + | } | |||
| 438 | + | ||||
| 439 | + | var rules []map[string]json.RawMessage | |||
| 440 | + | if err := json.Unmarshal(data, &rules); err != nil { | |||
| 441 | + | return fmt.Errorf("json parse failed: %w", err) | |||
| 442 | + | } | |||
| 443 | + | if len(rules) == 0 { | |||
| 444 | + | return fmt.Errorf("empty rule set") | |||
| 445 | + | } | |||
| 446 | + | ||||
| 447 | + | for i, rule := range rules { | |||
| 448 | + | trigger, ok := rule["trigger"] | |||
| 449 | + | if !ok || len(trigger) == 0 || string(trigger) == "null" { | |||
| 450 | + | return fmt.Errorf("rule[%d] missing trigger", i) | |||
| 451 | + | } | |||
| 452 | + | action, ok := rule["action"] | |||
| 453 | + | if !ok || len(action) == 0 || string(action) == "null" { | |||
| 454 | + | return fmt.Errorf("rule[%d] missing action", i) | |||
| 455 | + | } | |||
| 456 | + | var actionObj struct { | |||
| 457 | + | Type string `json:"type"` | |||
| 458 | + | } | |||
| 459 | + | if err := json.Unmarshal(action, &actionObj); err != nil { | |||
| 460 | + | return fmt.Errorf("rule[%d] action parse failed: %w", i, err) | |||
| 461 | + | } | |||
| 462 | + | if actionObj.Type == "" { | |||
| 463 | + | return fmt.Errorf("rule[%d] action.type missing", i) | |||
| 464 | + | } | |||
| 465 | + | } | |||
| 466 | + | ||||
| 467 | + | return nil | |||
| 468 | + | } | |||
| 469 | + | ||||
| 470 | + | func (m *Manager) quarantineInvalidFile(ctx context.Context, srcPath string, cause string) error { | |||
| 471 | + | log := logging.FromContext(ctx).With(). | |||
| 472 | + | Str("component", "filter-manager"). | |||
| 473 | + | Logger() | |||
| 474 | + | ||||
| 475 | + | quarantineDir := filepath.Join(m.jsonDir, "quarantine") | |||
| 476 | + | if err := os.MkdirAll(quarantineDir, quarantinePerm); err != nil { | |||
| 477 | + | return fmt.Errorf("create quarantine dir failed: %w", err) | |||
| 478 | + | } | |||
| 479 | + | ||||
| 480 | + | baseName := filepath.Base(srcPath) | |||
| 481 | + | quarantinePath := filepath.Join( | |||
| 482 | + | quarantineDir, | |||
| 483 | + | fmt.Sprintf("%d-%s", time.Now().UnixNano(), baseName), | |||
| 484 | + | ) | |||
| 485 | + | if err := os.Rename(srcPath, quarantinePath); err != nil { | |||
| 486 | + | return fmt.Errorf("move to quarantine failed: %w", err) | |||
| 487 | + | } | |||
| 488 | + | ||||
| 489 | + | log.Error(). | |||
| 490 | + | Str("path", srcPath). | |||
| 491 | + | Str("quarantine_path", quarantinePath). | |||
| 492 | + | Str("cause", cause). | |||
| 493 | + | Msg("quarantined invalid filter file") | |||
| 494 | + | ||||
| 495 | + | return nil | |||
| 496 | + | } | |||
| 497 | + | ||||
| 375 | // Enable enables content filtering. | 498 | // Enable enables content filtering. | ||
| 376 | func (m *Manager) Enable(ctx context.Context) { | 499 | func (m *Manager) Enable(ctx context.Context) { | ||
| 377 | log := logging.FromContext(ctx).With(). | 500 | log := logging.FromContext(ctx).With(). | ||
M internal/infrastructure/filtering/manager_test.go +117 -4
| @@ -2,6 +2,7 @@ package filtering_test | |||
| 2 | 2 | ||
| 3 | 3 | import ( | |
| 4 | 4 | "context" | |
| 5 | + | "errors" | |
| 5 | 6 | "os" | |
| 6 | 7 | "path/filepath" | |
| 7 | 8 | "sync" | |
| @@ -11,6 +12,7 @@ import ( | |||
| 11 | 12 | "github.com/bnema/dumber/internal/infrastructure/filtering" | |
| 12 | 13 | "github.com/bnema/dumber/internal/infrastructure/filtering/mocks" | |
| 13 | 14 | "github.com/bnema/dumber/internal/logging" | |
| 15 | + | "github.com/bnema/puregotk-webkit/webkit" | |
| 14 | 16 | "github.com/stretchr/testify/assert" | |
| 15 | 17 | "github.com/stretchr/testify/mock" | |
| 16 | 18 | "github.com/stretchr/testify/require" | |
| @@ -54,7 +56,7 @@ func TestManager_LoadAsync_ChecksCacheFirst(t *testing.T) { | |||
| 54 | 56 | ||
| 55 | 57 | mockStore.EXPECT(). | |
| 56 | 58 | Compile(mock.Anything, filtering.FilterIdentifier, mock.Anything). | |
| 57 | - | Return(nil, nil) | |
| 59 | + | Return(&webkit.UserContentFilter{}, nil) | |
| 58 | 60 | ||
| 59 | 61 | mockDownloader.EXPECT(). | |
| 60 | 62 | GetCachedManifest(). | |
| @@ -144,7 +146,7 @@ func TestManager_LoadAsync_DownloadsWhenCacheMiss(t *testing.T) { | |||
| 144 | 146 | // Compile should be called | |
| 145 | 147 | mockStore.EXPECT(). | |
| 146 | 148 | Compile(mock.Anything, filtering.FilterIdentifier, mock.Anything). | |
| 147 | - | Return(nil, nil) | |
| 149 | + | Return(&webkit.UserContentFilter{}, nil) | |
| 148 | 150 | ||
| 149 | 151 | // GetCachedManifest for version | |
| 150 | 152 | mockDownloader.EXPECT(). | |
| @@ -241,7 +243,7 @@ func TestManager_CheckForUpdates_DownloadsWhenNewVersionAvailable(t *testing.T) | |||
| 241 | 243 | // Compile called | |
| 242 | 244 | mockStore.EXPECT(). | |
| 243 | 245 | Compile(mock.Anything, filtering.FilterIdentifier, mock.Anything). | |
| 244 | - | Return(nil, nil) | |
| 246 | + | Return(&webkit.UserContentFilter{}, nil) | |
| 245 | 247 | ||
| 246 | 248 | // GetCachedManifest for version | |
| 247 | 249 | mockDownloader.EXPECT(). | |
| @@ -451,7 +453,7 @@ func TestManager_StatusCallback_CalledOnStateChange(t *testing.T) { | |||
| 451 | 453 | Return([]string{jsonFile}, nil) | |
| 452 | 454 | mockStore.EXPECT(). | |
| 453 | 455 | Compile(mock.Anything, filtering.FilterIdentifier, mock.Anything). | |
| 454 | - | Return(nil, nil) | |
| 456 | + | Return(&webkit.UserContentFilter{}, nil) | |
| 455 | 457 | mockDownloader.EXPECT(). | |
| 456 | 458 | GetCachedManifest(). | |
| 457 | 459 | Return(&filtering.Manifest{Version: "test"}, nil) | |
| @@ -494,3 +496,114 @@ func TestManager_StatusCallback_CalledOnStateChange(t *testing.T) { | |||
| 494 | 496 | defer mu.Unlock() | |
| 495 | 497 | assert.GreaterOrEqual(t, callCount, 2, "Expected callback to be called at least twice") | |
| 496 | 498 | } | |
| 499 | + | ||
| 500 | + | func TestManager_LoadAsync_QuarantinesInvalidDownloadedPayload(t *testing.T) { | |
| 501 | + | tmpDir := t.TempDir() | |
| 502 | + | storeDir := filepath.Join(tmpDir, "store") | |
| 503 | + | jsonDir := filepath.Join(tmpDir, "json") | |
| 504 | + | ||
| 505 | + | invalidJSONFile := filepath.Join(jsonDir, "combined-part1.json") | |
| 506 | + | require.NoError(t, os.MkdirAll(jsonDir, 0o755)) | |
| 507 | + | require.NoError(t, os.WriteFile(invalidJSONFile, []byte(`{"invalid":`), 0o644)) | |
| 508 | + | ||
| 509 | + | mockStore := mocks.NewMockFilterStore(t) | |
| 510 | + | mockDownloader := mocks.NewMockFilterDownloader(t) | |
| 511 | + | ||
| 512 | + | mockStore.EXPECT(). | |
| 513 | + | HasCompiledFilter(mock.Anything, filtering.FilterIdentifier). | |
| 514 | + | Return(false) | |
| 515 | + | ||
| 516 | + | mockDownloader.EXPECT(). | |
| 517 | + | DownloadFilters(mock.Anything, mock.Anything). | |
| 518 | + | Return([]string{invalidJSONFile}, nil) | |
| 519 | + | ||
| 520 | + | mgr, err := filtering.NewManager(filtering.ManagerConfig{ | |
| 521 | + | StoreDir: storeDir, | |
| 522 | + | JSONDir: jsonDir, | |
| 523 | + | Enabled: true, | |
| 524 | + | AutoUpdate: false, | |
| 525 | + | Store: mockStore, | |
| 526 | + | Downloader: mockDownloader, | |
| 527 | + | }) | |
| 528 | + | require.NoError(t, err) | |
| 529 | + | ||
| 530 | + | ctx := testContext() | |
| 531 | + | require.NoError(t, mgr.Initialize(ctx)) | |
| 532 | + | ||
| 533 | + | mgr.LoadAsync(ctx) | |
| 534 | + | ||
| 535 | + | require.Eventually(t, func() bool { | |
| 536 | + | return mgr.Status().State == filtering.StateError | |
| 537 | + | }, 3*time.Second, 50*time.Millisecond) | |
| 538 | + | assert.Equal(t, "Invalid downloaded filters", mgr.Status().Message) | |
| 539 | + | ||
| 540 | + | _, statErr := os.Stat(invalidJSONFile) | |
| 541 | + | assert.ErrorIs(t, statErr, os.ErrNotExist) | |
| 542 | + | ||
| 543 | + | quarantineDir := filepath.Join(jsonDir, "quarantine") | |
| 544 | + | entries, err := os.ReadDir(quarantineDir) | |
| 545 | + | require.NoError(t, err) | |
| 546 | + | require.Len(t, entries, 1) | |
| 547 | + | } | |
| 548 | + | ||
| 549 | + | func TestManager_CheckForUpdates_CompileFailureKeepsActiveFilter(t *testing.T) { | |
| 550 | + | tmpDir := t.TempDir() | |
| 551 | + | storeDir := filepath.Join(tmpDir, "store") | |
| 552 | + | jsonDir := filepath.Join(tmpDir, "json") | |
| 553 | + | ||
| 554 | + | validJSON := `[{"trigger":{"url-filter":"test"},"action":{"type":"block"}}]` | |
| 555 | + | jsonFile := filepath.Join(jsonDir, "combined-part1.json") | |
| 556 | + | require.NoError(t, os.MkdirAll(jsonDir, 0o755)) | |
| 557 | + | require.NoError(t, os.WriteFile(jsonFile, []byte(validJSON), 0o644)) | |
| 558 | + | ||
| 559 | + | mockStore := mocks.NewMockFilterStore(t) | |
| 560 | + | mockDownloader := mocks.NewMockFilterDownloader(t) | |
| 561 | + | ||
| 562 | + | activeFilter := &webkit.UserContentFilter{} | |
| 563 | + | ||
| 564 | + | mockStore.EXPECT(). | |
| 565 | + | HasCompiledFilter(mock.Anything, filtering.FilterIdentifier). | |
| 566 | + | Return(true).Once() | |
| 567 | + | mockStore.EXPECT(). | |
| 568 | + | Load(mock.Anything, filtering.FilterIdentifier). | |
| 569 | + | Return(activeFilter, nil).Once() | |
| 570 | + | mockDownloader.EXPECT(). | |
| 571 | + | GetCachedManifest(). | |
| 572 | + | Return(&filtering.Manifest{Version: "2025.12.19"}, nil).Once() | |
| 573 | + | mockDownloader.EXPECT(). | |
| 574 | + | IsCacheStale(filtering.CacheMaxAge). | |
| 575 | + | Return(false).Once() | |
| 576 | + | ||
| 577 | + | mockDownloader.EXPECT(). | |
| 578 | + | NeedsUpdate(mock.Anything). | |
| 579 | + | Return(true, nil).Once() | |
| 580 | + | mockDownloader.EXPECT(). | |
| 581 | + | DownloadFilters(mock.Anything, mock.Anything). | |
| 582 | + | Return([]string{jsonFile}, nil).Once() | |
| 583 | + | mockStore.EXPECT(). | |
| 584 | + | Compile(mock.Anything, filtering.FilterIdentifier, mock.Anything). | |
| 585 | + | Return(nil, errors.New("compile failed")).Once() | |
| 586 | + | ||
| 587 | + | mgr, err := filtering.NewManager(filtering.ManagerConfig{ | |
| 588 | + | StoreDir: storeDir, | |
| 589 | + | JSONDir: jsonDir, | |
| 590 | + | Enabled: true, | |
| 591 | + | AutoUpdate: true, | |
| 592 | + | Store: mockStore, | |
| 593 | + | Downloader: mockDownloader, | |
| 594 | + | }) | |
| 595 | + | require.NoError(t, err) | |
| 596 | + | ||
| 597 | + | ctx := testContext() | |
| 598 | + | require.NoError(t, mgr.Initialize(ctx)) | |
| 599 | + | mgr.LoadAsync(ctx) | |
| 600 | + | ||
| 601 | + | require.Eventually(t, func() bool { | |
| 602 | + | return mgr.GetFilter() != nil | |
| 603 | + | }, 3*time.Second, 50*time.Millisecond) | |
| 604 | + | ||
| 605 | + | err = mgr.CheckForUpdates(ctx) | |
| 606 | + | require.NoError(t, err) | |
| 607 | + | assert.Same(t, activeFilter, mgr.GetFilter()) | |
| 608 | + | assert.Equal(t, filtering.StateActive, mgr.Status().State) | |
| 609 | + | } | |
| @@ -2,6 +2,7 @@ package filtering_test | |||||
| 2 | 2 | ||||
| 3 | import ( | 3 | import ( | ||
| 4 | "context" | 4 | "context" | ||
| 5 | + | "errors" | |||
| 5 | "os" | 6 | "os" | ||
| 6 | "path/filepath" | 7 | "path/filepath" | ||
| 7 | "sync" | 8 | "sync" | ||
| @@ -11,6 +12,7 @@ import ( | |||||
| 11 | "github.com/bnema/dumber/internal/infrastructure/filtering" | 12 | "github.com/bnema/dumber/internal/infrastructure/filtering" | ||
| 12 | "github.com/bnema/dumber/internal/infrastructure/filtering/mocks" | 13 | "github.com/bnema/dumber/internal/infrastructure/filtering/mocks" | ||
| 13 | "github.com/bnema/dumber/internal/logging" | 14 | "github.com/bnema/dumber/internal/logging" | ||
| 15 | + | "github.com/bnema/puregotk-webkit/webkit" | |||
| 14 | "github.com/stretchr/testify/assert" | 16 | "github.com/stretchr/testify/assert" | ||
| 15 | "github.com/stretchr/testify/mock" | 17 | "github.com/stretchr/testify/mock" | ||
| 16 | "github.com/stretchr/testify/require" | 18 | "github.com/stretchr/testify/require" | ||
| @@ -54,7 +56,7 @@ func TestManager_LoadAsync_ChecksCacheFirst(t *testing.T) { | |||||
| 54 | 56 | ||||
| 55 | mockStore.EXPECT(). | 57 | mockStore.EXPECT(). | ||
| 56 | Compile(mock.Anything, filtering.FilterIdentifier, mock.Anything). | 58 | Compile(mock.Anything, filtering.FilterIdentifier, mock.Anything). | ||
| 57 | - | Return(nil, nil) | 59 | + | Return(&webkit.UserContentFilter{}, nil) |
| 58 | 60 | ||||
| 59 | mockDownloader.EXPECT(). | 61 | mockDownloader.EXPECT(). | ||
| 60 | GetCachedManifest(). | 62 | GetCachedManifest(). | ||
| @@ -144,7 +146,7 @@ func TestManager_LoadAsync_DownloadsWhenCacheMiss(t *testing.T) { | |||||
| 144 | // Compile should be called | 146 | // Compile should be called | ||
| 145 | mockStore.EXPECT(). | 147 | mockStore.EXPECT(). | ||
| 146 | Compile(mock.Anything, filtering.FilterIdentifier, mock.Anything). | 148 | Compile(mock.Anything, filtering.FilterIdentifier, mock.Anything). | ||
| 147 | - | Return(nil, nil) | 149 | + | Return(&webkit.UserContentFilter{}, nil) |
| 148 | 150 | ||||
| 149 | // GetCachedManifest for version | 151 | // GetCachedManifest for version | ||
| 150 | mockDownloader.EXPECT(). | 152 | mockDownloader.EXPECT(). | ||
| @@ -241,7 +243,7 @@ func TestManager_CheckForUpdates_DownloadsWhenNewVersionAvailable(t *testing.T) | |||||
| 241 | // Compile called | 243 | // Compile called | ||
| 242 | mockStore.EXPECT(). | 244 | mockStore.EXPECT(). | ||
| 243 | Compile(mock.Anything, filtering.FilterIdentifier, mock.Anything). | 245 | Compile(mock.Anything, filtering.FilterIdentifier, mock.Anything). | ||
| 244 | - | Return(nil, nil) | 246 | + | Return(&webkit.UserContentFilter{}, nil) |
| 245 | 247 | ||||
| 246 | // GetCachedManifest for version | 248 | // GetCachedManifest for version | ||
| 247 | mockDownloader.EXPECT(). | 249 | mockDownloader.EXPECT(). | ||
| @@ -451,7 +453,7 @@ func TestManager_StatusCallback_CalledOnStateChange(t *testing.T) { | |||||
| 451 | Return([]string{jsonFile}, nil) | 453 | Return([]string{jsonFile}, nil) | ||
| 452 | mockStore.EXPECT(). | 454 | mockStore.EXPECT(). | ||
| 453 | Compile(mock.Anything, filtering.FilterIdentifier, mock.Anything). | 455 | Compile(mock.Anything, filtering.FilterIdentifier, mock.Anything). | ||
| 454 | - | Return(nil, nil) | 456 | + | Return(&webkit.UserContentFilter{}, nil) |
| 455 | mockDownloader.EXPECT(). | 457 | mockDownloader.EXPECT(). | ||
| 456 | GetCachedManifest(). | 458 | GetCachedManifest(). | ||
| 457 | Return(&filtering.Manifest{Version: "test"}, nil) | 459 | Return(&filtering.Manifest{Version: "test"}, nil) | ||
| @@ -494,3 +496,114 @@ func TestManager_StatusCallback_CalledOnStateChange(t *testing.T) { | |||||
| 494 | defer mu.Unlock() | 496 | defer mu.Unlock() | ||
| 495 | assert.GreaterOrEqual(t, callCount, 2, "Expected callback to be called at least twice") | 497 | assert.GreaterOrEqual(t, callCount, 2, "Expected callback to be called at least twice") | ||
| 496 | } | 498 | } | ||
| 499 | + | ||||
| 500 | + | func TestManager_LoadAsync_QuarantinesInvalidDownloadedPayload(t *testing.T) { | |||
| 501 | + | tmpDir := t.TempDir() | |||
| 502 | + | storeDir := filepath.Join(tmpDir, "store") | |||
| 503 | + | jsonDir := filepath.Join(tmpDir, "json") | |||
| 504 | + | ||||
| 505 | + | invalidJSONFile := filepath.Join(jsonDir, "combined-part1.json") | |||
| 506 | + | require.NoError(t, os.MkdirAll(jsonDir, 0o755)) | |||
| 507 | + | require.NoError(t, os.WriteFile(invalidJSONFile, []byte(`{"invalid":`), 0o644)) | |||
| 508 | + | ||||
| 509 | + | mockStore := mocks.NewMockFilterStore(t) | |||
| 510 | + | mockDownloader := mocks.NewMockFilterDownloader(t) | |||
| 511 | + | ||||
| 512 | + | mockStore.EXPECT(). | |||
| 513 | + | HasCompiledFilter(mock.Anything, filtering.FilterIdentifier). | |||
| 514 | + | Return(false) | |||
| 515 | + | ||||
| 516 | + | mockDownloader.EXPECT(). | |||
| 517 | + | DownloadFilters(mock.Anything, mock.Anything). | |||
| 518 | + | Return([]string{invalidJSONFile}, nil) | |||
| 519 | + | ||||
| 520 | + | mgr, err := filtering.NewManager(filtering.ManagerConfig{ | |||
| 521 | + | StoreDir: storeDir, | |||
| 522 | + | JSONDir: jsonDir, | |||
| 523 | + | Enabled: true, | |||
| 524 | + | AutoUpdate: false, | |||
| 525 | + | Store: mockStore, | |||
| 526 | + | Downloader: mockDownloader, | |||
| 527 | + | }) | |||
| 528 | + | require.NoError(t, err) | |||
| 529 | + | ||||
| 530 | + | ctx := testContext() | |||
| 531 | + | require.NoError(t, mgr.Initialize(ctx)) | |||
| 532 | + | ||||
| 533 | + | mgr.LoadAsync(ctx) | |||
| 534 | + | ||||
| 535 | + | require.Eventually(t, func() bool { | |||
| 536 | + | return mgr.Status().State == filtering.StateError | |||
| 537 | + | }, 3*time.Second, 50*time.Millisecond) | |||
| 538 | + | assert.Equal(t, "Invalid downloaded filters", mgr.Status().Message) | |||
| 539 | + | ||||
| 540 | + | _, statErr := os.Stat(invalidJSONFile) | |||
| 541 | + | assert.ErrorIs(t, statErr, os.ErrNotExist) | |||
| 542 | + | ||||
| 543 | + | quarantineDir := filepath.Join(jsonDir, "quarantine") | |||
| 544 | + | entries, err := os.ReadDir(quarantineDir) | |||
| 545 | + | require.NoError(t, err) | |||
| 546 | + | require.Len(t, entries, 1) | |||
| 547 | + | } | |||
| 548 | + | ||||
| 549 | + | func TestManager_CheckForUpdates_CompileFailureKeepsActiveFilter(t *testing.T) { | |||
| 550 | + | tmpDir := t.TempDir() | |||
| 551 | + | storeDir := filepath.Join(tmpDir, "store") | |||
| 552 | + | jsonDir := filepath.Join(tmpDir, "json") | |||
| 553 | + | ||||
| 554 | + | validJSON := `[{"trigger":{"url-filter":"test"},"action":{"type":"block"}}]` | |||
| 555 | + | jsonFile := filepath.Join(jsonDir, "combined-part1.json") | |||
| 556 | + | require.NoError(t, os.MkdirAll(jsonDir, 0o755)) | |||
| 557 | + | require.NoError(t, os.WriteFile(jsonFile, []byte(validJSON), 0o644)) | |||
| 558 | + | ||||
| 559 | + | mockStore := mocks.NewMockFilterStore(t) | |||
| 560 | + | mockDownloader := mocks.NewMockFilterDownloader(t) | |||
| 561 | + | ||||
| 562 | + | activeFilter := &webkit.UserContentFilter{} | |||
| 563 | + | ||||
| 564 | + | mockStore.EXPECT(). | |||
| 565 | + | HasCompiledFilter(mock.Anything, filtering.FilterIdentifier). | |||
| 566 | + | Return(true).Once() | |||
| 567 | + | mockStore.EXPECT(). | |||
| 568 | + | Load(mock.Anything, filtering.FilterIdentifier). | |||
| 569 | + | Return(activeFilter, nil).Once() | |||
| 570 | + | mockDownloader.EXPECT(). | |||
| 571 | + | GetCachedManifest(). | |||
| 572 | + | Return(&filtering.Manifest{Version: "2025.12.19"}, nil).Once() | |||
| 573 | + | mockDownloader.EXPECT(). | |||
| 574 | + | IsCacheStale(filtering.CacheMaxAge). | |||
| 575 | + | Return(false).Once() | |||
| 576 | + | ||||
| 577 | + | mockDownloader.EXPECT(). | |||
| 578 | + | NeedsUpdate(mock.Anything). | |||
| 579 | + | Return(true, nil).Once() | |||
| 580 | + | mockDownloader.EXPECT(). | |||
| 581 | + | DownloadFilters(mock.Anything, mock.Anything). | |||
| 582 | + | Return([]string{jsonFile}, nil).Once() | |||
| 583 | + | mockStore.EXPECT(). | |||
| 584 | + | Compile(mock.Anything, filtering.FilterIdentifier, mock.Anything). | |||
| 585 | + | Return(nil, errors.New("compile failed")).Once() | |||
| 586 | + | ||||
| 587 | + | mgr, err := filtering.NewManager(filtering.ManagerConfig{ | |||
| 588 | + | StoreDir: storeDir, | |||
| 589 | + | JSONDir: jsonDir, | |||
| 590 | + | Enabled: true, | |||
| 591 | + | AutoUpdate: true, | |||
| 592 | + | Store: mockStore, | |||
| 593 | + | Downloader: mockDownloader, | |||
| 594 | + | }) | |||
| 595 | + | require.NoError(t, err) | |||
| 596 | + | ||||
| 597 | + | ctx := testContext() | |||
| 598 | + | require.NoError(t, mgr.Initialize(ctx)) | |||
| 599 | + | mgr.LoadAsync(ctx) | |||
| 600 | + | ||||
| 601 | + | require.Eventually(t, func() bool { | |||
| 602 | + | return mgr.GetFilter() != nil | |||
| 603 | + | }, 3*time.Second, 50*time.Millisecond) | |||
| 604 | + | ||||
| 605 | + | err = mgr.CheckForUpdates(ctx) | |||
| 606 | + | require.NoError(t, err) | |||
| 607 | + | assert.Same(t, activeFilter, mgr.GetFilter()) | |||
| 608 | + | assert.Equal(t, filtering.StateActive, mgr.Status().State) | |||
| 609 | + | } | |||
M internal/infrastructure/webkit/scheme_handler.go +126 -0
| @@ -5,6 +5,7 @@ import ( | |||
| 5 | 5 | "embed" | |
| 6 | 6 | "encoding/json" | |
| 7 | 7 | "fmt" | |
| 8 | + | "html" | |
| 8 | 9 | "io/fs" | |
| 9 | 10 | "mime" | |
| 10 | 11 | "net/http" | |
| @@ -28,6 +29,7 @@ const ( | |||
| 28 | 29 | HomePath = "home" | |
| 29 | 30 | ConfigPath = "config" | |
| 30 | 31 | ErrorPath = "error" | |
| 32 | + | CrashPath = "crash" | |
| 31 | 33 | IndexHTML = "index.html" | |
| 32 | 34 | ) | |
| 33 | 35 | ||
| @@ -161,6 +163,19 @@ func (h *DumbSchemeHandler) registerDefaults() { | |||
| 161 | 163 | } | |
| 162 | 164 | })) | |
| 163 | 165 | ||
| 166 | + | // Crash page (web process termination fallback) | |
| 167 | + | h.RegisterPage("/crash", PageHandlerFunc(func(req *SchemeRequest) *SchemeResponse { | |
| 168 | + | if req.Method != "" && req.Method != "GET" { | |
| 169 | + | return nil | |
| 170 | + | } | |
| 171 | + | originalURI := crashOriginalURI(req.URI) | |
| 172 | + | return &SchemeResponse{ | |
| 173 | + | Data: []byte(buildCrashPageHTML(originalURI)), | |
| 174 | + | ContentType: "text/html; charset=utf-8", | |
| 175 | + | StatusCode: http.StatusOK, | |
| 176 | + | } | |
| 177 | + | })) | |
| 178 | + | ||
| 164 | 179 | // API: Get current config (used by dumb://config) | |
| 165 | 180 | h.RegisterPage("/api/config", PageHandlerFunc(func(req *SchemeRequest) *SchemeResponse { | |
| 166 | 181 | if req.Method != "GET" { | |
| @@ -180,6 +195,117 @@ func (h *DumbSchemeHandler) registerDefaults() { | |||
| 180 | 195 | })) | |
| 181 | 196 | } | |
| 182 | 197 | ||
| 198 | + | func crashOriginalURI(requestURI string) string { | |
| 199 | + | if requestURI == "" { | |
| 200 | + | return "" | |
| 201 | + | } | |
| 202 | + | parsed, err := url.Parse(requestURI) | |
| 203 | + | if err != nil { | |
| 204 | + | return "" | |
| 205 | + | } | |
| 206 | + | return strings.TrimSpace(parsed.Query().Get("url")) | |
| 207 | + | } | |
| 208 | + | ||
| 209 | + | func buildCrashPageHTML(originalURI string) string { | |
| 210 | + | escapedOriginal := html.EscapeString(originalURI) | |
| 211 | + | reloadAction := "window.location.reload();" | |
| 212 | + | if originalURI != "" { | |
| 213 | + | reloadAction = "window.location.href = targetUrl;" | |
| 214 | + | } | |
| 215 | + | return fmt.Sprintf(`<!DOCTYPE html> | |
| 216 | + | <html lang="en"> | |
| 217 | + | <head> | |
| 218 | + | <meta charset="utf-8"> | |
| 219 | + | <meta name="viewport" content="width=device-width, initial-scale=1"> | |
| 220 | + | <title>Renderer crashed</title> | |
| 221 | + | <style> | |
| 222 | + | :root { | |
| 223 | + | color-scheme: dark; | |
| 224 | + | font-family: "IBM Plex Sans", "Segoe UI", sans-serif; | |
| 225 | + | } | |
| 226 | + | body { | |
| 227 | + | margin: 0; | |
| 228 | + | min-height: 100vh; | |
| 229 | + | display: flex; | |
| 230 | + | align-items: center; | |
| 231 | + | justify-content: center; | |
| 232 | + | background: radial-gradient(circle at top, #253447, #101622 55%%); | |
| 233 | + | color: #f2f6fa; | |
| 234 | + | padding: 24px; | |
| 235 | + | } | |
| 236 | + | .card { | |
| 237 | + | width: min(640px, 100%%); | |
| 238 | + | background: rgba(10, 16, 26, 0.86); | |
| 239 | + | border: 1px solid rgba(144, 173, 205, 0.35); | |
| 240 | + | border-radius: 16px; | |
| 241 | + | box-shadow: 0 24px 64px rgba(0, 0, 0, 0.45); | |
| 242 | + | padding: 28px; | |
| 243 | + | } | |
| 244 | + | h1 { | |
| 245 | + | margin: 0 0 12px; | |
| 246 | + | font-size: 1.8rem; | |
| 247 | + | } | |
| 248 | + | p { | |
| 249 | + | margin: 0 0 16px; | |
| 250 | + | line-height: 1.5; | |
| 251 | + | color: #c8d6e6; | |
| 252 | + | } | |
| 253 | + | .url { | |
| 254 | + | margin: 0 0 20px; | |
| 255 | + | padding: 12px; | |
| 256 | + | border-radius: 10px; | |
| 257 | + | background: rgba(26, 38, 56, 0.85); | |
| 258 | + | border: 1px solid rgba(139, 167, 194, 0.28); | |
| 259 | + | font-family: "IBM Plex Mono", "Fira Code", monospace; | |
| 260 | + | font-size: 0.92rem; | |
| 261 | + | overflow-wrap: anywhere; | |
| 262 | + | } | |
| 263 | + | .actions { | |
| 264 | + | display: flex; | |
| 265 | + | gap: 12px; | |
| 266 | + | flex-wrap: wrap; | |
| 267 | + | } | |
| 268 | + | button { | |
| 269 | + | border: 0; | |
| 270 | + | border-radius: 10px; | |
| 271 | + | padding: 10px 16px; | |
| 272 | + | cursor: pointer; | |
| 273 | + | font-size: 0.95rem; | |
| 274 | + | font-weight: 600; | |
| 275 | + | } | |
| 276 | + | .primary { | |
| 277 | + | background: #4dd0e1; | |
| 278 | + | color: #061018; | |
| 279 | + | } | |
| 280 | + | .secondary { | |
| 281 | + | background: #233346; | |
| 282 | + | color: #d6e5f5; | |
| 283 | + | } | |
| 284 | + | </style> | |
| 285 | + | </head> | |
| 286 | + | <body> | |
| 287 | + | <div class="card"> | |
| 288 | + | <h1>Renderer process ended</h1> | |
| 289 | + | <p>The current page was interrupted. You can reload it to continue browsing.</p> | |
| 290 | + | <div class="url">%s</div> | |
| 291 | + | <div class="actions"> | |
| 292 | + | <button class="primary" id="reload-btn">Reload page</button> | |
| 293 | + | <button class="secondary" id="stay-btn">Stay on this page</button> | |
| 294 | + | </div> | |
| 295 | + | </div> | |
| 296 | + | <script> | |
| 297 | + | const targetUrl = %q; | |
| 298 | + | document.getElementById('reload-btn').addEventListener('click', function() { | |
| 299 | + | %s | |
| 300 | + | }); | |
| 301 | + | document.getElementById('stay-btn').addEventListener('click', function() { | |
| 302 | + | window.location.href = 'dumb://home/crash?url=' + encodeURIComponent(targetUrl); | |
| 303 | + | }); | |
| 304 | + | </script> | |
| 305 | + | </body> | |
| 306 | + | </html>`, escapedOriginal, originalURI, reloadAction) | |
| 307 | + | } | |
| 308 | + | ||
| 183 | 309 | func (h *DumbSchemeHandler) buildConfigResponse(cfg *config.Config) *SchemeResponse { | |
| 184 | 310 | // Get hardware info for display and profile resolution | |
| 185 | 311 | // Use background context since survey results are cached and we don't want | |
| @@ -5,6 +5,7 @@ import ( | |||||
| 5 | "embed" | 5 | "embed" | ||
| 6 | "encoding/json" | 6 | "encoding/json" | ||
| 7 | "fmt" | 7 | "fmt" | ||
| 8 | + | "html" | |||
| 8 | "io/fs" | 9 | "io/fs" | ||
| 9 | "mime" | 10 | "mime" | ||
| 10 | "net/http" | 11 | "net/http" | ||
| @@ -28,6 +29,7 @@ const ( | |||||
| 28 | HomePath = "home" | 29 | HomePath = "home" | ||
| 29 | ConfigPath = "config" | 30 | ConfigPath = "config" | ||
| 30 | ErrorPath = "error" | 31 | ErrorPath = "error" | ||
| 32 | + | CrashPath = "crash" | |||
| 31 | IndexHTML = "index.html" | 33 | IndexHTML = "index.html" | ||
| 32 | ) | 34 | ) | ||
| 33 | 35 | ||||
| @@ -161,6 +163,19 @@ func (h *DumbSchemeHandler) registerDefaults() { | |||||
| 161 | } | 163 | } | ||
| 162 | })) | 164 | })) | ||
| 163 | 165 | ||||
| 166 | + | // Crash page (web process termination fallback) | |||
| 167 | + | h.RegisterPage("/crash", PageHandlerFunc(func(req *SchemeRequest) *SchemeResponse { | |||
| 168 | + | if req.Method != "" && req.Method != "GET" { | |||
| 169 | + | return nil | |||
| 170 | + | } | |||
| 171 | + | originalURI := crashOriginalURI(req.URI) | |||
| 172 | + | return &SchemeResponse{ | |||
| 173 | + | Data: []byte(buildCrashPageHTML(originalURI)), | |||
| 174 | + | ContentType: "text/html; charset=utf-8", | |||
| 175 | + | StatusCode: http.StatusOK, | |||
| 176 | + | } | |||
| 177 | + | })) | |||
| 178 | + | ||||
| 164 | // API: Get current config (used by dumb://config) | 179 | // API: Get current config (used by dumb://config) | ||
| 165 | h.RegisterPage("/api/config", PageHandlerFunc(func(req *SchemeRequest) *SchemeResponse { | 180 | h.RegisterPage("/api/config", PageHandlerFunc(func(req *SchemeRequest) *SchemeResponse { | ||
| 166 | if req.Method != "GET" { | 181 | if req.Method != "GET" { | ||
| @@ -180,6 +195,117 @@ func (h *DumbSchemeHandler) registerDefaults() { | |||||
| 180 | })) | 195 | })) | ||
| 181 | } | 196 | } | ||
| 182 | 197 | ||||
| 198 | + | func crashOriginalURI(requestURI string) string { | |||
| 199 | + | if requestURI == "" { | |||
| 200 | + | return "" | |||
| 201 | + | } | |||
| 202 | + | parsed, err := url.Parse(requestURI) | |||
| 203 | + | if err != nil { | |||
| 204 | + | return "" | |||
| 205 | + | } | |||
| 206 | + | return strings.TrimSpace(parsed.Query().Get("url")) | |||
| 207 | + | } | |||
| 208 | + | ||||
| 209 | + | func buildCrashPageHTML(originalURI string) string { | |||
| 210 | + | escapedOriginal := html.EscapeString(originalURI) | |||
| 211 | + | reloadAction := "window.location.reload();" | |||
| 212 | + | if originalURI != "" { | |||
| 213 | + | reloadAction = "window.location.href = targetUrl;" | |||
| 214 | + | } | |||
| 215 | + | return fmt.Sprintf(`<!DOCTYPE html> | |||
| 216 | + | <html lang="en"> | |||
| 217 | + | <head> | |||
| 218 | + | <meta charset="utf-8"> | |||
| 219 | + | <meta name="viewport" content="width=device-width, initial-scale=1"> | |||
| 220 | + | <title>Renderer crashed</title> | |||
| 221 | + | <style> | |||
| 222 | + | :root { | |||
| 223 | + | color-scheme: dark; | |||
| 224 | + | font-family: "IBM Plex Sans", "Segoe UI", sans-serif; | |||
| 225 | + | } | |||
| 226 | + | body { | |||
| 227 | + | margin: 0; | |||
| 228 | + | min-height: 100vh; | |||
| 229 | + | display: flex; | |||
| 230 | + | align-items: center; | |||
| 231 | + | justify-content: center; | |||
| 232 | + | background: radial-gradient(circle at top, #253447, #101622 55%%); | |||
| 233 | + | color: #f2f6fa; | |||
| 234 | + | padding: 24px; | |||
| 235 | + | } | |||
| 236 | + | .card { | |||
| 237 | + | width: min(640px, 100%%); | |||
| 238 | + | background: rgba(10, 16, 26, 0.86); | |||
| 239 | + | border: 1px solid rgba(144, 173, 205, 0.35); | |||
| 240 | + | border-radius: 16px; | |||
| 241 | + | box-shadow: 0 24px 64px rgba(0, 0, 0, 0.45); | |||
| 242 | + | padding: 28px; | |||
| 243 | + | } | |||
| 244 | + | h1 { | |||
| 245 | + | margin: 0 0 12px; | |||
| 246 | + | font-size: 1.8rem; | |||
| 247 | + | } | |||
| 248 | + | p { | |||
| 249 | + | margin: 0 0 16px; | |||
| 250 | + | line-height: 1.5; | |||
| 251 | + | color: #c8d6e6; | |||
| 252 | + | } | |||
| 253 | + | .url { | |||
| 254 | + | margin: 0 0 20px; | |||
| 255 | + | padding: 12px; | |||
| 256 | + | border-radius: 10px; | |||
| 257 | + | background: rgba(26, 38, 56, 0.85); | |||
| 258 | + | border: 1px solid rgba(139, 167, 194, 0.28); | |||
| 259 | + | font-family: "IBM Plex Mono", "Fira Code", monospace; | |||
| 260 | + | font-size: 0.92rem; | |||
| 261 | + | overflow-wrap: anywhere; | |||
| 262 | + | } | |||
| 263 | + | .actions { | |||
| 264 | + | display: flex; | |||
| 265 | + | gap: 12px; | |||
| 266 | + | flex-wrap: wrap; | |||
| 267 | + | } | |||
| 268 | + | button { | |||
| 269 | + | border: 0; | |||
| 270 | + | border-radius: 10px; | |||
| 271 | + | padding: 10px 16px; | |||
| 272 | + | cursor: pointer; | |||
| 273 | + | font-size: 0.95rem; | |||
| 274 | + | font-weight: 600; | |||
| 275 | + | } | |||
| 276 | + | .primary { | |||
| 277 | + | background: #4dd0e1; | |||
| 278 | + | color: #061018; | |||
| 279 | + | } | |||
| 280 | + | .secondary { | |||
| 281 | + | background: #233346; | |||
| 282 | + | color: #d6e5f5; | |||
| 283 | + | } | |||
| 284 | + | </style> | |||
| 285 | + | </head> | |||
| 286 | + | <body> | |||
| 287 | + | <div class="card"> | |||
| 288 | + | <h1>Renderer process ended</h1> | |||
| 289 | + | <p>The current page was interrupted. You can reload it to continue browsing.</p> | |||
| 290 | + | <div class="url">%s</div> | |||
| 291 | + | <div class="actions"> | |||
| 292 | + | <button class="primary" id="reload-btn">Reload page</button> | |||
| 293 | + | <button class="secondary" id="stay-btn">Stay on this page</button> | |||
| 294 | + | </div> | |||
| 295 | + | </div> | |||
| 296 | + | <script> | |||
| 297 | + | const targetUrl = %q; | |||
| 298 | + | document.getElementById('reload-btn').addEventListener('click', function() { | |||
| 299 | + | %s | |||
| 300 | + | }); | |||
| 301 | + | document.getElementById('stay-btn').addEventListener('click', function() { | |||
| 302 | + | window.location.href = 'dumb://home/crash?url=' + encodeURIComponent(targetUrl); | |||
| 303 | + | }); | |||
| 304 | + | </script> | |||
| 305 | + | </body> | |||
| 306 | + | </html>`, escapedOriginal, originalURI, reloadAction) | |||
| 307 | + | } | |||
| 308 | + | ||||
| 183 | func (h *DumbSchemeHandler) buildConfigResponse(cfg *config.Config) *SchemeResponse { | 309 | func (h *DumbSchemeHandler) buildConfigResponse(cfg *config.Config) *SchemeResponse { | ||
| 184 | // Get hardware info for display and profile resolution | 310 | // Get hardware info for display and profile resolution | ||
| 185 | // Use background context since survey results are cached and we don't want | 311 | // Use background context since survey results are cached and we don't want | ||
A internal/infrastructure/webkit/scheme_handler_test.go +48 -0
| @@ -0,0 +1,48 @@ | |||
| 1 | + | package webkit | |
| 2 | + | ||
| 3 | + | import ( | |
| 4 | + | "context" | |
| 5 | + | "testing" | |
| 6 | + | ||
| 7 | + | "github.com/stretchr/testify/assert" | |
| 8 | + | "github.com/stretchr/testify/require" | |
| 9 | + | ) | |
| 10 | + | ||
| 11 | + | func TestCrashOriginalURI(t *testing.T) { | |
| 12 | + | assert.Equal(t, "https://example.com/path?q=1", crashOriginalURI("dumb://home/crash?url=https%3A%2F%2Fexample.com%2Fpath%3Fq%3D1")) | |
| 13 | + | assert.Equal(t, "", crashOriginalURI("dumb://home/crash")) | |
| 14 | + | assert.Equal(t, "", crashOriginalURI("%%%")) | |
| 15 | + | } | |
| 16 | + | ||
| 17 | + | func TestBuildCrashPageHTMLIncludesReloadTarget(t *testing.T) { | |
| 18 | + | body := buildCrashPageHTML("https://example.com/foo?a=1&b=2") | |
| 19 | + | assert.Contains(t, body, "window.location.href = targetUrl;") | |
| 20 | + | assert.Contains(t, body, "https://example.com/foo?a=1&b=2") | |
| 21 | + | } | |
| 22 | + | ||
| 23 | + | func TestBuildCrashPageHTMLFallbackReloadWithoutTarget(t *testing.T) { | |
| 24 | + | body := buildCrashPageHTML("") | |
| 25 | + | assert.Contains(t, body, "window.location.reload();") | |
| 26 | + | } | |
| 27 | + | ||
| 28 | + | func TestRegisterDefaultsIncludesCrashHandler(t *testing.T) { | |
| 29 | + | handler := NewDumbSchemeHandler(context.Background()) | |
| 30 | + | require.NotNil(t, handler) | |
| 31 | + | ||
| 32 | + | handler.mu.RLock() | |
| 33 | + | crashHandler, ok := handler.handlers["/crash"] | |
| 34 | + | handler.mu.RUnlock() | |
| 35 | + | require.True(t, ok) | |
| 36 | + | require.NotNil(t, crashHandler) | |
| 37 | + | ||
| 38 | + | resp := crashHandler.Handle(&SchemeRequest{ | |
| 39 | + | URI: "dumb://home/crash?url=https%3A%2F%2Fexample.com", | |
| 40 | + | Path: "/crash", | |
| 41 | + | Method: "GET", | |
| 42 | + | Scheme: "dumb", | |
| 43 | + | }) | |
| 44 | + | require.NotNil(t, resp) | |
| 45 | + | assert.Equal(t, 200, resp.StatusCode) | |
| 46 | + | assert.Equal(t, "text/html; charset=utf-8", resp.ContentType) | |
| 47 | + | assert.Contains(t, string(resp.Data), "Renderer process ended") | |
| 48 | + | } | |
| @@ -0,0 +1,48 @@ | |||||
| 1 | + | package webkit | |||
| 2 | + | ||||
| 3 | + | import ( | |||
| 4 | + | "context" | |||
| 5 | + | "testing" | |||
| 6 | + | ||||
| 7 | + | "github.com/stretchr/testify/assert" | |||
| 8 | + | "github.com/stretchr/testify/require" | |||
| 9 | + | ) | |||
| 10 | + | ||||
| 11 | + | func TestCrashOriginalURI(t *testing.T) { | |||
| 12 | + | assert.Equal(t, "https://example.com/path?q=1", crashOriginalURI("dumb://home/crash?url=https%3A%2F%2Fexample.com%2Fpath%3Fq%3D1")) | |||
| 13 | + | assert.Equal(t, "", crashOriginalURI("dumb://home/crash")) | |||
| 14 | + | assert.Equal(t, "", crashOriginalURI("%%%")) | |||
| 15 | + | } | |||
| 16 | + | ||||
| 17 | + | func TestBuildCrashPageHTMLIncludesReloadTarget(t *testing.T) { | |||
| 18 | + | body := buildCrashPageHTML("https://example.com/foo?a=1&b=2") | |||
| 19 | + | assert.Contains(t, body, "window.location.href = targetUrl;") | |||
| 20 | + | assert.Contains(t, body, "https://example.com/foo?a=1&b=2") | |||
| 21 | + | } | |||
| 22 | + | ||||
| 23 | + | func TestBuildCrashPageHTMLFallbackReloadWithoutTarget(t *testing.T) { | |||
| 24 | + | body := buildCrashPageHTML("") | |||
| 25 | + | assert.Contains(t, body, "window.location.reload();") | |||
| 26 | + | } | |||
| 27 | + | ||||
| 28 | + | func TestRegisterDefaultsIncludesCrashHandler(t *testing.T) { | |||
| 29 | + | handler := NewDumbSchemeHandler(context.Background()) | |||
| 30 | + | require.NotNil(t, handler) | |||
| 31 | + | ||||
| 32 | + | handler.mu.RLock() | |||
| 33 | + | crashHandler, ok := handler.handlers["/crash"] | |||
| 34 | + | handler.mu.RUnlock() | |||
| 35 | + | require.True(t, ok) | |||
| 36 | + | require.NotNil(t, crashHandler) | |||
| 37 | + | ||||
| 38 | + | resp := crashHandler.Handle(&SchemeRequest{ | |||
| 39 | + | URI: "dumb://home/crash?url=https%3A%2F%2Fexample.com", | |||
| 40 | + | Path: "/crash", | |||
| 41 | + | Method: "GET", | |||
| 42 | + | Scheme: "dumb", | |||
| 43 | + | }) | |||
| 44 | + | require.NotNil(t, resp) | |||
| 45 | + | assert.Equal(t, 200, resp.StatusCode) | |||
| 46 | + | assert.Equal(t, "text/html; charset=utf-8", resp.ContentType) | |||
| 47 | + | assert.Contains(t, string(resp.Data), "Renderer process ended") | |||
| 48 | + | } | |||
M internal/ui/coordinator/content.go +247 -49
| @@ -4,6 +4,7 @@ import ( | |||
| 4 | 4 | "context" | |
| 5 | 5 | "encoding/json" | |
| 6 | 6 | "fmt" | |
| 7 | + | "net/url" | |
| 7 | 8 | "strings" | |
| 8 | 9 | "sync" | |
| 9 | 10 | "time" | |
| @@ -20,12 +21,15 @@ import ( | |||
| 20 | 21 | "github.com/bnema/dumber/internal/ui/component" | |
| 21 | 22 | "github.com/bnema/dumber/internal/ui/input" | |
| 22 | 23 | "github.com/bnema/dumber/internal/ui/layout" | |
| 24 | + | webkitlib "github.com/bnema/puregotk-webkit/webkit" | |
| 23 | 25 | "github.com/jwijenbergh/puregotk/v4/gdk" | |
| 24 | 26 | ) | |
| 25 | 27 | ||
| 26 | 28 | const ( | |
| 27 | - | aboutBlankURI = "about:blank" | |
| 28 | - | logURLMaxLen = 80 | |
| 29 | + | aboutBlankURI = "about:blank" | |
| 30 | + | crashPageURI = "dumb://home/crash" | |
| 31 | + | logURLMaxLen = 80 | |
| 32 | + | oauthParentRefreshDebounce = 200 * time.Millisecond | |
| 29 | 33 | ||
| 30 | 34 | // Dark theme background color (#0a0a0b) as float32 RGBA values | |
| 31 | 35 | darkBgR = 0.039 | |
| @@ -34,6 +38,51 @@ const ( | |||
| 34 | 38 | darkBgA = 1.0 | |
| 35 | 39 | ) | |
| 36 | 40 | ||
| 41 | + | func shouldRenderCrashPage(reason webkitlib.WebProcessTerminationReason) bool { | |
| 42 | + | switch reason { | |
| 43 | + | case webkitlib.WebProcessCrashedValue, webkitlib.WebProcessExceededMemoryLimitValue: | |
| 44 | + | return true | |
| 45 | + | case webkitlib.WebProcessTerminatedByApiValue: | |
| 46 | + | return false | |
| 47 | + | default: | |
| 48 | + | return true | |
| 49 | + | } | |
| 50 | + | } | |
| 51 | + | ||
| 52 | + | func extractOriginalURIFromCrashPage(uri string) string { | |
| 53 | + | if uri == "" { | |
| 54 | + | return "" | |
| 55 | + | } | |
| 56 | + | ||
| 57 | + | parsed, err := url.Parse(uri) | |
| 58 | + | if err != nil { | |
| 59 | + | return uri | |
| 60 | + | } | |
| 61 | + | ||
| 62 | + | if parsed.Scheme != "dumb" || parsed.Host != webkit.HomePath { | |
| 63 | + | return uri | |
| 64 | + | } | |
| 65 | + | ||
| 66 | + | if strings.Trim(parsed.Path, "/") != "crash" { | |
| 67 | + | return uri | |
| 68 | + | } | |
| 69 | + | ||
| 70 | + | original := strings.TrimSpace(parsed.Query().Get("url")) | |
| 71 | + | if original == "" { | |
| 72 | + | return "" | |
| 73 | + | } | |
| 74 | + | return original | |
| 75 | + | } | |
| 76 | + | ||
| 77 | + | func buildCrashPageURI(originalURI string) string { | |
| 78 | + | if strings.TrimSpace(originalURI) == "" { | |
| 79 | + | return crashPageURI | |
| 80 | + | } | |
| 81 | + | query := url.Values{} | |
| 82 | + | query.Set("url", originalURI) | |
| 83 | + | return crashPageURI + "?" + query.Encode() | |
| 84 | + | } | |
| 85 | + | ||
| 37 | 86 | // ContentCoordinator manages WebView lifecycle, title tracking, and content attachment. | |
| 38 | 87 | type ContentCoordinator struct { | |
| 39 | 88 | pool *webkit.WebViewPool | |
| @@ -87,6 +136,8 @@ type ContentCoordinator struct { | |||
| 87 | 136 | factory *webkit.WebViewFactory | |
| 88 | 137 | popupConfig *config.PopupBehaviorConfig | |
| 89 | 138 | pendingPopups map[port.WebViewID]*PendingPopup | |
| 139 | + | popupOAuth map[port.WebViewID]*popupOAuthState | |
| 140 | + | popupRefresh map[entity.PaneID]*time.Timer | |
| 90 | 141 | popupMu sync.RWMutex | |
| 91 | 142 | ||
| 92 | 143 | // Callback to insert popup into workspace (avoids circular dependency) | |
| @@ -117,6 +168,14 @@ type pendingThemeUpdate struct { | |||
| 117 | 168 | cssText string | |
| 118 | 169 | } | |
| 119 | 170 | ||
| 171 | + | type popupOAuthState struct { | |
| 172 | + | ParentPaneID entity.PaneID | |
| 173 | + | CallbackURI string | |
| 174 | + | Success bool | |
| 175 | + | Error bool | |
| 176 | + | Seen bool | |
| 177 | + | } | |
| 178 | + | ||
| 120 | 179 | // NewContentCoordinator creates a new ContentCoordinator. | |
| 121 | 180 | func NewContentCoordinator( | |
| 122 | 181 | ctx context.Context, | |
| @@ -140,6 +199,8 @@ func NewContentCoordinator( | |||
| 140 | 199 | pendingReveal: make(map[entity.PaneID]bool), | |
| 141 | 200 | getActiveWS: getActiveWS, | |
| 142 | 201 | pendingPopups: make(map[port.WebViewID]*PendingPopup), | |
| 202 | + | popupOAuth: make(map[port.WebViewID]*popupOAuthState), | |
| 203 | + | popupRefresh: make(map[entity.PaneID]*time.Timer), | |
| 143 | 204 | } | |
| 144 | 205 | } | |
| 145 | 206 | ||
| @@ -1299,7 +1360,8 @@ func (c *ContentCoordinator) handlePopupCreate( | |||
| 1299 | 1360 | // Setup OAuth auto-close if configured | |
| 1300 | 1361 | if hasConfig && oauthEnabled && isOAuth { | |
| 1301 | 1362 | popupPane.AutoClose = true | |
| 1302 | - | c.setupOAuthAutoClose(ctx, paneID, popupWV) | |
| 1363 | + | c.trackOAuthPopup(popupID, parentPaneID) | |
| 1364 | + | c.setupOAuthAutoClose(ctx, paneID, popupID, popupWV) | |
| 1303 | 1365 | log.Debug().Str("pane_id", string(paneID)).Msg("OAuth auto-close enabled for popup") | |
| 1304 | 1366 | } | |
| 1305 | 1367 | ||
| @@ -1323,9 +1385,9 @@ func (c *ContentCoordinator) handlePopupCreate( | |||
| 1323 | 1385 | popupWV.OnReadyToShow = func() { | |
| 1324 | 1386 | c.handlePopupReadyToShow(ctx, popupID) | |
| 1325 | 1387 | } | |
| 1326 | - | popupWV.OnClose = func() { | |
| 1388 | + | popupWV.OnClose = composeOnClose(popupWV.OnClose, func() { | |
| 1327 | 1389 | c.handlePopupClose(ctx, popupID) | |
| 1328 | - | } | |
| 1390 | + | }) | |
| 1329 | 1391 | ||
| 1330 | 1392 | log.Info(). | |
| 1331 | 1393 | Uint64("popup_id", uint64(popupID)). | |
| @@ -1386,6 +1448,7 @@ func (c *ContentCoordinator) handlePopupClose(ctx context.Context, popupID port. | |||
| 1386 | 1448 | c.popupMu.Unlock() | |
| 1387 | 1449 | ||
| 1388 | 1450 | if wasPending && pending != nil { | |
| 1451 | + | c.handlePopupOAuthClose(ctx, popupID) | |
| 1389 | 1452 | // Was never shown, just destroy | |
| 1390 | 1453 | pending.WebView.Destroy() | |
| 1391 | 1454 | log.Debug().Msg("destroyed pending popup that was never shown") | |
| @@ -1402,6 +1465,7 @@ func (c *ContentCoordinator) handlePopupClose(ctx context.Context, popupID port. | |||
| 1402 | 1465 | } | |
| 1403 | 1466 | ||
| 1404 | 1467 | if paneID == "" { | |
| 1468 | + | c.handlePopupOAuthClose(ctx, popupID) | |
| 1405 | 1469 | log.Warn().Msg("popup close: could not find pane for webview") | |
| 1406 | 1470 | return | |
| 1407 | 1471 | } | |
| @@ -1413,6 +1477,8 @@ func (c *ContentCoordinator) handlePopupClose(ctx context.Context, popupID port. | |||
| 1413 | 1477 | } | |
| 1414 | 1478 | } | |
| 1415 | 1479 | ||
| 1480 | + | c.handlePopupOAuthClose(ctx, popupID) | |
| 1481 | + | ||
| 1416 | 1482 | // Release the WebView (this will clean up tracking) | |
| 1417 | 1483 | c.ReleaseWebView(ctx, paneID) | |
| 1418 | 1484 | ||
| @@ -1421,6 +1487,8 @@ func (c *ContentCoordinator) handlePopupClose(ctx context.Context, popupID port. | |||
| 1421 | 1487 | ||
| 1422 | 1488 | // setupWebViewCallbacks configures standard callbacks and popup handling. | |
| 1423 | 1489 | func (c *ContentCoordinator) setupWebViewCallbacks(ctx context.Context, paneID entity.PaneID, wv *webkit.WebView) { | |
| 1490 | + | log := logging.FromContext(ctx) | |
| 1491 | + | ||
| 1424 | 1492 | // Title changes | |
| 1425 | 1493 | wv.OnTitleChanged = func(title string) { | |
| 1426 | 1494 | c.onTitleChanged(ctx, paneID, title) | |
| @@ -1491,6 +1559,36 @@ func (c *ContentCoordinator) setupWebViewCallbacks(ctx context.Context, paneID e | |||
| 1491 | 1559 | c.onLinkHover(paneID, uri) | |
| 1492 | 1560 | } | |
| 1493 | 1561 | ||
| 1562 | + | wv.OnWebProcessTerminated = func(reason webkitlib.WebProcessTerminationReason, reasonLabel string, uri string) { | |
| 1563 | + | originalURI := extractOriginalURIFromCrashPage(uri) | |
| 1564 | + | if !shouldRenderCrashPage(reason) { | |
| 1565 | + | log.Info(). | |
| 1566 | + | Str("pane_id", string(paneID)). | |
| 1567 | + | Str("reason", reasonLabel). | |
| 1568 | + | Str("uri", uri). | |
| 1569 | + | Msg("web process termination handled without crash page") | |
| 1570 | + | return | |
| 1571 | + | } | |
| 1572 | + | ||
| 1573 | + | crashURI := buildCrashPageURI(originalURI) | |
| 1574 | + | log.Warn(). | |
| 1575 | + | Str("pane_id", string(paneID)). | |
| 1576 | + | Str("reason", reasonLabel). | |
| 1577 | + | Str("uri", uri). | |
| 1578 | + | Str("crash_uri", crashURI). | |
| 1579 | + | Msg("web process terminated, redirecting to crash page") | |
| 1580 | + | ||
| 1581 | + | if err := wv.LoadURI(ctx, crashURI); err != nil { | |
| 1582 | + | log.Error(). | |
| 1583 | + | Err(err). | |
| 1584 | + | Str("pane_id", string(paneID)). | |
| 1585 | + | Str("reason", reasonLabel). | |
| 1586 | + | Str("uri", uri). | |
| 1587 | + | Str("crash_uri", crashURI). | |
| 1588 | + | Msg("failed to load crash page after web process termination") | |
| 1589 | + | } | |
| 1590 | + | } | |
| 1591 | + | ||
| 1494 | 1592 | // Fullscreen handlers for idle inhibition | |
| 1495 | 1593 | c.setupIdleInhibitionHandlers(ctx, paneID, wv) | |
| 1496 | 1594 | ||
| @@ -1503,12 +1601,19 @@ func (c *ContentCoordinator) setupWebViewCallbacks(ctx context.Context, paneID e | |||
| 1503 | 1601 | // For providers using postMessage (like Google Sign-In), we rely on the provider calling | |
| 1504 | 1602 | // window.close() which triggers WebKit's close signal. | |
| 1505 | 1603 | // A long safety timeout (30s) catches popups that get stuck. | |
| 1506 | - | func (c *ContentCoordinator) setupOAuthAutoClose(ctx context.Context, paneID entity.PaneID, wv *webkit.WebView) { | |
| 1604 | + | func (c *ContentCoordinator) setupOAuthAutoClose( | |
| 1605 | + | ctx context.Context, | |
| 1606 | + | paneID entity.PaneID, | |
| 1607 | + | popupID port.WebViewID, | |
| 1608 | + | wv *webkit.WebView, | |
| 1609 | + | ) { | |
| 1507 | 1610 | log := logging.FromContext(ctx) | |
| 1508 | 1611 | ||
| 1509 | 1612 | // Safety timeout - only triggers if popup gets stuck (provider should close via window.close) | |
| 1510 | 1613 | var safetyTimer *time.Timer | |
| 1511 | 1614 | var safetyTimerMu sync.Mutex | |
| 1615 | + | var cancelSafetyTimerOnce sync.Once | |
| 1616 | + | var requestCloseOnce sync.Once | |
| 1512 | 1617 | const oauthSafetyTimeout = 30 * time.Second | |
| 1513 | 1618 | ||
| 1514 | 1619 | startSafetyTimer := func() { | |
| @@ -1526,69 +1631,162 @@ func (c *ContentCoordinator) setupOAuthAutoClose(ctx context.Context, paneID ent | |||
| 1526 | 1631 | } | |
| 1527 | 1632 | ||
| 1528 | 1633 | cancelSafetyTimer := func() { | |
| 1529 | - | safetyTimerMu.Lock() | |
| 1530 | - | defer safetyTimerMu.Unlock() | |
| 1531 | - | if safetyTimer != nil { | |
| 1532 | - | safetyTimer.Stop() | |
| 1533 | - | safetyTimer = nil | |
| 1534 | - | } | |
| 1634 | + | cancelSafetyTimerOnce.Do(func() { | |
| 1635 | + | safetyTimerMu.Lock() | |
| 1636 | + | defer safetyTimerMu.Unlock() | |
| 1637 | + | if safetyTimer != nil { | |
| 1638 | + | safetyTimer.Stop() | |
| 1639 | + | safetyTimer = nil | |
| 1640 | + | } | |
| 1641 | + | }) | |
| 1535 | 1642 | } | |
| 1536 | 1643 | ||
| 1537 | - | // Start safety timer immediately | |
| 1538 | - | startSafetyTimer() | |
| 1539 | - | ||
| 1540 | - | // Wrap the existing OnURIChanged to also check for OAuth callbacks | |
| 1541 | - | originalURIChanged := wv.OnURIChanged | |
| 1542 | - | wv.OnURIChanged = func(uri string) { | |
| 1543 | - | // Call original handler first | |
| 1544 | - | if originalURIChanged != nil { | |
| 1545 | - | originalURIChanged(uri) | |
| 1546 | - | } | |
| 1547 | - | ||
| 1548 | - | // Check for OAuth callback (URL-based detection) | |
| 1549 | - | if ShouldAutoClose(uri) { | |
| 1550 | - | cancelSafetyTimer() | |
| 1551 | - | log.Info().Str("pane", string(paneID)).Msg("oauth callback detected, closing") | |
| 1644 | + | requestOAuthClose := func(uri string, reason string) { | |
| 1645 | + | c.capturePopupOAuthState(popupID, uri) | |
| 1646 | + | cancelSafetyTimer() | |
| 1647 | + | log.Info(). | |
| 1648 | + | Str("pane", string(paneID)). | |
| 1649 | + | Str("reason", reason). | |
| 1650 | + | Msg("oauth callback detected, closing") | |
| 1651 | + | requestCloseOnce.Do(func() { | |
| 1552 | 1652 | go func() { | |
| 1553 | 1653 | time.Sleep(500 * time.Millisecond) | |
| 1554 | 1654 | if wv != nil && !wv.IsDestroyed() { | |
| 1555 | 1655 | wv.Close() | |
| 1556 | 1656 | } | |
| 1557 | 1657 | }() | |
| 1558 | - | } | |
| 1658 | + | }) | |
| 1559 | 1659 | } | |
| 1560 | 1660 | ||
| 1561 | - | // Monitor load events for URL-based detection | |
| 1562 | - | originalLoadChanged := wv.OnLoadChanged | |
| 1563 | - | wv.OnLoadChanged = func(event webkit.LoadEvent) { | |
| 1564 | - | if originalLoadChanged != nil { | |
| 1565 | - | originalLoadChanged(event) | |
| 1661 | + | // Start safety timer immediately. | |
| 1662 | + | startSafetyTimer() | |
| 1663 | + | ||
| 1664 | + | // Wrap OnURIChanged to check for OAuth callbacks. | |
| 1665 | + | wv.OnURIChanged = composeOnURIChanged(wv.OnURIChanged, func(uri string) { | |
| 1666 | + | if ShouldAutoClose(uri) { | |
| 1667 | + | requestOAuthClose(uri, "uri_changed") | |
| 1566 | 1668 | } | |
| 1669 | + | }) | |
| 1567 | 1670 | ||
| 1568 | - | // Check URL on commit for immediate detection | |
| 1671 | + | // Monitor load events for URL-based detection. | |
| 1672 | + | wv.OnLoadChanged = composeOnLoadChanged(wv.OnLoadChanged, func(event webkit.LoadEvent) { | |
| 1569 | 1673 | if event == webkit.LoadCommitted { | |
| 1570 | 1674 | uri := wv.URI() | |
| 1571 | 1675 | if ShouldAutoClose(uri) { | |
| 1572 | - | cancelSafetyTimer() | |
| 1573 | - | log.Info().Str("pane", string(paneID)).Msg("oauth callback on load, closing") | |
| 1574 | - | go func() { | |
| 1575 | - | time.Sleep(500 * time.Millisecond) | |
| 1576 | - | if wv != nil && !wv.IsDestroyed() { | |
| 1577 | - | wv.Close() | |
| 1578 | - | } | |
| 1579 | - | }() | |
| 1676 | + | requestOAuthClose(uri, "load_committed") | |
| 1580 | 1677 | } | |
| 1581 | 1678 | } | |
| 1582 | - | } | |
| 1679 | + | }) | |
| 1583 | 1680 | ||
| 1584 | - | // Wrap OnClose to cancel safety timer when popup closes normally | |
| 1585 | - | originalOnClose := wv.OnClose | |
| 1586 | - | wv.OnClose = func() { | |
| 1681 | + | // Cancel safety timer on any close path. | |
| 1682 | + | wv.OnClose = composeOnClose(func() { | |
| 1587 | 1683 | cancelSafetyTimer() | |
| 1588 | - | if originalOnClose != nil { | |
| 1589 | - | originalOnClose() | |
| 1590 | - | } | |
| 1684 | + | }, wv.OnClose) | |
| 1685 | + | } | |
| 1686 | + | ||
| 1687 | + | func (c *ContentCoordinator) trackOAuthPopup(popupID port.WebViewID, parentPaneID entity.PaneID) { | |
| 1688 | + | c.popupMu.Lock() | |
| 1689 | + | defer c.popupMu.Unlock() | |
| 1690 | + | if c.popupOAuth == nil { | |
| 1691 | + | c.popupOAuth = make(map[port.WebViewID]*popupOAuthState) | |
| 1692 | + | } | |
| 1693 | + | c.popupOAuth[popupID] = &popupOAuthState{ | |
| 1694 | + | ParentPaneID: parentPaneID, | |
| 1695 | + | } | |
| 1696 | + | } | |
| 1697 | + | ||
| 1698 | + | func (c *ContentCoordinator) capturePopupOAuthState(popupID port.WebViewID, uri string) { | |
| 1699 | + | c.popupMu.Lock() | |
| 1700 | + | defer c.popupMu.Unlock() | |
| 1701 | + | ||
| 1702 | + | state, ok := c.popupOAuth[popupID] | |
| 1703 | + | if !ok { | |
| 1704 | + | return | |
| 1705 | + | } | |
| 1706 | + | ||
| 1707 | + | state.Seen = true | |
| 1708 | + | state.CallbackURI = uri | |
| 1709 | + | state.Success = IsOAuthSuccess(uri) | |
| 1710 | + | state.Error = IsOAuthError(uri) | |
| 1711 | + | } | |
| 1712 | + | ||
| 1713 | + | func (c *ContentCoordinator) handlePopupOAuthClose(ctx context.Context, popupID port.WebViewID) { | |
| 1714 | + | log := logging.FromContext(ctx) | |
| 1715 | + | ||
| 1716 | + | c.popupMu.Lock() | |
| 1717 | + | state, ok := c.popupOAuth[popupID] | |
| 1718 | + | if ok { | |
| 1719 | + | delete(c.popupOAuth, popupID) | |
| 1720 | + | } | |
| 1721 | + | c.popupMu.Unlock() | |
| 1722 | + | ||
| 1723 | + | if !ok || state == nil || !state.Seen { | |
| 1724 | + | return | |
| 1725 | + | } | |
| 1726 | + | ||
| 1727 | + | log.Debug(). | |
| 1728 | + | Uint64("popup_id", uint64(popupID)). | |
| 1729 | + | Str("parent_pane_id", string(state.ParentPaneID)). | |
| 1730 | + | Bool("oauth_success", state.Success). | |
| 1731 | + | Bool("oauth_error", state.Error). | |
| 1732 | + | Msg("popup oauth result captured on close") | |
| 1733 | + | ||
| 1734 | + | if !state.Success || state.ParentPaneID == "" { | |
| 1735 | + | return | |
| 1736 | + | } | |
| 1737 | + | ||
| 1738 | + | c.scheduleParentPaneRefresh(ctx, state.ParentPaneID, popupID) | |
| 1739 | + | } | |
| 1740 | + | ||
| 1741 | + | func (c *ContentCoordinator) scheduleParentPaneRefresh( | |
| 1742 | + | ctx context.Context, | |
| 1743 | + | parentPaneID entity.PaneID, | |
| 1744 | + | popupID port.WebViewID, | |
| 1745 | + | ) { | |
| 1746 | + | c.popupMu.Lock() | |
| 1747 | + | if c.popupRefresh == nil { | |
| 1748 | + | c.popupRefresh = make(map[entity.PaneID]*time.Timer) | |
| 1749 | + | } | |
| 1750 | + | if existing := c.popupRefresh[parentPaneID]; existing != nil { | |
| 1751 | + | existing.Stop() | |
| 1591 | 1752 | } | |
| 1753 | + | c.popupRefresh[parentPaneID] = time.AfterFunc(oauthParentRefreshDebounce, func() { | |
| 1754 | + | c.popupMu.Lock() | |
| 1755 | + | delete(c.popupRefresh, parentPaneID) | |
| 1756 | + | c.popupMu.Unlock() | |
| 1757 | + | c.refreshPaneAfterOAuth(ctx, parentPaneID, popupID) | |
| 1758 | + | }) | |
| 1759 | + | c.popupMu.Unlock() | |
| 1760 | + | } | |
| 1761 | + | ||
| 1762 | + | func (c *ContentCoordinator) refreshPaneAfterOAuth( | |
| 1763 | + | ctx context.Context, | |
| 1764 | + | parentPaneID entity.PaneID, | |
| 1765 | + | popupID port.WebViewID, | |
| 1766 | + | ) { | |
| 1767 | + | log := logging.FromContext(ctx) | |
| 1768 | + | wv := c.webViews[parentPaneID] | |
| 1769 | + | if wv == nil || wv.IsDestroyed() { | |
| 1770 | + | log.Debug(). | |
| 1771 | + | Str("parent_pane_id", string(parentPaneID)). | |
| 1772 | + | Uint64("popup_id", uint64(popupID)). | |
| 1773 | + | Msg("skipping parent pane refresh after oauth close: parent webview unavailable") | |
| 1774 | + | return | |
| 1775 | + | } | |
| 1776 | + | ||
| 1777 | + | if err := wv.Reload(ctx); err != nil { | |
| 1778 | + | log.Warn(). | |
| 1779 | + | Err(err). | |
| 1780 | + | Str("parent_pane_id", string(parentPaneID)). | |
| 1781 | + | Uint64("popup_id", uint64(popupID)). | |
| 1782 | + | Msg("failed parent pane refresh after oauth popup close") | |
| 1783 | + | return | |
| 1784 | + | } | |
| 1785 | + | ||
| 1786 | + | log.Info(). | |
| 1787 | + | Str("parent_pane_id", string(parentPaneID)). | |
| 1788 | + | Uint64("popup_id", uint64(popupID)). | |
| 1789 | + | Msg("refreshed parent pane after oauth popup success") | |
| 1592 | 1790 | } | |
| 1593 | 1791 | ||
| 1594 | 1792 | // handleLinkMiddleClick handles middle-click / Ctrl+click on links. | |
| @@ -4,6 +4,7 @@ import ( | |||||
| 4 | "context" | 4 | "context" | ||
| 5 | "encoding/json" | 5 | "encoding/json" | ||
| 6 | "fmt" | 6 | "fmt" | ||
| 7 | + | "net/url" | |||
| 7 | "strings" | 8 | "strings" | ||
| 8 | "sync" | 9 | "sync" | ||
| 9 | "time" | 10 | "time" | ||
| @@ -20,12 +21,15 @@ import ( | |||||
| 20 | "github.com/bnema/dumber/internal/ui/component" | 21 | "github.com/bnema/dumber/internal/ui/component" | ||
| 21 | "github.com/bnema/dumber/internal/ui/input" | 22 | "github.com/bnema/dumber/internal/ui/input" | ||
| 22 | "github.com/bnema/dumber/internal/ui/layout" | 23 | "github.com/bnema/dumber/internal/ui/layout" | ||
| 24 | + | webkitlib "github.com/bnema/puregotk-webkit/webkit" | |||
| 23 | "github.com/jwijenbergh/puregotk/v4/gdk" | 25 | "github.com/jwijenbergh/puregotk/v4/gdk" | ||
| 24 | ) | 26 | ) | ||
| 25 | 27 | ||||
| 26 | const ( | 28 | const ( | ||
| 27 | - | aboutBlankURI = "about:blank" | 29 | + | aboutBlankURI = "about:blank" |
| 28 | - | logURLMaxLen = 80 | 30 | + | crashPageURI = "dumb://home/crash" |
| 31 | + | logURLMaxLen = 80 | |||
| 32 | + | oauthParentRefreshDebounce = 200 * time.Millisecond | |||
| 29 | 33 | ||||
| 30 | // Dark theme background color (#0a0a0b) as float32 RGBA values | 34 | // Dark theme background color (#0a0a0b) as float32 RGBA values | ||
| 31 | darkBgR = 0.039 | 35 | darkBgR = 0.039 | ||
| @@ -34,6 +38,51 @@ const ( | |||||
| 34 | darkBgA = 1.0 | 38 | darkBgA = 1.0 | ||
| 35 | ) | 39 | ) | ||
| 36 | 40 | ||||
| 41 | + | func shouldRenderCrashPage(reason webkitlib.WebProcessTerminationReason) bool { | |||
| 42 | + | switch reason { | |||
| 43 | + | case webkitlib.WebProcessCrashedValue, webkitlib.WebProcessExceededMemoryLimitValue: | |||
| 44 | + | return true | |||
| 45 | + | case webkitlib.WebProcessTerminatedByApiValue: | |||
| 46 | + | return false | |||
| 47 | + | default: | |||
| 48 | + | return true | |||
| 49 | + | } | |||
| 50 | + | } | |||
| 51 | + | ||||
| 52 | + | func extractOriginalURIFromCrashPage(uri string) string { | |||
| 53 | + | if uri == "" { | |||
| 54 | + | return "" | |||
| 55 | + | } | |||
| 56 | + | ||||
| 57 | + | parsed, err := url.Parse(uri) | |||
| 58 | + | if err != nil { | |||
| 59 | + | return uri | |||
| 60 | + | } | |||
| 61 | + | ||||
| 62 | + | if parsed.Scheme != "dumb" || parsed.Host != webkit.HomePath { | |||
| 63 | + | return uri | |||
| 64 | + | } | |||
| 65 | + | ||||
| 66 | + | if strings.Trim(parsed.Path, "/") != "crash" { | |||
| 67 | + | return uri | |||
| 68 | + | } | |||
| 69 | + | ||||
| 70 | + | original := strings.TrimSpace(parsed.Query().Get("url")) | |||
| 71 | + | if original == "" { | |||
| 72 | + | return "" | |||
| 73 | + | } | |||
| 74 | + | return original | |||
| 75 | + | } | |||
| 76 | + | ||||
| 77 | + | func buildCrashPageURI(originalURI string) string { | |||
| 78 | + | if strings.TrimSpace(originalURI) == "" { | |||
| 79 | + | return crashPageURI | |||
| 80 | + | } | |||
| 81 | + | query := url.Values{} | |||
| 82 | + | query.Set("url", originalURI) | |||
| 83 | + | return crashPageURI + "?" + query.Encode() | |||
| 84 | + | } | |||
| 85 | + | ||||
| 37 | // ContentCoordinator manages WebView lifecycle, title tracking, and content attachment. | 86 | // ContentCoordinator manages WebView lifecycle, title tracking, and content attachment. | ||
| 38 | type ContentCoordinator struct { | 87 | type ContentCoordinator struct { | ||
| 39 | pool *webkit.WebViewPool | 88 | pool *webkit.WebViewPool | ||
| @@ -87,6 +136,8 @@ type ContentCoordinator struct { | |||||
| 87 | factory *webkit.WebViewFactory | 136 | factory *webkit.WebViewFactory | ||
| 88 | popupConfig *config.PopupBehaviorConfig | 137 | popupConfig *config.PopupBehaviorConfig | ||
| 89 | pendingPopups map[port.WebViewID]*PendingPopup | 138 | pendingPopups map[port.WebViewID]*PendingPopup | ||
| 139 | + | popupOAuth map[port.WebViewID]*popupOAuthState | |||
| 140 | + | popupRefresh map[entity.PaneID]*time.Timer | |||
| 90 | popupMu sync.RWMutex | 141 | popupMu sync.RWMutex | ||
| 91 | 142 | ||||
| 92 | // Callback to insert popup into workspace (avoids circular dependency) | 143 | // Callback to insert popup into workspace (avoids circular dependency) | ||
| @@ -117,6 +168,14 @@ type pendingThemeUpdate struct { | |||||
| 117 | cssText string | 168 | cssText string | ||
| 118 | } | 169 | } | ||
| 119 | 170 | ||||
| 171 | + | type popupOAuthState struct { | |||
| 172 | + | ParentPaneID entity.PaneID | |||
| 173 | + | CallbackURI string | |||
| 174 | + | Success bool | |||
| 175 | + | Error bool | |||
| 176 | + | Seen bool | |||
| 177 | + | } | |||
| 178 | + | ||||
| 120 | // NewContentCoordinator creates a new ContentCoordinator. | 179 | // NewContentCoordinator creates a new ContentCoordinator. | ||
| 121 | func NewContentCoordinator( | 180 | func NewContentCoordinator( | ||
| 122 | ctx context.Context, | 181 | ctx context.Context, | ||
| @@ -140,6 +199,8 @@ func NewContentCoordinator( | |||||
| 140 | pendingReveal: make(map[entity.PaneID]bool), | 199 | pendingReveal: make(map[entity.PaneID]bool), | ||
| 141 | getActiveWS: getActiveWS, | 200 | getActiveWS: getActiveWS, | ||
| 142 | pendingPopups: make(map[port.WebViewID]*PendingPopup), | 201 | pendingPopups: make(map[port.WebViewID]*PendingPopup), | ||
| 202 | + | popupOAuth: make(map[port.WebViewID]*popupOAuthState), | |||
| 203 | + | popupRefresh: make(map[entity.PaneID]*time.Timer), | |||
| 143 | } | 204 | } | ||
| 144 | } | 205 | } | ||
| 145 | 206 | ||||
| @@ -1299,7 +1360,8 @@ func (c *ContentCoordinator) handlePopupCreate( | |||||
| 1299 | // Setup OAuth auto-close if configured | 1360 | // Setup OAuth auto-close if configured | ||
| 1300 | if hasConfig && oauthEnabled && isOAuth { | 1361 | if hasConfig && oauthEnabled && isOAuth { | ||
| 1301 | popupPane.AutoClose = true | 1362 | popupPane.AutoClose = true | ||
| 1302 | - | c.setupOAuthAutoClose(ctx, paneID, popupWV) | 1363 | + | c.trackOAuthPopup(popupID, parentPaneID) |
| 1364 | + | c.setupOAuthAutoClose(ctx, paneID, popupID, popupWV) | |||
| 1303 | log.Debug().Str("pane_id", string(paneID)).Msg("OAuth auto-close enabled for popup") | 1365 | log.Debug().Str("pane_id", string(paneID)).Msg("OAuth auto-close enabled for popup") | ||
| 1304 | } | 1366 | } | ||
| 1305 | 1367 | ||||
| @@ -1323,9 +1385,9 @@ func (c *ContentCoordinator) handlePopupCreate( | |||||
| 1323 | popupWV.OnReadyToShow = func() { | 1385 | popupWV.OnReadyToShow = func() { | ||
| 1324 | c.handlePopupReadyToShow(ctx, popupID) | 1386 | c.handlePopupReadyToShow(ctx, popupID) | ||
| 1325 | } | 1387 | } | ||
| 1326 | - | popupWV.OnClose = func() { | 1388 | + | popupWV.OnClose = composeOnClose(popupWV.OnClose, func() { |
| 1327 | c.handlePopupClose(ctx, popupID) | 1389 | c.handlePopupClose(ctx, popupID) | ||
| 1328 | - | } | 1390 | + | }) |
| 1329 | 1391 | ||||
| 1330 | log.Info(). | 1392 | log.Info(). | ||
| 1331 | Uint64("popup_id", uint64(popupID)). | 1393 | Uint64("popup_id", uint64(popupID)). | ||
| @@ -1386,6 +1448,7 @@ func (c *ContentCoordinator) handlePopupClose(ctx context.Context, popupID port. | |||||
| 1386 | c.popupMu.Unlock() | 1448 | c.popupMu.Unlock() | ||
| 1387 | 1449 | ||||
| 1388 | if wasPending && pending != nil { | 1450 | if wasPending && pending != nil { | ||
| 1451 | + | c.handlePopupOAuthClose(ctx, popupID) | |||
| 1389 | // Was never shown, just destroy | 1452 | // Was never shown, just destroy | ||
| 1390 | pending.WebView.Destroy() | 1453 | pending.WebView.Destroy() | ||
| 1391 | log.Debug().Msg("destroyed pending popup that was never shown") | 1454 | log.Debug().Msg("destroyed pending popup that was never shown") | ||
| @@ -1402,6 +1465,7 @@ func (c *ContentCoordinator) handlePopupClose(ctx context.Context, popupID port. | |||||
| 1402 | } | 1465 | } | ||
| 1403 | 1466 | ||||
| 1404 | if paneID == "" { | 1467 | if paneID == "" { | ||
| 1468 | + | c.handlePopupOAuthClose(ctx, popupID) | |||
| 1405 | log.Warn().Msg("popup close: could not find pane for webview") | 1469 | log.Warn().Msg("popup close: could not find pane for webview") | ||
| 1406 | return | 1470 | return | ||
| 1407 | } | 1471 | } | ||
| @@ -1413,6 +1477,8 @@ func (c *ContentCoordinator) handlePopupClose(ctx context.Context, popupID port. | |||||
| 1413 | } | 1477 | } | ||
| 1414 | } | 1478 | } | ||
| 1415 | 1479 | ||||
| 1480 | + | c.handlePopupOAuthClose(ctx, popupID) | |||
| 1481 | + | ||||
| 1416 | // Release the WebView (this will clean up tracking) | 1482 | // Release the WebView (this will clean up tracking) | ||
| 1417 | c.ReleaseWebView(ctx, paneID) | 1483 | c.ReleaseWebView(ctx, paneID) | ||
| 1418 | 1484 | ||||
| @@ -1421,6 +1487,8 @@ func (c *ContentCoordinator) handlePopupClose(ctx context.Context, popupID port. | |||||
| 1421 | 1487 | ||||
| 1422 | // setupWebViewCallbacks configures standard callbacks and popup handling. | 1488 | // setupWebViewCallbacks configures standard callbacks and popup handling. | ||
| 1423 | func (c *ContentCoordinator) setupWebViewCallbacks(ctx context.Context, paneID entity.PaneID, wv *webkit.WebView) { | 1489 | func (c *ContentCoordinator) setupWebViewCallbacks(ctx context.Context, paneID entity.PaneID, wv *webkit.WebView) { | ||
| 1490 | + | log := logging.FromContext(ctx) | |||
| 1491 | + | ||||
| 1424 | // Title changes | 1492 | // Title changes | ||
| 1425 | wv.OnTitleChanged = func(title string) { | 1493 | wv.OnTitleChanged = func(title string) { | ||
| 1426 | c.onTitleChanged(ctx, paneID, title) | 1494 | c.onTitleChanged(ctx, paneID, title) | ||
| @@ -1491,6 +1559,36 @@ func (c *ContentCoordinator) setupWebViewCallbacks(ctx context.Context, paneID e | |||||
| 1491 | c.onLinkHover(paneID, uri) | 1559 | c.onLinkHover(paneID, uri) | ||
| 1492 | } | 1560 | } | ||
| 1493 | 1561 | ||||
| 1562 | + | wv.OnWebProcessTerminated = func(reason webkitlib.WebProcessTerminationReason, reasonLabel string, uri string) { | |||
| 1563 | + | originalURI := extractOriginalURIFromCrashPage(uri) | |||
| 1564 | + | if !shouldRenderCrashPage(reason) { | |||
| 1565 | + | log.Info(). | |||
| 1566 | + | Str("pane_id", string(paneID)). | |||
| 1567 | + | Str("reason", reasonLabel). | |||
| 1568 | + | Str("uri", uri). | |||
| 1569 | + | Msg("web process termination handled without crash page") | |||
| 1570 | + | return | |||
| 1571 | + | } | |||
| 1572 | + | ||||
| 1573 | + | crashURI := buildCrashPageURI(originalURI) | |||
| 1574 | + | log.Warn(). | |||
| 1575 | + | Str("pane_id", string(paneID)). | |||
| 1576 | + | Str("reason", reasonLabel). | |||
| 1577 | + | Str("uri", uri). | |||
| 1578 | + | Str("crash_uri", crashURI). | |||
| 1579 | + | Msg("web process terminated, redirecting to crash page") | |||
| 1580 | + | ||||
| 1581 | + | if err := wv.LoadURI(ctx, crashURI); err != nil { | |||
| 1582 | + | log.Error(). | |||
| 1583 | + | Err(err). | |||
| 1584 | + | Str("pane_id", string(paneID)). | |||
| 1585 | + | Str("reason", reasonLabel). | |||
| 1586 | + | Str("uri", uri). | |||
| 1587 | + | Str("crash_uri", crashURI). | |||
| 1588 | + | Msg("failed to load crash page after web process termination") | |||
| 1589 | + | } | |||
| 1590 | + | } | |||
| 1591 | + | ||||
| 1494 | // Fullscreen handlers for idle inhibition | 1592 | // Fullscreen handlers for idle inhibition | ||
| 1495 | c.setupIdleInhibitionHandlers(ctx, paneID, wv) | 1593 | c.setupIdleInhibitionHandlers(ctx, paneID, wv) | ||
| 1496 | 1594 | ||||
| @@ -1503,12 +1601,19 @@ func (c *ContentCoordinator) setupWebViewCallbacks(ctx context.Context, paneID e | |||||
| 1503 | // For providers using postMessage (like Google Sign-In), we rely on the provider calling | 1601 | // For providers using postMessage (like Google Sign-In), we rely on the provider calling | ||
| 1504 | // window.close() which triggers WebKit's close signal. | 1602 | // window.close() which triggers WebKit's close signal. | ||
| 1505 | // A long safety timeout (30s) catches popups that get stuck. | 1603 | // A long safety timeout (30s) catches popups that get stuck. | ||
| 1506 | - | func (c *ContentCoordinator) setupOAuthAutoClose(ctx context.Context, paneID entity.PaneID, wv *webkit.WebView) { | 1604 | + | func (c *ContentCoordinator) setupOAuthAutoClose( |
| 1605 | + | ctx context.Context, | |||
| 1606 | + | paneID entity.PaneID, | |||
| 1607 | + | popupID port.WebViewID, | |||
| 1608 | + | wv *webkit.WebView, | |||
| 1609 | + | ) { | |||
| 1507 | log := logging.FromContext(ctx) | 1610 | log := logging.FromContext(ctx) | ||
| 1508 | 1611 | ||||
| 1509 | // Safety timeout - only triggers if popup gets stuck (provider should close via window.close) | 1612 | // Safety timeout - only triggers if popup gets stuck (provider should close via window.close) | ||
| 1510 | var safetyTimer *time.Timer | 1613 | var safetyTimer *time.Timer | ||
| 1511 | var safetyTimerMu sync.Mutex | 1614 | var safetyTimerMu sync.Mutex | ||
| 1615 | + | var cancelSafetyTimerOnce sync.Once | |||
| 1616 | + | var requestCloseOnce sync.Once | |||
| 1512 | const oauthSafetyTimeout = 30 * time.Second | 1617 | const oauthSafetyTimeout = 30 * time.Second | ||
| 1513 | 1618 | ||||
| 1514 | startSafetyTimer := func() { | 1619 | startSafetyTimer := func() { | ||
| @@ -1526,69 +1631,162 @@ func (c *ContentCoordinator) setupOAuthAutoClose(ctx context.Context, paneID ent | |||||
| 1526 | } | 1631 | } | ||
| 1527 | 1632 | ||||
| 1528 | cancelSafetyTimer := func() { | 1633 | cancelSafetyTimer := func() { | ||
| 1529 | - | safetyTimerMu.Lock() | 1634 | + | cancelSafetyTimerOnce.Do(func() { |
| 1530 | - | defer safetyTimerMu.Unlock() | 1635 | + | safetyTimerMu.Lock() |
| 1531 | - | if safetyTimer != nil { | 1636 | + | defer safetyTimerMu.Unlock() |
| 1532 | - | safetyTimer.Stop() | 1637 | + | if safetyTimer != nil { |
| 1533 | - | safetyTimer = nil | 1638 | + | safetyTimer.Stop() |
| 1534 | - | } | 1639 | + | safetyTimer = nil |
| 1640 | + | } | |||
| 1641 | + | }) | |||
| 1535 | } | 1642 | } | ||
| 1536 | 1643 | ||||
| 1537 | - | // Start safety timer immediately | 1644 | + | requestOAuthClose := func(uri string, reason string) { |
| 1538 | - | startSafetyTimer() | 1645 | + | c.capturePopupOAuthState(popupID, uri) |
| 1539 | - | 1646 | + | cancelSafetyTimer() | |
| 1540 | - | // Wrap the existing OnURIChanged to also check for OAuth callbacks | 1647 | + | log.Info(). |
| 1541 | - | originalURIChanged := wv.OnURIChanged | 1648 | + | Str("pane", string(paneID)). |
| 1542 | - | wv.OnURIChanged = func(uri string) { | 1649 | + | Str("reason", reason). |
| 1543 | - | // Call original handler first | 1650 | + | Msg("oauth callback detected, closing") |
| 1544 | - | if originalURIChanged != nil { | 1651 | + | requestCloseOnce.Do(func() { |
| 1545 | - | originalURIChanged(uri) | |||
| 1546 | - | } | |||
| 1547 | - | ||||
| 1548 | - | // Check for OAuth callback (URL-based detection) | |||
| 1549 | - | if ShouldAutoClose(uri) { | |||
| 1550 | - | cancelSafetyTimer() | |||
| 1551 | - | log.Info().Str("pane", string(paneID)).Msg("oauth callback detected, closing") | |||
| 1552 | go func() { | 1652 | go func() { | ||
| 1553 | time.Sleep(500 * time.Millisecond) | 1653 | time.Sleep(500 * time.Millisecond) | ||
| 1554 | if wv != nil && !wv.IsDestroyed() { | 1654 | if wv != nil && !wv.IsDestroyed() { | ||
| 1555 | wv.Close() | 1655 | wv.Close() | ||
| 1556 | } | 1656 | } | ||
| 1557 | }() | 1657 | }() | ||
| 1558 | - | } | 1658 | + | }) |
| 1559 | } | 1659 | } | ||
| 1560 | 1660 | ||||
| 1561 | - | // Monitor load events for URL-based detection | 1661 | + | // Start safety timer immediately. |
| 1562 | - | originalLoadChanged := wv.OnLoadChanged | 1662 | + | startSafetyTimer() |
| 1563 | - | wv.OnLoadChanged = func(event webkit.LoadEvent) { | 1663 | + | |
| 1564 | - | if originalLoadChanged != nil { | 1664 | + | // Wrap OnURIChanged to check for OAuth callbacks. |
| 1565 | - | originalLoadChanged(event) | 1665 | + | wv.OnURIChanged = composeOnURIChanged(wv.OnURIChanged, func(uri string) { |
| 1666 | + | if ShouldAutoClose(uri) { | |||
| 1667 | + | requestOAuthClose(uri, "uri_changed") | |||
| 1566 | } | 1668 | } | ||
| 1669 | + | }) | |||
| 1567 | 1670 | ||||
| 1568 | - | // Check URL on commit for immediate detection | 1671 | + | // Monitor load events for URL-based detection. |
| 1672 | + | wv.OnLoadChanged = composeOnLoadChanged(wv.OnLoadChanged, func(event webkit.LoadEvent) { | |||
| 1569 | if event == webkit.LoadCommitted { | 1673 | if event == webkit.LoadCommitted { | ||
| 1570 | uri := wv.URI() | 1674 | uri := wv.URI() | ||
| 1571 | if ShouldAutoClose(uri) { | 1675 | if ShouldAutoClose(uri) { | ||
| 1572 | - | cancelSafetyTimer() | 1676 | + | requestOAuthClose(uri, "load_committed") |
| 1573 | - | log.Info().Str("pane", string(paneID)).Msg("oauth callback on load, closing") | |||
| 1574 | - | go func() { | |||
| 1575 | - | time.Sleep(500 * time.Millisecond) | |||
| 1576 | - | if wv != nil && !wv.IsDestroyed() { | |||
| 1577 | - | wv.Close() | |||
| 1578 | - | } | |||
| 1579 | - | }() | |||
| 1580 | } | 1677 | } | ||
| 1581 | } | 1678 | } | ||
| 1582 | - | } | 1679 | + | }) |
| 1583 | 1680 | ||||
| 1584 | - | // Wrap OnClose to cancel safety timer when popup closes normally | 1681 | + | // Cancel safety timer on any close path. |
| 1585 | - | originalOnClose := wv.OnClose | 1682 | + | wv.OnClose = composeOnClose(func() { |
| 1586 | - | wv.OnClose = func() { | |||
| 1587 | cancelSafetyTimer() | 1683 | cancelSafetyTimer() | ||
| 1588 | - | if originalOnClose != nil { | 1684 | + | }, wv.OnClose) |
| 1589 | - | originalOnClose() | 1685 | + | } |
| 1590 | - | } | 1686 | + | |
| 1687 | + | func (c *ContentCoordinator) trackOAuthPopup(popupID port.WebViewID, parentPaneID entity.PaneID) { | |||
| 1688 | + | c.popupMu.Lock() | |||
| 1689 | + | defer c.popupMu.Unlock() | |||
| 1690 | + | if c.popupOAuth == nil { | |||
| 1691 | + | c.popupOAuth = make(map[port.WebViewID]*popupOAuthState) | |||
| 1692 | + | } | |||
| 1693 | + | c.popupOAuth[popupID] = &popupOAuthState{ | |||
| 1694 | + | ParentPaneID: parentPaneID, | |||
| 1695 | + | } | |||
| 1696 | + | } | |||
| 1697 | + | ||||
| 1698 | + | func (c *ContentCoordinator) capturePopupOAuthState(popupID port.WebViewID, uri string) { | |||
| 1699 | + | c.popupMu.Lock() | |||
| 1700 | + | defer c.popupMu.Unlock() | |||
| 1701 | + | ||||
| 1702 | + | state, ok := c.popupOAuth[popupID] | |||
| 1703 | + | if !ok { | |||
| 1704 | + | return | |||
| 1705 | + | } | |||
| 1706 | + | ||||
| 1707 | + | state.Seen = true | |||
| 1708 | + | state.CallbackURI = uri | |||
| 1709 | + | state.Success = IsOAuthSuccess(uri) | |||
| 1710 | + | state.Error = IsOAuthError(uri) | |||
| 1711 | + | } | |||
| 1712 | + | ||||
| 1713 | + | func (c *ContentCoordinator) handlePopupOAuthClose(ctx context.Context, popupID port.WebViewID) { | |||
| 1714 | + | log := logging.FromContext(ctx) | |||
| 1715 | + | ||||
| 1716 | + | c.popupMu.Lock() | |||
| 1717 | + | state, ok := c.popupOAuth[popupID] | |||
| 1718 | + | if ok { | |||
| 1719 | + | delete(c.popupOAuth, popupID) | |||
| 1720 | + | } | |||
| 1721 | + | c.popupMu.Unlock() | |||
| 1722 | + | ||||
| 1723 | + | if !ok || state == nil || !state.Seen { | |||
| 1724 | + | return | |||
| 1725 | + | } | |||
| 1726 | + | ||||
| 1727 | + | log.Debug(). | |||
| 1728 | + | Uint64("popup_id", uint64(popupID)). | |||
| 1729 | + | Str("parent_pane_id", string(state.ParentPaneID)). | |||
| 1730 | + | Bool("oauth_success", state.Success). | |||
| 1731 | + | Bool("oauth_error", state.Error). | |||
| 1732 | + | Msg("popup oauth result captured on close") | |||
| 1733 | + | ||||
| 1734 | + | if !state.Success || state.ParentPaneID == "" { | |||
| 1735 | + | return | |||
| 1736 | + | } | |||
| 1737 | + | ||||
| 1738 | + | c.scheduleParentPaneRefresh(ctx, state.ParentPaneID, popupID) | |||
| 1739 | + | } | |||
| 1740 | + | ||||
| 1741 | + | func (c *ContentCoordinator) scheduleParentPaneRefresh( | |||
| 1742 | + | ctx context.Context, | |||
| 1743 | + | parentPaneID entity.PaneID, | |||
| 1744 | + | popupID port.WebViewID, | |||
| 1745 | + | ) { | |||
| 1746 | + | c.popupMu.Lock() | |||
| 1747 | + | if c.popupRefresh == nil { | |||
| 1748 | + | c.popupRefresh = make(map[entity.PaneID]*time.Timer) | |||
| 1749 | + | } | |||
| 1750 | + | if existing := c.popupRefresh[parentPaneID]; existing != nil { | |||
| 1751 | + | existing.Stop() | |||
| 1591 | } | 1752 | } | ||
| 1753 | + | c.popupRefresh[parentPaneID] = time.AfterFunc(oauthParentRefreshDebounce, func() { | |||
| 1754 | + | c.popupMu.Lock() | |||
| 1755 | + | delete(c.popupRefresh, parentPaneID) | |||
| 1756 | + | c.popupMu.Unlock() | |||
| 1757 | + | c.refreshPaneAfterOAuth(ctx, parentPaneID, popupID) | |||
| 1758 | + | }) | |||
| 1759 | + | c.popupMu.Unlock() | |||
| 1760 | + | } | |||
| 1761 | + | ||||
| 1762 | + | func (c *ContentCoordinator) refreshPaneAfterOAuth( | |||
| 1763 | + | ctx context.Context, | |||
| 1764 | + | parentPaneID entity.PaneID, | |||
| 1765 | + | popupID port.WebViewID, | |||
| 1766 | + | ) { | |||
| 1767 | + | log := logging.FromContext(ctx) | |||
| 1768 | + | wv := c.webViews[parentPaneID] | |||
| 1769 | + | if wv == nil || wv.IsDestroyed() { | |||
| 1770 | + | log.Debug(). | |||
| 1771 | + | Str("parent_pane_id", string(parentPaneID)). | |||
| 1772 | + | Uint64("popup_id", uint64(popupID)). | |||
| 1773 | + | Msg("skipping parent pane refresh after oauth close: parent webview unavailable") | |||
| 1774 | + | return | |||
| 1775 | + | } | |||
| 1776 | + | ||||
| 1777 | + | if err := wv.Reload(ctx); err != nil { | |||
| 1778 | + | log.Warn(). | |||
| 1779 | + | Err(err). | |||
| 1780 | + | Str("parent_pane_id", string(parentPaneID)). | |||
| 1781 | + | Uint64("popup_id", uint64(popupID)). | |||
| 1782 | + | Msg("failed parent pane refresh after oauth popup close") | |||
| 1783 | + | return | |||
| 1784 | + | } | |||
| 1785 | + | ||||
| 1786 | + | log.Info(). | |||
| 1787 | + | Str("parent_pane_id", string(parentPaneID)). | |||
| 1788 | + | Uint64("popup_id", uint64(popupID)). | |||
| 1789 | + | Msg("refreshed parent pane after oauth popup success") | |||
| 1592 | } | 1790 | } | ||
| 1593 | 1791 | ||||
| 1594 | // handleLinkMiddleClick handles middle-click / Ctrl+click on links. | 1792 | // handleLinkMiddleClick handles middle-click / Ctrl+click on links. | ||
A internal/ui/coordinator/content_termination_test.go +27 -0
| @@ -0,0 +1,27 @@ | |||
| 1 | + | package coordinator | |
| 2 | + | ||
| 3 | + | import ( | |
| 4 | + | "testing" | |
| 5 | + | ||
| 6 | + | webkitlib "github.com/bnema/puregotk-webkit/webkit" | |
| 7 | + | "github.com/stretchr/testify/assert" | |
| 8 | + | ) | |
| 9 | + | ||
| 10 | + | func TestShouldRenderCrashPage(t *testing.T) { | |
| 11 | + | assert.True(t, shouldRenderCrashPage(webkitlib.WebProcessCrashedValue)) | |
| 12 | + | assert.True(t, shouldRenderCrashPage(webkitlib.WebProcessExceededMemoryLimitValue)) | |
| 13 | + | assert.False(t, shouldRenderCrashPage(webkitlib.WebProcessTerminatedByApiValue)) | |
| 14 | + | assert.True(t, shouldRenderCrashPage(webkitlib.WebProcessTerminationReason(99))) | |
| 15 | + | } | |
| 16 | + | ||
| 17 | + | func TestExtractOriginalURIFromCrashPage(t *testing.T) { | |
| 18 | + | assert.Equal(t, "https://example.com/path?q=1", extractOriginalURIFromCrashPage("dumb://home/crash?url=https%3A%2F%2Fexample.com%2Fpath%3Fq%3D1")) | |
| 19 | + | assert.Equal(t, "", extractOriginalURIFromCrashPage("dumb://home/crash")) | |
| 20 | + | assert.Equal(t, "https://example.com", extractOriginalURIFromCrashPage("https://example.com")) | |
| 21 | + | assert.Equal(t, "%%%bad", extractOriginalURIFromCrashPage("%%%bad")) | |
| 22 | + | } | |
| 23 | + | ||
| 24 | + | func TestBuildCrashPageURI(t *testing.T) { | |
| 25 | + | assert.Equal(t, "dumb://home/crash", buildCrashPageURI("")) | |
| 26 | + | assert.Equal(t, "dumb://home/crash?url=https%3A%2F%2Fexample.com%2Ffoo%3Fa%3D1%26b%3D2", buildCrashPageURI("https://example.com/foo?a=1&b=2")) | |
| 27 | + | } | |
| @@ -0,0 +1,27 @@ | |||||
| 1 | + | package coordinator | |||
| 2 | + | ||||
| 3 | + | import ( | |||
| 4 | + | "testing" | |||
| 5 | + | ||||
| 6 | + | webkitlib "github.com/bnema/puregotk-webkit/webkit" | |||
| 7 | + | "github.com/stretchr/testify/assert" | |||
| 8 | + | ) | |||
| 9 | + | ||||
| 10 | + | func TestShouldRenderCrashPage(t *testing.T) { | |||
| 11 | + | assert.True(t, shouldRenderCrashPage(webkitlib.WebProcessCrashedValue)) | |||
| 12 | + | assert.True(t, shouldRenderCrashPage(webkitlib.WebProcessExceededMemoryLimitValue)) | |||
| 13 | + | assert.False(t, shouldRenderCrashPage(webkitlib.WebProcessTerminatedByApiValue)) | |||
| 14 | + | assert.True(t, shouldRenderCrashPage(webkitlib.WebProcessTerminationReason(99))) | |||
| 15 | + | } | |||
| 16 | + | ||||
| 17 | + | func TestExtractOriginalURIFromCrashPage(t *testing.T) { | |||
| 18 | + | assert.Equal(t, "https://example.com/path?q=1", extractOriginalURIFromCrashPage("dumb://home/crash?url=https%3A%2F%2Fexample.com%2Fpath%3Fq%3D1")) | |||
| 19 | + | assert.Equal(t, "", extractOriginalURIFromCrashPage("dumb://home/crash")) | |||
| 20 | + | assert.Equal(t, "https://example.com", extractOriginalURIFromCrashPage("https://example.com")) | |||
| 21 | + | assert.Equal(t, "%%%bad", extractOriginalURIFromCrashPage("%%%bad")) | |||
| 22 | + | } | |||
| 23 | + | ||||
| 24 | + | func TestBuildCrashPageURI(t *testing.T) { | |||
| 25 | + | assert.Equal(t, "dumb://home/crash", buildCrashPageURI("")) | |||
| 26 | + | assert.Equal(t, "dumb://home/crash?url=https%3A%2F%2Fexample.com%2Ffoo%3Fa%3D1%26b%3D2", buildCrashPageURI("https://example.com/foo?a=1&b=2")) | |||
| 27 | + | } | |||
M internal/ui/coordinator/oauth.go +44 -0
| @@ -2,6 +2,8 @@ package coordinator | |||
| 2 | 2 | ||
| 3 | 3 | import ( | |
| 4 | 4 | "strings" | |
| 5 | + | ||
| 6 | + | "github.com/bnema/dumber/internal/infrastructure/webkit" | |
| 5 | 7 | ) | |
| 6 | 8 | ||
| 7 | 9 | // oauthFlowPatterns are URL patterns that indicate an OAuth flow is in progress. | |
| @@ -138,3 +140,45 @@ func IsOAuthError(url string) bool { | |||
| 138 | 140 | lower := strings.ToLower(url) | |
| 139 | 141 | return strings.Contains(lower, "error=") | |
| 140 | 142 | } | |
| 143 | + | ||
| 144 | + | func composeOnClose(existing func(), next func()) func() { | |
| 145 | + | if existing == nil { | |
| 146 | + | return next | |
| 147 | + | } | |
| 148 | + | if next == nil { | |
| 149 | + | return existing | |
| 150 | + | } | |
| 151 | + | return func() { | |
| 152 | + | existing() | |
| 153 | + | next() | |
| 154 | + | } | |
| 155 | + | } | |
| 156 | + | ||
| 157 | + | func composeOnURIChanged(existing func(string), next func(string)) func(string) { | |
| 158 | + | if existing == nil { | |
| 159 | + | return next | |
| 160 | + | } | |
| 161 | + | if next == nil { | |
| 162 | + | return existing | |
| 163 | + | } | |
| 164 | + | return func(uri string) { | |
| 165 | + | existing(uri) | |
| 166 | + | next(uri) | |
| 167 | + | } | |
| 168 | + | } | |
| 169 | + | ||
| 170 | + | func composeOnLoadChanged( | |
| 171 | + | existing func(webkit.LoadEvent), | |
| 172 | + | next func(webkit.LoadEvent), | |
| 173 | + | ) func(webkit.LoadEvent) { | |
| 174 | + | if existing == nil { | |
| 175 | + | return next | |
| 176 | + | } | |
| 177 | + | if next == nil { | |
| 178 | + | return existing | |
| 179 | + | } | |
| 180 | + | return func(event webkit.LoadEvent) { | |
| 181 | + | existing(event) | |
| 182 | + | next(event) | |
| 183 | + | } | |
| 184 | + | } | |
| @@ -2,6 +2,8 @@ package coordinator | |||||
| 2 | 2 | ||||
| 3 | import ( | 3 | import ( | ||
| 4 | "strings" | 4 | "strings" | ||
| 5 | + | ||||
| 6 | + | "github.com/bnema/dumber/internal/infrastructure/webkit" | |||
| 5 | ) | 7 | ) | ||
| 6 | 8 | ||||
| 7 | // oauthFlowPatterns are URL patterns that indicate an OAuth flow is in progress. | 9 | // oauthFlowPatterns are URL patterns that indicate an OAuth flow is in progress. | ||
| @@ -138,3 +140,45 @@ func IsOAuthError(url string) bool { | |||||
| 138 | lower := strings.ToLower(url) | 140 | lower := strings.ToLower(url) | ||
| 139 | return strings.Contains(lower, "error=") | 141 | return strings.Contains(lower, "error=") | ||
| 140 | } | 142 | } | ||
| 143 | + | ||||
| 144 | + | func composeOnClose(existing func(), next func()) func() { | |||
| 145 | + | if existing == nil { | |||
| 146 | + | return next | |||
| 147 | + | } | |||
| 148 | + | if next == nil { | |||
| 149 | + | return existing | |||
| 150 | + | } | |||
| 151 | + | return func() { | |||
| 152 | + | existing() | |||
| 153 | + | next() | |||
| 154 | + | } | |||
| 155 | + | } | |||
| 156 | + | ||||
| 157 | + | func composeOnURIChanged(existing func(string), next func(string)) func(string) { | |||
| 158 | + | if existing == nil { | |||
| 159 | + | return next | |||
| 160 | + | } | |||
| 161 | + | if next == nil { | |||
| 162 | + | return existing | |||
| 163 | + | } | |||
| 164 | + | return func(uri string) { | |||
| 165 | + | existing(uri) | |||
| 166 | + | next(uri) | |||
| 167 | + | } | |||
| 168 | + | } | |||
| 169 | + | ||||
| 170 | + | func composeOnLoadChanged( | |||
| 171 | + | existing func(webkit.LoadEvent), | |||
| 172 | + | next func(webkit.LoadEvent), | |||
| 173 | + | ) func(webkit.LoadEvent) { | |||
| 174 | + | if existing == nil { | |||
| 175 | + | return next | |||
| 176 | + | } | |||
| 177 | + | if next == nil { | |||
| 178 | + | return existing | |||
| 179 | + | } | |||
| 180 | + | return func(event webkit.LoadEvent) { | |||
| 181 | + | existing(event) | |||
| 182 | + | next(event) | |||
| 183 | + | } | |||
| 184 | + | } | |||
A internal/ui/coordinator/oauth_test.go +110 -0
| @@ -0,0 +1,110 @@ | |||
| 1 | + | package coordinator | |
| 2 | + | ||
| 3 | + | import ( | |
| 4 | + | "context" | |
| 5 | + | "testing" | |
| 6 | + | "time" | |
| 7 | + | ||
| 8 | + | "github.com/stretchr/testify/assert" | |
| 9 | + | ||
| 10 | + | "github.com/bnema/dumber/internal/application/port" | |
| 11 | + | "github.com/bnema/dumber/internal/domain/entity" | |
| 12 | + | "github.com/bnema/dumber/internal/infrastructure/webkit" | |
| 13 | + | ) | |
| 14 | + | ||
| 15 | + | func TestComposeOnClose_Order(t *testing.T) { | |
| 16 | + | var calls []string | |
| 17 | + | ||
| 18 | + | composed := composeOnClose( | |
| 19 | + | func() { calls = append(calls, "existing") }, | |
| 20 | + | func() { calls = append(calls, "next") }, | |
| 21 | + | ) | |
| 22 | + | composed() | |
| 23 | + | ||
| 24 | + | assert.Equal(t, []string{"existing", "next"}, calls) | |
| 25 | + | } | |
| 26 | + | ||
| 27 | + | func TestComposeOnURIChanged_Order(t *testing.T) { | |
| 28 | + | var calls []string | |
| 29 | + | ||
| 30 | + | composed := composeOnURIChanged( | |
| 31 | + | func(_ string) { calls = append(calls, "existing") }, | |
| 32 | + | func(_ string) { calls = append(calls, "next") }, | |
| 33 | + | ) | |
| 34 | + | composed("https://example.com") | |
| 35 | + | ||
| 36 | + | assert.Equal(t, []string{"existing", "next"}, calls) | |
| 37 | + | } | |
| 38 | + | ||
| 39 | + | func TestComposeOnLoadChanged_Order(t *testing.T) { | |
| 40 | + | var calls []string | |
| 41 | + | ||
| 42 | + | composed := composeOnLoadChanged( | |
| 43 | + | func(_ webkit.LoadEvent) { calls = append(calls, "existing") }, | |
| 44 | + | func(_ webkit.LoadEvent) { calls = append(calls, "next") }, | |
| 45 | + | ) | |
| 46 | + | composed(webkit.LoadCommitted) | |
| 47 | + | ||
| 48 | + | assert.Equal(t, []string{"existing", "next"}, calls) | |
| 49 | + | } | |
| 50 | + | ||
| 51 | + | func TestHandlePopupOAuthClose_SuccessSchedulesParentRefresh(t *testing.T) { | |
| 52 | + | parentPaneID := entity.PaneID("parent-pane") | |
| 53 | + | popupID := port.WebViewID(101) | |
| 54 | + | ||
| 55 | + | c := &ContentCoordinator{ | |
| 56 | + | webViews: make(map[entity.PaneID]*webkit.WebView), | |
| 57 | + | popupOAuth: make(map[port.WebViewID]*popupOAuthState), | |
| 58 | + | popupRefresh: make(map[entity.PaneID]*time.Timer), | |
| 59 | + | } | |
| 60 | + | ||
| 61 | + | c.trackOAuthPopup(popupID, parentPaneID) | |
| 62 | + | c.capturePopupOAuthState(popupID, "https://app/callback?code=123") | |
| 63 | + | c.handlePopupOAuthClose(context.Background(), popupID) | |
| 64 | + | ||
| 65 | + | c.popupMu.RLock() | |
| 66 | + | _, exists := c.popupOAuth[popupID] | |
| 67 | + | refreshTimer := c.popupRefresh[parentPaneID] | |
| 68 | + | c.popupMu.RUnlock() | |
| 69 | + | ||
| 70 | + | assert.False(t, exists, "oauth state should be removed after close handling") | |
| 71 | + | assert.NotNil(t, refreshTimer, "successful oauth close should schedule parent refresh") | |
| 72 | + | ||
| 73 | + | waitFor(t, time.Second, func() bool { | |
| 74 | + | c.popupMu.RLock() | |
| 75 | + | defer c.popupMu.RUnlock() | |
| 76 | + | return c.popupRefresh[parentPaneID] == nil | |
| 77 | + | }) | |
| 78 | + | } | |
| 79 | + | ||
| 80 | + | func TestHandlePopupOAuthClose_ErrorDoesNotScheduleParentRefresh(t *testing.T) { | |
| 81 | + | parentPaneID := entity.PaneID("parent-pane") | |
| 82 | + | popupID := port.WebViewID(102) | |
| 83 | + | ||
| 84 | + | c := &ContentCoordinator{ | |
| 85 | + | webViews: make(map[entity.PaneID]*webkit.WebView), | |
| 86 | + | popupOAuth: make(map[port.WebViewID]*popupOAuthState), | |
| 87 | + | popupRefresh: make(map[entity.PaneID]*time.Timer), | |
| 88 | + | } | |
| 89 | + | ||
| 90 | + | c.trackOAuthPopup(popupID, parentPaneID) | |
| 91 | + | c.capturePopupOAuthState(popupID, "https://app/callback?error=access_denied") | |
| 92 | + | c.handlePopupOAuthClose(context.Background(), popupID) | |
| 93 | + | ||
| 94 | + | c.popupMu.RLock() | |
| 95 | + | defer c.popupMu.RUnlock() | |
| 96 | + | assert.Nil(t, c.popupRefresh[parentPaneID], "oauth errors should not refresh parent pane") | |
| 97 | + | } | |
| 98 | + | ||
| 99 | + | func waitFor(t *testing.T, timeout time.Duration, condition func() bool) { | |
| 100 | + | t.Helper() | |
| 101 | + | ||
| 102 | + | deadline := time.Now().Add(timeout) | |
| 103 | + | for time.Now().Before(deadline) { | |
| 104 | + | if condition() { | |
| 105 | + | return | |
| 106 | + | } | |
| 107 | + | time.Sleep(20 * time.Millisecond) | |
| 108 | + | } | |
| 109 | + | assert.True(t, condition(), "condition not met before timeout") | |
| 110 | + | } | |
| @@ -0,0 +1,110 @@ | |||||
| 1 | + | package coordinator | |||
| 2 | + | ||||
| 3 | + | import ( | |||
| 4 | + | "context" | |||
| 5 | + | "testing" | |||
| 6 | + | "time" | |||
| 7 | + | ||||
| 8 | + | "github.com/stretchr/testify/assert" | |||
| 9 | + | ||||
| 10 | + | "github.com/bnema/dumber/internal/application/port" | |||
| 11 | + | "github.com/bnema/dumber/internal/domain/entity" | |||
| 12 | + | "github.com/bnema/dumber/internal/infrastructure/webkit" | |||
| 13 | + | ) | |||
| 14 | + | ||||
| 15 | + | func TestComposeOnClose_Order(t *testing.T) { | |||
| 16 | + | var calls []string | |||
| 17 | + | ||||
| 18 | + | composed := composeOnClose( | |||
| 19 | + | func() { calls = append(calls, "existing") }, | |||
| 20 | + | func() { calls = append(calls, "next") }, | |||
| 21 | + | ) | |||
| 22 | + | composed() | |||
| 23 | + | ||||
| 24 | + | assert.Equal(t, []string{"existing", "next"}, calls) | |||
| 25 | + | } | |||
| 26 | + | ||||
| 27 | + | func TestComposeOnURIChanged_Order(t *testing.T) { | |||
| 28 | + | var calls []string | |||
| 29 | + | ||||
| 30 | + | composed := composeOnURIChanged( | |||
| 31 | + | func(_ string) { calls = append(calls, "existing") }, | |||
| 32 | + | func(_ string) { calls = append(calls, "next") }, | |||
| 33 | + | ) | |||
| 34 | + | composed("https://example.com") | |||
| 35 | + | ||||
| 36 | + | assert.Equal(t, []string{"existing", "next"}, calls) | |||
| 37 | + | } | |||
| 38 | + | ||||
| 39 | + | func TestComposeOnLoadChanged_Order(t *testing.T) { | |||
| 40 | + | var calls []string | |||
| 41 | + | ||||
| 42 | + | composed := composeOnLoadChanged( | |||
| 43 | + | func(_ webkit.LoadEvent) { calls = append(calls, "existing") }, | |||
| 44 | + | func(_ webkit.LoadEvent) { calls = append(calls, "next") }, | |||
| 45 | + | ) | |||
| 46 | + | composed(webkit.LoadCommitted) | |||
| 47 | + | ||||
| 48 | + | assert.Equal(t, []string{"existing", "next"}, calls) | |||
| 49 | + | } | |||
| 50 | + | ||||
| 51 | + | func TestHandlePopupOAuthClose_SuccessSchedulesParentRefresh(t *testing.T) { | |||
| 52 | + | parentPaneID := entity.PaneID("parent-pane") | |||
| 53 | + | popupID := port.WebViewID(101) | |||
| 54 | + | ||||
| 55 | + | c := &ContentCoordinator{ | |||
| 56 | + | webViews: make(map[entity.PaneID]*webkit.WebView), | |||
| 57 | + | popupOAuth: make(map[port.WebViewID]*popupOAuthState), | |||
| 58 | + | popupRefresh: make(map[entity.PaneID]*time.Timer), | |||
| 59 | + | } | |||
| 60 | + | ||||
| 61 | + | c.trackOAuthPopup(popupID, parentPaneID) | |||
| 62 | + | c.capturePopupOAuthState(popupID, "https://app/callback?code=123") | |||
| 63 | + | c.handlePopupOAuthClose(context.Background(), popupID) | |||
| 64 | + | ||||
| 65 | + | c.popupMu.RLock() | |||
| 66 | + | _, exists := c.popupOAuth[popupID] | |||
| 67 | + | refreshTimer := c.popupRefresh[parentPaneID] | |||
| 68 | + | c.popupMu.RUnlock() | |||
| 69 | + | ||||
| 70 | + | assert.False(t, exists, "oauth state should be removed after close handling") | |||
| 71 | + | assert.NotNil(t, refreshTimer, "successful oauth close should schedule parent refresh") | |||
| 72 | + | ||||
| 73 | + | waitFor(t, time.Second, func() bool { | |||
| 74 | + | c.popupMu.RLock() | |||
| 75 | + | defer c.popupMu.RUnlock() | |||
| 76 | + | return c.popupRefresh[parentPaneID] == nil | |||
| 77 | + | }) | |||
| 78 | + | } | |||
| 79 | + | ||||
| 80 | + | func TestHandlePopupOAuthClose_ErrorDoesNotScheduleParentRefresh(t *testing.T) { | |||
| 81 | + | parentPaneID := entity.PaneID("parent-pane") | |||
| 82 | + | popupID := port.WebViewID(102) | |||
| 83 | + | ||||
| 84 | + | c := &ContentCoordinator{ | |||
| 85 | + | webViews: make(map[entity.PaneID]*webkit.WebView), | |||
| 86 | + | popupOAuth: make(map[port.WebViewID]*popupOAuthState), | |||
| 87 | + | popupRefresh: make(map[entity.PaneID]*time.Timer), | |||
| 88 | + | } | |||
| 89 | + | ||||
| 90 | + | c.trackOAuthPopup(popupID, parentPaneID) | |||
| 91 | + | c.capturePopupOAuthState(popupID, "https://app/callback?error=access_denied") | |||
| 92 | + | c.handlePopupOAuthClose(context.Background(), popupID) | |||
| 93 | + | ||||
| 94 | + | c.popupMu.RLock() | |||
| 95 | + | defer c.popupMu.RUnlock() | |||
| 96 | + | assert.Nil(t, c.popupRefresh[parentPaneID], "oauth errors should not refresh parent pane") | |||
| 97 | + | } | |||
| 98 | + | ||||
| 99 | + | func waitFor(t *testing.T, timeout time.Duration, condition func() bool) { | |||
| 100 | + | t.Helper() | |||
| 101 | + | ||||
| 102 | + | deadline := time.Now().Add(timeout) | |||
| 103 | + | for time.Now().Before(deadline) { | |||
| 104 | + | if condition() { | |||
| 105 | + | return | |||
| 106 | + | } | |||
| 107 | + | time.Sleep(20 * time.Millisecond) | |||
| 108 | + | } | |||
| 109 | + | assert.True(t, condition(), "condition not met before timeout") | |||
| 110 | + | } | |||