perf(cef): coalesce favicon discovery and script injection (#432)
* perf(cef): coalesce favicon discovery and script injection * test(omnibox): isolate asynchronous suggestion updates * test(omnibox): avoid native widget for paste replay state * test(omnibox): keep navigation state tests headless * fix(cef): keep content injection and favicons on the committed document Pin load-end content scripts to the document sequence validated on the GTK thread. A replacement commit before the scripts reach the CEF UI thread now drops them instead of installing them into the new document. Revalidate document sequence, page URL, browser identity, and callback owner inside the queued favicon GTK callbacks before emitting candidates. Record the document whose favicon source scan completed so a repeated loading-finished notification does not repeat a full-document transfer, and reset it when a new document commits. * refactor(ui): replace omnibox test seams with owned dependencies Extract the ghost-echo predicate so the headless test exercises production logic, and move suggestion scheduling onto the Omnibox itself. Tests inject a synchronous scheduler before starting the history worker instead of swapping a package global that the worker could still read after cleanup.
- commit
- 06172784d31298059666474c33305f27662200a3
- author
- bnema · · 2026-09-12 12:43 UTC
- committer
- GitHub · · 2026-09-12 12:43 UTC
- parent
- b7b10b9
- signature
- GPG Unverified
B5690EEEBB952194The key is unknown - refs
- tag: v0.32.0
8 files · +355 -156
M internal/infrastructure/cef/content_injector.go +37 -29
| @@ -3,6 +3,7 @@ package cef | |||
| 3 | 3 | import ( | |
| 4 | 4 | "context" | |
| 5 | 5 | "fmt" | |
| 6 | + | "strings" | |
| 6 | 7 | "sync" | |
| 7 | 8 | ||
| 8 | 9 | "github.com/bnema/dumber/internal/infrastructure/webutil" | |
| @@ -206,7 +207,7 @@ func (ci *contentInjector) InjectThemeCSS(ctx context.Context, css string) error | |||
| 206 | 207 | // Broadcast to all active webviews. | |
| 207 | 208 | ci.engine.activeWebViews.Range(func(_, value any) bool { | |
| 208 | 209 | if wv, ok := value.(*WebView); ok { | |
| 209 | - | ci.injectCSS(wv, "dumber-theme-vars", css) | |
| 210 | + | ci.injectCSS(wv, 0, "dumber-theme-vars", css) | |
| 210 | 211 | } | |
| 211 | 212 | return true | |
| 212 | 213 | }) | |
| @@ -225,7 +226,7 @@ func (ci *contentInjector) InjectFindHighlightCSS(ctx context.Context, css strin | |||
| 225 | 226 | ||
| 226 | 227 | ci.engine.activeWebViews.Range(func(_, value any) bool { | |
| 227 | 228 | if wv, ok := value.(*WebView); ok { | |
| 228 | - | ci.injectCSS(wv, "dumber-find-highlight", css) | |
| 229 | + | ci.injectCSS(wv, 0, "dumber-find-highlight", css) | |
| 229 | 230 | } | |
| 230 | 231 | return true | |
| 231 | 232 | }) | |
| @@ -245,7 +246,7 @@ func (ci *contentInjector) RefreshScripts(ctx context.Context, wv port.WebView) | |||
| 245 | 246 | return nil | |
| 246 | 247 | } | |
| 247 | 248 | ||
| 248 | - | ci.onLoadEnd(cefWV) | |
| 249 | + | ci.onLoadEnd(cefWV, 0) | |
| 249 | 250 | return nil | |
| 250 | 251 | } | |
| 251 | 252 | ||
| @@ -257,31 +258,36 @@ func (ci *contentInjector) RefreshScripts(ctx context.Context, wv port.WebView) | |||
| 257 | 258 | // committed while the callback was queued). Ambiguous cases fall back to skipping rather than | |
| 258 | 259 | // installing blind: an event captured without a browser cannot prove | |
| 259 | 260 | // identity, and the current browser gets its own load-end installation. | |
| 260 | - | // The single snapshot bounds the residual check-then-install window to | |
| 261 | - | // nanoseconds, and any race there can only repeat idempotent scripts. | |
| 262 | - | // Repeated same-document load-ends still install. | |
| 261 | + | // The installation is pinned to the validated document sequence, so a | |
| 262 | + | // replacement commit between validation and the CEF UI thread drops the | |
| 263 | + | // scripts instead of injecting them into the new document. Repeated | |
| 264 | + | // notifications for the same committed document are coalesced. | |
| 263 | 265 | func (ci *contentInjector) onLoadEndForEvent(wv *WebView, event injectionEvent) { | |
| 264 | 266 | if wv == nil || ci == nil || event.browserID == noBrowserID { | |
| 265 | 267 | return | |
| 266 | 268 | } | |
| 267 | 269 | wv.mu.RLock() | |
| 268 | - | browser, currentIntent := wv.browser, wv.pendingIntentID | |
| 270 | + | browser := wv.browser | |
| 269 | 271 | wv.mu.RUnlock() | |
| 270 | - | if browser == nil { | |
| 272 | + | if browser == nil || browser.GetIdentifier() != event.browserID { | |
| 271 | 273 | return | |
| 272 | 274 | } | |
| 273 | - | if browser.GetIdentifier() != event.browserID { | |
| 275 | + | wv.mu.Lock() | |
| 276 | + | if wv.browser != browser || wv.pendingIntentID != event.intentID || event.documentSeq == 0 || | |
| 277 | + | wv.documentSeq != event.documentSeq || strings.TrimSpace(wv.uri) != event.frameURL || | |
| 278 | + | wv.installedDocumentSeq == event.documentSeq { | |
| 279 | + | wv.mu.Unlock() | |
| 274 | 280 | return | |
| 275 | 281 | } | |
| 276 | - | if currentIntent != event.intentID { | |
| 277 | - | return | |
| 278 | - | } | |
| 279 | - | ci.onLoadEnd(wv) | |
| 282 | + | wv.installedDocumentSeq = event.documentSeq | |
| 283 | + | wv.mu.Unlock() | |
| 284 | + | ci.onLoadEnd(wv, event.documentSeq) | |
| 280 | 285 | } | |
| 281 | 286 | ||
| 282 | 287 | // onLoadEnd is called from the load handler after a page finishes loading. | |
| 283 | 288 | // It injects the appropriate scripts based on whether the page is internal. | |
| 284 | - | func (ci *contentInjector) onLoadEnd(wv *WebView) { | |
| 289 | + | // A documentSeq of 0 installs unconditionally (manual refresh paths). | |
| 290 | + | func (ci *contentInjector) onLoadEnd(wv *WebView, documentSeq uint64) { | |
| 285 | 291 | uri := wv.URI() | |
| 286 | 292 | isInternal := isConceptualInternalURL(uri) || isActualInternalURL(uri) | |
| 287 | 293 | ||
| @@ -296,21 +302,21 @@ func (ci *contentInjector) onLoadEnd(wv *WebView) { | |||
| 296 | 302 | if ci.colorResolver != nil { | |
| 297 | 303 | prefersDark = ci.colorResolver.Resolve().PrefersDark | |
| 298 | 304 | } | |
| 299 | - | ci.injectDarkModeScript(wv, prefersDark) | |
| 300 | - | ci.injectMessageBridgeShim(wv) | |
| 305 | + | ci.injectDarkModeScript(wv, documentSeq, prefersDark) | |
| 306 | + | ci.injectMessageBridgeShim(wv, documentSeq) | |
| 301 | 307 | if themeCSS != "" { | |
| 302 | - | ci.injectCSS(wv, "dumber-theme-vars", themeCSS) | |
| 308 | + | ci.injectCSS(wv, documentSeq, "dumber-theme-vars", themeCSS) | |
| 303 | 309 | } | |
| 304 | 310 | } | |
| 305 | 311 | ||
| 306 | 312 | // All pages get find highlight CSS if set. | |
| 307 | 313 | if findCSS != "" { | |
| 308 | - | ci.injectCSS(wv, "dumber-find-highlight", findCSS) | |
| 314 | + | ci.injectCSS(wv, documentSeq, "dumber-find-highlight", findCSS) | |
| 309 | 315 | } | |
| 310 | 316 | ||
| 311 | 317 | // All pages get custom scrollbar styling with auto-hide. | |
| 312 | - | ci.injectCSS(wv, "dumber-scrollbar", scrollbarCSS) | |
| 313 | - | wv.RunJavaScript(context.Background(), scrollbarAutoHideJS) | |
| 318 | + | ci.injectCSS(wv, documentSeq, "dumber-scrollbar", scrollbarCSS) | |
| 319 | + | wv.RunJavaScriptForDocument(documentSeq, scrollbarAutoHideJS) | |
| 314 | 320 | ||
| 315 | 321 | // Clipboard copy/cut, editable focus sync, and synthetic popup proxies still | |
| 316 | 322 | // need a JS bridge in OSR mode while the native renderer bridge remains disabled. | |
| @@ -322,18 +328,20 @@ func (ci *contentInjector) onLoadEnd(wv *WebView) { | |||
| 322 | 328 | } | |
| 323 | 329 | logging.FromContext(ctx).Warn().Msg("cef: skipped clipboard bridge injection — nonce generation failed") | |
| 324 | 330 | } else { | |
| 325 | - | wv.RunJavaScript(context.Background(), buildTrustedPageFetchBridgeJS(bridgeNonce)) | |
| 331 | + | wv.RunJavaScriptForDocument(documentSeq, buildTrustedPageFetchBridgeJS(bridgeNonce)) | |
| 326 | 332 | } | |
| 327 | 333 | ||
| 328 | 334 | // Video playback diagnostic — logs video element state changes. | |
| 329 | 335 | // Gated behind DUMBER_VIDEO_DIAG=1 environment variable. | |
| 330 | 336 | if ci.videoDiagnosticsEnabled { | |
| 331 | - | wv.RunJavaScript(context.Background(), videoDiagnosticJS) | |
| 337 | + | wv.RunJavaScriptForDocument(documentSeq, videoDiagnosticJS) | |
| 332 | 338 | } | |
| 333 | 339 | } | |
| 334 | 340 | ||
| 335 | - | // injectCSS injects a CSS string as a <style> element via JavaScript. | |
| 336 | - | func (ci *contentInjector) injectCSS(wv *WebView, id, css string) { | |
| 341 | + | // injectCSS injects a CSS string as a <style> element via JavaScript. The | |
| 342 | + | // style is pinned to documentSeq when the caller knows the committed document | |
| 343 | + | // sequence; 0 installs unconditionally. | |
| 344 | + | func (ci *contentInjector) injectCSS(wv *WebView, documentSeq uint64, id, css string) { | |
| 337 | 345 | escapedID := webutil.EscapeForJSString(id) | |
| 338 | 346 | escaped := webutil.EscapeForJSString(css) | |
| 339 | 347 | script := fmt.Sprintf(`(function(){ | |
| @@ -342,19 +350,19 @@ func (ci *contentInjector) injectCSS(wv *WebView, id, css string) { | |||
| 342 | 350 | el.textContent = '%s'; | |
| 343 | 351 | })();`, escapedID, escapedID, escaped) | |
| 344 | 352 | ||
| 345 | - | wv.RunJavaScript(context.Background(), script) | |
| 353 | + | wv.RunJavaScriptForDocument(documentSeq, script) | |
| 346 | 354 | } | |
| 347 | 355 | ||
| 348 | 356 | // injectDarkModeScript sets dark/light class on <html> and patches matchMedia | |
| 349 | 357 | // for prefers-color-scheme queries on internal pages. | |
| 350 | - | func (ci *contentInjector) injectDarkModeScript(wv *WebView, prefersDark bool) { | |
| 351 | - | wv.RunJavaScript(context.Background(), webutil.DarkModeScript(prefersDark, "__dumber_cef_prefers_dark")) | |
| 358 | + | func (ci *contentInjector) injectDarkModeScript(wv *WebView, documentSeq uint64, prefersDark bool) { | |
| 359 | + | wv.RunJavaScriptForDocument(documentSeq, webutil.DarkModeScript(prefersDark, "__dumber_cef_prefers_dark")) | |
| 352 | 360 | } | |
| 353 | 361 | ||
| 354 | 362 | // injectMessageBridgeShim injects the window.dumber.postMessage JS client shim | |
| 355 | 363 | // so internal pages can communicate with Go handlers via fetch. | |
| 356 | 364 | // The message body is base64-encoded into the X-Dumber-Body header to work | |
| 357 | 365 | // around purego-cef's unexported PostData element wrapper. | |
| 358 | - | func (ci *contentInjector) injectMessageBridgeShim(wv *WebView) { | |
| 359 | - | wv.RunJavaScript(context.Background(), MessageBridgeJS) | |
| 366 | + | func (ci *contentInjector) injectMessageBridgeShim(wv *WebView, documentSeq uint64) { | |
| 367 | + | wv.RunJavaScriptForDocument(documentSeq, MessageBridgeJS) | |
| 360 | 368 | } | |
| @@ -3,6 +3,7 @@ package cef | |||||
| 3 | import ( | 3 | import ( | ||
| 4 | "context" | 4 | "context" | ||
| 5 | "fmt" | 5 | "fmt" | ||
| 6 | + | "strings" | |||
| 6 | "sync" | 7 | "sync" | ||
| 7 | 8 | ||||
| 8 | "github.com/bnema/dumber/internal/infrastructure/webutil" | 9 | "github.com/bnema/dumber/internal/infrastructure/webutil" | ||
| @@ -206,7 +207,7 @@ func (ci *contentInjector) InjectThemeCSS(ctx context.Context, css string) error | |||||
| 206 | // Broadcast to all active webviews. | 207 | // Broadcast to all active webviews. | ||
| 207 | ci.engine.activeWebViews.Range(func(_, value any) bool { | 208 | ci.engine.activeWebViews.Range(func(_, value any) bool { | ||
| 208 | if wv, ok := value.(*WebView); ok { | 209 | if wv, ok := value.(*WebView); ok { | ||
| 209 | - | ci.injectCSS(wv, "dumber-theme-vars", css) | 210 | + | ci.injectCSS(wv, 0, "dumber-theme-vars", css) |
| 210 | } | 211 | } | ||
| 211 | return true | 212 | return true | ||
| 212 | }) | 213 | }) | ||
| @@ -225,7 +226,7 @@ func (ci *contentInjector) InjectFindHighlightCSS(ctx context.Context, css strin | |||||
| 225 | 226 | ||||
| 226 | ci.engine.activeWebViews.Range(func(_, value any) bool { | 227 | ci.engine.activeWebViews.Range(func(_, value any) bool { | ||
| 227 | if wv, ok := value.(*WebView); ok { | 228 | if wv, ok := value.(*WebView); ok { | ||
| 228 | - | ci.injectCSS(wv, "dumber-find-highlight", css) | 229 | + | ci.injectCSS(wv, 0, "dumber-find-highlight", css) |
| 229 | } | 230 | } | ||
| 230 | return true | 231 | return true | ||
| 231 | }) | 232 | }) | ||
| @@ -245,7 +246,7 @@ func (ci *contentInjector) RefreshScripts(ctx context.Context, wv port.WebView) | |||||
| 245 | return nil | 246 | return nil | ||
| 246 | } | 247 | } | ||
| 247 | 248 | ||||
| 248 | - | ci.onLoadEnd(cefWV) | 249 | + | ci.onLoadEnd(cefWV, 0) |
| 249 | return nil | 250 | return nil | ||
| 250 | } | 251 | } | ||
| 251 | 252 | ||||
| @@ -257,31 +258,36 @@ func (ci *contentInjector) RefreshScripts(ctx context.Context, wv port.WebView) | |||||
| 257 | // committed while the callback was queued). Ambiguous cases fall back to skipping rather than | 258 | // committed while the callback was queued). Ambiguous cases fall back to skipping rather than | ||
| 258 | // installing blind: an event captured without a browser cannot prove | 259 | // installing blind: an event captured without a browser cannot prove | ||
| 259 | // identity, and the current browser gets its own load-end installation. | 260 | // identity, and the current browser gets its own load-end installation. | ||
| 260 | - | // The single snapshot bounds the residual check-then-install window to | 261 | + | // The installation is pinned to the validated document sequence, so a |
| 261 | - | // nanoseconds, and any race there can only repeat idempotent scripts. | 262 | + | // replacement commit between validation and the CEF UI thread drops the |
| 262 | - | // Repeated same-document load-ends still install. | 263 | + | // scripts instead of injecting them into the new document. Repeated |
| 264 | + | // notifications for the same committed document are coalesced. | |||
| 263 | func (ci *contentInjector) onLoadEndForEvent(wv *WebView, event injectionEvent) { | 265 | func (ci *contentInjector) onLoadEndForEvent(wv *WebView, event injectionEvent) { | ||
| 264 | if wv == nil || ci == nil || event.browserID == noBrowserID { | 266 | if wv == nil || ci == nil || event.browserID == noBrowserID { | ||
| 265 | return | 267 | return | ||
| 266 | } | 268 | } | ||
| 267 | wv.mu.RLock() | 269 | wv.mu.RLock() | ||
| 268 | - | browser, currentIntent := wv.browser, wv.pendingIntentID | 270 | + | browser := wv.browser |
| 269 | wv.mu.RUnlock() | 271 | wv.mu.RUnlock() | ||
| 270 | - | if browser == nil { | 272 | + | if browser == nil || browser.GetIdentifier() != event.browserID { |
| 271 | return | 273 | return | ||
| 272 | } | 274 | } | ||
| 273 | - | if browser.GetIdentifier() != event.browserID { | 275 | + | wv.mu.Lock() |
| 276 | + | if wv.browser != browser || wv.pendingIntentID != event.intentID || event.documentSeq == 0 || | |||
| 277 | + | wv.documentSeq != event.documentSeq || strings.TrimSpace(wv.uri) != event.frameURL || | |||
| 278 | + | wv.installedDocumentSeq == event.documentSeq { | |||
| 279 | + | wv.mu.Unlock() | |||
| 274 | return | 280 | return | ||
| 275 | } | 281 | } | ||
| 276 | - | if currentIntent != event.intentID { | 282 | + | wv.installedDocumentSeq = event.documentSeq |
| 277 | - | return | 283 | + | wv.mu.Unlock() |
| 278 | - | } | 284 | + | ci.onLoadEnd(wv, event.documentSeq) |
| 279 | - | ci.onLoadEnd(wv) | |||
| 280 | } | 285 | } | ||
| 281 | 286 | ||||
| 282 | // onLoadEnd is called from the load handler after a page finishes loading. | 287 | // onLoadEnd is called from the load handler after a page finishes loading. | ||
| 283 | // It injects the appropriate scripts based on whether the page is internal. | 288 | // It injects the appropriate scripts based on whether the page is internal. | ||
| 284 | - | func (ci *contentInjector) onLoadEnd(wv *WebView) { | 289 | + | // A documentSeq of 0 installs unconditionally (manual refresh paths). |
| 290 | + | func (ci *contentInjector) onLoadEnd(wv *WebView, documentSeq uint64) { | |||
| 285 | uri := wv.URI() | 291 | uri := wv.URI() | ||
| 286 | isInternal := isConceptualInternalURL(uri) || isActualInternalURL(uri) | 292 | isInternal := isConceptualInternalURL(uri) || isActualInternalURL(uri) | ||
| 287 | 293 | ||||
| @@ -296,21 +302,21 @@ func (ci *contentInjector) onLoadEnd(wv *WebView) { | |||||
| 296 | if ci.colorResolver != nil { | 302 | if ci.colorResolver != nil { | ||
| 297 | prefersDark = ci.colorResolver.Resolve().PrefersDark | 303 | prefersDark = ci.colorResolver.Resolve().PrefersDark | ||
| 298 | } | 304 | } | ||
| 299 | - | ci.injectDarkModeScript(wv, prefersDark) | 305 | + | ci.injectDarkModeScript(wv, documentSeq, prefersDark) |
| 300 | - | ci.injectMessageBridgeShim(wv) | 306 | + | ci.injectMessageBridgeShim(wv, documentSeq) |
| 301 | if themeCSS != "" { | 307 | if themeCSS != "" { | ||
| 302 | - | ci.injectCSS(wv, "dumber-theme-vars", themeCSS) | 308 | + | ci.injectCSS(wv, documentSeq, "dumber-theme-vars", themeCSS) |
| 303 | } | 309 | } | ||
| 304 | } | 310 | } | ||
| 305 | 311 | ||||
| 306 | // All pages get find highlight CSS if set. | 312 | // All pages get find highlight CSS if set. | ||
| 307 | if findCSS != "" { | 313 | if findCSS != "" { | ||
| 308 | - | ci.injectCSS(wv, "dumber-find-highlight", findCSS) | 314 | + | ci.injectCSS(wv, documentSeq, "dumber-find-highlight", findCSS) |
| 309 | } | 315 | } | ||
| 310 | 316 | ||||
| 311 | // All pages get custom scrollbar styling with auto-hide. | 317 | // All pages get custom scrollbar styling with auto-hide. | ||
| 312 | - | ci.injectCSS(wv, "dumber-scrollbar", scrollbarCSS) | 318 | + | ci.injectCSS(wv, documentSeq, "dumber-scrollbar", scrollbarCSS) |
| 313 | - | wv.RunJavaScript(context.Background(), scrollbarAutoHideJS) | 319 | + | wv.RunJavaScriptForDocument(documentSeq, scrollbarAutoHideJS) |
| 314 | 320 | ||||
| 315 | // Clipboard copy/cut, editable focus sync, and synthetic popup proxies still | 321 | // Clipboard copy/cut, editable focus sync, and synthetic popup proxies still | ||
| 316 | // need a JS bridge in OSR mode while the native renderer bridge remains disabled. | 322 | // need a JS bridge in OSR mode while the native renderer bridge remains disabled. | ||
| @@ -322,18 +328,20 @@ func (ci *contentInjector) onLoadEnd(wv *WebView) { | |||||
| 322 | } | 328 | } | ||
| 323 | logging.FromContext(ctx).Warn().Msg("cef: skipped clipboard bridge injection — nonce generation failed") | 329 | logging.FromContext(ctx).Warn().Msg("cef: skipped clipboard bridge injection — nonce generation failed") | ||
| 324 | } else { | 330 | } else { | ||
| 325 | - | wv.RunJavaScript(context.Background(), buildTrustedPageFetchBridgeJS(bridgeNonce)) | 331 | + | wv.RunJavaScriptForDocument(documentSeq, buildTrustedPageFetchBridgeJS(bridgeNonce)) |
| 326 | } | 332 | } | ||
| 327 | 333 | ||||
| 328 | // Video playback diagnostic — logs video element state changes. | 334 | // Video playback diagnostic — logs video element state changes. | ||
| 329 | // Gated behind DUMBER_VIDEO_DIAG=1 environment variable. | 335 | // Gated behind DUMBER_VIDEO_DIAG=1 environment variable. | ||
| 330 | if ci.videoDiagnosticsEnabled { | 336 | if ci.videoDiagnosticsEnabled { | ||
| 331 | - | wv.RunJavaScript(context.Background(), videoDiagnosticJS) | 337 | + | wv.RunJavaScriptForDocument(documentSeq, videoDiagnosticJS) |
| 332 | } | 338 | } | ||
| 333 | } | 339 | } | ||
| 334 | 340 | ||||
| 335 | - | // injectCSS injects a CSS string as a <style> element via JavaScript. | 341 | + | // injectCSS injects a CSS string as a <style> element via JavaScript. The |
| 336 | - | func (ci *contentInjector) injectCSS(wv *WebView, id, css string) { | 342 | + | // style is pinned to documentSeq when the caller knows the committed document |
| 343 | + | // sequence; 0 installs unconditionally. | |||
| 344 | + | func (ci *contentInjector) injectCSS(wv *WebView, documentSeq uint64, id, css string) { | |||
| 337 | escapedID := webutil.EscapeForJSString(id) | 345 | escapedID := webutil.EscapeForJSString(id) | ||
| 338 | escaped := webutil.EscapeForJSString(css) | 346 | escaped := webutil.EscapeForJSString(css) | ||
| 339 | script := fmt.Sprintf(`(function(){ | 347 | script := fmt.Sprintf(`(function(){ | ||
| @@ -342,19 +350,19 @@ func (ci *contentInjector) injectCSS(wv *WebView, id, css string) { | |||||
| 342 | el.textContent = '%s'; | 350 | el.textContent = '%s'; | ||
| 343 | })();`, escapedID, escapedID, escaped) | 351 | })();`, escapedID, escapedID, escaped) | ||
| 344 | 352 | ||||
| 345 | - | wv.RunJavaScript(context.Background(), script) | 353 | + | wv.RunJavaScriptForDocument(documentSeq, script) |
| 346 | } | 354 | } | ||
| 347 | 355 | ||||
| 348 | // injectDarkModeScript sets dark/light class on <html> and patches matchMedia | 356 | // injectDarkModeScript sets dark/light class on <html> and patches matchMedia | ||
| 349 | // for prefers-color-scheme queries on internal pages. | 357 | // for prefers-color-scheme queries on internal pages. | ||
| 350 | - | func (ci *contentInjector) injectDarkModeScript(wv *WebView, prefersDark bool) { | 358 | + | func (ci *contentInjector) injectDarkModeScript(wv *WebView, documentSeq uint64, prefersDark bool) { |
| 351 | - | wv.RunJavaScript(context.Background(), webutil.DarkModeScript(prefersDark, "__dumber_cef_prefers_dark")) | 359 | + | wv.RunJavaScriptForDocument(documentSeq, webutil.DarkModeScript(prefersDark, "__dumber_cef_prefers_dark")) |
| 352 | } | 360 | } | ||
| 353 | 361 | ||||
| 354 | // injectMessageBridgeShim injects the window.dumber.postMessage JS client shim | 362 | // injectMessageBridgeShim injects the window.dumber.postMessage JS client shim | ||
| 355 | // so internal pages can communicate with Go handlers via fetch. | 363 | // so internal pages can communicate with Go handlers via fetch. | ||
| 356 | // The message body is base64-encoded into the X-Dumber-Body header to work | 364 | // The message body is base64-encoded into the X-Dumber-Body header to work | ||
| 357 | // around purego-cef's unexported PostData element wrapper. | 365 | // around purego-cef's unexported PostData element wrapper. | ||
| 358 | - | func (ci *contentInjector) injectMessageBridgeShim(wv *WebView) { | 366 | + | func (ci *contentInjector) injectMessageBridgeShim(wv *WebView, documentSeq uint64) { |
| 359 | - | wv.RunJavaScript(context.Background(), MessageBridgeJS) | 367 | + | wv.RunJavaScriptForDocument(documentSeq, MessageBridgeJS) |
| 360 | } | 368 | } | ||
M internal/infrastructure/cef/favicon_discovery_test.go +70 -4
| @@ -60,11 +60,13 @@ func TestCEFOnFaviconUrlchangeForwardsOrderedCandidates(t *testing.T) { | |||
| 60 | 60 | ||
| 61 | 61 | var page string | |
| 62 | 62 | var icons []string | |
| 63 | - | wv := &WebView{ctx: context.Background(), uri: "https://example.com/docs/page", callbacks: &port.WebViewCallbacks{ | |
| 63 | + | browser := cefmocks.NewMockBrowser(t) | |
| 64 | + | browser.EXPECT().GetIdentifier().Return(int32(7)).Maybe() | |
| 65 | + | wv := &WebView{ctx: context.Background(), uri: "https://example.com/docs/page", browser: browser, documentSeq: 1, callbacks: &port.WebViewCallbacks{ | |
| 64 | 66 | OnFaviconURLChanged: func(p string, urls []string) { page, icons = p, urls }, | |
| 65 | 67 | }} | |
| 66 | 68 | ||
| 67 | - | (&handlerSet{wv: wv}).OnFaviconUrlchange(nil, purecef.StringList(42)) | |
| 69 | + | (&handlerSet{wv: wv}).OnFaviconUrlchange(browser, purecef.StringList(42)) | |
| 68 | 70 | ||
| 69 | 71 | require.Equal(t, "https://example.com/docs/page", page) | |
| 70 | 72 | require.Equal(t, []string{"https://example.com/favicon.png", "https://cdn.example.com/icon.ico"}, icons) | |
| @@ -77,6 +79,7 @@ func TestCEFLoadingFinishedDiscoversMetadataBeforeFallback(t *testing.T) { | |||
| 77 | 79 | ||
| 78 | 80 | browser := cefmocks.NewMockBrowser(t) | |
| 79 | 81 | frame := cefmocks.NewMockFrame(t) | |
| 82 | + | browser.EXPECT().GetIdentifier().Return(int32(7)).Maybe() | |
| 80 | 83 | browser.EXPECT().GetMainFrame().Return(frame) | |
| 81 | 84 | frame.EXPECT().GetSource(mock.Anything).Run(func(visitor purecef.StringVisitor) { | |
| 82 | 85 | visitor.Visit(`<html><head><link rel="icon" href="/meta.png"></head></html>`) | |
| @@ -84,7 +87,7 @@ func TestCEFLoadingFinishedDiscoversMetadataBeforeFallback(t *testing.T) { | |||
| 84 | 87 | ||
| 85 | 88 | var page string | |
| 86 | 89 | var icons []string | |
| 87 | - | wv := &WebView{ctx: context.Background(), uri: "https://example.com/docs/page", callbacks: &port.WebViewCallbacks{ | |
| 90 | + | wv := &WebView{ctx: context.Background(), uri: "https://example.com/docs/page", browser: browser, documentSeq: 1, callbacks: &port.WebViewCallbacks{ | |
| 88 | 91 | OnFaviconURLChanged: func(p string, urls []string) { page, icons = p, urls }, | |
| 89 | 92 | }} | |
| 90 | 93 | ||
| @@ -97,7 +100,7 @@ func TestCEFLoadingFinishedDiscoversMetadataBeforeFallback(t *testing.T) { | |||
| 97 | 100 | func TestCEFLoadingFinishedForwardsFallbackCandidate(t *testing.T) { | |
| 98 | 101 | var page string | |
| 99 | 102 | var icons []string | |
| 100 | - | wv := &WebView{ctx: context.Background(), uri: "https://example.com/docs/page", callbacks: &port.WebViewCallbacks{ | |
| 103 | + | wv := &WebView{ctx: context.Background(), uri: "https://example.com/docs/page", documentSeq: 1, callbacks: &port.WebViewCallbacks{ | |
| 101 | 104 | OnFaviconURLChanged: func(p string, urls []string) { page, icons = p, urls }, | |
| 102 | 105 | }} | |
| 103 | 106 | ||
| @@ -106,3 +109,66 @@ func TestCEFLoadingFinishedForwardsFallbackCandidate(t *testing.T) { | |||
| 106 | 109 | require.Equal(t, "https://example.com/docs/page", page) | |
| 107 | 110 | require.Equal(t, []string{"https://example.com/favicon.ico"}, icons) | |
| 108 | 111 | } | |
| 112 | + | ||
| 113 | + | func TestCEFLoadingFinishedSkipsSourceWhenEngineProvidedCandidates(t *testing.T) { | |
| 114 | + | prevDecode := decodeCEFStringList | |
| 115 | + | decodeCEFStringList = func(purecef.StringList) []string { return []string{"/engine.png"} } | |
| 116 | + | defer func() { decodeCEFStringList = prevDecode }() | |
| 117 | + | ||
| 118 | + | browser := cefmocks.NewMockBrowser(t) | |
| 119 | + | browser.EXPECT().GetIdentifier().Return(int32(7)).Maybe() | |
| 120 | + | calls := 0 | |
| 121 | + | wv := &WebView{ctx: context.Background(), uri: "https://example.com/page", browser: browser, documentSeq: 1, callbacks: &port.WebViewCallbacks{ | |
| 122 | + | OnFaviconURLChanged: func(string, []string) { calls++ }, | |
| 123 | + | }} | |
| 124 | + | h := &handlerSet{wv: wv} | |
| 125 | + | h.OnFaviconUrlchange(browser, purecef.StringList(1)) | |
| 126 | + | h.OnLoadingStateChange(browser, 0, 0, 0) | |
| 127 | + | ||
| 128 | + | require.Equal(t, 1, calls) | |
| 129 | + | } | |
| 130 | + | ||
| 131 | + | func TestCEFLoadingFinishedCoalescesPendingSourceDiscovery(t *testing.T) { | |
| 132 | + | prevNewStringVisitor := cefNewStringVisitor | |
| 133 | + | cefNewStringVisitor = func(visitor purecef.StringVisitor) purecef.StringVisitor { return visitor } | |
| 134 | + | defer func() { cefNewStringVisitor = prevNewStringVisitor }() | |
| 135 | + | ||
| 136 | + | browser := cefmocks.NewMockBrowser(t) | |
| 137 | + | frame := cefmocks.NewMockFrame(t) | |
| 138 | + | browser.EXPECT().GetMainFrame().Return(frame).Once() | |
| 139 | + | frame.EXPECT().GetSource(mock.Anything).Return().Once() | |
| 140 | + | wv := &WebView{ctx: context.Background(), uri: "https://example.com/page", documentSeq: 1, callbacks: &port.WebViewCallbacks{ | |
| 141 | + | OnFaviconURLChanged: func(string, []string) {}, | |
| 142 | + | }} | |
| 143 | + | h := &handlerSet{wv: wv} | |
| 144 | + | h.OnLoadingStateChange(browser, 0, 0, 0) | |
| 145 | + | h.OnLoadingStateChange(browser, 0, 0, 0) | |
| 146 | + | } | |
| 147 | + | ||
| 148 | + | func TestCEFLoadingFinishedDoesNotRepeatCompletedSourceDiscovery(t *testing.T) { | |
| 149 | + | prevNewStringVisitor := cefNewStringVisitor | |
| 150 | + | var visitor purecef.StringVisitor | |
| 151 | + | cefNewStringVisitor = func(v purecef.StringVisitor) purecef.StringVisitor { | |
| 152 | + | visitor = v | |
| 153 | + | return v | |
| 154 | + | } | |
| 155 | + | defer func() { cefNewStringVisitor = prevNewStringVisitor }() | |
| 156 | + | ||
| 157 | + | browser := cefmocks.NewMockBrowser(t) | |
| 158 | + | frame := cefmocks.NewMockFrame(t) | |
| 159 | + | browser.EXPECT().GetIdentifier().Return(int32(7)).Maybe() | |
| 160 | + | browser.EXPECT().GetMainFrame().Return(frame).Once() | |
| 161 | + | frame.EXPECT().GetSource(mock.Anything).Return().Once() | |
| 162 | + | wv := &WebView{ctx: context.Background(), uri: "https://example.com/page", documentSeq: 1, callbacks: &port.WebViewCallbacks{ | |
| 163 | + | OnFaviconURLChanged: func(string, []string) {}, | |
| 164 | + | }} | |
| 165 | + | h := &handlerSet{wv: wv} | |
| 166 | + | ||
| 167 | + | h.OnLoadingStateChange(browser, 0, 0, 0) | |
| 168 | + | require.NotNil(t, visitor, "loading-finished must start source discovery") | |
| 169 | + | visitor.Visit(`<html><head><link rel="icon" href="/meta.png"></head></html>`) | |
| 170 | + | ||
| 171 | + | // The same document finishing again must not repeat the full-document | |
| 172 | + | // transfer; the GetMainFrame and GetSource expectations are Once. | |
| 173 | + | h.OnLoadingStateChange(browser, 0, 0, 0) | |
| 174 | + | } | |
| @@ -60,11 +60,13 @@ func TestCEFOnFaviconUrlchangeForwardsOrderedCandidates(t *testing.T) { | |||||
| 60 | 60 | ||||
| 61 | var page string | 61 | var page string | ||
| 62 | var icons []string | 62 | var icons []string | ||
| 63 | - | wv := &WebView{ctx: context.Background(), uri: "https://example.com/docs/page", callbacks: &port.WebViewCallbacks{ | 63 | + | browser := cefmocks.NewMockBrowser(t) |
| 64 | + | browser.EXPECT().GetIdentifier().Return(int32(7)).Maybe() | |||
| 65 | + | wv := &WebView{ctx: context.Background(), uri: "https://example.com/docs/page", browser: browser, documentSeq: 1, callbacks: &port.WebViewCallbacks{ | |||
| 64 | OnFaviconURLChanged: func(p string, urls []string) { page, icons = p, urls }, | 66 | OnFaviconURLChanged: func(p string, urls []string) { page, icons = p, urls }, | ||
| 65 | }} | 67 | }} | ||
| 66 | 68 | ||||
| 67 | - | (&handlerSet{wv: wv}).OnFaviconUrlchange(nil, purecef.StringList(42)) | 69 | + | (&handlerSet{wv: wv}).OnFaviconUrlchange(browser, purecef.StringList(42)) |
| 68 | 70 | ||||
| 69 | require.Equal(t, "https://example.com/docs/page", page) | 71 | require.Equal(t, "https://example.com/docs/page", page) | ||
| 70 | require.Equal(t, []string{"https://example.com/favicon.png", "https://cdn.example.com/icon.ico"}, icons) | 72 | require.Equal(t, []string{"https://example.com/favicon.png", "https://cdn.example.com/icon.ico"}, icons) | ||
| @@ -77,6 +79,7 @@ func TestCEFLoadingFinishedDiscoversMetadataBeforeFallback(t *testing.T) { | |||||
| 77 | 79 | ||||
| 78 | browser := cefmocks.NewMockBrowser(t) | 80 | browser := cefmocks.NewMockBrowser(t) | ||
| 79 | frame := cefmocks.NewMockFrame(t) | 81 | frame := cefmocks.NewMockFrame(t) | ||
| 82 | + | browser.EXPECT().GetIdentifier().Return(int32(7)).Maybe() | |||
| 80 | browser.EXPECT().GetMainFrame().Return(frame) | 83 | browser.EXPECT().GetMainFrame().Return(frame) | ||
| 81 | frame.EXPECT().GetSource(mock.Anything).Run(func(visitor purecef.StringVisitor) { | 84 | frame.EXPECT().GetSource(mock.Anything).Run(func(visitor purecef.StringVisitor) { | ||
| 82 | visitor.Visit(`<html><head><link rel="icon" href="/meta.png"></head></html>`) | 85 | visitor.Visit(`<html><head><link rel="icon" href="/meta.png"></head></html>`) | ||
| @@ -84,7 +87,7 @@ func TestCEFLoadingFinishedDiscoversMetadataBeforeFallback(t *testing.T) { | |||||
| 84 | 87 | ||||
| 85 | var page string | 88 | var page string | ||
| 86 | var icons []string | 89 | var icons []string | ||
| 87 | - | wv := &WebView{ctx: context.Background(), uri: "https://example.com/docs/page", callbacks: &port.WebViewCallbacks{ | 90 | + | wv := &WebView{ctx: context.Background(), uri: "https://example.com/docs/page", browser: browser, documentSeq: 1, callbacks: &port.WebViewCallbacks{ |
| 88 | OnFaviconURLChanged: func(p string, urls []string) { page, icons = p, urls }, | 91 | OnFaviconURLChanged: func(p string, urls []string) { page, icons = p, urls }, | ||
| 89 | }} | 92 | }} | ||
| 90 | 93 | ||||
| @@ -97,7 +100,7 @@ func TestCEFLoadingFinishedDiscoversMetadataBeforeFallback(t *testing.T) { | |||||
| 97 | func TestCEFLoadingFinishedForwardsFallbackCandidate(t *testing.T) { | 100 | func TestCEFLoadingFinishedForwardsFallbackCandidate(t *testing.T) { | ||
| 98 | var page string | 101 | var page string | ||
| 99 | var icons []string | 102 | var icons []string | ||
| 100 | - | wv := &WebView{ctx: context.Background(), uri: "https://example.com/docs/page", callbacks: &port.WebViewCallbacks{ | 103 | + | wv := &WebView{ctx: context.Background(), uri: "https://example.com/docs/page", documentSeq: 1, callbacks: &port.WebViewCallbacks{ |
| 101 | OnFaviconURLChanged: func(p string, urls []string) { page, icons = p, urls }, | 104 | OnFaviconURLChanged: func(p string, urls []string) { page, icons = p, urls }, | ||
| 102 | }} | 105 | }} | ||
| 103 | 106 | ||||
| @@ -106,3 +109,66 @@ func TestCEFLoadingFinishedForwardsFallbackCandidate(t *testing.T) { | |||||
| 106 | require.Equal(t, "https://example.com/docs/page", page) | 109 | require.Equal(t, "https://example.com/docs/page", page) | ||
| 107 | require.Equal(t, []string{"https://example.com/favicon.ico"}, icons) | 110 | require.Equal(t, []string{"https://example.com/favicon.ico"}, icons) | ||
| 108 | } | 111 | } | ||
| 112 | + | ||||
| 113 | + | func TestCEFLoadingFinishedSkipsSourceWhenEngineProvidedCandidates(t *testing.T) { | |||
| 114 | + | prevDecode := decodeCEFStringList | |||
| 115 | + | decodeCEFStringList = func(purecef.StringList) []string { return []string{"/engine.png"} } | |||
| 116 | + | defer func() { decodeCEFStringList = prevDecode }() | |||
| 117 | + | ||||
| 118 | + | browser := cefmocks.NewMockBrowser(t) | |||
| 119 | + | browser.EXPECT().GetIdentifier().Return(int32(7)).Maybe() | |||
| 120 | + | calls := 0 | |||
| 121 | + | wv := &WebView{ctx: context.Background(), uri: "https://example.com/page", browser: browser, documentSeq: 1, callbacks: &port.WebViewCallbacks{ | |||
| 122 | + | OnFaviconURLChanged: func(string, []string) { calls++ }, | |||
| 123 | + | }} | |||
| 124 | + | h := &handlerSet{wv: wv} | |||
| 125 | + | h.OnFaviconUrlchange(browser, purecef.StringList(1)) | |||
| 126 | + | h.OnLoadingStateChange(browser, 0, 0, 0) | |||
| 127 | + | ||||
| 128 | + | require.Equal(t, 1, calls) | |||
| 129 | + | } | |||
| 130 | + | ||||
| 131 | + | func TestCEFLoadingFinishedCoalescesPendingSourceDiscovery(t *testing.T) { | |||
| 132 | + | prevNewStringVisitor := cefNewStringVisitor | |||
| 133 | + | cefNewStringVisitor = func(visitor purecef.StringVisitor) purecef.StringVisitor { return visitor } | |||
| 134 | + | defer func() { cefNewStringVisitor = prevNewStringVisitor }() | |||
| 135 | + | ||||
| 136 | + | browser := cefmocks.NewMockBrowser(t) | |||
| 137 | + | frame := cefmocks.NewMockFrame(t) | |||
| 138 | + | browser.EXPECT().GetMainFrame().Return(frame).Once() | |||
| 139 | + | frame.EXPECT().GetSource(mock.Anything).Return().Once() | |||
| 140 | + | wv := &WebView{ctx: context.Background(), uri: "https://example.com/page", documentSeq: 1, callbacks: &port.WebViewCallbacks{ | |||
| 141 | + | OnFaviconURLChanged: func(string, []string) {}, | |||
| 142 | + | }} | |||
| 143 | + | h := &handlerSet{wv: wv} | |||
| 144 | + | h.OnLoadingStateChange(browser, 0, 0, 0) | |||
| 145 | + | h.OnLoadingStateChange(browser, 0, 0, 0) | |||
| 146 | + | } | |||
| 147 | + | ||||
| 148 | + | func TestCEFLoadingFinishedDoesNotRepeatCompletedSourceDiscovery(t *testing.T) { | |||
| 149 | + | prevNewStringVisitor := cefNewStringVisitor | |||
| 150 | + | var visitor purecef.StringVisitor | |||
| 151 | + | cefNewStringVisitor = func(v purecef.StringVisitor) purecef.StringVisitor { | |||
| 152 | + | visitor = v | |||
| 153 | + | return v | |||
| 154 | + | } | |||
| 155 | + | defer func() { cefNewStringVisitor = prevNewStringVisitor }() | |||
| 156 | + | ||||
| 157 | + | browser := cefmocks.NewMockBrowser(t) | |||
| 158 | + | frame := cefmocks.NewMockFrame(t) | |||
| 159 | + | browser.EXPECT().GetIdentifier().Return(int32(7)).Maybe() | |||
| 160 | + | browser.EXPECT().GetMainFrame().Return(frame).Once() | |||
| 161 | + | frame.EXPECT().GetSource(mock.Anything).Return().Once() | |||
| 162 | + | wv := &WebView{ctx: context.Background(), uri: "https://example.com/page", documentSeq: 1, callbacks: &port.WebViewCallbacks{ | |||
| 163 | + | OnFaviconURLChanged: func(string, []string) {}, | |||
| 164 | + | }} | |||
| 165 | + | h := &handlerSet{wv: wv} | |||
| 166 | + | ||||
| 167 | + | h.OnLoadingStateChange(browser, 0, 0, 0) | |||
| 168 | + | require.NotNil(t, visitor, "loading-finished must start source discovery") | |||
| 169 | + | visitor.Visit(`<html><head><link rel="icon" href="/meta.png"></head></html>`) | |||
| 170 | + | ||||
| 171 | + | // The same document finishing again must not repeat the full-document | |||
| 172 | + | // transfer; the GetMainFrame and GetSource expectations are Once. | |||
| 173 | + | h.OnLoadingStateChange(browser, 0, 0, 0) | |||
| 174 | + | } | |||
M internal/infrastructure/cef/injection_event_test.go +54 -14
| @@ -5,6 +5,7 @@ import ( | |||
| 5 | 5 | "sync" | |
| 6 | 6 | "testing" | |
| 7 | 7 | ||
| 8 | + | purecef "github.com/bnema/purego-cef/cef" | |
| 8 | 9 | cefmocks "github.com/bnema/purego-cef/cef/mocks" | |
| 9 | 10 | "github.com/stretchr/testify/mock" | |
| 10 | 11 | "github.com/stretchr/testify/require" | |
| @@ -36,7 +37,7 @@ func newInjectionHarnessWithID(t *testing.T, uri string, browserID int32) *injec | |||
| 36 | 37 | h.browser = cefmocks.NewMockBrowser(t) | |
| 37 | 38 | h.browser.EXPECT().GetMainFrame().Return(frame).Maybe() | |
| 38 | 39 | h.browser.EXPECT().GetIdentifier().Return(browserID).Maybe() | |
| 39 | - | h.wv = &WebView{ctx: context.Background(), browser: h.browser} | |
| 40 | + | h.wv = &WebView{ctx: context.Background(), browser: h.browser, documentSeq: 1} | |
| 40 | 41 | h.wv.uri = uri | |
| 41 | 42 | return h | |
| 42 | 43 | } | |
| @@ -57,7 +58,7 @@ func testInjector() *contentInjector { | |||
| 57 | 58 | // bridge shim belongs to external pages. | |
| 58 | 59 | func TestInjectionEvent_ExternalPageScriptSet(t *testing.T) { | |
| 59 | 60 | h := newInjectionHarness(t, "https://example.com/page") | |
| 60 | - | testInjector().onLoadEndForEvent(h.wv, h.wv.captureInjectionEvent(h.browser)) | |
| 61 | + | testInjector().onLoadEndForEvent(h.wv, h.wv.captureInjectionEvent(h.browser, h.wv.uri)) | |
| 61 | 62 | ||
| 62 | 63 | require.Equal(t, 4, h.scriptCount(), "external install must stay a fixed small script set") | |
| 63 | 64 | h.mu.Lock() | |
| @@ -76,7 +77,7 @@ func TestInjectionEvent_ExternalPageScriptSet(t *testing.T) { | |||
| 76 | 77 | // internal document, including dark-mode handling and the message bridge. | |
| 77 | 78 | func TestInjectionEvent_InternalPageScriptSet(t *testing.T) { | |
| 78 | 79 | h := newInjectionHarness(t, "dumb://home") | |
| 79 | - | testInjector().onLoadEndForEvent(h.wv, h.wv.captureInjectionEvent(h.browser)) | |
| 80 | + | testInjector().onLoadEndForEvent(h.wv, h.wv.captureInjectionEvent(h.browser, h.wv.uri)) | |
| 80 | 81 | ||
| 81 | 82 | require.Equal(t, 7, h.scriptCount(), "internal install must stay a fixed small script set") | |
| 82 | 83 | } | |
| @@ -85,7 +86,7 @@ func TestInjectionEvent_InternalPageScriptSet(t *testing.T) { | |||
| 85 | 86 | // during process swap: the GTK-dispatched callback must install nothing. | |
| 86 | 87 | func TestInjectionEvent_StaleBrowserSkipped(t *testing.T) { | |
| 87 | 88 | h := newInjectionHarnessWithID(t, "https://example.com/a", 7) | |
| 88 | - | event := h.wv.captureInjectionEvent(h.browser) | |
| 89 | + | event := h.wv.captureInjectionEvent(h.browser, h.wv.uri) | |
| 89 | 90 | ||
| 90 | 91 | replacement := cefmocks.NewMockBrowser(t) | |
| 91 | 92 | replacement.EXPECT().GetIdentifier().Return(int32(8)).Maybe() | |
| @@ -102,7 +103,7 @@ func TestInjectionEvent_StaleBrowserSkipped(t *testing.T) { | |||
| 102 | 103 | // superseded intent. | |
| 103 | 104 | func TestInjectionEvent_StaleIntentSkipped(t *testing.T) { | |
| 104 | 105 | h := newInjectionHarness(t, "https://example.com/a") | |
| 105 | - | event := h.wv.captureInjectionEvent(h.browser) | |
| 106 | + | event := h.wv.captureInjectionEvent(h.browser, h.wv.uri) | |
| 106 | 107 | ||
| 107 | 108 | h.wv.mu.Lock() | |
| 108 | 109 | h.wv.pendingIntentID++ | |
| @@ -112,19 +113,58 @@ func TestInjectionEvent_StaleIntentSkipped(t *testing.T) { | |||
| 112 | 113 | require.Zero(t, h.scriptCount(), "superseded intent must not install") | |
| 113 | 114 | } | |
| 114 | 115 | ||
| 115 | - | // TestInjectionEvent_CurrentInstalls verifies matching identity installs | |
| 116 | - | // exactly as the legacy path, including repeated same-document load-ends | |
| 117 | - | // whose scripts are idempotent by element ID. | |
| 116 | + | // TestInjectionEvent_CurrentInstalls verifies matching identity installs once | |
| 117 | + | // for a committed document. Repeated load-end notifications for that document | |
| 118 | + | // must not repeat the GTK-to-CEF script round trips. | |
| 118 | 119 | func TestInjectionEvent_CurrentInstalls(t *testing.T) { | |
| 119 | 120 | h := newInjectionHarness(t, "https://example.com/a") | |
| 120 | 121 | ci := testInjector() | |
| 121 | 122 | ||
| 122 | - | ci.onLoadEndForEvent(h.wv, h.wv.captureInjectionEvent(h.browser)) | |
| 123 | + | ci.onLoadEndForEvent(h.wv, h.wv.captureInjectionEvent(h.browser, h.wv.uri)) | |
| 123 | 124 | first := h.scriptCount() | |
| 124 | 125 | require.Positive(t, first) | |
| 125 | 126 | ||
| 126 | - | ci.onLoadEndForEvent(h.wv, h.wv.captureInjectionEvent(h.browser)) | |
| 127 | - | require.Equal(t, 2*first, h.scriptCount(), "repeated same-document events reinstall idempotent scripts") | |
| 127 | + | ci.onLoadEndForEvent(h.wv, h.wv.captureInjectionEvent(h.browser, h.wv.uri)) | |
| 128 | + | require.Equal(t, first, h.scriptCount(), "repeated same-document events must be coalesced") | |
| 129 | + | ||
| 130 | + | h.wv.mu.Lock() | |
| 131 | + | h.wv.documentSeq++ | |
| 132 | + | h.wv.mu.Unlock() | |
| 133 | + | ci.onLoadEndForEvent(h.wv, h.wv.captureInjectionEvent(h.browser, h.wv.uri)) | |
| 134 | + | require.Equal(t, 2*first, h.scriptCount(), "a new committed document must receive scripts") | |
| 135 | + | } | |
| 136 | + | ||
| 137 | + | // TestInjectionEvent_ReplacedDocumentSkipsQueuedScripts reproduces a replacement | |
| 138 | + | // navigation committing between validation and the CEF UI thread: scripts | |
| 139 | + | // pinned to the validated document must be dropped instead of landing in the | |
| 140 | + | // new document. | |
| 141 | + | func TestInjectionEvent_ReplacedDocumentSkipsQueuedScripts(t *testing.T) { | |
| 142 | + | h := newInjectionHarness(t, "https://example.com/a") | |
| 143 | + | ||
| 144 | + | var queued []purecef.Task | |
| 145 | + | prevNewTask, prevPostTask := cefNewTask, cefPostTask | |
| 146 | + | cefNewTask = func(task purecef.Task) purecef.Task { return task } | |
| 147 | + | cefPostTask = func(_ purecef.ThreadID, task purecef.Task) int32 { | |
| 148 | + | queued = append(queued, task) | |
| 149 | + | return 1 | |
| 150 | + | } | |
| 151 | + | t.Cleanup(func() { cefNewTask, cefPostTask = prevNewTask, prevPostTask }) | |
| 152 | + | ||
| 153 | + | // A non-nil engine queues scripts for the CEF UI thread instead of running | |
| 154 | + | // them inline, which is what opens the window the pin has to close. | |
| 155 | + | h.wv.engine = &Engine{} | |
| 156 | + | testInjector().onLoadEndForEvent(h.wv, h.wv.captureInjectionEvent(h.browser, h.wv.uri)) | |
| 157 | + | require.NotEmpty(t, queued, "a validated load-end must queue its installation") | |
| 158 | + | ||
| 159 | + | // The replacement document commits before the queued scripts run. | |
| 160 | + | h.wv.mu.Lock() | |
| 161 | + | h.wv.documentSeq++ | |
| 162 | + | h.wv.mu.Unlock() | |
| 163 | + | for _, task := range queued { | |
| 164 | + | task.Execute() | |
| 165 | + | } | |
| 166 | + | ||
| 167 | + | require.Zero(t, h.scriptCount(), "scripts pinned to a replaced document must not install") | |
| 128 | 168 | } | |
| 129 | 169 | ||
| 130 | 170 | // TestInjectionEvent_NilCaptureSkips verifies the conservative fallback: a | |
| @@ -132,7 +172,7 @@ func TestInjectionEvent_CurrentInstalls(t *testing.T) { | |||
| 132 | 172 | // nothing; the current browser gets its own load-end installation. | |
| 133 | 173 | func TestInjectionEvent_NilCaptureSkips(t *testing.T) { | |
| 134 | 174 | h := newInjectionHarness(t, "https://example.com/a") | |
| 135 | - | event := h.wv.captureInjectionEvent(nil) | |
| 175 | + | event := h.wv.captureInjectionEvent(nil, h.wv.uri) | |
| 136 | 176 | require.Equal(t, noBrowserID, event.browserID) | |
| 137 | 177 | ||
| 138 | 178 | testInjector().onLoadEndForEvent(h.wv, event) | |
| @@ -143,7 +183,7 @@ func TestInjectionEvent_NilCaptureSkips(t *testing.T) { | |||
| 143 | 183 | // destroyed browser installs nothing instead of dereferencing it. | |
| 144 | 184 | func TestInjectionEvent_NilCurrentBrowserSkips(t *testing.T) { | |
| 145 | 185 | h := newInjectionHarness(t, "https://example.com/a") | |
| 146 | - | event := h.wv.captureInjectionEvent(h.browser) | |
| 186 | + | event := h.wv.captureInjectionEvent(h.browser, h.wv.uri) | |
| 147 | 187 | ||
| 148 | 188 | h.wv.mu.Lock() | |
| 149 | 189 | h.wv.browser = nil | |
| @@ -167,7 +207,7 @@ func TestInjectionEvent_StaleCallbackBrowserSkipped(t *testing.T) { | |||
| 167 | 207 | h.wv.browser = replacement | |
| 168 | 208 | h.wv.mu.Unlock() | |
| 169 | 209 | ||
| 170 | - | event := h.wv.captureInjectionEvent(oldCallbackBrowser) | |
| 210 | + | event := h.wv.captureInjectionEvent(oldCallbackBrowser, h.wv.uri) | |
| 171 | 211 | require.Equal(t, int32(7), event.browserID, "callback-sourced capture must keep the old identity") | |
| 172 | 212 | testInjector().onLoadEndForEvent(h.wv, event) | |
| 173 | 213 | require.Zero(t, h.scriptCount(), "old-callback event must not install against the replacement browser") | |
| @@ -5,6 +5,7 @@ import ( | |||||
| 5 | "sync" | 5 | "sync" | ||
| 6 | "testing" | 6 | "testing" | ||
| 7 | 7 | ||||
| 8 | + | purecef "github.com/bnema/purego-cef/cef" | |||
| 8 | cefmocks "github.com/bnema/purego-cef/cef/mocks" | 9 | cefmocks "github.com/bnema/purego-cef/cef/mocks" | ||
| 9 | "github.com/stretchr/testify/mock" | 10 | "github.com/stretchr/testify/mock" | ||
| 10 | "github.com/stretchr/testify/require" | 11 | "github.com/stretchr/testify/require" | ||
| @@ -36,7 +37,7 @@ func newInjectionHarnessWithID(t *testing.T, uri string, browserID int32) *injec | |||||
| 36 | h.browser = cefmocks.NewMockBrowser(t) | 37 | h.browser = cefmocks.NewMockBrowser(t) | ||
| 37 | h.browser.EXPECT().GetMainFrame().Return(frame).Maybe() | 38 | h.browser.EXPECT().GetMainFrame().Return(frame).Maybe() | ||
| 38 | h.browser.EXPECT().GetIdentifier().Return(browserID).Maybe() | 39 | h.browser.EXPECT().GetIdentifier().Return(browserID).Maybe() | ||
| 39 | - | h.wv = &WebView{ctx: context.Background(), browser: h.browser} | 40 | + | h.wv = &WebView{ctx: context.Background(), browser: h.browser, documentSeq: 1} |
| 40 | h.wv.uri = uri | 41 | h.wv.uri = uri | ||
| 41 | return h | 42 | return h | ||
| 42 | } | 43 | } | ||
| @@ -57,7 +58,7 @@ func testInjector() *contentInjector { | |||||
| 57 | // bridge shim belongs to external pages. | 58 | // bridge shim belongs to external pages. | ||
| 58 | func TestInjectionEvent_ExternalPageScriptSet(t *testing.T) { | 59 | func TestInjectionEvent_ExternalPageScriptSet(t *testing.T) { | ||
| 59 | h := newInjectionHarness(t, "https://example.com/page") | 60 | h := newInjectionHarness(t, "https://example.com/page") | ||
| 60 | - | testInjector().onLoadEndForEvent(h.wv, h.wv.captureInjectionEvent(h.browser)) | 61 | + | testInjector().onLoadEndForEvent(h.wv, h.wv.captureInjectionEvent(h.browser, h.wv.uri)) |
| 61 | 62 | ||||
| 62 | require.Equal(t, 4, h.scriptCount(), "external install must stay a fixed small script set") | 63 | require.Equal(t, 4, h.scriptCount(), "external install must stay a fixed small script set") | ||
| 63 | h.mu.Lock() | 64 | h.mu.Lock() | ||
| @@ -76,7 +77,7 @@ func TestInjectionEvent_ExternalPageScriptSet(t *testing.T) { | |||||
| 76 | // internal document, including dark-mode handling and the message bridge. | 77 | // internal document, including dark-mode handling and the message bridge. | ||
| 77 | func TestInjectionEvent_InternalPageScriptSet(t *testing.T) { | 78 | func TestInjectionEvent_InternalPageScriptSet(t *testing.T) { | ||
| 78 | h := newInjectionHarness(t, "dumb://home") | 79 | h := newInjectionHarness(t, "dumb://home") | ||
| 79 | - | testInjector().onLoadEndForEvent(h.wv, h.wv.captureInjectionEvent(h.browser)) | 80 | + | testInjector().onLoadEndForEvent(h.wv, h.wv.captureInjectionEvent(h.browser, h.wv.uri)) |
| 80 | 81 | ||||
| 81 | require.Equal(t, 7, h.scriptCount(), "internal install must stay a fixed small script set") | 82 | require.Equal(t, 7, h.scriptCount(), "internal install must stay a fixed small script set") | ||
| 82 | } | 83 | } | ||
| @@ -85,7 +86,7 @@ func TestInjectionEvent_InternalPageScriptSet(t *testing.T) { | |||||
| 85 | // during process swap: the GTK-dispatched callback must install nothing. | 86 | // during process swap: the GTK-dispatched callback must install nothing. | ||
| 86 | func TestInjectionEvent_StaleBrowserSkipped(t *testing.T) { | 87 | func TestInjectionEvent_StaleBrowserSkipped(t *testing.T) { | ||
| 87 | h := newInjectionHarnessWithID(t, "https://example.com/a", 7) | 88 | h := newInjectionHarnessWithID(t, "https://example.com/a", 7) | ||
| 88 | - | event := h.wv.captureInjectionEvent(h.browser) | 89 | + | event := h.wv.captureInjectionEvent(h.browser, h.wv.uri) |
| 89 | 90 | ||||
| 90 | replacement := cefmocks.NewMockBrowser(t) | 91 | replacement := cefmocks.NewMockBrowser(t) | ||
| 91 | replacement.EXPECT().GetIdentifier().Return(int32(8)).Maybe() | 92 | replacement.EXPECT().GetIdentifier().Return(int32(8)).Maybe() | ||
| @@ -102,7 +103,7 @@ func TestInjectionEvent_StaleBrowserSkipped(t *testing.T) { | |||||
| 102 | // superseded intent. | 103 | // superseded intent. | ||
| 103 | func TestInjectionEvent_StaleIntentSkipped(t *testing.T) { | 104 | func TestInjectionEvent_StaleIntentSkipped(t *testing.T) { | ||
| 104 | h := newInjectionHarness(t, "https://example.com/a") | 105 | h := newInjectionHarness(t, "https://example.com/a") | ||
| 105 | - | event := h.wv.captureInjectionEvent(h.browser) | 106 | + | event := h.wv.captureInjectionEvent(h.browser, h.wv.uri) |
| 106 | 107 | ||||
| 107 | h.wv.mu.Lock() | 108 | h.wv.mu.Lock() | ||
| 108 | h.wv.pendingIntentID++ | 109 | h.wv.pendingIntentID++ | ||
| @@ -112,19 +113,58 @@ func TestInjectionEvent_StaleIntentSkipped(t *testing.T) { | |||||
| 112 | require.Zero(t, h.scriptCount(), "superseded intent must not install") | 113 | require.Zero(t, h.scriptCount(), "superseded intent must not install") | ||
| 113 | } | 114 | } | ||
| 114 | 115 | ||||
| 115 | - | // TestInjectionEvent_CurrentInstalls verifies matching identity installs | 116 | + | // TestInjectionEvent_CurrentInstalls verifies matching identity installs once |
| 116 | - | // exactly as the legacy path, including repeated same-document load-ends | 117 | + | // for a committed document. Repeated load-end notifications for that document |
| 117 | - | // whose scripts are idempotent by element ID. | 118 | + | // must not repeat the GTK-to-CEF script round trips. |
| 118 | func TestInjectionEvent_CurrentInstalls(t *testing.T) { | 119 | func TestInjectionEvent_CurrentInstalls(t *testing.T) { | ||
| 119 | h := newInjectionHarness(t, "https://example.com/a") | 120 | h := newInjectionHarness(t, "https://example.com/a") | ||
| 120 | ci := testInjector() | 121 | ci := testInjector() | ||
| 121 | 122 | ||||
| 122 | - | ci.onLoadEndForEvent(h.wv, h.wv.captureInjectionEvent(h.browser)) | 123 | + | ci.onLoadEndForEvent(h.wv, h.wv.captureInjectionEvent(h.browser, h.wv.uri)) |
| 123 | first := h.scriptCount() | 124 | first := h.scriptCount() | ||
| 124 | require.Positive(t, first) | 125 | require.Positive(t, first) | ||
| 125 | 126 | ||||
| 126 | - | ci.onLoadEndForEvent(h.wv, h.wv.captureInjectionEvent(h.browser)) | 127 | + | ci.onLoadEndForEvent(h.wv, h.wv.captureInjectionEvent(h.browser, h.wv.uri)) |
| 127 | - | require.Equal(t, 2*first, h.scriptCount(), "repeated same-document events reinstall idempotent scripts") | 128 | + | require.Equal(t, first, h.scriptCount(), "repeated same-document events must be coalesced") |
| 129 | + | ||||
| 130 | + | h.wv.mu.Lock() | |||
| 131 | + | h.wv.documentSeq++ | |||
| 132 | + | h.wv.mu.Unlock() | |||
| 133 | + | ci.onLoadEndForEvent(h.wv, h.wv.captureInjectionEvent(h.browser, h.wv.uri)) | |||
| 134 | + | require.Equal(t, 2*first, h.scriptCount(), "a new committed document must receive scripts") | |||
| 135 | + | } | |||
| 136 | + | ||||
| 137 | + | // TestInjectionEvent_ReplacedDocumentSkipsQueuedScripts reproduces a replacement | |||
| 138 | + | // navigation committing between validation and the CEF UI thread: scripts | |||
| 139 | + | // pinned to the validated document must be dropped instead of landing in the | |||
| 140 | + | // new document. | |||
| 141 | + | func TestInjectionEvent_ReplacedDocumentSkipsQueuedScripts(t *testing.T) { | |||
| 142 | + | h := newInjectionHarness(t, "https://example.com/a") | |||
| 143 | + | ||||
| 144 | + | var queued []purecef.Task | |||
| 145 | + | prevNewTask, prevPostTask := cefNewTask, cefPostTask | |||
| 146 | + | cefNewTask = func(task purecef.Task) purecef.Task { return task } | |||
| 147 | + | cefPostTask = func(_ purecef.ThreadID, task purecef.Task) int32 { | |||
| 148 | + | queued = append(queued, task) | |||
| 149 | + | return 1 | |||
| 150 | + | } | |||
| 151 | + | t.Cleanup(func() { cefNewTask, cefPostTask = prevNewTask, prevPostTask }) | |||
| 152 | + | ||||
| 153 | + | // A non-nil engine queues scripts for the CEF UI thread instead of running | |||
| 154 | + | // them inline, which is what opens the window the pin has to close. | |||
| 155 | + | h.wv.engine = &Engine{} | |||
| 156 | + | testInjector().onLoadEndForEvent(h.wv, h.wv.captureInjectionEvent(h.browser, h.wv.uri)) | |||
| 157 | + | require.NotEmpty(t, queued, "a validated load-end must queue its installation") | |||
| 158 | + | ||||
| 159 | + | // The replacement document commits before the queued scripts run. | |||
| 160 | + | h.wv.mu.Lock() | |||
| 161 | + | h.wv.documentSeq++ | |||
| 162 | + | h.wv.mu.Unlock() | |||
| 163 | + | for _, task := range queued { | |||
| 164 | + | task.Execute() | |||
| 165 | + | } | |||
| 166 | + | ||||
| 167 | + | require.Zero(t, h.scriptCount(), "scripts pinned to a replaced document must not install") | |||
| 128 | } | 168 | } | ||
| 129 | 169 | ||||
| 130 | // TestInjectionEvent_NilCaptureSkips verifies the conservative fallback: a | 170 | // TestInjectionEvent_NilCaptureSkips verifies the conservative fallback: a | ||
| @@ -132,7 +172,7 @@ func TestInjectionEvent_CurrentInstalls(t *testing.T) { | |||||
| 132 | // nothing; the current browser gets its own load-end installation. | 172 | // nothing; the current browser gets its own load-end installation. | ||
| 133 | func TestInjectionEvent_NilCaptureSkips(t *testing.T) { | 173 | func TestInjectionEvent_NilCaptureSkips(t *testing.T) { | ||
| 134 | h := newInjectionHarness(t, "https://example.com/a") | 174 | h := newInjectionHarness(t, "https://example.com/a") | ||
| 135 | - | event := h.wv.captureInjectionEvent(nil) | 175 | + | event := h.wv.captureInjectionEvent(nil, h.wv.uri) |
| 136 | require.Equal(t, noBrowserID, event.browserID) | 176 | require.Equal(t, noBrowserID, event.browserID) | ||
| 137 | 177 | ||||
| 138 | testInjector().onLoadEndForEvent(h.wv, event) | 178 | testInjector().onLoadEndForEvent(h.wv, event) | ||
| @@ -143,7 +183,7 @@ func TestInjectionEvent_NilCaptureSkips(t *testing.T) { | |||||
| 143 | // destroyed browser installs nothing instead of dereferencing it. | 183 | // destroyed browser installs nothing instead of dereferencing it. | ||
| 144 | func TestInjectionEvent_NilCurrentBrowserSkips(t *testing.T) { | 184 | func TestInjectionEvent_NilCurrentBrowserSkips(t *testing.T) { | ||
| 145 | h := newInjectionHarness(t, "https://example.com/a") | 185 | h := newInjectionHarness(t, "https://example.com/a") | ||
| 146 | - | event := h.wv.captureInjectionEvent(h.browser) | 186 | + | event := h.wv.captureInjectionEvent(h.browser, h.wv.uri) |
| 147 | 187 | ||||
| 148 | h.wv.mu.Lock() | 188 | h.wv.mu.Lock() | ||
| 149 | h.wv.browser = nil | 189 | h.wv.browser = nil | ||
| @@ -167,7 +207,7 @@ func TestInjectionEvent_StaleCallbackBrowserSkipped(t *testing.T) { | |||||
| 167 | h.wv.browser = replacement | 207 | h.wv.browser = replacement | ||
| 168 | h.wv.mu.Unlock() | 208 | h.wv.mu.Unlock() | ||
| 169 | 209 | ||||
| 170 | - | event := h.wv.captureInjectionEvent(oldCallbackBrowser) | 210 | + | event := h.wv.captureInjectionEvent(oldCallbackBrowser, h.wv.uri) |
| 171 | require.Equal(t, int32(7), event.browserID, "callback-sourced capture must keep the old identity") | 211 | require.Equal(t, int32(7), event.browserID, "callback-sourced capture must keep the old identity") | ||
| 172 | testInjector().onLoadEndForEvent(h.wv, event) | 212 | testInjector().onLoadEndForEvent(h.wv, event) | ||
| 173 | require.Zero(t, h.scriptCount(), "old-callback event must not install against the replacement browser") | 213 | require.Zero(t, h.scriptCount(), "old-callback event must not install against the replacement browser") | ||
M internal/infrastructure/cef/webview.go +53 -16
| @@ -274,10 +274,23 @@ type WebView struct { | |||
| 274 | 274 | // Last known hover URI for middle-click → new tab. | |
| 275 | 275 | lastHoverURI string | |
| 276 | 276 | ||
| 277 | - | // Favicon source visitors are retained until CEF calls them back because | |
| 278 | - | // GetSource delivers asynchronously through a C callback pointer. | |
| 279 | - | faviconSourceVisitorsMu sync.Mutex | |
| 280 | - | faviconSourceVisitors []faviconSourceVisitorRetention | |
| 277 | + | // Favicon discovery state is scoped to the current main-frame document. | |
| 278 | + | // Source visitors are retained until CEF calls them back because GetSource | |
| 279 | + | // delivers asynchronously through a C callback pointer. | |
| 280 | + | faviconSourceVisitorsMu sync.Mutex | |
| 281 | + | faviconSourceVisitors []faviconSourceVisitorRetention | |
| 282 | + | faviconEngineDocumentSeq uint64 | |
| 283 | + | faviconSourcePendingToken uint64 | |
| 284 | + | faviconSourceRequestSerial uint64 | |
| 285 | + | // faviconSourceCompletedDocumentSeq records the document whose source scan | |
| 286 | + | // already ran, so a repeated loading-finished notification does not repeat a | |
| 287 | + | // full-document transfer. It stays separate from faviconEngineDocumentSeq so | |
| 288 | + | // engine-provided favicons remain accepted for the current document. | |
| 289 | + | faviconSourceCompletedDocumentSeq uint64 | |
| 290 | + | ||
| 291 | + | // Content installation is deduplicated per committed main-frame document. | |
| 292 | + | documentSeq uint64 | |
| 293 | + | installedDocumentSeq uint64 | |
| 281 | 294 | ||
| 282 | 295 | // Load diagnostics state (mutex-protected). | |
| 283 | 296 | loadDiagSeq uint64 | |
| @@ -1186,15 +1199,28 @@ func (wv *WebView) handleNativePopupAborted() { | |||
| 1186 | 1199 | ||
| 1187 | 1200 | // RunJavaScript executes a script in the main world. Fire-and-forget. | |
| 1188 | 1201 | func (wv *WebView) RunJavaScript(_ context.Context, script string) { | |
| 1202 | + | wv.runJavaScriptForDocument(0, script) | |
| 1203 | + | } | |
| 1204 | + | ||
| 1205 | + | // RunJavaScriptForDocument executes a script pinned to one committed | |
| 1206 | + | // main-frame document. The script is dropped when a newer document has | |
| 1207 | + | // committed before it reaches the CEF UI thread, so a stale load-end callback | |
| 1208 | + | // cannot install its content into a replacement document. A documentSeq of 0 | |
| 1209 | + | // means unpinned. | |
| 1210 | + | func (wv *WebView) RunJavaScriptForDocument(documentSeq uint64, script string) { | |
| 1211 | + | wv.runJavaScriptForDocument(documentSeq, script) | |
| 1212 | + | } | |
| 1213 | + | ||
| 1214 | + | func (wv *WebView) runJavaScriptForDocument(documentSeq uint64, script string) { | |
| 1189 | 1215 | if wv.destroyed.Load() { | |
| 1190 | 1216 | return | |
| 1191 | 1217 | } | |
| 1192 | 1218 | if wv.engine == nil { | |
| 1193 | - | wv.executeJavaScriptNow(script) | |
| 1219 | + | wv.executeJavaScript(documentSeq, script) | |
| 1194 | 1220 | return | |
| 1195 | 1221 | } | |
| 1196 | 1222 | task := cefNewTask(cefTaskFunc(func() { | |
| 1197 | - | wv.executeJavaScriptNow(script) | |
| 1223 | + | wv.executeJavaScript(documentSeq, script) | |
| 1198 | 1224 | })) | |
| 1199 | 1225 | if task == nil { | |
| 1200 | 1226 | return | |
| @@ -1203,15 +1229,23 @@ func (wv *WebView) RunJavaScript(_ context.Context, script string) { | |||
| 1203 | 1229 | } | |
| 1204 | 1230 | ||
| 1205 | 1231 | func (wv *WebView) executeJavaScriptNow(script string) { | |
| 1232 | + | wv.executeJavaScript(0, script) | |
| 1233 | + | } | |
| 1234 | + | ||
| 1235 | + | func (wv *WebView) executeJavaScript(documentSeq uint64, script string) { | |
| 1206 | 1236 | if wv == nil || wv.destroyed.Load() { | |
| 1207 | 1237 | return | |
| 1208 | 1238 | } | |
| 1209 | 1239 | wv.mu.RLock() | |
| 1210 | 1240 | browser := wv.browser | |
| 1241 | + | currentDocumentSeq := wv.documentSeq | |
| 1211 | 1242 | wv.mu.RUnlock() | |
| 1212 | 1243 | if browser == nil { | |
| 1213 | 1244 | return | |
| 1214 | 1245 | } | |
| 1246 | + | if documentSeq != 0 && currentDocumentSeq != documentSeq { | |
| 1247 | + | return | |
| 1248 | + | } | |
| 1215 | 1249 | if frame := browser.GetMainFrame(); frame != nil { | |
| 1216 | 1250 | frame.ExecuteJavaScript(script, "", 0) | |
| 1217 | 1251 | } | |
| @@ -2260,8 +2294,10 @@ func (wv *WebView) claimPendingNavigationSubmission(intentID uint64, browserID i | |||
| 2260 | 2294 | // identity. noBrowserID marks captures taken while no browser was | |
| 2261 | 2295 | // attached. The current URL alone never proves document identity. | |
| 2262 | 2296 | type injectionEvent struct { | |
| 2263 | - | browserID int32 | |
| 2264 | - | intentID uint64 | |
| 2297 | + | browserID int32 | |
| 2298 | + | intentID uint64 | |
| 2299 | + | documentSeq uint64 | |
| 2300 | + | frameURL string | |
| 2265 | 2301 | } | |
| 2266 | 2302 | ||
| 2267 | 2303 | // noBrowserID marks an injection event captured while no browser was | |
| @@ -2275,20 +2311,21 @@ const noBrowserID = int32(-1) | |||
| 2275 | 2311 | // detection work: a queued event from a replaced browser keeps the OLD | |
| 2276 | 2312 | // identifier and fails revalidation, while stamping the current wv.browser | |
| 2277 | 2313 | // would bless it. Same-browser navigations are distinguished by the pending | |
| 2278 | - | // intent generation; repeated same-document load-ends share it and still | |
| 2279 | - | // install (their scripts are idempotent). | |
| 2280 | - | func (wv *WebView) captureInjectionEvent(browser purecef.Browser) injectionEvent { | |
| 2281 | - | if wv == nil { | |
| 2282 | - | return injectionEvent{browserID: noBrowserID} | |
| 2283 | - | } | |
| 2284 | - | if browser == nil { | |
| 2314 | + | // intent generation and committed document. Repeated load-end notifications | |
| 2315 | + | // for the same document share the sequence and are installed once. | |
| 2316 | + | func (wv *WebView) captureInjectionEvent(browser purecef.Browser, frameURL string) injectionEvent { | |
| 2317 | + | if wv == nil || browser == nil { | |
| 2285 | 2318 | return injectionEvent{browserID: noBrowserID} | |
| 2286 | 2319 | } | |
| 2287 | 2320 | browserID := browser.GetIdentifier() | |
| 2288 | 2321 | wv.mu.RLock() | |
| 2289 | 2322 | intentID := wv.pendingIntentID | |
| 2323 | + | documentSeq := wv.documentSeq | |
| 2290 | 2324 | wv.mu.RUnlock() | |
| 2291 | - | return injectionEvent{browserID: browserID, intentID: intentID} | |
| 2325 | + | return injectionEvent{ | |
| 2326 | + | browserID: browserID, intentID: intentID, documentSeq: documentSeq, | |
| 2327 | + | frameURL: strings.TrimSpace(frameURL), | |
| 2328 | + | } | |
| 2292 | 2329 | } | |
| 2293 | 2330 | ||
| 2294 | 2331 | func pendingURIEquivalent(a, b string) bool { | |
| @@ -274,10 +274,23 @@ type WebView struct { | |||||
| 274 | // Last known hover URI for middle-click → new tab. | 274 | // Last known hover URI for middle-click → new tab. | ||
| 275 | lastHoverURI string | 275 | lastHoverURI string | ||
| 276 | 276 | ||||
| 277 | - | // Favicon source visitors are retained until CEF calls them back because | 277 | + | // Favicon discovery state is scoped to the current main-frame document. |
| 278 | - | // GetSource delivers asynchronously through a C callback pointer. | 278 | + | // Source visitors are retained until CEF calls them back because GetSource |
| 279 | - | faviconSourceVisitorsMu sync.Mutex | 279 | + | // delivers asynchronously through a C callback pointer. |
| 280 | - | faviconSourceVisitors []faviconSourceVisitorRetention | 280 | + | faviconSourceVisitorsMu sync.Mutex |
| 281 | + | faviconSourceVisitors []faviconSourceVisitorRetention | |||
| 282 | + | faviconEngineDocumentSeq uint64 | |||
| 283 | + | faviconSourcePendingToken uint64 | |||
| 284 | + | faviconSourceRequestSerial uint64 | |||
| 285 | + | // faviconSourceCompletedDocumentSeq records the document whose source scan | |||
| 286 | + | // already ran, so a repeated loading-finished notification does not repeat a | |||
| 287 | + | // full-document transfer. It stays separate from faviconEngineDocumentSeq so | |||
| 288 | + | // engine-provided favicons remain accepted for the current document. | |||
| 289 | + | faviconSourceCompletedDocumentSeq uint64 | |||
| 290 | + | ||||
| 291 | + | // Content installation is deduplicated per committed main-frame document. | |||
| 292 | + | documentSeq uint64 | |||
| 293 | + | installedDocumentSeq uint64 | |||
| 281 | 294 | ||||
| 282 | // Load diagnostics state (mutex-protected). | 295 | // Load diagnostics state (mutex-protected). | ||
| 283 | loadDiagSeq uint64 | 296 | loadDiagSeq uint64 | ||
| @@ -1186,15 +1199,28 @@ func (wv *WebView) handleNativePopupAborted() { | |||||
| 1186 | 1199 | ||||
| 1187 | // RunJavaScript executes a script in the main world. Fire-and-forget. | 1200 | // RunJavaScript executes a script in the main world. Fire-and-forget. | ||
| 1188 | func (wv *WebView) RunJavaScript(_ context.Context, script string) { | 1201 | func (wv *WebView) RunJavaScript(_ context.Context, script string) { | ||
| 1202 | + | wv.runJavaScriptForDocument(0, script) | |||
| 1203 | + | } | |||
| 1204 | + | ||||
| 1205 | + | // RunJavaScriptForDocument executes a script pinned to one committed | |||
| 1206 | + | // main-frame document. The script is dropped when a newer document has | |||
| 1207 | + | // committed before it reaches the CEF UI thread, so a stale load-end callback | |||
| 1208 | + | // cannot install its content into a replacement document. A documentSeq of 0 | |||
| 1209 | + | // means unpinned. | |||
| 1210 | + | func (wv *WebView) RunJavaScriptForDocument(documentSeq uint64, script string) { | |||
| 1211 | + | wv.runJavaScriptForDocument(documentSeq, script) | |||
| 1212 | + | } | |||
| 1213 | + | ||||
| 1214 | + | func (wv *WebView) runJavaScriptForDocument(documentSeq uint64, script string) { | |||
| 1189 | if wv.destroyed.Load() { | 1215 | if wv.destroyed.Load() { | ||
| 1190 | return | 1216 | return | ||
| 1191 | } | 1217 | } | ||
| 1192 | if wv.engine == nil { | 1218 | if wv.engine == nil { | ||
| 1193 | - | wv.executeJavaScriptNow(script) | 1219 | + | wv.executeJavaScript(documentSeq, script) |
| 1194 | return | 1220 | return | ||
| 1195 | } | 1221 | } | ||
| 1196 | task := cefNewTask(cefTaskFunc(func() { | 1222 | task := cefNewTask(cefTaskFunc(func() { | ||
| 1197 | - | wv.executeJavaScriptNow(script) | 1223 | + | wv.executeJavaScript(documentSeq, script) |
| 1198 | })) | 1224 | })) | ||
| 1199 | if task == nil { | 1225 | if task == nil { | ||
| 1200 | return | 1226 | return | ||
| @@ -1203,15 +1229,23 @@ func (wv *WebView) RunJavaScript(_ context.Context, script string) { | |||||
| 1203 | } | 1229 | } | ||
| 1204 | 1230 | ||||
| 1205 | func (wv *WebView) executeJavaScriptNow(script string) { | 1231 | func (wv *WebView) executeJavaScriptNow(script string) { | ||
| 1232 | + | wv.executeJavaScript(0, script) | |||
| 1233 | + | } | |||
| 1234 | + | ||||
| 1235 | + | func (wv *WebView) executeJavaScript(documentSeq uint64, script string) { | |||
| 1206 | if wv == nil || wv.destroyed.Load() { | 1236 | if wv == nil || wv.destroyed.Load() { | ||
| 1207 | return | 1237 | return | ||
| 1208 | } | 1238 | } | ||
| 1209 | wv.mu.RLock() | 1239 | wv.mu.RLock() | ||
| 1210 | browser := wv.browser | 1240 | browser := wv.browser | ||
| 1241 | + | currentDocumentSeq := wv.documentSeq | |||
| 1211 | wv.mu.RUnlock() | 1242 | wv.mu.RUnlock() | ||
| 1212 | if browser == nil { | 1243 | if browser == nil { | ||
| 1213 | return | 1244 | return | ||
| 1214 | } | 1245 | } | ||
| 1246 | + | if documentSeq != 0 && currentDocumentSeq != documentSeq { | |||
| 1247 | + | return | |||
| 1248 | + | } | |||
| 1215 | if frame := browser.GetMainFrame(); frame != nil { | 1249 | if frame := browser.GetMainFrame(); frame != nil { | ||
| 1216 | frame.ExecuteJavaScript(script, "", 0) | 1250 | frame.ExecuteJavaScript(script, "", 0) | ||
| 1217 | } | 1251 | } | ||
| @@ -2260,8 +2294,10 @@ func (wv *WebView) claimPendingNavigationSubmission(intentID uint64, browserID i | |||||
| 2260 | // identity. noBrowserID marks captures taken while no browser was | 2294 | // identity. noBrowserID marks captures taken while no browser was | ||
| 2261 | // attached. The current URL alone never proves document identity. | 2295 | // attached. The current URL alone never proves document identity. | ||
| 2262 | type injectionEvent struct { | 2296 | type injectionEvent struct { | ||
| 2263 | - | browserID int32 | 2297 | + | browserID int32 |
| 2264 | - | intentID uint64 | 2298 | + | intentID uint64 |
| 2299 | + | documentSeq uint64 | |||
| 2300 | + | frameURL string | |||
| 2265 | } | 2301 | } | ||
| 2266 | 2302 | ||||
| 2267 | // noBrowserID marks an injection event captured while no browser was | 2303 | // noBrowserID marks an injection event captured while no browser was | ||
| @@ -2275,20 +2311,21 @@ const noBrowserID = int32(-1) | |||||
| 2275 | // detection work: a queued event from a replaced browser keeps the OLD | 2311 | // detection work: a queued event from a replaced browser keeps the OLD | ||
| 2276 | // identifier and fails revalidation, while stamping the current wv.browser | 2312 | // identifier and fails revalidation, while stamping the current wv.browser | ||
| 2277 | // would bless it. Same-browser navigations are distinguished by the pending | 2313 | // would bless it. Same-browser navigations are distinguished by the pending | ||
| 2278 | - | // intent generation; repeated same-document load-ends share it and still | 2314 | + | // intent generation and committed document. Repeated load-end notifications |
| 2279 | - | // install (their scripts are idempotent). | 2315 | + | // for the same document share the sequence and are installed once. |
| 2280 | - | func (wv *WebView) captureInjectionEvent(browser purecef.Browser) injectionEvent { | 2316 | + | func (wv *WebView) captureInjectionEvent(browser purecef.Browser, frameURL string) injectionEvent { |
| 2281 | - | if wv == nil { | 2317 | + | if wv == nil || browser == nil { |
| 2282 | - | return injectionEvent{browserID: noBrowserID} | |||
| 2283 | - | } | |||
| 2284 | - | if browser == nil { | |||
| 2285 | return injectionEvent{browserID: noBrowserID} | 2318 | return injectionEvent{browserID: noBrowserID} | ||
| 2286 | } | 2319 | } | ||
| 2287 | browserID := browser.GetIdentifier() | 2320 | browserID := browser.GetIdentifier() | ||
| 2288 | wv.mu.RLock() | 2321 | wv.mu.RLock() | ||
| 2289 | intentID := wv.pendingIntentID | 2322 | intentID := wv.pendingIntentID | ||
| 2323 | + | documentSeq := wv.documentSeq | |||
| 2290 | wv.mu.RUnlock() | 2324 | wv.mu.RUnlock() | ||
| 2291 | - | return injectionEvent{browserID: browserID, intentID: intentID} | 2325 | + | return injectionEvent{ |
| 2326 | + | browserID: browserID, intentID: intentID, documentSeq: documentSeq, | |||
| 2327 | + | frameURL: strings.TrimSpace(frameURL), | |||
| 2328 | + | } | |||
| 2292 | } | 2329 | } | ||
| 2293 | 2330 | ||||
| 2294 | func pendingURIEquivalent(a, b string) bool { | 2331 | func pendingURIEquivalent(a, b string) bool { | ||
M internal/infrastructure/cef/webview_handlers.go +78 -5
| @@ -178,26 +178,59 @@ func (h *handlerSet) OnTitleChange(_ purecef.Browser, title string) { | |||
| 178 | 178 | h.wv.updateTitle(title) | |
| 179 | 179 | } | |
| 180 | 180 | ||
| 181 | - | func (h *handlerSet) OnFaviconUrlchange(_ purecef.Browser, iconURLs purecef.StringList) { | |
| 181 | + | func (h *handlerSet) OnFaviconUrlchange(browser purecef.Browser, iconURLs purecef.StringList) { | |
| 182 | 182 | if h == nil || h.wv == nil { | |
| 183 | 183 | return | |
| 184 | 184 | } | |
| 185 | 185 | h.wv.mu.RLock() | |
| 186 | - | pageURL := h.wv.uri | |
| 187 | - | cb := h.wv.callbacks | |
| 186 | + | pageURL, cb, currentBrowser, documentSeq := h.wv.uri, h.wv.callbacks, h.wv.browser, h.wv.documentSeq | |
| 188 | 187 | h.wv.mu.RUnlock() | |
| 189 | - | if cb == nil || cb.OnFaviconURLChanged == nil { | |
| 188 | + | if cb == nil || cb.OnFaviconURLChanged == nil || browser == nil || currentBrowser == nil || | |
| 189 | + | browser.GetIdentifier() != currentBrowser.GetIdentifier() { | |
| 190 | 190 | return | |
| 191 | 191 | } | |
| 192 | 192 | candidates := resolveFaviconCandidates(pageURL, decodeCEFStringList(iconURLs)) | |
| 193 | 193 | if len(candidates) == 0 { | |
| 194 | 194 | return | |
| 195 | 195 | } | |
| 196 | + | h.wv.mu.Lock() | |
| 197 | + | if h.wv.documentSeq != documentSeq || h.wv.uri != pageURL { | |
| 198 | + | h.wv.mu.Unlock() | |
| 199 | + | return | |
| 200 | + | } | |
| 201 | + | h.wv.faviconEngineDocumentSeq = documentSeq | |
| 202 | + | h.wv.mu.Unlock() | |
| 196 | 203 | h.wv.runOnGTK(func() { | |
| 204 | + | if !h.faviconCallbackCurrent(cb, browser, documentSeq, pageURL) { | |
| 205 | + | return | |
| 206 | + | } | |
| 197 | 207 | cb.OnFaviconURLChanged(pageURL, candidates) | |
| 198 | 208 | }) | |
| 199 | 209 | } | |
| 200 | 210 | ||
| 211 | + | // faviconCallbackCurrent reports whether a queued favicon callback still | |
| 212 | + | // belongs to the current committed document, callback owner, and browser. The | |
| 213 | + | // GTK hop outlives the state it was queued for, so callbacks revalidate before | |
| 214 | + | // handing candidates to the UI. | |
| 215 | + | func (h *handlerSet) faviconCallbackCurrent(cb *port.WebViewCallbacks, browser purecef.Browser, documentSeq uint64, pageURL string) bool { | |
| 216 | + | if h == nil || h.wv == nil || cb == nil { | |
| 217 | + | return false | |
| 218 | + | } | |
| 219 | + | h.wv.mu.RLock() | |
| 220 | + | currentBrowser := h.wv.browser | |
| 221 | + | current := h.wv.callbacks == cb && h.wv.documentSeq == documentSeq && h.wv.uri == pageURL | |
| 222 | + | h.wv.mu.RUnlock() | |
| 223 | + | if !current { | |
| 224 | + | return false | |
| 225 | + | } | |
| 226 | + | if browser == nil { | |
| 227 | + | // No captured browser identity to compare; the document checks above | |
| 228 | + | // still bound the callback. | |
| 229 | + | return true | |
| 230 | + | } | |
| 231 | + | return currentBrowser != nil && currentBrowser.GetIdentifier() == browser.GetIdentifier() | |
| 232 | + | } | |
| 233 | + | ||
| 201 | 234 | // OnFullscreenModeChange toggles the fullscreen atomic and fires callbacks. | |
| 202 | 235 | func (h *handlerSet) OnFullscreenModeChange(_ purecef.Browser, fullscreen int32) { | |
| 203 | 236 | entering := fullscreen != 0 | |
| @@ -403,21 +436,45 @@ func (h *handlerSet) emitDiscoveredFaviconCandidates(browser purecef.Browser, pa | |||
| 403 | 436 | if h == nil || h.wv == nil || cb == nil || cb.OnFaviconURLChanged == nil { | |
| 404 | 437 | return | |
| 405 | 438 | } | |
| 439 | + | h.wv.mu.Lock() | |
| 440 | + | documentSeq := h.wv.documentSeq | |
| 441 | + | if h.wv.faviconEngineDocumentSeq == documentSeq || h.wv.faviconSourceCompletedDocumentSeq == documentSeq || | |
| 442 | + | h.wv.faviconSourcePendingToken != 0 || documentSeq == 0 { | |
| 443 | + | h.wv.mu.Unlock() | |
| 444 | + | return | |
| 445 | + | } | |
| 446 | + | h.wv.faviconSourceRequestSerial++ | |
| 447 | + | token := h.wv.faviconSourceRequestSerial | |
| 448 | + | h.wv.faviconSourcePendingToken = token | |
| 449 | + | h.wv.mu.Unlock() | |
| 450 | + | finish := func() { | |
| 451 | + | h.wv.mu.Lock() | |
| 452 | + | if h.wv.documentSeq == documentSeq && h.wv.faviconSourcePendingToken == token { | |
| 453 | + | h.wv.faviconSourcePendingToken = 0 | |
| 454 | + | h.wv.faviconSourceCompletedDocumentSeq = documentSeq | |
| 455 | + | } | |
| 456 | + | h.wv.mu.Unlock() | |
| 457 | + | } | |
| 406 | 458 | fallback := fallbackFaviconCandidates(pageURL) | |
| 407 | 459 | emit := func(candidates []string) { | |
| 408 | 460 | if len(candidates) == 0 { | |
| 409 | 461 | return | |
| 410 | 462 | } | |
| 411 | 463 | h.wv.runOnGTK(func() { | |
| 464 | + | if !h.faviconCallbackCurrent(cb, browser, documentSeq, pageURL) { | |
| 465 | + | return | |
| 466 | + | } | |
| 412 | 467 | cb.OnFaviconURLChanged(pageURL, candidates) | |
| 413 | 468 | }) | |
| 414 | 469 | } | |
| 415 | 470 | if browser == nil { | |
| 471 | + | finish() | |
| 416 | 472 | emit(fallback) | |
| 417 | 473 | return | |
| 418 | 474 | } | |
| 419 | 475 | frame := browser.GetMainFrame() | |
| 420 | 476 | if frame == nil { | |
| 477 | + | finish() | |
| 421 | 478 | emit(fallback) | |
| 422 | 479 | return | |
| 423 | 480 | } | |
| @@ -429,6 +486,16 @@ func (h *handlerSet) emitDiscoveredFaviconCandidates(browser purecef.Browser, pa | |||
| 429 | 486 | release() | |
| 430 | 487 | } | |
| 431 | 488 | }) | |
| 489 | + | finish() | |
| 490 | + | if !h.faviconCallbackCurrent(cb, browser, documentSeq, pageURL) { | |
| 491 | + | return | |
| 492 | + | } | |
| 493 | + | h.wv.mu.RLock() | |
| 494 | + | engineWon := h.wv.faviconEngineDocumentSeq == documentSeq | |
| 495 | + | h.wv.mu.RUnlock() | |
| 496 | + | if engineWon { | |
| 497 | + | return | |
| 498 | + | } | |
| 432 | 499 | candidates := DiscoverFaviconCandidates(pageURL, source) | |
| 433 | 500 | if len(candidates) == 0 { | |
| 434 | 501 | candidates = fallback | |
| @@ -442,6 +509,7 @@ func (h *handlerSet) emitDiscoveredFaviconCandidates(browser purecef.Browser, pa | |||
| 442 | 509 | release() | |
| 443 | 510 | } | |
| 444 | 511 | }) | |
| 512 | + | finish() | |
| 445 | 513 | }) | |
| 446 | 514 | frame.GetSource(visitor) | |
| 447 | 515 | } | |
| @@ -457,6 +525,11 @@ func (h *handlerSet) OnLoadStart(_ purecef.Browser, frame purecef.Frame, _ purec | |||
| 457 | 525 | // (same-page, error, and helper-driven commits). Main frame only. | |
| 458 | 526 | if h.wv != nil { | |
| 459 | 527 | h.wv.invalidateScrollMotion() | |
| 528 | + | h.wv.mu.Lock() | |
| 529 | + | h.wv.documentSeq++ | |
| 530 | + | h.wv.faviconSourcePendingToken = 0 | |
| 531 | + | h.wv.faviconSourceCompletedDocumentSeq = 0 | |
| 532 | + | h.wv.mu.Unlock() | |
| 460 | 533 | } | |
| 461 | 534 | bridgeNonce := h.wv.ensureBridgeNonce() | |
| 462 | 535 | if openerBridgeScript := h.wv.popupOpenerBridgeScript(bridgeNonce); openerBridgeScript != "" { | |
| @@ -535,7 +608,7 @@ func (h *handlerSet) OnLoadEnd(browser purecef.Browser, frame purecef.Frame, htt | |||
| 535 | 608 | // Capture navigation identity for the GTK hop from the callback's | |
| 536 | 609 | // browser: a process swap or a newer navigation can stale this | |
| 537 | 610 | // event before it runs. | |
| 538 | - | event := h.wv.captureInjectionEvent(browser) | |
| 611 | + | event := h.wv.captureInjectionEvent(browser, frameURL) | |
| 539 | 612 | h.wv.runOnGTK(func() { | |
| 540 | 613 | h.wv.engine.contentInj.onLoadEndForEvent(h.wv, event) | |
| 541 | 614 | }) | |
| @@ -178,26 +178,59 @@ func (h *handlerSet) OnTitleChange(_ purecef.Browser, title string) { | |||||
| 178 | h.wv.updateTitle(title) | 178 | h.wv.updateTitle(title) | ||
| 179 | } | 179 | } | ||
| 180 | 180 | ||||
| 181 | - | func (h *handlerSet) OnFaviconUrlchange(_ purecef.Browser, iconURLs purecef.StringList) { | 181 | + | func (h *handlerSet) OnFaviconUrlchange(browser purecef.Browser, iconURLs purecef.StringList) { |
| 182 | if h == nil || h.wv == nil { | 182 | if h == nil || h.wv == nil { | ||
| 183 | return | 183 | return | ||
| 184 | } | 184 | } | ||
| 185 | h.wv.mu.RLock() | 185 | h.wv.mu.RLock() | ||
| 186 | - | pageURL := h.wv.uri | 186 | + | pageURL, cb, currentBrowser, documentSeq := h.wv.uri, h.wv.callbacks, h.wv.browser, h.wv.documentSeq |
| 187 | - | cb := h.wv.callbacks | |||
| 188 | h.wv.mu.RUnlock() | 187 | h.wv.mu.RUnlock() | ||
| 189 | - | if cb == nil || cb.OnFaviconURLChanged == nil { | 188 | + | if cb == nil || cb.OnFaviconURLChanged == nil || browser == nil || currentBrowser == nil || |
| 189 | + | browser.GetIdentifier() != currentBrowser.GetIdentifier() { | |||
| 190 | return | 190 | return | ||
| 191 | } | 191 | } | ||
| 192 | candidates := resolveFaviconCandidates(pageURL, decodeCEFStringList(iconURLs)) | 192 | candidates := resolveFaviconCandidates(pageURL, decodeCEFStringList(iconURLs)) | ||
| 193 | if len(candidates) == 0 { | 193 | if len(candidates) == 0 { | ||
| 194 | return | 194 | return | ||
| 195 | } | 195 | } | ||
| 196 | + | h.wv.mu.Lock() | |||
| 197 | + | if h.wv.documentSeq != documentSeq || h.wv.uri != pageURL { | |||
| 198 | + | h.wv.mu.Unlock() | |||
| 199 | + | return | |||
| 200 | + | } | |||
| 201 | + | h.wv.faviconEngineDocumentSeq = documentSeq | |||
| 202 | + | h.wv.mu.Unlock() | |||
| 196 | h.wv.runOnGTK(func() { | 203 | h.wv.runOnGTK(func() { | ||
| 204 | + | if !h.faviconCallbackCurrent(cb, browser, documentSeq, pageURL) { | |||
| 205 | + | return | |||
| 206 | + | } | |||
| 197 | cb.OnFaviconURLChanged(pageURL, candidates) | 207 | cb.OnFaviconURLChanged(pageURL, candidates) | ||
| 198 | }) | 208 | }) | ||
| 199 | } | 209 | } | ||
| 200 | 210 | ||||
| 211 | + | // faviconCallbackCurrent reports whether a queued favicon callback still | |||
| 212 | + | // belongs to the current committed document, callback owner, and browser. The | |||
| 213 | + | // GTK hop outlives the state it was queued for, so callbacks revalidate before | |||
| 214 | + | // handing candidates to the UI. | |||
| 215 | + | func (h *handlerSet) faviconCallbackCurrent(cb *port.WebViewCallbacks, browser purecef.Browser, documentSeq uint64, pageURL string) bool { | |||
| 216 | + | if h == nil || h.wv == nil || cb == nil { | |||
| 217 | + | return false | |||
| 218 | + | } | |||
| 219 | + | h.wv.mu.RLock() | |||
| 220 | + | currentBrowser := h.wv.browser | |||
| 221 | + | current := h.wv.callbacks == cb && h.wv.documentSeq == documentSeq && h.wv.uri == pageURL | |||
| 222 | + | h.wv.mu.RUnlock() | |||
| 223 | + | if !current { | |||
| 224 | + | return false | |||
| 225 | + | } | |||
| 226 | + | if browser == nil { | |||
| 227 | + | // No captured browser identity to compare; the document checks above | |||
| 228 | + | // still bound the callback. | |||
| 229 | + | return true | |||
| 230 | + | } | |||
| 231 | + | return currentBrowser != nil && currentBrowser.GetIdentifier() == browser.GetIdentifier() | |||
| 232 | + | } | |||
| 233 | + | ||||
| 201 | // OnFullscreenModeChange toggles the fullscreen atomic and fires callbacks. | 234 | // OnFullscreenModeChange toggles the fullscreen atomic and fires callbacks. | ||
| 202 | func (h *handlerSet) OnFullscreenModeChange(_ purecef.Browser, fullscreen int32) { | 235 | func (h *handlerSet) OnFullscreenModeChange(_ purecef.Browser, fullscreen int32) { | ||
| 203 | entering := fullscreen != 0 | 236 | entering := fullscreen != 0 | ||
| @@ -403,21 +436,45 @@ func (h *handlerSet) emitDiscoveredFaviconCandidates(browser purecef.Browser, pa | |||||
| 403 | if h == nil || h.wv == nil || cb == nil || cb.OnFaviconURLChanged == nil { | 436 | if h == nil || h.wv == nil || cb == nil || cb.OnFaviconURLChanged == nil { | ||
| 404 | return | 437 | return | ||
| 405 | } | 438 | } | ||
| 439 | + | h.wv.mu.Lock() | |||
| 440 | + | documentSeq := h.wv.documentSeq | |||
| 441 | + | if h.wv.faviconEngineDocumentSeq == documentSeq || h.wv.faviconSourceCompletedDocumentSeq == documentSeq || | |||
| 442 | + | h.wv.faviconSourcePendingToken != 0 || documentSeq == 0 { | |||
| 443 | + | h.wv.mu.Unlock() | |||
| 444 | + | return | |||
| 445 | + | } | |||
| 446 | + | h.wv.faviconSourceRequestSerial++ | |||
| 447 | + | token := h.wv.faviconSourceRequestSerial | |||
| 448 | + | h.wv.faviconSourcePendingToken = token | |||
| 449 | + | h.wv.mu.Unlock() | |||
| 450 | + | finish := func() { | |||
| 451 | + | h.wv.mu.Lock() | |||
| 452 | + | if h.wv.documentSeq == documentSeq && h.wv.faviconSourcePendingToken == token { | |||
| 453 | + | h.wv.faviconSourcePendingToken = 0 | |||
| 454 | + | h.wv.faviconSourceCompletedDocumentSeq = documentSeq | |||
| 455 | + | } | |||
| 456 | + | h.wv.mu.Unlock() | |||
| 457 | + | } | |||
| 406 | fallback := fallbackFaviconCandidates(pageURL) | 458 | fallback := fallbackFaviconCandidates(pageURL) | ||
| 407 | emit := func(candidates []string) { | 459 | emit := func(candidates []string) { | ||
| 408 | if len(candidates) == 0 { | 460 | if len(candidates) == 0 { | ||
| 409 | return | 461 | return | ||
| 410 | } | 462 | } | ||
| 411 | h.wv.runOnGTK(func() { | 463 | h.wv.runOnGTK(func() { | ||
| 464 | + | if !h.faviconCallbackCurrent(cb, browser, documentSeq, pageURL) { | |||
| 465 | + | return | |||
| 466 | + | } | |||
| 412 | cb.OnFaviconURLChanged(pageURL, candidates) | 467 | cb.OnFaviconURLChanged(pageURL, candidates) | ||
| 413 | }) | 468 | }) | ||
| 414 | } | 469 | } | ||
| 415 | if browser == nil { | 470 | if browser == nil { | ||
| 471 | + | finish() | |||
| 416 | emit(fallback) | 472 | emit(fallback) | ||
| 417 | return | 473 | return | ||
| 418 | } | 474 | } | ||
| 419 | frame := browser.GetMainFrame() | 475 | frame := browser.GetMainFrame() | ||
| 420 | if frame == nil { | 476 | if frame == nil { | ||
| 477 | + | finish() | |||
| 421 | emit(fallback) | 478 | emit(fallback) | ||
| 422 | return | 479 | return | ||
| 423 | } | 480 | } | ||
| @@ -429,6 +486,16 @@ func (h *handlerSet) emitDiscoveredFaviconCandidates(browser purecef.Browser, pa | |||||
| 429 | release() | 486 | release() | ||
| 430 | } | 487 | } | ||
| 431 | }) | 488 | }) | ||
| 489 | + | finish() | |||
| 490 | + | if !h.faviconCallbackCurrent(cb, browser, documentSeq, pageURL) { | |||
| 491 | + | return | |||
| 492 | + | } | |||
| 493 | + | h.wv.mu.RLock() | |||
| 494 | + | engineWon := h.wv.faviconEngineDocumentSeq == documentSeq | |||
| 495 | + | h.wv.mu.RUnlock() | |||
| 496 | + | if engineWon { | |||
| 497 | + | return | |||
| 498 | + | } | |||
| 432 | candidates := DiscoverFaviconCandidates(pageURL, source) | 499 | candidates := DiscoverFaviconCandidates(pageURL, source) | ||
| 433 | if len(candidates) == 0 { | 500 | if len(candidates) == 0 { | ||
| 434 | candidates = fallback | 501 | candidates = fallback | ||
| @@ -442,6 +509,7 @@ func (h *handlerSet) emitDiscoveredFaviconCandidates(browser purecef.Browser, pa | |||||
| 442 | release() | 509 | release() | ||
| 443 | } | 510 | } | ||
| 444 | }) | 511 | }) | ||
| 512 | + | finish() | |||
| 445 | }) | 513 | }) | ||
| 446 | frame.GetSource(visitor) | 514 | frame.GetSource(visitor) | ||
| 447 | } | 515 | } | ||
| @@ -457,6 +525,11 @@ func (h *handlerSet) OnLoadStart(_ purecef.Browser, frame purecef.Frame, _ purec | |||||
| 457 | // (same-page, error, and helper-driven commits). Main frame only. | 525 | // (same-page, error, and helper-driven commits). Main frame only. | ||
| 458 | if h.wv != nil { | 526 | if h.wv != nil { | ||
| 459 | h.wv.invalidateScrollMotion() | 527 | h.wv.invalidateScrollMotion() | ||
| 528 | + | h.wv.mu.Lock() | |||
| 529 | + | h.wv.documentSeq++ | |||
| 530 | + | h.wv.faviconSourcePendingToken = 0 | |||
| 531 | + | h.wv.faviconSourceCompletedDocumentSeq = 0 | |||
| 532 | + | h.wv.mu.Unlock() | |||
| 460 | } | 533 | } | ||
| 461 | bridgeNonce := h.wv.ensureBridgeNonce() | 534 | bridgeNonce := h.wv.ensureBridgeNonce() | ||
| 462 | if openerBridgeScript := h.wv.popupOpenerBridgeScript(bridgeNonce); openerBridgeScript != "" { | 535 | if openerBridgeScript := h.wv.popupOpenerBridgeScript(bridgeNonce); openerBridgeScript != "" { | ||
| @@ -535,7 +608,7 @@ func (h *handlerSet) OnLoadEnd(browser purecef.Browser, frame purecef.Frame, htt | |||||
| 535 | // Capture navigation identity for the GTK hop from the callback's | 608 | // Capture navigation identity for the GTK hop from the callback's | ||
| 536 | // browser: a process swap or a newer navigation can stale this | 609 | // browser: a process swap or a newer navigation can stale this | ||
| 537 | // event before it runs. | 610 | // event before it runs. | ||
| 538 | - | event := h.wv.captureInjectionEvent(browser) | 611 | + | event := h.wv.captureInjectionEvent(browser, frameURL) |
| 539 | h.wv.runOnGTK(func() { | 612 | h.wv.runOnGTK(func() { | ||
| 540 | h.wv.engine.contentInj.onLoadEndForEvent(h.wv, event) | 613 | h.wv.engine.contentInj.onLoadEndForEvent(h.wv, event) | ||
| 541 | }) | 614 | }) | ||
M internal/ui/component/omnibox.go +32 -8
| @@ -184,6 +184,10 @@ type Omnibox struct { | |||
| 184 | 184 | saveInitialBehaviorFn func(context.Context, entity.OmniboxInitialBehavior) error | |
| 185 | 185 | ctx context.Context | |
| 186 | 186 | ||
| 187 | + | // scheduleSuggestionUpdate posts suggestion refreshes to the GTK main loop. | |
| 188 | + | // Tests inject a synchronous scheduler; nil falls back to glib.IdleAdd. | |
| 189 | + | scheduleSuggestionUpdate func(fn func()) | |
| 190 | + | ||
| 187 | 191 | // Callbacks | |
| 188 | 192 | onNavigate func(ctx context.Context, url string) error | |
| 189 | 193 | onClose func() | |
| @@ -1188,6 +1192,15 @@ func (o *Omnibox) handleCtrlNumberShortcut(keyval, keycode uint, ctrl bool) bool | |||
| 1188 | 1192 | return false | |
| 1189 | 1193 | } | |
| 1190 | 1194 | ||
| 1195 | + | // isGhostEcho reports whether the entry text is the debounced echo of Dumber's | |
| 1196 | + | // own ghost completion: realInput plus the suffix currently displayed. A | |
| 1197 | + | // matching echo must leave the ghost state untouched, but it cannot be told | |
| 1198 | + | // apart from a paste that replaced the selected suffix with the same text, so a | |
| 1199 | + | // deferred Enter still wins and the ghost state is rebuilt from the paste. | |
| 1200 | + | func isGhostEcho(entryText, realInput, ghostSuffix string) bool { | |
| 1201 | + | return ghostSuffix != "" && entryText == realInput+ghostSuffix | |
| 1202 | + | } | |
| 1203 | + | ||
| 1191 | 1204 | // onEntryChanged handles text input changes with debouncing. | |
| 1192 | 1205 | // When this fires, GTK has already processed the keystroke — if ghost text was | |
| 1193 | 1206 | // selected and the user typed, the selection was replaced naturally by GTK. | |
| @@ -1213,7 +1226,7 @@ func (o *Omnibox) onEntryChanged() { | |||
| 1213 | 1226 | // paste that replaced the selected ghost suffix with the same text, so a | |
| 1214 | 1227 | // deferred Enter always wins and the ghost state is rebuilt from the paste. | |
| 1215 | 1228 | o.mu.RLock() | |
| 1216 | - | ghostEcho := o.ghostSuffix != "" && entryText == o.realInput+o.ghostSuffix | |
| 1229 | + | ghostEcho := isGhostEcho(entryText, o.realInput, o.ghostSuffix) | |
| 1217 | 1230 | o.mu.RUnlock() | |
| 1218 | 1231 | if ghostEcho && !deferredSubmit { | |
| 1219 | 1232 | return | |
| @@ -3050,16 +3063,27 @@ func (o *Omnibox) UpdateZoomIndicator(factor float64) { | |||
| 3050 | 3063 | glib.IdleAdd(&cb, 0) | |
| 3051 | 3064 | } | |
| 3052 | 3065 | ||
| 3066 | + | // scheduleSuggestionUpdateOnGTK posts a suggestion refresh to the GTK main | |
| 3067 | + | // loop as a one-shot callback. | |
| 3068 | + | func scheduleSuggestionUpdateOnGTK(fn func()) { | |
| 3069 | + | var cb glib.SourceFunc = func(uintptr) bool { | |
| 3070 | + | fn() | |
| 3071 | + | return false | |
| 3072 | + | } | |
| 3073 | + | glib.IdleAdd(&cb, 0) | |
| 3074 | + | } | |
| 3075 | + | ||
| 3053 | 3076 | // idleAddUpdateSuggestions schedules updateSuggestions on the GTK main thread. | |
| 3054 | 3077 | func (o *Omnibox) idleAddUpdateSuggestions(suggestions []Suggestion, query string, token uint64) { | |
| 3055 | - | var cb glib.SourceFunc = func(data uintptr) bool { | |
| 3056 | - | if !o.isSearchTokenCurrent(token) { | |
| 3057 | - | return false | |
| 3058 | - | } | |
| 3059 | - | o.updateSuggestions(suggestions, query) | |
| 3060 | - | return false // One-shot callback | |
| 3078 | + | schedule := o.scheduleSuggestionUpdate | |
| 3079 | + | if schedule == nil { | |
| 3080 | + | schedule = scheduleSuggestionUpdateOnGTK | |
| 3061 | 3081 | } | |
| 3062 | - | glib.IdleAdd(&cb, 0) | |
| 3082 | + | schedule(func() { | |
| 3083 | + | if o.isSearchTokenCurrent(token) { | |
| 3084 | + | o.updateSuggestions(suggestions, query) | |
| 3085 | + | } | |
| 3086 | + | }) | |
| 3063 | 3087 | } | |
| 3064 | 3088 | ||
| 3065 | 3089 | // getFavoriteURLs returns a set of all favorited URLs for batch lookup. | |
| @@ -184,6 +184,10 @@ type Omnibox struct { | |||||
| 184 | saveInitialBehaviorFn func(context.Context, entity.OmniboxInitialBehavior) error | 184 | saveInitialBehaviorFn func(context.Context, entity.OmniboxInitialBehavior) error | ||
| 185 | ctx context.Context | 185 | ctx context.Context | ||
| 186 | 186 | ||||
| 187 | + | // scheduleSuggestionUpdate posts suggestion refreshes to the GTK main loop. | |||
| 188 | + | // Tests inject a synchronous scheduler; nil falls back to glib.IdleAdd. | |||
| 189 | + | scheduleSuggestionUpdate func(fn func()) | |||
| 190 | + | ||||
| 187 | // Callbacks | 191 | // Callbacks | ||
| 188 | onNavigate func(ctx context.Context, url string) error | 192 | onNavigate func(ctx context.Context, url string) error | ||
| 189 | onClose func() | 193 | onClose func() | ||
| @@ -1188,6 +1192,15 @@ func (o *Omnibox) handleCtrlNumberShortcut(keyval, keycode uint, ctrl bool) bool | |||||
| 1188 | return false | 1192 | return false | ||
| 1189 | } | 1193 | } | ||
| 1190 | 1194 | ||||
| 1195 | + | // isGhostEcho reports whether the entry text is the debounced echo of Dumber's | |||
| 1196 | + | // own ghost completion: realInput plus the suffix currently displayed. A | |||
| 1197 | + | // matching echo must leave the ghost state untouched, but it cannot be told | |||
| 1198 | + | // apart from a paste that replaced the selected suffix with the same text, so a | |||
| 1199 | + | // deferred Enter still wins and the ghost state is rebuilt from the paste. | |||
| 1200 | + | func isGhostEcho(entryText, realInput, ghostSuffix string) bool { | |||
| 1201 | + | return ghostSuffix != "" && entryText == realInput+ghostSuffix | |||
| 1202 | + | } | |||
| 1203 | + | ||||
| 1191 | // onEntryChanged handles text input changes with debouncing. | 1204 | // onEntryChanged handles text input changes with debouncing. | ||
| 1192 | // When this fires, GTK has already processed the keystroke — if ghost text was | 1205 | // When this fires, GTK has already processed the keystroke — if ghost text was | ||
| 1193 | // selected and the user typed, the selection was replaced naturally by GTK. | 1206 | // selected and the user typed, the selection was replaced naturally by GTK. | ||
| @@ -1213,7 +1226,7 @@ func (o *Omnibox) onEntryChanged() { | |||||
| 1213 | // paste that replaced the selected ghost suffix with the same text, so a | 1226 | // paste that replaced the selected ghost suffix with the same text, so a | ||
| 1214 | // deferred Enter always wins and the ghost state is rebuilt from the paste. | 1227 | // deferred Enter always wins and the ghost state is rebuilt from the paste. | ||
| 1215 | o.mu.RLock() | 1228 | o.mu.RLock() | ||
| 1216 | - | ghostEcho := o.ghostSuffix != "" && entryText == o.realInput+o.ghostSuffix | 1229 | + | ghostEcho := isGhostEcho(entryText, o.realInput, o.ghostSuffix) |
| 1217 | o.mu.RUnlock() | 1230 | o.mu.RUnlock() | ||
| 1218 | if ghostEcho && !deferredSubmit { | 1231 | if ghostEcho && !deferredSubmit { | ||
| 1219 | return | 1232 | return | ||
| @@ -3050,16 +3063,27 @@ func (o *Omnibox) UpdateZoomIndicator(factor float64) { | |||||
| 3050 | glib.IdleAdd(&cb, 0) | 3063 | glib.IdleAdd(&cb, 0) | ||
| 3051 | } | 3064 | } | ||
| 3052 | 3065 | ||||
| 3066 | + | // scheduleSuggestionUpdateOnGTK posts a suggestion refresh to the GTK main | |||
| 3067 | + | // loop as a one-shot callback. | |||
| 3068 | + | func scheduleSuggestionUpdateOnGTK(fn func()) { | |||
| 3069 | + | var cb glib.SourceFunc = func(uintptr) bool { | |||
| 3070 | + | fn() | |||
| 3071 | + | return false | |||
| 3072 | + | } | |||
| 3073 | + | glib.IdleAdd(&cb, 0) | |||
| 3074 | + | } | |||
| 3075 | + | ||||
| 3053 | // idleAddUpdateSuggestions schedules updateSuggestions on the GTK main thread. | 3076 | // idleAddUpdateSuggestions schedules updateSuggestions on the GTK main thread. | ||
| 3054 | func (o *Omnibox) idleAddUpdateSuggestions(suggestions []Suggestion, query string, token uint64) { | 3077 | func (o *Omnibox) idleAddUpdateSuggestions(suggestions []Suggestion, query string, token uint64) { | ||
| 3055 | - | var cb glib.SourceFunc = func(data uintptr) bool { | 3078 | + | schedule := o.scheduleSuggestionUpdate |
| 3056 | - | if !o.isSearchTokenCurrent(token) { | 3079 | + | if schedule == nil { |
| 3057 | - | return false | 3080 | + | schedule = scheduleSuggestionUpdateOnGTK |
| 3058 | - | } | |||
| 3059 | - | o.updateSuggestions(suggestions, query) | |||
| 3060 | - | return false // One-shot callback | |||
| 3061 | } | 3081 | } | ||
| 3062 | - | glib.IdleAdd(&cb, 0) | 3082 | + | schedule(func() { |
| 3083 | + | if o.isSearchTokenCurrent(token) { | |||
| 3084 | + | o.updateSuggestions(suggestions, query) | |||
| 3085 | + | } | |||
| 3086 | + | }) | |||
| 3063 | } | 3087 | } | ||
| 3064 | 3088 | ||||
| 3065 | // getFavoriteURLs returns a set of all favorited URLs for batch lookup. | 3089 | // getFavoriteURLs returns a set of all favorited URLs for batch lookup. | ||
M internal/ui/component/omnibox_initial_behavior_test.go +10 -0
| @@ -153,6 +153,12 @@ func TestOmniboxToggleInitialBehaviorPreference_RefreshesEmptyHistory(t *testing | |||
| 153 | 153 | assert.Equal(t, 1, refreshCalls) | |
| 154 | 154 | } | |
| 155 | 155 | ||
| 156 | + | // omniboxSuggestionUpdatesInline makes suggestion refreshes run synchronously | |
| 157 | + | // on the calling goroutine instead of through the GTK main loop. | |
| 158 | + | func omniboxSuggestionUpdatesInline(o *Omnibox) { | |
| 159 | + | o.scheduleSuggestionUpdate = func(fn func()) { fn() } | |
| 160 | + | } | |
| 161 | + | ||
| 156 | 162 | func TestOmniboxLoadInitialHistory_UsesCapturedInitialBehavior(t *testing.T) { | |
| 157 | 163 | repo := repomocks.NewMockHistoryRepository(t) | |
| 158 | 164 | done := make(chan struct{}) | |
| @@ -168,6 +174,7 @@ func TestOmniboxLoadInitialHistory_UsesCapturedInitialBehavior(t *testing.T) { | |||
| 168 | 174 | initialBehavior: entity.OmniboxInitialBehaviorRecent, | |
| 169 | 175 | ctx: context.Background(), | |
| 170 | 176 | } | |
| 177 | + | omniboxSuggestionUpdatesInline(o) | |
| 171 | 178 | ||
| 172 | 179 | o.loadInitialHistory(1) | |
| 173 | 180 | o.initialBehavior = entity.OmniboxInitialBehaviorMostVisited | |
| @@ -195,6 +202,7 @@ func TestOmniboxLoadInitialHistory_MostVisitedUsesThirtyDayWindow(t *testing.T) | |||
| 195 | 202 | mostVisitedDays: 30, | |
| 196 | 203 | ctx: context.Background(), | |
| 197 | 204 | } | |
| 205 | + | omniboxSuggestionUpdatesInline(o) | |
| 198 | 206 | ||
| 199 | 207 | o.loadInitialHistory(1) | |
| 200 | 208 | ||
| @@ -221,6 +229,7 @@ func TestOmniboxLoadInitialHistory_MostVisitedUsesConfiguredWindow(t *testing.T) | |||
| 221 | 229 | mostVisitedDays: 7, | |
| 222 | 230 | ctx: context.Background(), | |
| 223 | 231 | } | |
| 232 | + | omniboxSuggestionUpdatesInline(o) | |
| 224 | 233 | ||
| 225 | 234 | o.loadInitialHistory(1) | |
| 226 | 235 | ||
| @@ -247,6 +256,7 @@ func TestOmniboxLoadInitialHistory_MostVisitedZeroWindowUsesAllHistory(t *testin | |||
| 247 | 256 | mostVisitedDays: 0, | |
| 248 | 257 | ctx: context.Background(), | |
| 249 | 258 | } | |
| 259 | + | omniboxSuggestionUpdatesInline(o) | |
| 250 | 260 | ||
| 251 | 261 | o.loadInitialHistory(1) | |
| 252 | 262 | ||
| @@ -153,6 +153,12 @@ func TestOmniboxToggleInitialBehaviorPreference_RefreshesEmptyHistory(t *testing | |||||
| 153 | assert.Equal(t, 1, refreshCalls) | 153 | assert.Equal(t, 1, refreshCalls) | ||
| 154 | } | 154 | } | ||
| 155 | 155 | ||||
| 156 | + | // omniboxSuggestionUpdatesInline makes suggestion refreshes run synchronously | |||
| 157 | + | // on the calling goroutine instead of through the GTK main loop. | |||
| 158 | + | func omniboxSuggestionUpdatesInline(o *Omnibox) { | |||
| 159 | + | o.scheduleSuggestionUpdate = func(fn func()) { fn() } | |||
| 160 | + | } | |||
| 161 | + | ||||
| 156 | func TestOmniboxLoadInitialHistory_UsesCapturedInitialBehavior(t *testing.T) { | 162 | func TestOmniboxLoadInitialHistory_UsesCapturedInitialBehavior(t *testing.T) { | ||
| 157 | repo := repomocks.NewMockHistoryRepository(t) | 163 | repo := repomocks.NewMockHistoryRepository(t) | ||
| 158 | done := make(chan struct{}) | 164 | done := make(chan struct{}) | ||
| @@ -168,6 +174,7 @@ func TestOmniboxLoadInitialHistory_UsesCapturedInitialBehavior(t *testing.T) { | |||||
| 168 | initialBehavior: entity.OmniboxInitialBehaviorRecent, | 174 | initialBehavior: entity.OmniboxInitialBehaviorRecent, | ||
| 169 | ctx: context.Background(), | 175 | ctx: context.Background(), | ||
| 170 | } | 176 | } | ||
| 177 | + | omniboxSuggestionUpdatesInline(o) | |||
| 171 | 178 | ||||
| 172 | o.loadInitialHistory(1) | 179 | o.loadInitialHistory(1) | ||
| 173 | o.initialBehavior = entity.OmniboxInitialBehaviorMostVisited | 180 | o.initialBehavior = entity.OmniboxInitialBehaviorMostVisited | ||
| @@ -195,6 +202,7 @@ func TestOmniboxLoadInitialHistory_MostVisitedUsesThirtyDayWindow(t *testing.T) | |||||
| 195 | mostVisitedDays: 30, | 202 | mostVisitedDays: 30, | ||
| 196 | ctx: context.Background(), | 203 | ctx: context.Background(), | ||
| 197 | } | 204 | } | ||
| 205 | + | omniboxSuggestionUpdatesInline(o) | |||
| 198 | 206 | ||||
| 199 | o.loadInitialHistory(1) | 207 | o.loadInitialHistory(1) | ||
| 200 | 208 | ||||
| @@ -221,6 +229,7 @@ func TestOmniboxLoadInitialHistory_MostVisitedUsesConfiguredWindow(t *testing.T) | |||||
| 221 | mostVisitedDays: 7, | 229 | mostVisitedDays: 7, | ||
| 222 | ctx: context.Background(), | 230 | ctx: context.Background(), | ||
| 223 | } | 231 | } | ||
| 232 | + | omniboxSuggestionUpdatesInline(o) | |||
| 224 | 233 | ||||
| 225 | o.loadInitialHistory(1) | 234 | o.loadInitialHistory(1) | ||
| 226 | 235 | ||||
| @@ -247,6 +256,7 @@ func TestOmniboxLoadInitialHistory_MostVisitedZeroWindowUsesAllHistory(t *testin | |||||
| 247 | mostVisitedDays: 0, | 256 | mostVisitedDays: 0, | ||
| 248 | ctx: context.Background(), | 257 | ctx: context.Background(), | ||
| 249 | } | 258 | } | ||
| 259 | + | omniboxSuggestionUpdatesInline(o) | |||
| 250 | 260 | ||||
| 251 | o.loadInitialHistory(1) | 261 | o.loadInitialHistory(1) | ||
| 252 | 262 | ||||
M internal/ui/component/omnibox_navigation_state_test.go +21 -80
| @@ -1,11 +1,9 @@ | |||
| 1 | 1 | package component | |
| 2 | 2 | ||
| 3 | 3 | import ( | |
| 4 | - | "context" | |
| 5 | 4 | "testing" | |
| 6 | 5 | ||
| 7 | 6 | "github.com/bnema/puregotk/v4/gdk" | |
| 8 | - | "github.com/bnema/puregotk/v4/gtk" | |
| 9 | 7 | ) | |
| 10 | 8 | ||
| 11 | 9 | func TestShouldPreferTypedURLNavigation(t *testing.T) { | |
| @@ -86,92 +84,35 @@ func TestPasteSubmissionStateResetCancelsDeferredEnter(t *testing.T) { | |||
| 86 | 84 | } | |
| 87 | 85 | } | |
| 88 | 86 | ||
| 89 | - | // omniboxPasteHarness drives the omnibox paste path against a real GTK entry. | |
| 90 | - | type omniboxPasteHarness struct { | |
| 91 | - | omnibox *Omnibox | |
| 92 | - | navigated []string | |
| 93 | - | } | |
| 94 | - | ||
| 95 | - | func newOmniboxPasteHarness(t *testing.T) *omniboxPasteHarness { | |
| 96 | - | t.Helper() | |
| 97 | - | if !gtk.InitCheck() { | |
| 98 | - | t.Skip("GTK native display prerequisite unavailable (gtk.InitCheck returned false)") | |
| 99 | - | } | |
| 100 | - | entry := gtk.NewSearchEntry() | |
| 101 | - | if entry == nil { | |
| 102 | - | t.Fatal("failed to create a GTK search entry") | |
| 103 | - | } | |
| 104 | - | harness := &omniboxPasteHarness{} | |
| 105 | - | harness.omnibox = &Omnibox{ | |
| 106 | - | ctx: context.Background(), | |
| 107 | - | entry: entry, | |
| 108 | - | onNavigate: func(_ context.Context, targetURL string) error { | |
| 109 | - | harness.navigated = append(harness.navigated, targetURL) | |
| 110 | - | return nil | |
| 111 | - | }, | |
| 112 | - | } | |
| 113 | - | return harness | |
| 114 | - | } | |
| 115 | - | ||
| 116 | - | // pasteShortcut reproduces what the capture-phase key controller does on Ctrl+V. | |
| 117 | - | func (h *omniboxPasteHarness) pasteShortcut() { | |
| 118 | - | h.omnibox.mu.Lock() | |
| 119 | - | defer h.omnibox.mu.Unlock() | |
| 120 | - | h.omnibox.pasteSubmit.beginPaste(h.omnibox.entry.GetText()) | |
| 121 | - | } | |
| 122 | - | ||
| 123 | - | // showGhostSuffix displays input+suffix with the suffix selected, as the ghost | |
| 124 | - | // completion does, without emitting entry notifications. | |
| 125 | - | func (h *omniboxPasteHarness) showGhostSuffix(input, suffix string) { | |
| 126 | - | h.omnibox.entry.SetText(input + suffix) | |
| 127 | - | h.omnibox.mu.Lock() | |
| 128 | - | defer h.omnibox.mu.Unlock() | |
| 129 | - | h.omnibox.realInput = input | |
| 130 | - | h.omnibox.ghostSuffix = suffix | |
| 131 | - | h.omnibox.selectedIndex = -1 | |
| 132 | - | } | |
| 133 | - | ||
| 134 | - | func TestOmniboxReplaysDeferredEnterWhenPasteKeepsTheGhostSuffixText(t *testing.T) { | |
| 135 | - | harness := newOmniboxPasteHarness(t) | |
| 136 | - | harness.showGhostSuffix("https://example.com/gu", "ide") | |
| 87 | + | func TestPasteSubmissionStateReplaysEnterWhenPasteKeepsGhostSuffixText(t *testing.T) { | |
| 88 | + | state := pasteSubmissionState{} | |
| 89 | + | fullText := "https://example.com/guide" | |
| 90 | + | state.beginPaste(fullText) | |
| 137 | 91 | ||
| 138 | - | // Ctrl+V with clipboard content equal to the whole entry text, then Enter | |
| 139 | - | // before GTK delivers the pasted text. | |
| 140 | - | harness.pasteShortcut() | |
| 141 | - | harness.omnibox.handleSubmitKeyPress() | |
| 142 | - | if len(harness.navigated) != 0 { | |
| 143 | - | t.Fatalf("Enter must be deferred while the clipboard read is in flight, got %v", harness.navigated) | |
| 92 | + | if submit, _ := state.requestSubmit(fullText); submit { | |
| 93 | + | t.Fatal("Enter must be deferred while the clipboard replacement is pending") | |
| 144 | 94 | } | |
| 145 | - | ||
| 146 | - | // GTK reports the replacement. The text is unchanged, so it also matches the | |
| 147 | - | // ghost echo pattern; the pasted text still has to win. | |
| 148 | - | harness.omnibox.onEntryChanged() | |
| 149 | - | ||
| 150 | - | if len(harness.navigated) != 1 || harness.navigated[0] != "https://example.com/guide" { | |
| 151 | - | t.Fatalf("the deferred Enter must navigate once the paste lands, got %v", harness.navigated) | |
| 95 | + | if !state.textChanged() { | |
| 96 | + | t.Fatal("an unchanged replacement matching ghost text must replay deferred Enter") | |
| 152 | 97 | } | |
| 153 | - | if harness.omnibox.hasGhost() { | |
| 154 | - | t.Fatal("the pasted text must replace the ghost suffix") | |
| 98 | + | if state.textChanged() { | |
| 99 | + | t.Fatal("the replay must be consumed exactly once") | |
| 155 | 100 | } | |
| 156 | 101 | } | |
| 157 | 102 | ||
| 158 | - | func TestOmniboxGhostEchoAloneKeepsItsBehavior(t *testing.T) { | |
| 159 | - | harness := newOmniboxPasteHarness(t) | |
| 160 | - | harness.showGhostSuffix("https://example.com/gu", "ide") | |
| 103 | + | func TestGhostEchoDetectionKeepsSelfGeneratedCompletion(t *testing.T) { | |
| 104 | + | realInput := "https://example.com/gu" | |
| 105 | + | ghostSuffix := "ide" | |
| 106 | + | entryText := realInput + ghostSuffix | |
| 161 | 107 | ||
| 162 | - | harness.omnibox.onEntryChanged() | |
| 163 | - | ||
| 164 | - | if len(harness.navigated) != 0 { | |
| 165 | - | t.Fatalf("a self-generated ghost echo must not navigate, got %v", harness.navigated) | |
| 108 | + | if !isGhostEcho(entryText, realInput, ghostSuffix) { | |
| 109 | + | t.Fatal("the unchanged completion text must be recognized as a ghost echo") | |
| 110 | + | } | |
| 111 | + | if isGhostEcho(entryText+"/x", realInput, ghostSuffix) { | |
| 112 | + | t.Fatal("text beyond the completion must not be treated as a ghost echo") | |
| 166 | 113 | } | |
| 167 | - | harness.omnibox.mu.RLock() | |
| 168 | - | defer harness.omnibox.mu.RUnlock() | |
| 169 | - | if harness.omnibox.realInput != "https://example.com/gu" || harness.omnibox.ghostSuffix != "ide" { | |
| 170 | - | t.Fatalf( | |
| 171 | - | "a ghost echo must keep the ghost state, got input=%q suffix=%q", | |
| 172 | - | harness.omnibox.realInput, | |
| 173 | - | harness.omnibox.ghostSuffix, | |
| 174 | - | ) | |
| 114 | + | if isGhostEcho(entryText, realInput, "") { | |
| 115 | + | t.Fatal("absent ghost text must not be treated as a ghost echo") | |
| 175 | 116 | } | |
| 176 | 117 | } | |
| 177 | 118 | ||
| @@ -1,11 +1,9 @@ | |||||
| 1 | package component | 1 | package component | ||
| 2 | 2 | ||||
| 3 | import ( | 3 | import ( | ||
| 4 | - | "context" | |||
| 5 | "testing" | 4 | "testing" | ||
| 6 | 5 | ||||
| 7 | "github.com/bnema/puregotk/v4/gdk" | 6 | "github.com/bnema/puregotk/v4/gdk" | ||
| 8 | - | "github.com/bnema/puregotk/v4/gtk" | |||
| 9 | ) | 7 | ) | ||
| 10 | 8 | ||||
| 11 | func TestShouldPreferTypedURLNavigation(t *testing.T) { | 9 | func TestShouldPreferTypedURLNavigation(t *testing.T) { | ||
| @@ -86,92 +84,35 @@ func TestPasteSubmissionStateResetCancelsDeferredEnter(t *testing.T) { | |||||
| 86 | } | 84 | } | ||
| 87 | } | 85 | } | ||
| 88 | 86 | ||||
| 89 | - | // omniboxPasteHarness drives the omnibox paste path against a real GTK entry. | 87 | + | func TestPasteSubmissionStateReplaysEnterWhenPasteKeepsGhostSuffixText(t *testing.T) { |
| 90 | - | type omniboxPasteHarness struct { | 88 | + | state := pasteSubmissionState{} |
| 91 | - | omnibox *Omnibox | 89 | + | fullText := "https://example.com/guide" |
| 92 | - | navigated []string | 90 | + | state.beginPaste(fullText) |
| 93 | - | } | |||
| 94 | - | ||||
| 95 | - | func newOmniboxPasteHarness(t *testing.T) *omniboxPasteHarness { | |||
| 96 | - | t.Helper() | |||
| 97 | - | if !gtk.InitCheck() { | |||
| 98 | - | t.Skip("GTK native display prerequisite unavailable (gtk.InitCheck returned false)") | |||
| 99 | - | } | |||
| 100 | - | entry := gtk.NewSearchEntry() | |||
| 101 | - | if entry == nil { | |||
| 102 | - | t.Fatal("failed to create a GTK search entry") | |||
| 103 | - | } | |||
| 104 | - | harness := &omniboxPasteHarness{} | |||
| 105 | - | harness.omnibox = &Omnibox{ | |||
| 106 | - | ctx: context.Background(), | |||
| 107 | - | entry: entry, | |||
| 108 | - | onNavigate: func(_ context.Context, targetURL string) error { | |||
| 109 | - | harness.navigated = append(harness.navigated, targetURL) | |||
| 110 | - | return nil | |||
| 111 | - | }, | |||
| 112 | - | } | |||
| 113 | - | return harness | |||
| 114 | - | } | |||
| 115 | - | ||||
| 116 | - | // pasteShortcut reproduces what the capture-phase key controller does on Ctrl+V. | |||
| 117 | - | func (h *omniboxPasteHarness) pasteShortcut() { | |||
| 118 | - | h.omnibox.mu.Lock() | |||
| 119 | - | defer h.omnibox.mu.Unlock() | |||
| 120 | - | h.omnibox.pasteSubmit.beginPaste(h.omnibox.entry.GetText()) | |||
| 121 | - | } | |||
| 122 | - | ||||
| 123 | - | // showGhostSuffix displays input+suffix with the suffix selected, as the ghost | |||
| 124 | - | // completion does, without emitting entry notifications. | |||
| 125 | - | func (h *omniboxPasteHarness) showGhostSuffix(input, suffix string) { | |||
| 126 | - | h.omnibox.entry.SetText(input + suffix) | |||
| 127 | - | h.omnibox.mu.Lock() | |||
| 128 | - | defer h.omnibox.mu.Unlock() | |||
| 129 | - | h.omnibox.realInput = input | |||
| 130 | - | h.omnibox.ghostSuffix = suffix | |||
| 131 | - | h.omnibox.selectedIndex = -1 | |||
| 132 | - | } | |||
| 133 | - | ||||
| 134 | - | func TestOmniboxReplaysDeferredEnterWhenPasteKeepsTheGhostSuffixText(t *testing.T) { | |||
| 135 | - | harness := newOmniboxPasteHarness(t) | |||
| 136 | - | harness.showGhostSuffix("https://example.com/gu", "ide") | |||
| 137 | 91 | ||||
| 138 | - | // Ctrl+V with clipboard content equal to the whole entry text, then Enter | 92 | + | if submit, _ := state.requestSubmit(fullText); submit { |
| 139 | - | // before GTK delivers the pasted text. | 93 | + | t.Fatal("Enter must be deferred while the clipboard replacement is pending") |
| 140 | - | harness.pasteShortcut() | |||
| 141 | - | harness.omnibox.handleSubmitKeyPress() | |||
| 142 | - | if len(harness.navigated) != 0 { | |||
| 143 | - | t.Fatalf("Enter must be deferred while the clipboard read is in flight, got %v", harness.navigated) | |||
| 144 | } | 94 | } | ||
| 145 | - | 95 | + | if !state.textChanged() { | |
| 146 | - | // GTK reports the replacement. The text is unchanged, so it also matches the | 96 | + | t.Fatal("an unchanged replacement matching ghost text must replay deferred Enter") |
| 147 | - | // ghost echo pattern; the pasted text still has to win. | |||
| 148 | - | harness.omnibox.onEntryChanged() | |||
| 149 | - | ||||
| 150 | - | if len(harness.navigated) != 1 || harness.navigated[0] != "https://example.com/guide" { | |||
| 151 | - | t.Fatalf("the deferred Enter must navigate once the paste lands, got %v", harness.navigated) | |||
| 152 | } | 97 | } | ||
| 153 | - | if harness.omnibox.hasGhost() { | 98 | + | if state.textChanged() { |
| 154 | - | t.Fatal("the pasted text must replace the ghost suffix") | 99 | + | t.Fatal("the replay must be consumed exactly once") |
| 155 | } | 100 | } | ||
| 156 | } | 101 | } | ||
| 157 | 102 | ||||
| 158 | - | func TestOmniboxGhostEchoAloneKeepsItsBehavior(t *testing.T) { | 103 | + | func TestGhostEchoDetectionKeepsSelfGeneratedCompletion(t *testing.T) { |
| 159 | - | harness := newOmniboxPasteHarness(t) | 104 | + | realInput := "https://example.com/gu" |
| 160 | - | harness.showGhostSuffix("https://example.com/gu", "ide") | 105 | + | ghostSuffix := "ide" |
| 106 | + | entryText := realInput + ghostSuffix | |||
| 161 | 107 | ||||
| 162 | - | harness.omnibox.onEntryChanged() | 108 | + | if !isGhostEcho(entryText, realInput, ghostSuffix) { |
| 163 | - | 109 | + | t.Fatal("the unchanged completion text must be recognized as a ghost echo") | |
| 164 | - | if len(harness.navigated) != 0 { | 110 | + | } |
| 165 | - | t.Fatalf("a self-generated ghost echo must not navigate, got %v", harness.navigated) | 111 | + | if isGhostEcho(entryText+"/x", realInput, ghostSuffix) { |
| 112 | + | t.Fatal("text beyond the completion must not be treated as a ghost echo") | |||
| 166 | } | 113 | } | ||
| 167 | - | harness.omnibox.mu.RLock() | 114 | + | if isGhostEcho(entryText, realInput, "") { |
| 168 | - | defer harness.omnibox.mu.RUnlock() | 115 | + | t.Fatal("absent ghost text must not be treated as a ghost echo") |
| 169 | - | if harness.omnibox.realInput != "https://example.com/gu" || harness.omnibox.ghostSuffix != "ide" { | |||
| 170 | - | t.Fatalf( | |||
| 171 | - | "a ghost echo must keep the ghost state, got input=%q suffix=%q", | |||
| 172 | - | harness.omnibox.realInput, | |||
| 173 | - | harness.omnibox.ghostSuffix, | |||
| 174 | - | ) | |||
| 175 | } | 116 | } | ||
| 176 | } | 117 | } | ||
| 177 | 118 | ||||