chore(core): document Chrome 143 upgrade decisions per PR review

Address inline review comments on PR #2187:

- src/browser.js: split the --disable-features comment into one bullet
  per feature. Each bullet documents the specific Chrome >=128/143
  behavior that broke and why disabling is the right fix
- src/install.js: add the procedure for picking per-platform revision
  numbers (chromiumdash + snapshot index) so future upgrades don't
  have to re-derive it
- test/helpers/server.js: only short-circuit /favicon.ico to 204 when
  the test hasn't supplied an explicit reply for it. Tests that want
  favicon-as-asset (none today, but future-proof) keep working via
  `server.reply('/favicon.ico', ...)`
- test/snapshot.test.js: expand the iframe srcdoc-serialization comment
  with a literal example of each form so the regex's "either form"
  intent is obvious

No behavioral change.

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>

Author: Manoj-Katta

packages/core/src/browser.js

@@ -20,14 +20,28 @@ export class Browser extends EventEmitter {
   #lastid = 0;
 
   args = [
-    // disable: translate popup, optimization downloads, baseline site
-    // isolation (so cross-origin sub-resources and worker fetches stay in
-    // the main renderer for interception), HTTPS-first navigation blocking,
-    // and Local Network Access permission checks (Chrome 143+ blocks
-    // sub-resource requests to *.localhost / 127.0.0.1 / RFC1918 with
-    // `LocalNetworkAccessPermissionDenied` when the document was served
-    // via Fetch.fulfillRequest from asset discovery — there is no permission
-    // prompt available in headless to grant the access).
+    // Disable a set of Chrome features that break Percy's asset-discovery
+    // assumptions in Chrome >=128 new-headless (which became the default when
+    // we upgraded from v126 to v143):
+    //   - Translate, OptimizationGuideModelDownloading: chatty background
+    //     features that aren't relevant for headless asset capture
+    //   - site-per-process: baseline cross-site renderer-process isolation
+    //     (default-on in v143). Without this off, cross-origin sub-resources
+    //     run in a different renderer process / CDP target than the page,
+    //     so Percy's page-session Network/Fetch listeners never see their
+    //     events
+    //   - IsolateOrigins: stricter, per-origin variant of site-per-process;
+    //     the existing `--disable-site-isolation-trials` flag below only
+    //     covers *opt-in trials* and doesn't override the baseline
+    //   - HttpsFirstBalancedModeAutoEnable: Chrome 143+ auto-upgrades HTTP
+    //     navigation to HTTPS and blocks otherwise with ERR_BLOCKED_BY_CLIENT,
+    //     which would break HTTP asset discovery (local dev, CI, staging URLs)
+    //   - LocalNetworkAccessChecks: Chrome 143+ requires explicit user
+    //     permission for sub-resource requests to *.localhost / 127.0.0.1 /
+    //     RFC1918. When asset discovery serves the root via
+    //     Fetch.fulfillRequest, sub-resource requests fail with
+    //     LocalNetworkAccessPermissionDenied. Headless has no UI to grant
+    //     the permission, so disable the check.
     '--disable-features=Translate,OptimizationGuideModelDownloading,IsolateOrigins,site-per-process,HttpsFirstBalancedModeAutoEnable,LocalNetworkAccessChecks',
     // disable several subsystems which run network requests in the background
     '--disable-background-networking',

packages/core/src/install.js

@@ -175,7 +175,18 @@ export function chromium({
   });
 }
 
-// default chromium revisions corresponds to v143.0.7499.169
+// Default Chromium revisions, pinned to Chrome 143.0.7499.169 (branch base
+// position 1536371). Per-platform numbers below are the *closest available*
+// revision for each OS — Chromium snapshots aren't always published at the
+// exact base position, so pick the nearest one whose archive exists.
+//
+// To pick numbers for a future upgrade:
+//   1. Find the target Chrome version + branch base position at
+//      https://chromiumdash.appspot.com/releases?platform=Mac
+//   2. Find the closest available revision per platform via the snapshots index
+//      https://commondatastorage.googleapis.com/chromium-browser-snapshots/index.html
+//      (folders: Linux_x64/, Mac/, Mac_Arm/, Win_x64/, Win/)
+//   3. Verify each archive exists before pinning the number here.
 chromium.revisions = {
   linux: '1536366',
   win64: '1536376',

packages/core/test/helpers/server.js

@@ -28,14 +28,18 @@ export function createTestServer({ default: defaultReply, ...replies }, port = 8
   server.route(async (req, res, next) => {
     let pathname = req.url.pathname;
     if (req.url.search) pathname += req.url.search;
-    // Chrome >=128 new headless auto-requests /favicon.ico for every navigation.
-    // Reply 204 so the browser doesn't capture it as a snapshot resource, and
-    // skip it from the requests log so per-test request assertions stay stable.
-    if (req.url.pathname === '/favicon.ico') {
+    let reply = replies[pathname] || defaultReply;
+    // Chrome >=128 new-headless auto-fetches /favicon.ico on every navigation.
+    // We can't distinguish that from a page-issued <link rel="icon"> request —
+    // both arrive at the test server identically. So if the test didn't
+    // explicitly set a /favicon.ico reply (`server.reply('/favicon.ico', ...)`),
+    // fall back to a 204 instead of letting the catch-all default reply turn
+    // it into a captured resource. Tests that *do* care about favicon-as-asset
+    // continue to work because their explicit reply takes precedence.
+    if (req.url.pathname === '/favicon.ico' && !replies['/favicon.ico']) {
       return res.writeHead(204).end();
     }
     server.requests.push(req.body ? [pathname, req.body, req.headers] : [pathname, req.headers]);
-    let reply = replies[pathname] || defaultReply;
     return reply ? await reply(req, res) : next();
   });
 

packages/core/test/snapshot.test.js

@@ -1535,9 +1535,14 @@ describe('Snapshot', () => {
 
       await percy.idle();
 
-      // srcdoc HTML attribute serialization: pre-Chrome-128 left `<` `>` literal
-      // inside attribute values; Chrome >=128 entity-escapes them per HTML5 spec.
-      // Accept either form.
+      // The iframe's srcdoc attribute holds serialized child-frame HTML. Pre
+      // Chrome-128 (old headless) left `<` and `>` literal inside the attribute
+      // value, so the captured DOM looked like:
+      //   <iframe ... srcdoc="<!DOCTYPE html>...<p>Foo</p>...">
+      // Chrome >=128 (new headless) entity-escapes special chars per HTML5
+      // spec, so the same DOM looks like:
+      //   <iframe ... srcdoc="&lt;!DOCTYPE html&gt;...&lt;p&gt;Foo&lt;/p&gt;...">
+      // Both render identically in any browser. Accept either serialization.
       expect(Buffer.from((
         api.requests['/builds/123/resources'][0]
           .body.data.attributes['base64-content']