Update SSLClientSessionCache to handle stateful memory pressure [chromium/src : main]

0 views

Skip to first unread message

Patrick Monette (Gerrit)

unread,

Dec 5, 2025, 5:10:15 PM (2 days ago) Dec 5

to Chromium Metrics Reviews, Menard, Alexis, Mangesh Ghiware, Sophie Chang, Lei Zhang, Stephen Chenney, Mark Schillaci, Dirk Schulze, (Julie)Jeongeun Kim, Heron Yang, AyeAye, Matt Mueller, Chromium LUCI CQ, chromium...@chromium.org, webauthn...@chromium.org, vakh+safe_br...@chromium.org, drott+bl...@chromium.org, dullweb...@chromium.org, hanxi...@chromium.org, sloboda...@chromium.org, omnibox-...@chromium.org, dtseng...@chromium.org, ayman...@chromium.org, cblume...@chromium.org, cc-...@chromium.org, twelling...@chromium.org, nwoked...@chromium.org, apavlo...@chromium.org, mattsimm...@chromium.org, vasilii+watchlis...@chromium.org, asvitkine...@chromium.org, blink-revie...@chromium.org, christia...@chromium.org, ios-r...@chromium.org, tmartino+tran...@chromium.org, feature-me...@chromium.org, yusufo...@chromium.org, fmalit...@chromium.org, derinel+wat...@google.com, yuzo+...@chromium.org, telemetr...@chromium.org, jdonnel...@chromium.org, devtools...@chromium.org, andysjl...@chromium.org, gcasto+w...@chromium.org, dewitt...@chromium.org, thegreenf...@chromium.org, druber...@chromium.org, chromiumme...@microsoft.com, blink-reviews-p...@chromium.org, peilinwa...@google.com, blink-rev...@chromium.org, ender...@chromium.org, nektar...@chromium.org, kyungjunle...@google.com, language...@chromium.org, trewin...@google.com, penghuan...@chromium.org, extension...@chromium.org, lwinston+watc...@google.com, srahim...@chromium.org, josiah...@chromium.org, ios-revie...@chromium.org, mac-r...@chromium.org, dtapuska+...@chromium.org, blink-...@chromium.org, davidj...@chromium.org, yuezhang...@chromium.org, jmedle...@chromium.org, wychen...@chromium.org, blink-re...@chromium.org, agriev...@chromium.org, mfoltz+wa...@chromium.org, kinuko...@chromium.org, asvitki...@chromium.org, marq+...@chromium.org, msrame...@chromium.org, zackha...@chromium.org, donnd...@chromium.org, filesapp...@chromium.org, gangwu...@chromium.org, jdeblas...@chromium.org, mdjone...@chromium.org, meilian...@chromium.org, chromium-a...@chromium.org, francisjp...@google.com, android-web...@chromium.org, jatapiaro+wat...@google.com, browser-comp...@chromium.org, xinghui...@chromium.org, gogeral...@chromium.org, abigailbk...@google.com, blink-re...@chromium.org, blink-re...@chromium.org, fserb...@chromium.org, drott...@chromium.org, gavinp...@chromium.org, net-r...@chromium.org

Attention needed from Matt Mueller

Patrick Monette added 3 comments

Patchset-level comments

File-level comment, Patchset 8 (Latest):

Patrick Monette . resolved

Thanks for the review. You uncovered a major issue.

PTAnL

File base/memory/memory_pressure_listener_registry.cc

Line 154, Patchset 4: run_loop.Run();

Matt Mueller . resolved

This method is named as being async no longer async if it's running a loop to wait for the task to have finished

Patrick Monette

Good point. The name is mostly to mean that it's to be used for async memory pressure listeners, but I agree that it doesn't feel right. I ended up with another version in https://chromium-review.googlesource.com/c/chromium/src/+/7229564 where the function is truly async, but you can pass the QuitClosure as a parameter so the synchronous part is done by the calling code.

File net/ssl/ssl_client_session_cache.cc

Line 219, Patchset 4: cache_.UpdateMaxSize(0);

Matt Mueller . resolved

Does this actually work? The unittest does not actually test what happens when you try to insert an element after setting the max to zero. lru_cache special cases 0 as NO_AUTO_EVICT, meaning it no longer manages the size at all. (And if it didn't special case 0, it would cause an overflow at the line it does ` ShrinkToSize(max_size_ - 1);`)

Patrick Monette

Yeah I missed a big issue here. Thanks!

I needed more changes to LRUCache which I did in this CL: https://chromium-review.googlesource.com/c/chromium/src/+/7228584.

I also added a test to ensure inserting an element fails when the max is zero.

Open in Gerrit

Related details

Attention is currently required from:

Matt Mueller

Submit Requirements:

Code-Coverage
Code-Owners
Code-Review
Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Patrick Monette (Gerrit)

unread,

Dec 5, 2025, 10:31:26 PM (2 days ago) Dec 5

Attention needed from Matt Mueller

Patrick Monette added 1 comment

Patchset-level comments

File-level comment, Patchset 9 (Latest):

Patrick Monette . resolved

Unfortunately a bad rebase added a bunch of bots that keep failing, but the CQ passes.