Migrate ServiceProcessHost::Observer to per-instance callbacks [chromium/src : main]
0 views
Skip to first unread message
Emily Andrews (Gerrit)
Mar 27, 2026, 2:42:12 PM (6 days ago) Mar 27
to Alex Gough, Chromium LUCI CQ, AyeAye, chromium...@chromium.org, Rijubrata Bhaumik, alexmo...@chromium.org, chfreme...@chromium.org, creis...@chromium.org, feature-me...@chromium.org, jophba...@chromium.org, mfoltz+wa...@chromium.org, navigation...@chromium.org, net-r...@chromium.org
Attention needed from Alex Gough
Emily Andrews voted and added 1 comment![]( "Open in Gerrit")
Votes added by Emily Andrews
| Commit-Queue | +1 |
1 comment
Patchset-level comments
File-level comment, Patchset 5 (Latest):
Emily Andrews . resolved
Hey Alex, would you mind giving this a gander? I think the global Service Process Host Observer pattern is a little not performant and it would be better to register an observer for the process when you launch. I have follow-ups to fix up the users of the global observer.
Related details
Attention is currently required from:
- Alex Gough
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Alex Gough (Gerrit)
Mar 27, 2026, 7:24:51 PM (6 days ago) Mar 27
to Emily Andrews, Alex Gough, Chromium LUCI CQ, AyeAye, chromium...@chromium.org, Rijubrata Bhaumik, alexmo...@chromium.org, chfreme...@chromium.org, creis...@chromium.org, feature-me...@chromium.org, jophba...@chromium.org, mfoltz+wa...@chromium.org, navigation...@chromium.org, net-r...@chromium.org
Attention needed from Emily Andrews
Alex Gough voted and added 2 comments![]( "Open in Gerrit")
Votes added by Alex Gough
| Code-Review | +1 |
2 comments
Patchset-level comments
Alex Gough . resolved
lgtm w/nit
File content/public/browser/service_process_host.h
Line 112, Patchset 5 (Latest):
Options& WithObserver(base::WeakPtr<Observer> observer);Alex Gough . unresolved
do we need to stop multiple observers being added (e.g. CHECK if there already is one?) - people might think the builder can build a bunch of them.
Related details
Attention is currently required from:
- Emily Andrews
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. |
Emily Andrews (Gerrit)
Mar 27, 2026, 11:55:28 PM (6 days ago) Mar 27
to Avi Drissman, Alex Gough, Chromium LUCI CQ, AyeAye, chromium...@chromium.org, Rijubrata Bhaumik, alexmo...@chromium.org, chfreme...@chromium.org, creis...@chromium.org, feature-me...@chromium.org, jophba...@chromium.org, mfoltz+wa...@chromium.org, navigation...@chromium.org, net-r...@chromium.org
Attention needed from Alex Gough and Avi Drissman
Emily Andrews added 2 comments![]( "Open in Gerrit")
Patchset-level comments
File-level comment, Patchset 6 (Latest):
Emily Andrews . resolved
Hey Avi, would you mind taking a look at this?
File content/public/browser/service_process_host.h
do we need to stop multiple observers being added (e.g. CHECK if there already is one?) - people might think the builder can build a bunch of them.
Attention is currently required from:
- Alex Gough
- Avi Drissman
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Avi Drissman (Gerrit)
Mar 30, 2026, 10:42:32 AM (3 days ago) Mar 30
to Emily Andrews, Avi Drissman, Alex Gough, Chromium LUCI CQ, AyeAye, chromium...@chromium.org, Rijubrata Bhaumik, alexmo...@chromium.org, chfreme...@chromium.org, creis...@chromium.org, feature-me...@chromium.org, jophba...@chromium.org, mfoltz+wa...@chromium.org, navigation...@chromium.org, net-r...@chromium.org
Attention needed from Alex Gough and Emily Andrews
Avi Drissman voted Code-Review+1![]( "Open in Gerrit")
Attention is currently required from:
- Alex Gough
- Emily Andrews
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Emily Andrews (Gerrit)
Mar 30, 2026, 11:56:42 AM (3 days ago) Mar 30
to Avi Drissman, Alex Gough, Chromium LUCI CQ, AyeAye, chromium...@chromium.org, Rijubrata Bhaumik, alexmo...@chromium.org, chfreme...@chromium.org, creis...@chromium.org, feature-me...@chromium.org, jophba...@chromium.org, mfoltz+wa...@chromium.org, navigation...@chromium.org, net-r...@chromium.org
Attention needed from Alex Gough
Emily Andrews voted and added 1 comment![]( "Open in Gerrit")
Votes added by Emily Andrews
| Auto-Submit | +1 |
1 comment
Patchset-level comments
File-level comment, Patchset 7 (Latest):
Emily Andrews . resolved
@AlexGough Could you do one last sign off? I added an ObserverHub so that multiple things could subscribe to a registered observer because as I was migrating over the apps I discovered a need for that.
Related details
Attention is currently required from:
- Alex Gough
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Emily Andrews (Gerrit)
Mar 30, 2026, 2:37:39 PM (3 days ago) Mar 30
to Avi Drissman, Alex Gough, Chromium LUCI CQ, AyeAye, chromium...@chromium.org, Rijubrata Bhaumik, alexmo...@chromium.org, chfreme...@chromium.org, creis...@chromium.org, feature-me...@chromium.org, jophba...@chromium.org, mfoltz+wa...@chromium.org, navigation...@chromium.org, net-r...@chromium.org
Attention needed from Alex Gough and Avi Drissman
Emily Andrews voted and added 1 comment![]( "Open in Gerrit")
Votes added by Emily Andrews
| Auto-Submit | +1 |
1 comment
Patchset-level comments
File-level comment, Patchset 9 (Latest):
Emily Andrews . resolved
Just adding another test to validate restarting hooks up the ObserverHub properly.
Related details
Attention is currently required from:
- Alex Gough
- Avi Drissman
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Avi Drissman (Gerrit)
Mar 30, 2026, 4:01:24 PM (3 days ago) Mar 30
to Emily Andrews, Avi Drissman, Alex Gough, Chromium LUCI CQ, AyeAye, chromium...@chromium.org, Rijubrata Bhaumik, alexmo...@chromium.org, chfreme...@chromium.org, creis...@chromium.org, feature-me...@chromium.org, jophba...@chromium.org, mfoltz+wa...@chromium.org, navigation...@chromium.org, net-r...@chromium.org
Attention needed from Alex Gough and Emily Andrews
Avi Drissman voted and added 1 comment![]( "Open in Gerrit")
Votes added by Avi Drissman
| Code-Review | +1 |
| Commit-Queue | +2 |
1 comment
Patchset-level comments
Attention is currently required from:
- Alex Gough
- Emily Andrews
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Avi Drissman (Gerrit)
Mar 31, 2026, 11:37:17 AM (2 days ago) Mar 31
to Emily Andrews, Avi Drissman, Alex Gough, Chromium LUCI CQ, AyeAye, chromium...@chromium.org, Rijubrata Bhaumik, alexmo...@chromium.org, chfreme...@chromium.org, creis...@chromium.org, feature-me...@chromium.org, jophba...@chromium.org, mfoltz+wa...@chromium.org, navigation...@chromium.org, net-r...@chromium.org
Attention needed from Alex Gough and Emily Andrews
Avi Drissman added 1 comment![]( "Open in Gerrit")
Patchset-level comments
Avi Drissman . resolved
@alex please restamp
Alex Gough (Gerrit)
Mar 31, 2026, 2:26:43 PM (2 days ago) Mar 31
to Emily Andrews, Alex Gough, Avi Drissman, Chromium LUCI CQ, AyeAye, chromium...@chromium.org, Rijubrata Bhaumik, alexmo...@chromium.org, chfreme...@chromium.org, creis...@chromium.org, feature-me...@chromium.org, jophba...@chromium.org, mfoltz+wa...@chromium.org, navigation...@chromium.org, net-r...@chromium.org
Attention needed from Emily Andrews
Alex Gough voted and added 1 comment![]( "Open in Gerrit")
Votes added by Alex Gough
| Code-Review | +1 |
1 comment
Patchset-level comments
Attention is currently required from:
- Emily Andrews
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Emily Andrews (Gerrit)
Apr 1, 2026, 12:13:11 AM (yesterday) Apr 1
to Alex Gough, Avi Drissman, Chromium LUCI CQ, AyeAye, chromium...@chromium.org, Rijubrata Bhaumik, alexmo...@chromium.org, chfreme...@chromium.org, creis...@chromium.org, feature-me...@chromium.org, jophba...@chromium.org, mfoltz+wa...@chromium.org, navigation...@chromium.org, net-r...@chromium.org
Emily Andrews voted Commit-Queue+2![]( "Open in Gerrit")
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Emily Andrews (Gerrit)
1:48 PM (6 hours ago) 1:48 PM
to Alex Gough, Avi Drissman, Chromium LUCI CQ, AyeAye, chromium...@chromium.org, Rijubrata Bhaumik, alexmo...@chromium.org, chfreme...@chromium.org, creis...@chromium.org, feature-me...@chromium.org, jophba...@chromium.org, mfoltz+wa...@chromium.org, navigation...@chromium.org, net-r...@chromium.org
Emily Andrews voted Commit-Queue+2![]( "Open in Gerrit")
| Commit-Queue | +2 |
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)
3:06 PM (5 hours ago) 3:06 PM
to Emily Andrews, Alex Gough, Avi Drissman, AyeAye, chromium...@chromium.org, Rijubrata Bhaumik, alexmo...@chromium.org, chfreme...@chromium.org, creis...@chromium.org, feature-me...@chromium.org, jophba...@chromium.org, mfoltz+wa...@chromium.org, navigation...@chromium.org, net-r...@chromium.org
Chromium LUCI CQ submitted the change![]( "Open in Gerrit")
Unreviewed changes
9 is the latest approved patch-set.
No files were changed between the latest approved patch-set and the submitted one.
Change information
Commit message:
Migrate ServiceProcessHost::Observer to per-instance callbacks
The global ServiceProcessHost::Observer broadcasts every service process
event to every observer. Each observer must filter by service type, and
this will cause duplicate notifications when process pooling shares a
process across multiple services.
This change adds a WithObserver Option to ServiceProcessHost::Options so
there is no need to register for global updates and then filter on a
particular service name.
Bug: 41493609
Change-Id: I1c11c71aa036bde76ac58830e5c86f5a3021e0b8
Reviewed-by: Alex Gough <aj...@chromium.org>
Auto-Submit: Emily Andrews <emi...@microsoft.com>
Commit-Queue: Emily Andrews <emi...@microsoft.com>
Reviewed-by: Avi Drissman <a...@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1609413}
Files:
- M content/browser/service_host/service_process_host_browsertest.cc
- M content/browser/service_host/service_process_host_impl.cc
- M content/browser/service_host/service_process_tracker.cc
- M content/browser/service_host/service_process_tracker.h
- M content/browser/service_host/utility_process_client.cc
- M content/browser/service_host/utility_process_client.h
- M content/public/browser/BUILD.gn
- M content/public/browser/service_process_host.cc
- M content/public/browser/service_process_host.h
- A content/public/browser/service_process_observer_hub.h
Change size: L
Delta: 10 files changed, 546 insertions(+), 26 deletions(-)
Branch: refs/heads/main
Submit Requirements:
Code-Review: +1 by Alex Gough, +1 by Avi Drissman
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Reply all
Reply to author
Forward
0 new messages