memory_instrumentation: Add DetailedMetricsDelegate and Mojo fields [chromium/src : main]
Adam Walls (Gerrit)
Adam Walls abandoned this change.
Adam Walls abandoned this change![]( "Open in Gerrit")
Related details
Code-Coverage
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Adam Walls (Gerrit)
Adam Walls abandoned this change.
Adam Walls abandoned this change![]( "Open in Gerrit")
Related details
Code-Coverage
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Adam Walls (Gerrit)
Adam Walls added 1 comment![]( "Open in Gerrit")
namespace memory_instrumentation {
namespace features {Please fix this WARNING reported by ClangTidy: check: modernize-concat-nested-namespaces
nested namespaces can be concatenated...
check: modernize-concat-nested-namespaces
nested namespaces can be concatenated (https://clang.llvm.org/extra/clang-tidy/checks/modernize/concat-nested-namespaces.html)
(Note: You can add `Skip-Clang-Tidy-Checks: modernize-concat-nested-namespaces` footer to the CL description to skip the check)
(Lint observed on `linux-clang-tidy-rel`, but not on `android-clang-tidy-rel`)
Related details
- Chromium IPC Reviews
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
gwsq (Gerrit)
Message from gwsq![]( "Open in Gerrit")
From googleclient/chrome/chromium_gwsq/ipc/config.gwsq:
IPC: dch...@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): dch...@chromium.org
Reviewer source(s):
dch...@chromium.org is from context(googleclient/chrome/chromium_gwsq/ipc/config.gwsq)
Related details
- Daniel Cheng
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)
Daniel Cheng added 1 comment![]( "Open in Gerrit")
Well I am an owner for this code, someone who works on resource_coordinator should take a look at this.
(joenotcharles@ is an IPC reviewer and owns the code in question :)
Related details
- Adam Walls
- Joe Mason
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Joe Mason (Gerrit)
Joe Mason added 3 comments![]( "Open in Gerrit")
Why not use the existing MemoryDumpProvider class?
The linked bug doesn't tell me much. Is there a design doc for this?
The memory instrumentation system is already a huge tangle. I don't want to add another abstraction to it if the existing one can serve your needs.
uint64_t total_pss_kb = 0;Please use base::ByteSize for these, so there's no confusion about the units.
map<string, uint64> detailed_stats_kb;Please use mojo_base.mojom.ByteSize for this. (Other struct members don't use it because they haven't been migrated yet, but we should use it in new code.)
Related details
- Adam Walls
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. |
Adam Walls (Gerrit)
Adam Walls added 1 comment![]( "Open in Gerrit")
Why not use the existing MemoryDumpProvider class?
The linked bug doesn't tell me much. Is there a design doc for this?
The memory instrumentation system is already a huge tangle. I don't want to add another abstraction to it if the existing one can serve your needs.
Thanks for the feedback. Having worked with the service in the past few months, I totally understand! :)
Yes, we have a Design Doc for this work and have added you for visibility.
Please note that this DD is still under review within the team. Feel free to add feedback there or add any relevant reviewers.
Related details
- Joe Mason
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. |
Joe Mason (Gerrit)
Joe Mason added 1 comment![]( "Open in Gerrit")
Adam WallsWhy not use the existing MemoryDumpProvider class?
The linked bug doesn't tell me much. Is there a design doc for this?
The memory instrumentation system is already a huge tangle. I don't want to add another abstraction to it if the existing one can serve your needs.
Thanks for the feedback. Having worked with the service in the past few months, I totally understand! :)
Yes, we have a Design Doc for this work and have added you for visibility.
Please note that this DD is still under review within the team. Feel free to add feedback there or add any relevant reviewers.
- Adam Walls
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. |