Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: only access memory coordinator interface from browser process #31295

Merged
merged 1 commit into from
Oct 5, 2021

Conversation

deepak1556
Copy link
Member

@deepak1556 deepak1556 commented Oct 5, 2021

Description of Change

As part of the issue https://bugs.chromium.org/p/chromium/issues/detail?id=1251787 chromium refactored the memory coordinator interface to be only accessed from the browser process.

Refs https://chromium-review.googlesource.com/c/chromium/src/+/3174305

Currently implementation of process.getProcessMemoryInfo accesses the coordinator directly from the renderer process. This PR refactors the internal implementation to accommodate the upstream change while maintaining the user facing api and does not introduce any behavior changes.

Required for landing these security backports

/cc @ppontes

Checklist

Release Notes

Notes: none

@deepak1556 deepak1556 requested a review from a team as a code owner October 5, 2021 15:18
@electron-cation electron-cation bot added the new-pr 🌱 PR opened in the last 24 hours label Oct 5, 2021
@deepak1556 deepak1556 requested a review from a team October 5, 2021 15:18
@deepak1556 deepak1556 added target/12-x-y semver/patch backwards-compatible bug fixes labels Oct 5, 2021
@deepak1556 deepak1556 requested a review from a team October 5, 2021 15:19
@pushkin-
Copy link

pushkin- commented Oct 5, 2021

If I understand this change correctly, memory info is now going to be retrieved from the browser process, instead of the renderer process. Is this expected to degrade the response time of this API, then? (I want to use this API, but will call it many times per second so am sensitive to its performance)

@deepak1556
Copy link
Member Author

@pushkin- every child process including the browser registers itself as a client with the resource coordinator service that lives inside the browser process, the coordinator is interface that can be used to talk with the clients to get respective memory dumps. At the end of day, the clients are responsible to collect the dumps and send them to the coordinator.

This PR only changes the point of access to the coordinator interface, even before this PR accessing the coordinator from the renderer causes it talk to the browser process via mojo to get the interface. With this PR it makes it clear where the coordinator lives and there is no behavior change.

@deepak1556 deepak1556 merged commit 2a92d8f into main Oct 5, 2021
@deepak1556 deepak1556 deleted the robo/refactor_getprocessmemoryinfo_api branch October 5, 2021 22:30
@release-clerk
Copy link

release-clerk bot commented Oct 5, 2021

No Release Notes

@trop
Copy link
Contributor

trop bot commented Oct 5, 2021

I was unable to backport this PR to "12-x-y" cleanly;
you will need to perform this backport manually.

@trop
Copy link
Contributor

trop bot commented Oct 5, 2021

I was unable to backport this PR to "13-x-y" cleanly;
you will need to perform this backport manually.

@trop trop bot removed the target/13-x-y label Oct 5, 2021
@trop
Copy link
Contributor

trop bot commented Oct 5, 2021

I was unable to backport this PR to "14-x-y" cleanly;
you will need to perform this backport manually.

@trop
Copy link
Contributor

trop bot commented Oct 5, 2021

I was unable to backport this PR to "15-x-y" cleanly;
you will need to perform this backport manually.

@trop
Copy link
Contributor

trop bot commented Oct 6, 2021

@deepak1556 has manually backported this PR to "13-x-y", please check out #31305

@electron-cation electron-cation bot removed the new-pr 🌱 PR opened in the last 24 hours label Oct 6, 2021
deepak1556 added a commit that referenced this pull request Oct 6, 2021
…ss (#31305)

* refactor: only access memory coordinator interface from browser process (#31295)

Refs https://chromium-review.googlesource.com/c/chromium/src/+/3174305

* chore: fix build

* chore: fix lint
ppontes pushed a commit that referenced this pull request Oct 7, 2021
@trop
Copy link
Contributor

trop bot commented Oct 7, 2021

@ppontes has manually backported this PR to "12-x-y", please check out #31342

deepak1556 added a commit that referenced this pull request Oct 8, 2021
…ss (#31305) (#31342)

Backport of #31295

Co-authored-by: Robo <hop2deep@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
semver/patch backwards-compatible bug fixes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants