From 26a88a4b2ef09ca0a8e9b8f803aac1d30be0a9dc Mon Sep 17 00:00:00 2001 From: Lawrence Chen <54008264+lawrencecchen@users.noreply.github.com> Date: Fri, 20 Feb 2026 20:17:58 -0800 Subject: [PATCH] Fix favicon race during back/forward navigation --- Sources/Panels/BrowserPanel.swift | 34 ++++++++ ...t_browser_favicon_navigation_regression.py | 85 +++++++++++++++++++ 2 files changed, 119 insertions(+) create mode 100644 tests/test_browser_favicon_navigation_regression.py diff --git a/Sources/Panels/BrowserPanel.swift b/Sources/Panels/BrowserPanel.swift index 1a7e9736..b034113f 100644 --- a/Sources/Panels/BrowserPanel.swift +++ b/Sources/Panels/BrowserPanel.swift @@ -997,6 +997,7 @@ final class BrowserPanel: Panel, ObservableObject { private var loadingGeneration: Int = 0 private var faviconTask: Task? + private var faviconRefreshGeneration: Int = 0 private var lastFaviconURLString: String? private let minPageZoom: CGFloat = 0.25 private let maxPageZoom: CGFloat = 5.0 @@ -1228,9 +1229,17 @@ final class BrowserPanel: Panel, ObservableObject { guard let pageURL = webView.url else { return } guard let scheme = pageURL.scheme?.lowercased(), scheme == "http" || scheme == "https" else { return } + let pageURLString = pageURL.absoluteString + faviconRefreshGeneration &+= 1 + let refreshGeneration = faviconRefreshGeneration faviconTask = Task { @MainActor [weak self, weak webView] in guard let self, let webView else { return } + guard self.isCurrentFaviconRefresh( + generation: refreshGeneration, + pageURLString: pageURLString, + webView: webView + ) else { return } // Try to discover the best icon URL from the document. let js = """ @@ -1264,6 +1273,11 @@ final class BrowserPanel: Panel, ObservableObject { discoveredURL = u } } + guard self.isCurrentFaviconRefresh( + generation: refreshGeneration, + pageURLString: pageURLString, + webView: webView + ) else { return } let fallbackURL = URL(string: "/favicon.ico", relativeTo: pageURL) let iconURL = discoveredURL ?? fallbackURL @@ -1288,6 +1302,11 @@ final class BrowserPanel: Panel, ObservableObject { } catch { return } + guard self.isCurrentFaviconRefresh( + generation: refreshGeneration, + pageURLString: pageURLString, + webView: webView + ) else { return } guard let http = response as? HTTPURLResponse, (200..<300).contains(http.statusCode) else { @@ -1301,6 +1320,16 @@ final class BrowserPanel: Panel, ObservableObject { } } + private func isCurrentFaviconRefresh( + generation: Int, + pageURLString: String, + webView: WKWebView + ) -> Bool { + guard !Task.isCancelled else { return false } + guard generation == faviconRefreshGeneration else { return false } + return webView.url?.absoluteString == pageURLString + } + @MainActor private static func makeFaviconPNGData(from raw: Data, targetPx: Int) -> Data? { guard let image = NSImage(data: raw) else { return nil } @@ -1359,6 +1388,11 @@ final class BrowserPanel: Panel, ObservableObject { private func handleWebViewLoadingChanged(_ newValue: Bool) { if newValue { + // Any new load invalidates older favicon fetches, even for same-URL reloads. + faviconRefreshGeneration &+= 1 + faviconTask?.cancel() + faviconTask = nil + lastFaviconURLString = nil loadingGeneration &+= 1 loadingEndWorkItem?.cancel() loadingEndWorkItem = nil diff --git a/tests/test_browser_favicon_navigation_regression.py b/tests/test_browser_favicon_navigation_regression.py new file mode 100644 index 00000000..1073eef8 --- /dev/null +++ b/tests/test_browser_favicon_navigation_regression.py @@ -0,0 +1,85 @@ +#!/usr/bin/env python3 +"""Static regression checks for favicon sync during browser navigation. + +Guards the race fix where stale async favicon fetches must not overwrite the +icon after the user navigates (including back/forward and same-URL reloads). +""" + +from __future__ import annotations + +import subprocess +from pathlib import Path + + +def repo_root() -> Path: + result = subprocess.run( + ["git", "rev-parse", "--show-toplevel"], + capture_output=True, + text=True, + ) + if result.returncode == 0: + return Path(result.stdout.strip()) + return Path(__file__).resolve().parents[1] + + +def extract_block(source: str, signature: str) -> str: + start = source.find(signature) + if start < 0: + raise ValueError(f"Missing signature: {signature}") + brace_start = source.find("{", start) + if brace_start < 0: + raise ValueError(f"Missing opening brace for: {signature}") + depth = 0 + for idx in range(brace_start, len(source)): + char = source[idx] + if char == "{": + depth += 1 + elif char == "}": + depth -= 1 + if depth == 0: + return source[brace_start : idx + 1] + raise ValueError(f"Unbalanced braces for: {signature}") + + +def main() -> int: + root = repo_root() + failures: list[str] = [] + + panel_path = root / "Sources" / "Panels" / "BrowserPanel.swift" + panel_source = panel_path.read_text(encoding="utf-8") + + if "private var faviconRefreshGeneration: Int = 0" not in panel_source: + failures.append("BrowserPanel is missing faviconRefreshGeneration state") + + refresh_block = extract_block(panel_source, "private func refreshFavicon(from webView: WKWebView)") + if "let pageURLString = pageURL.absoluteString" not in refresh_block: + failures.append("refreshFavicon() no longer captures pageURLString for staleness checks") + if refresh_block.count("isCurrentFaviconRefresh(") < 3: + failures.append("refreshFavicon() no longer checks staleness at each async stage") + + current_guard_block = extract_block(panel_source, "private func isCurrentFaviconRefresh(") + if "generation == faviconRefreshGeneration" not in current_guard_block: + failures.append("isCurrentFaviconRefresh() no longer validates refresh generation") + if "webView.url?.absoluteString == pageURLString" not in current_guard_block: + failures.append("isCurrentFaviconRefresh() no longer validates active page URL") + + loading_block = extract_block(panel_source, "private func handleWebViewLoadingChanged(_ newValue: Bool)") + if "faviconRefreshGeneration &+= 1" not in loading_block: + failures.append("handleWebViewLoadingChanged() no longer invalidates old favicon refreshes") + if "faviconTask?.cancel()" not in loading_block: + failures.append("handleWebViewLoadingChanged() no longer cancels stale favicon tasks") + if "lastFaviconURLString = nil" not in loading_block: + failures.append("handleWebViewLoadingChanged() no longer resets favicon URL cache on new loads") + + if failures: + print("FAIL: browser favicon navigation regression guard failed") + for item in failures: + print(f" - {item}") + return 1 + + print("PASS: browser favicon navigation guard is in place") + return 0 + + +if __name__ == "__main__": + raise SystemExit(main())