Migrate blink::MemoryPressureListener to base::MemoryPressureListener [chromium/src : main]
38 views
Skip to first unread message
Patrick Monette (Gerrit)
Nov 17, 2025, 11:44:03 AM (5 days ago) Nov 17
to Francois Pierre Doray, Menard, Alexis, Stephen Chenney, Dirk Schulze, Kentaro Hara, Raphael Kubo da Costa, AyeAye, Chromium LUCI CQ, chromium...@chromium.org, Nate Chapin, apavlo...@chromium.org, blink-re...@chromium.org, blink-re...@chromium.org, blink-reviews-p...@chromium.org, blink-revie...@chromium.org, blink-rev...@chromium.org, gavinp...@chromium.org, fserb...@chromium.org, fmalit...@chromium.org, drott+bl...@chromium.org, blink-...@chromium.org, gavinp...@chromium.org, kinuko...@chromium.org, loading-re...@chromium.org, loading...@chromium.org
Attention needed from Francois Pierre Doray
Patrick Monette added 1 comment![]( "Open in Gerrit")
Patchset-level comments
File-level comment, Patchset 35 (Latest):
Patrick Monette . resolved
+Francois. Please take a first look.
Related details
Attention is currently required from:
- Francois Pierre Doray
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Francois Pierre Doray (Gerrit)
Nov 17, 2025, 4:43:56 PM (5 days ago) Nov 17
to Patrick Monette, Menard, Alexis, Stephen Chenney, Dirk Schulze, Kentaro Hara, Raphael Kubo da Costa, AyeAye, Chromium LUCI CQ, chromium...@chromium.org, Nate Chapin, apavlo...@chromium.org, blink-re...@chromium.org, blink-re...@chromium.org, blink-reviews-p...@chromium.org, blink-revie...@chromium.org, blink-rev...@chromium.org, gavinp...@chromium.org, fserb...@chromium.org, fmalit...@chromium.org, drott+bl...@chromium.org, blink-...@chromium.org, gavinp...@chromium.org, kinuko...@chromium.org, loading-re...@chromium.org, loading...@chromium.org
Attention needed from Patrick Monette
Francois Pierre Doray voted Code-Review+1![]( "Open in Gerrit")
Attention is currently required from:
- Patrick Monette
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Patrick Monette (Gerrit)
Nov 18, 2025, 11:14:40 AM (4 days ago) Nov 18
to Daniel Cheng, Francois Pierre Doray, Menard, Alexis, Stephen Chenney, Dirk Schulze, Kentaro Hara, Raphael Kubo da Costa, AyeAye, Chromium LUCI CQ, chromium...@chromium.org, Nate Chapin, apavlo...@chromium.org, blink-re...@chromium.org, blink-re...@chromium.org, blink-reviews-p...@chromium.org, blink-revie...@chromium.org, blink-rev...@chromium.org, gavinp...@chromium.org, fserb...@chromium.org, fmalit...@chromium.org, drott+bl...@chromium.org, blink-...@chromium.org, gavinp...@chromium.org, kinuko...@chromium.org, loading-re...@chromium.org, loading...@chromium.org
Attention needed from Daniel Cheng
Patrick Monette added 1 comment![]( "Open in Gerrit")
Patchset-level comments
File-level comment, Patchset 36 (Latest):
Patrick Monette . resolved
PTAL
Last generic cleanup CL for this.
Related details
Attention is currently required from:
- Daniel Cheng
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Daniel Cheng (Gerrit)
Nov 18, 2025, 1:28:00 PM (4 days ago) Nov 18
to Patrick Monette, Daniel Cheng, Francois Pierre Doray, Menard, Alexis, Stephen Chenney, Dirk Schulze, Kentaro Hara, Raphael Kubo da Costa, AyeAye, Chromium LUCI CQ, chromium...@chromium.org, Nate Chapin, apavlo...@chromium.org, blink-re...@chromium.org, blink-re...@chromium.org, blink-reviews-p...@chromium.org, blink-revie...@chromium.org, blink-rev...@chromium.org, gavinp...@chromium.org, fserb...@chromium.org, fmalit...@chromium.org, drott+bl...@chromium.org, blink-...@chromium.org, gavinp...@chromium.org, kinuko...@chromium.org, loading-re...@chromium.org, loading...@chromium.org
Attention needed from Patrick Monette
Daniel Cheng added 2 comments![]( "Open in Gerrit")
File third_party/blink/renderer/core/html/canvas/unique_font_selector.cc
Line 17, Patchset 36 (Parent):
if (base_selector != nullptr && IsMainThread()) {Daniel Cheng . unresolved
I'm assuming that it's correct to remove the IsMainThread() guard here because that was a limitation of the previous API.
File third_party/blink/renderer/platform/bindings/parkable_string_manager.cc
Line 541, Patchset 36 (Latest):
memory_pressure_listener_registration_->Dispose();Daniel Cheng . unresolved
This is kind of ugly. Do we need to re-register here at all? What happens if we don't?
Related details
Attention is currently required from:
- Patrick Monette
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Patrick Monette (Gerrit)
Nov 18, 2025, 1:44:28 PM (4 days ago) Nov 18
to Daniel Cheng, Francois Pierre Doray, Menard, Alexis, Stephen Chenney, Dirk Schulze, Kentaro Hara, Raphael Kubo da Costa, AyeAye, Chromium LUCI CQ, chromium...@chromium.org, Nate Chapin, apavlo...@chromium.org, blink-re...@chromium.org, blink-re...@chromium.org, blink-reviews-p...@chromium.org, blink-revie...@chromium.org, blink-rev...@chromium.org, gavinp...@chromium.org, fserb...@chromium.org, fmalit...@chromium.org, drott+bl...@chromium.org, blink-...@chromium.org, gavinp...@chromium.org, kinuko...@chromium.org, loading-re...@chromium.org, loading...@chromium.org
Attention needed from Daniel Cheng
Patrick Monette added 2 comments![]( "Open in Gerrit")
File third_party/blink/renderer/core/html/canvas/unique_font_selector.cc
Line 17, Patchset 36 (Parent):
if (base_selector != nullptr && IsMainThread()) {Daniel Cheng . resolved
I'm assuming that it's correct to remove the IsMainThread() guard here because that was a limitation of the previous API.
Patrick Monette
Yes now that blink::MemoryPressureListenerRegistration is always an async registration, it works on all threads now.
File third_party/blink/renderer/platform/bindings/parkable_string_manager.cc
This is kind of ugly. Do we need to re-register here at all? What happens if we don't?
Patrick Monette
Your comment made me realize I don't have to use the blink version of MemoryPressureListenerRegistration here, as ParkableStringManager is not garbage collected.
To answer the question though, it would have CHECK'ed since the blink version forces user to call Dispose().
Related details
Attention is currently required from:
- Daniel Cheng
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Daniel Cheng (Gerrit)
Nov 18, 2025, 7:22:44 PM (4 days ago) Nov 18
to Patrick Monette, Daniel Cheng, Francois Pierre Doray, Menard, Alexis, Stephen Chenney, Dirk Schulze, Kentaro Hara, Raphael Kubo da Costa, AyeAye, Chromium LUCI CQ, chromium...@chromium.org, Nate Chapin, apavlo...@chromium.org, blink-re...@chromium.org, blink-re...@chromium.org, blink-reviews-p...@chromium.org, blink-revie...@chromium.org, blink-rev...@chromium.org, gavinp...@chromium.org, fserb...@chromium.org, fmalit...@chromium.org, drott+bl...@chromium.org, blink-...@chromium.org, gavinp...@chromium.org, kinuko...@chromium.org, loading-re...@chromium.org, loading...@chromium.org
Attention needed from Patrick Monette
Daniel Cheng voted and added 2 comments![]( "Open in Gerrit")
Votes added by Daniel Cheng
| Code-Review | +1 |
2 comments
Patchset-level comments
File third_party/blink/renderer/platform/bindings/parkable_string_manager.cc
Line 541, Patchset 36:
memory_pressure_listener_registration_->Dispose();Daniel Cheng . unresolved
Patrick MonetteThis is kind of ugly. Do we need to re-register here at all? What happens if we don't?
Your comment made me realize I don't have to use the blink version of MemoryPressureListenerRegistration here, as ParkableStringManager is not garbage collected.
To answer the question though, it would have CHECK'ed since the blink version forces user to call Dispose().
Daniel Cheng
Sorry, my question wasn't precise. I meant:
"Do we need to reset this at all? If so, why?"
I'm still wondering about that, and I'll assume there's a good reason, but I'm curious what the answer is :)
Related details
Attention is currently required from:
- Patrick Monette
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Patrick Monette (Gerrit)
Nov 19, 2025, 1:22:16 PM (3 days ago) Nov 19
to Daniel Cheng, Francois Pierre Doray, Menard, Alexis, Stephen Chenney, Dirk Schulze, Kentaro Hara, Raphael Kubo da Costa, AyeAye, Chromium LUCI CQ, chromium...@chromium.org, Nate Chapin, apavlo...@chromium.org, blink-re...@chromium.org, blink-re...@chromium.org, blink-reviews-p...@chromium.org, blink-revie...@chromium.org, blink-rev...@chromium.org, gavinp...@chromium.org, fserb...@chromium.org, fmalit...@chromium.org, drott+bl...@chromium.org, blink-...@chromium.org, gavinp...@chromium.org, kinuko...@chromium.org, loading-re...@chromium.org, loading...@chromium.org
Patrick Monette voted and added 2 comments![]( "Open in Gerrit")
Votes added by Patrick Monette
| Commit-Queue | +2 |
2 comments
Patchset-level comments
Patrick Monette . resolved
Thanks!
File third_party/blink/renderer/platform/bindings/parkable_string_manager.cc
Line 541, Patchset 36:
memory_pressure_listener_registration_->Dispose();Daniel Cheng . resolved
Patrick MonetteThis is kind of ugly. Do we need to re-register here at all? What happens if we don't?
Daniel ChengYour comment made me realize I don't have to use the blink version of MemoryPressureListenerRegistration here, as ParkableStringManager is not garbage collected.
To answer the question though, it would have CHECK'ed since the blink version forces user to call Dispose().
Sorry, my question wasn't precise. I meant:
"Do we need to reset this at all? If so, why?"I'm still wondering about that, and I'll assume there's a good reason, but I'm curious what the answer is :)
Patrick Monette
Sorry I misunderstood the question!
I don't like the pattern of ResetForTesting(), but since that's the current design I'm staying with it. Resetting the registration ensures that any async notifications in the message queue that was sent before ResetForTesting() will not affect the next test.
Related details
Attention set is empty
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Chromium LUCI CQ (Gerrit)
Nov 19, 2025, 1:26:41 PM (3 days ago) Nov 19
to Patrick Monette, Daniel Cheng, Francois Pierre Doray, Menard, Alexis, Stephen Chenney, Dirk Schulze, Kentaro Hara, Raphael Kubo da Costa, AyeAye, chromium...@chromium.org, Nate Chapin, apavlo...@chromium.org, blink-re...@chromium.org, blink-re...@chromium.org, blink-reviews-p...@chromium.org, blink-revie...@chromium.org, blink-rev...@chromium.org, gavinp...@chromium.org, fserb...@chromium.org, fmalit...@chromium.org, drott+bl...@chromium.org, blink-...@chromium.org, gavinp...@chromium.org, kinuko...@chromium.org, loading-re...@chromium.org, loading...@chromium.org
Chromium LUCI CQ submitted the change![]( "Open in Gerrit")
Change information
Commit message:
Migrate blink::MemoryPressureListener to base::MemoryPressureListener
A future change will remove it, so that there is only one single
interface for listening to memory pressure in the code base.
Bug: 436324601
Change-Id: I822ad3cb579c90fd15b02c762aba61af791abfd2
Reviewed-by: Francois Pierre Doray <fdo...@chromium.org>
Reviewed-by: Daniel Cheng <dch...@chromium.org>
Commit-Queue: Patrick Monette <pmon...@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1547326}
Files:
- M base/memory/memory_pressure_listener.h
- M third_party/blink/renderer/core/css/threaded/text_renderer_threaded_test.cc
- M third_party/blink/renderer/core/html/canvas/DEPS
- M third_party/blink/renderer/core/html/canvas/unique_font_selector.cc
- M third_party/blink/renderer/core/html/canvas/unique_font_selector.h
- M third_party/blink/renderer/core/loader/resource/font_resource.cc
- M third_party/blink/renderer/core/loader/resource/image_resource.cc
- M third_party/blink/renderer/platform/bindings/parkable_string_manager.cc
- M third_party/blink/renderer/platform/bindings/parkable_string_manager.h
- M third_party/blink/renderer/platform/bindings/parkable_string_test.cc
- M third_party/blink/renderer/platform/fonts/plain_text_painter.cc
- M third_party/blink/renderer/platform/fonts/plain_text_painter.h
- M third_party/blink/renderer/platform/fonts/plain_text_painter_test.cc
- M third_party/blink/renderer/platform/loader/fetch/memory_cache.cc
- M third_party/blink/renderer/platform/loader/fetch/memory_cache.h
- M third_party/blink/renderer/platform/loader/fetch/resource.cc
- M third_party/blink/renderer/platform/loader/fetch/resource.h
- M third_party/blink/renderer/platform/loader/fetch/resource_fetcher.cc
- M third_party/blink/renderer/platform/loader/fetch/resource_fetcher.h
- M third_party/blink/tools/blinkpy/presubmit/audit_non_blink_usage.py
Change size: L
Delta: 20 files changed, 179 insertions(+), 121 deletions(-)
Branch: refs/heads/main
Submit Requirements:
Code-Review: +1 by Daniel Cheng, +1 by Francois Pierre Doray
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Reply all
Reply to author
Forward
0 new messages