Remove dead code from webcore C++ bindings - #35052
Conversation
ServerTimingParser and HeaderFieldTokenizer were brought in from WebKit alongside PerformanceServerTiming in #15341 but never wired up: parseServerTiming() has no callers (ResourceTiming.cpp includes the header but populates server timing from m_serverTiming directly), and HeaderFieldTokenizer's only consumer is ServerTimingParser. Also removes two unreferenced static functions that -Wno-unused-function was hiding: resolverFunctionCallback in bindings.cpp and containsCORSUnsafeRequestHeaderBytes in HTTPParsers.cpp (whose only call site has been commented out).
WalkthroughChangesThe change removes obsolete JSC and WebCore helper code, including Server-Timing parsing APIs and implementations, an unused resolver callback, a CORS header helper, and an unused include. Obsolete helper cleanup
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
Comment |
|
No |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
LGTM — pure dead-code removal, all four symbols confirmed unreferenced. The inline nit about ServerTiming::setParameter is a valid "one more hop" cleanup but non-blocking.
Extended reasoning...
Overview
This PR deletes 323 lines of unreferenced C++ from src/jsc/bindings/: two whole files (ServerTimingParser.{cpp,h}, HeaderFieldTokenizer.{cpp,h}), two static functions (resolverFunctionCallback in bindings.cpp, containsCORSUnsafeRequestHeaderBytes in HTTPParsers.cpp), and one dangling #include in ResourceTiming.cpp. No behavior is changed — every removed symbol was already unreachable.
Verification
I independently confirmed each deletion via repo-wide grep:
resolverFunctionCallback: zero hits.containsCORSUnsafeRequestHeaderBytes: only remaining hit is inside the commented-out block atHTTPParsers.cpp:909, matching the PR description.HeaderFieldTokenizer,ServerTimingParser,parseServerTiming: zero hits anywhere in the tree (including build config — the C++ source list is glob-generated byscripts/build/codegen.ts, so no manual list needs updating).RFC7230.h, previously included byHeaderFieldTokenizer.cpp, is still used byHTTPParsers.cppandHTTPHeaderField.{h,cpp}, so it is not stranded.
Security risks
None. This deletes code that was never called; nothing on any live request path is touched.
Level of scrutiny
Low. Mechanical dead-code removal in vendored WebCore glue, with a stated bun bd clean build and passing performance/headers tests. The only thing that could go wrong is a stale reference from generated code or a hand-maintained source list, and both were checked.
Other factors
The bug-hunter nit is accurate: deleting parseServerTiming() also strands ServerTiming::setParameter() and the single-arg ServerTiming(String&&) ctor. That's a fair follow-on per the "delete dead code in the same PR that makes it dead" rule, but it doesn't affect correctness of what's here and can be folded in or done separately. Not a reason to hold the PR.
ServerTiming::setParameter() and the 1-arg/3-arg ServerTiming constructors were only ever called by ServerTimingParser::parseServerTiming(), which the previous commit removed. Only the 5-arg constructor (used by isolatedCopy) remains.
There was a problem hiding this comment.
LGTM — pure dead-code removal, and the follow-up commit cleaned up the stranded ServerTiming members flagged earlier.
What was reviewed:
- Re-grepped
src/and repo root for every removed symbol (HeaderFieldTokenizer,ServerTimingParser,parseServerTiming,resolverFunctionCallback,containsCORSUnsafeRequestHeaderBytes,setParameter) — no remaining references outside the commented-out CORS block. - Confirmed no build-system file lists the deleted
.cpp/.hby name (source list is glob-generated). - Checked
RFC7230.h(previously included byHeaderFieldTokenizer.cpp) is still used byRFC7230.cpp, so not newly stranded.
Extended reasoning...
Overview
This PR deletes ~356 lines of unreferenced C++ from src/jsc/bindings/: four whole files (ServerTimingParser.{cpp,h}, HeaderFieldTokenizer.{cpp,h}), two static functions (resolverFunctionCallback in bindings.cpp, containsCORSUnsafeRequestHeaderBytes in HTTPParsers.cpp), one dangling #include, and — after my earlier feedback — the transitively-dead ServerTiming::setParameter() plus the 1-arg/3-arg ServerTiming constructors. Nothing is added or modified; every hunk is a deletion.
Security risks
None. All removed code was already unreachable at runtime: two file-local static functions with no call sites, a parser class only reachable via another deleted function, and struct members whose sole caller was that same deleted parser. Deleting dead code cannot change observable behavior. The only line touching security-adjacent semantics is containsCORSUnsafeRequestHeaderBytes, whose sole reference is inside a commented-out block in isCrossOriginSafeRequestHeader — that function unconditionally return true; both before and after this PR.
Level of scrutiny
Low. This is a mechanical dead-code sweep in the same shape as recently-merged #35002, #34878, and #34837. The correctness bar is "does anything still reference these symbols?" — I verified via repo-wide rg that nothing does (the one hit for containsCORSUnsafeRequestHeaderBytes is the commented-out call at HTTPParsers.cpp:909). No explicit build source list names the deleted files, so the glob-generated cxx-sources.txt/NativeFilenameCPP will regenerate cleanly. The PR body reports bun bd links and the Performance/Headers suites pass.
Other factors
My prior inline comment about stranded ServerTiming helpers was fully addressed in 06841ab (the author also caught the unused 3-arg ctor). I checked for further transitive stranding: RFC7230.h (formerly included by the deleted tokenizer) is still consumed by RFC7230.cpp, and the remaining 5-arg ServerTiming ctor is still used by both isolatedCopy() overloads. No new tests are expected for pure deletions with no behavior change, consistent with the cited precedent PRs.
Brings in #35002 (remove ~39k lines of dead Rust) and its follow-ups (#35019, #35052, #35152, #35225, #35293, #35326). The binary-size check compares against current main; this branch was 137 commits behind, so it still carried the dead code main dropped and registered as +630KB..+1.7MB on x64 while aarch64 linux showed -513KB/-601KB (different dead-code elimination outcomes per target). This stack's own native contribution is 19 files / +392 -45 lines; src/js is net -977 lines (domain.ts +692 vs fast-utf8-stream.ts -856 etc). Also: drop the hoisted pbkdf2 .bind handlers back to closures (review nit), and take main's expectations.txt since #34741 audited the stale ASAN entries.
2 participants
Removes 356 lines of unreferenced C++ from
src/jsc/bindings/.Deleted files
src/jsc/bindings/webcore/ServerTimingParser.{cpp,h}(117 lines): the only export,ServerTimingParser::parseServerTiming(), has no callers.ResourceTiming.cppincluded the header but builds itsPerformanceServerTimingentries fromm_serverTimingdirectly without ever invoking the parser. Brought in alongside thePerformanceServerTimingstubs in Stubperformance.markResourceTiming, addPerformanceResourceTiming,PerformanceServerTiming#15341 but never wired up.src/jsc/bindings/webcore/HeaderFieldTokenizer.{cpp,h}(187 lines): theHeaderFieldTokenizerclass is only instantiated byServerTimingParser::parseServerTiming()(above).rg HeaderFieldTokenizer src/ build/debug/codegen/returns nothing else.Removed members and static functions
ServerTiming::setParameter()and the 1-arg / 3-argServerTimingconstructors inwebcore/ServerTiming.{h,cpp}: their only caller wasparseServerTiming(). Only the 5-arg constructor used byisolatedCopy()remains.resolverFunctionCallbackinbindings.cpp:static, trivially returnsjsUndefined(), zero references anywhere in the tree.containsCORSUnsafeRequestHeaderBytesinwebcore/HTTPParsers.cpp:static, its only call site is inside a commented-out block at line ~909. The build sets-Wno-unused-function, so this never produced a warning.Also drops the now-dangling
#include "ServerTimingParser.h"fromResourceTiming.cpp.Verification
rgfor each removed symbol acrosssrc/,src/codegen/, andbuild/debug/codegen/returns only the definitions themselves (and the globbedcxx-sources.txt/NativeFilenameCPPtype union, which regenerate).bun bdbuilds clean.bun bd test test/js/web/timers/performance-entries.test.ts test/js/web/timers/performance.test.js test/js/web/fetch/headers.test.tspasses (107 tests).