Fix favicon race during back/forward navigation
This commit is contained in:
parent
51a0b3222c
commit
26a88a4b2e
2 changed files with 119 additions and 0 deletions
|
|
@ -997,6 +997,7 @@ final class BrowserPanel: Panel, ObservableObject {
|
|||
private var loadingGeneration: Int = 0
|
||||
|
||||
private var faviconTask: Task<Void, Never>?
|
||||
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
|
||||
|
|
|
|||
85
tests/test_browser_favicon_navigation_regression.py
Normal file
85
tests/test_browser_favicon_navigation_regression.py
Normal file
|
|
@ -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())
|
||||
Loading…
Add table
Add a link
Reference in a new issue