perf(browser): always bulk clear cookies, not only when nothing is excluded

The fast path only fired when the jar held no Google cookie, but this import
exists for Google, so a Google cookie is normally present and the common case
still paid one native remove per cookie. Measured on Electron 43, clearData
excludeOrigins preserves the whole google.com registrable family -- host,
leading-dot, subdomain and partitioned cookies -- so the bulk call is safe with
cookies to keep, and the gate was never load-bearing for preservation.

Drop the caller-supplied exclusion predicate so the preserved set has exactly
one source of truth: a predicate that disagreed with NON_TRANSPLANTABLE_DOMAINS
would have deleted cookies the bulk call is meant to keep. The per-cookie loop
stays as the fallback for a rejected bulk clear, carrying the same exclusion.

src/main/browser/browser-cookie-import-partition-rollback.electron.test.ts

@@ -35,7 +35,7 @@ function buildFixtureMain(policyPath: string, resultPath: string): string {
   return `
 const { app, BrowserWindow, session } = require('electron')
 const { writeFileSync } = require('node:fs')
-const { removeAllCookiesExcept, isNonTransplantableCookieDomain } = require(${JSON.stringify(policyPath)})
+const { removeTransplantableCookies } = require(${JSON.stringify(policyPath)})
 const resultPath = ${JSON.stringify(resultPath)}
 let currentStep = 'starting'
 const mark = (step) => {
@@ -93,18 +93,20 @@ async function run() {
     }
   }
 
+  // Why: the bulk clear is the ordinary path now, so rejecting it is what routes this fixture onto
+  // the per-cookie fallback where a partial failure — and the old rollback — could happen at all.
   let bulkClearCalls = 0
   const clearSession = {
     cookies: store,
-    clearData: async (options) => {
+    clearData: async () => {
       bulkClearCalls++
-      return targetSession.clearData(options)
+      throw new Error('forced bulk clear failure')
     }
   }
 
   let clearError = null
   try {
-    await removeAllCookiesExcept(clearSession, (cookie) => isNonTransplantableCookieDomain(cookie.domain ?? ''))
+    await removeTransplantableCookies(clearSession)
   } catch (error) {
     clearError = String(error?.message || error)
   }
@@ -180,8 +182,9 @@ describe('non-Google partitioned cookie under a failed Electron cookie clear', (
     const result = await runFixture()
 
     expect(result.beforePartitionKey).toEqual(EXPECTED_PARTITION_KEY)
-    // Why (STA-4065): a bulk clear here would make every assertion below vacuous.
-    expect(result.bulkClearCalls).toBe(0)
+    // Why (STA-4065): the fallback is only reachable once the bulk clear has been tried and failed;
+    // if it ever succeeded here the partial-failure assertions below would be vacuous.
+    expect(result.bulkClearCalls).toBe(1)
     expect(result.remainingExcluded.map(({ name }) => name)).toEqual(['SID'])
     expect(result.clearError).toContain('Could not clear existing cookies')
     // STA-4061: rollback rebuilt this cookie through cookies.set, which drops partitionKey.

src/main/browser/browser-cookie-import-policy.test.ts

@@ -6,7 +6,7 @@ import {
   isNonTransplantableCookieDomain,
   NON_TRANSPLANTABLE_HOST_KEY_SQL,
   normalizeCookieDomain,
-  removeAllCookiesExcept,
+  removeTransplantableCookies,
   replaceCookiesForImportedDomains
 } from './browser-cookie-import-policy'
 
@@ -214,7 +214,9 @@ type CookieClearMocks = {
   clearData: Mock
 }
 
-describe('removeAllCookiesExcept', () => {
+describe('removeTransplantableCookies', () => {
+  const rejectingBulkClear = () => vi.fn().mockRejectedValue(new Error('storage busy'))
+
   function clearSession(cookies: Cookie[], overrides: Partial<CookieClearMocks> = {}) {
     const store = {
       get: vi.fn().mockResolvedValue(cookies),
@@ -232,10 +234,9 @@ describe('removeAllCookiesExcept', () => {
     }
   }
 
-  const excludesNonTransplantable = (existingCookie: Cookie): boolean =>
-    isNonTransplantableCookieDomain(existingCookie.domain ?? '')
-
-  it('never removes or reconstructs excluded Google cookies', async () => {
+  // Why (STA-4065): the bulk call is the ordinary path even when the jar holds cookies to keep —
+  // excludeOrigins preserves the whole google.com family, verified against real Electron.
+  it('clears a jar holding Google cookies in one call that excludes them', async () => {
     const { session, remove, set, clearData } = clearSession([
       cookie('.google.com', 'SID'),
       cookie('accounts.google.com', 'ACCOUNT'),
@@ -243,13 +244,12 @@ describe('removeAllCookiesExcept', () => {
       cookie('other.test', 'tracker', '/scoped')
     ])
 
-    await removeAllCookiesExcept(session, excludesNonTransplantable)
+    await removeTransplantableCookies(session)
 
-    expect(clearData).not.toHaveBeenCalled()
-    expect(remove.mock.calls).toEqual([
-      ['https://example.com/', 'session'],
-      ['https://other.test/scoped', 'tracker']
+    expect(clearData.mock.calls).toEqual([
+      [{ dataTypes: ['cookies'], excludeOrigins: ['https://google.com'] }]
     ])
+    expect(remove).not.toHaveBeenCalled()
     expect(set).not.toHaveBeenCalled()
   })
 
@@ -260,7 +260,7 @@ describe('removeAllCookiesExcept', () => {
       cookie('notgoogle.com', 'lookalike')
     ])
 
-    await removeAllCookiesExcept(session, excludesNonTransplantable)
+    await removeTransplantableCookies(session)
 
     expect(clearData.mock.calls).toEqual([
       [{ dataTypes: ['cookies'], excludeOrigins: ['https://google.com'] }]
@@ -272,7 +272,7 @@ describe('removeAllCookiesExcept', () => {
   it('touches nothing when the jar is already empty', async () => {
     const { session, remove, clearData } = clearSession([])
 
-    await removeAllCookiesExcept(session, excludesNonTransplantable)
+    await removeTransplantableCookies(session)
 
     expect(clearData).not.toHaveBeenCalled()
     expect(remove).not.toHaveBeenCalled()
@@ -283,7 +283,7 @@ describe('removeAllCookiesExcept', () => {
       { ...cookie('.example.com', 'session'), domain: '' }
     ])
 
-    await removeAllCookiesExcept(session, excludesNonTransplantable)
+    await removeTransplantableCookies(session)
 
     expect(clearData).toHaveBeenCalledOnce()
   })
@@ -291,10 +291,10 @@ describe('removeAllCookiesExcept', () => {
   it('falls back to per-cookie removal when the bulk clear rejects', async () => {
     const { session, remove, clearData } = clearSession(
       [cookie('.example.com', 'session'), cookie('other.test', 'tracker')],
-      { clearData: vi.fn().mockRejectedValue(new Error('storage busy')) }
+      { clearData: rejectingBulkClear() }
     )
 
-    await removeAllCookiesExcept(session, excludesNonTransplantable)
+    await removeTransplantableCookies(session)
 
     expect(clearData).toHaveBeenCalledOnce()
     expect(remove.mock.calls).toEqual([
@@ -303,6 +303,24 @@ describe('removeAllCookiesExcept', () => {
     ])
   })
 
+  // Why: the fallback carries the same exclusion as the bulk call, so a rejected clearData must
+  // not become the path that finally deletes a live Google session.
+  it('still preserves Google cookies on the per-cookie fallback', async () => {
+    const { session, remove, set } = clearSession(
+      [
+        cookie('.google.com', 'SID'),
+        cookie('accounts.google.com', 'ACCOUNT'),
+        cookie('.example.com', 'session')
+      ],
+      { clearData: rejectingBulkClear() }
+    )
+
+    await removeTransplantableCookies(session)
+
+    expect(remove.mock.calls).toEqual([['https://example.com/', 'session']])
+    expect(set).not.toHaveBeenCalled()
+  })
+
   // Why (STA-4061): reconstructing a removed cookie loses its partition key, and the snapshot
   // cannot say which cookies had one, so a failed clear must stay failed.
   it('never reconstructs removed cookies when another removal fails', async () => {
@@ -314,6 +332,7 @@ describe('removeAllCookiesExcept', () => {
         cookie('.example.com', 'third', '/three')
       ],
       {
+        clearData: rejectingBulkClear(),
         remove: vi.fn().mockImplementation(async (_url: string, name: string) => {
           if (name === 'second') {
             throw new Error('store unavailable')
@@ -322,7 +341,7 @@ describe('removeAllCookiesExcept', () => {
       }
     )
 
-    await expect(removeAllCookiesExcept(session, excludesNonTransplantable)).rejects.toThrow(
+    await expect(removeTransplantableCookies(session)).rejects.toThrow(
       'the session was left partially cleared'
     )
     expect(remove).toHaveBeenCalledTimes(3)
@@ -342,6 +361,7 @@ describe('removeAllCookiesExcept', () => {
         ...Array.from({ length: 12 }, (_, index) => cookie('.example.com', `${index}`))
       ],
       {
+        clearData: rejectingBulkClear(),
         remove: vi.fn().mockImplementation(async () => {
           active++
           maxActive = Math.max(maxActive, active)
@@ -351,7 +371,7 @@ describe('removeAllCookiesExcept', () => {
       }
     )
 
-    const clearing = removeAllCookiesExcept(session, excludesNonTransplantable)
+    const clearing = removeTransplantableCookies(session)
     await vi.waitFor(() => expect(remove).toHaveBeenCalledTimes(8))
     expect(maxActive).toBe(8)
     releaseRemovals?.()
@@ -373,6 +393,7 @@ describe('removeAllCookiesExcept', () => {
         { ...cookie('example.com', 'session'), hostOnly: true }
       ],
       {
+        clearData: rejectingBulkClear(),
         remove: vi
           .fn()
           .mockImplementationOnce(() => firstReleased)
@@ -380,7 +401,7 @@ describe('removeAllCookiesExcept', () => {
       }
     )
 
-    const clearing = removeAllCookiesExcept(session, excludesNonTransplantable)
+    const clearing = removeTransplantableCookies(session)
     await vi.waitFor(() => expect(remove).toHaveBeenCalledOnce())
     releaseFirst?.()
     await clearing

src/main/browser/browser-cookie-import-policy.ts

@@ -211,32 +211,36 @@ export type CookieClearSession = {
 // can downgrade a partitioned (CHIPS) cookie into an unpartitioned one — and nothing in the
 // snapshot says which cookies are at risk. A partially cleared jar is a retryable import failure;
 // a downgraded cookie is unrecoverable auth-state corruption that survives restart.
-export async function removeAllCookiesExcept(
-  targetSession: CookieClearSession,
-  isExcluded: (cookie: Cookie) => boolean
+// Why (STA-4065): the exclusion is module state rather than a parameter so the predicate and the
+// origins the bulk clear preserves cannot drift apart — a caller-supplied predicate that disagreed
+// with NON_TRANSPLANTABLE_DOMAINS would silently delete a cookie the bulk call is meant to keep.
+export async function removeTransplantableCookies(
+  targetSession: CookieClearSession
 ): Promise<void> {
   const store = targetSession.cookies
   const existingCookies = await store.get({})
+  if (existingCookies.length === 0) {
+    return
+  }
 
-  // Why: an unfiltered get() is the whole jar, so no exclusion in it means nothing to preserve and
-  // one bulk clear can replace a remove() per cookie. An empty jar needs neither.
-  if (existingCookies.length > 0 && !existingCookies.some(isExcluded)) {
-    try {
-      // Why: exclusions protect a non-transplantable cookie arriving between snapshot and clear.
-      await targetSession.clearData({
-        dataTypes: ['cookies'],
-        excludeOrigins: NON_TRANSPLANTABLE_CLEAR_EXCLUDED_ORIGINS
-      })
-      return
-    } catch {
-      // Why: a rejected bulk clear can still have emptied part of the jar, so fall through to the
-      // per-cookie path and let it re-remove whatever survived.
-    }
+  // Why (STA-4065): measured on Electron 43, excludeOrigins preserves the whole registrable
+  // family — host, leading-dot, subdomain, and partitioned Google cookies — so one call replaces a
+  // remove() per cookie even when the jar holds cookies to keep. That is the ordinary case here:
+  // this import exists for Google, so a Google cookie is usually present.
+  try {
+    await targetSession.clearData({
+      dataTypes: ['cookies'],
+      excludeOrigins: NON_TRANSPLANTABLE_CLEAR_EXCLUDED_ORIGINS
+    })
+    return
+  } catch {
+    // Why: a rejected bulk clear can still have emptied part of the jar, so fall through to the
+    // per-cookie path and let it re-remove whatever survived.
   }
 
   const removableGroups = new Map<string, { cookie: Cookie; url: string }[]>()
   for (const cookie of existingCookies) {
-    if (isExcluded(cookie)) {
+    if (isNonTransplantableCookieDomain(cookie.domain ?? '')) {
       continue
     }
     const domain = cookie.domain ? normalizeCookieDomain(cookie.domain) : null

src/main/browser/browser-cookie-import.ts

@@ -80,7 +80,7 @@ import {
   NON_TRANSPLANTABLE_HOST_KEY_SQL,
   normalizeCookieDomain,
   normalizeCookieImportDomain,
-  removeAllCookiesExcept,
+  removeTransplantableCookies,
   replaceCookiesForImportedDomains,
   restoreImportedDomainCookies,
   type CookieImportMode
@@ -1780,9 +1780,7 @@ export async function importCookiesFromBrowser(
     // Why: clear stale cookies first; mixing them with the imported set makes sites reject the
     // session. Non-transplantable families are exempt — nothing was imported for them, and their
     // live session is the only one that works.
-    await removeAllCookiesExcept(targetSession, (cookie) =>
-      isNonTransplantableCookieDomain(cookie.domain ?? '')
-    )
+    await removeTransplantableCookies(targetSession)
     diag(
       `  cleared existing session cookies before loading ${decryptedCookies.length} imported cookies`
     )