Migrate AutomationEventRouter to ChildProcessId [chromium/src : main]
gwsq (Gerrit)
Message from gwsq![]( "Open in Gerrit")
From googleclient/chrome/chromium_gwsq/ipc/config.gwsq:
IPC: ort...@chromium.org
📎 It looks like you’re making a possibly security-sensitive change! 📎 IPC security review isn’t a rubberstamp, so your friendly security reviewer will need a fair amount of context to review your CL effectively. Please review your CL description and code comments to make sure they provide context for someone unfamiliar with your project/area. Pay special attention to where data comes from and which processes it flows between (and their privilege levels). Feel free to point your security reviewer at design docs, bugs, or other links if you can’t reasonably make a self-contained CL description. (Also see https://cbea.ms/git-commit/).
IPC reviewer(s): ort...@chromium.org
Reviewer source(s):
ort...@chromium.org is from context(googleclient/chrome/chromium_gwsq/ipc/config.gwsq)
Related details
- Giovanni Ortuno Urquidi
- Katie D
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Giovanni Ortuno Urquidi (Gerrit)
Giovanni Ortuno Urquidi added 1 comment![]( "Open in Gerrit")
content::ChildProcessId::FromUnsafeValue(Why do we use FromUnsafeValue here? Neither the comment on the method nor the bug really give much context.
I'm assuming a follow up change will change `service_worker_version_info.process_id` to use a ChildProcessId and then this won't be needed?
Related details
- Christopher Staite
- Katie D
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. |
Christopher Staite (Gerrit)
Christopher Staite added 1 comment![]( "Open in Gerrit")
content::ChildProcessId::FromUnsafeValue(Why do we use FromUnsafeValue here? Neither the comment on the method nor the bug really give much context.
I'm assuming a follow up change will change `service_worker_version_info.process_id` to use a ChildProcessId and then this won't be needed?
Correct, the end game for this patch set is in https://crrev.com/c/7621713, however that's rather large and so I've broken it up to get there slowly. I will be coming back to rectify this soon.
Related details
- Giovanni Ortuno Urquidi
- Katie D
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. |
Giovanni Ortuno Urquidi (Gerrit)
Giovanni Ortuno Urquidi voted and added 2 comments![]( "Open in Gerrit")
Votes added by Giovanni Ortuno Urquidi
| Code-Review | +1 |
2 comments
content::ChildProcessId::FromUnsafeValue(Christopher StaiteWhy do we use FromUnsafeValue here? Neither the comment on the method nor the bug really give much context.
I'm assuming a follow up change will change `service_worker_version_info.process_id` to use a ChildProcessId and then this won't be needed?
Correct, the end game for this patch set is in https://crrev.com/c/7621713, however that's rather large and so I've broken it up to get there slowly. I will be coming back to rectify this soon.
- Christopher Staite
- Katie D
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Katie D (Gerrit)
Katie D voted and added 1 comment![]( "Open in Gerrit")
Votes added by Katie D
| Code-Review | +1 |
1 comment
Sorry I missed this. extensions/browser/api/automation_internal LGTM
Related details
- Christopher Staite
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Christopher Staite (Gerrit)
Christopher Staite added 1 comment![]( "Open in Gerrit")
Thank you. Could I get a CQ+2 pleasse?
Related details
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Christopher Staite (Gerrit)
Christopher Staite voted Commit-Queue+2![]( "Open in Gerrit")
| Commit-Queue | +2 |
Related details
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Christopher Staite (Gerrit)
Christopher Staite voted Commit-Queue+2![]( "Open in Gerrit")
| Commit-Queue | +2 |
Related details
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)
Chromium LUCI CQ submitted the change with unreviewed changes![]( "Open in Gerrit")
Unreviewed changes
3 is the latest approved patch-set.
The change was submitted with unreviewed changes in the following files:
```
The name of the file: chrome/browser/extensions/chrome_content_browser_client_extensions_part_bindings.cc
Insertions: 2, Deletions: 4.
The diff is too large to show. Please review the diff.
```
Change information
Migrate AutomationEventRouter to ChildProcessId
Update `AutomationEventRouter` to use the strongly typed
`content::ChildProcessId` instead of raw integers for process IDs. This
change is part of a broader effort to replace unsafe integer process IDs
with strongly typed identifiers to improve type safety and reduce the
risk of ID mix-ups.
Since the function `RegisterListenerForOneTree` is unused, it was
removed rather than ported.
- M chrome/browser/extensions/chrome_content_browser_client_extensions_part_bindings.cc
- M extensions/browser/api/automation_internal/automation_event_router.cc
- M extensions/browser/api/automation_internal/automation_event_router.h
- M extensions/browser/api/automation_internal/automation_internal_api.cc
Code-Review: +1 by Katie D, +1 by Giovanni Ortuno Urquidi
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |