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

feat: allow setting code cache directory #31154

Merged
merged 4 commits into from Mar 15, 2022
Merged

feat: allow setting code cache directory #31154

merged 4 commits into from Mar 15, 2022

Conversation

miniak
Copy link
Contributor

@miniak miniak commented Sep 27, 2021

Description of Change

Add ses.setCodeCachePath() API for setting code cache directory.

When transitioning to a node-free renderer environment apps which previously relied on creating v8 cached data via the vm module would store the data at version based locations of the format like <user-data-dir>/<commit-sha-or-version> so that the cache can be invalidated on updates. When relying on blink to now handle the cache creation and invalidation, this api is useful to serve as a hard reset when the cache goes into a non-recoverable state.

Checklist

Release Notes

Notes: Added ses.setCodeCachePath() API for setting code cache directory.

@miniak miniak added semver/minor backwards-compatible functionality target/16-x-y labels Sep 27, 2021
@electron-cation electron-cation bot added new-pr 🌱 PR opened in the last 24 hours api-review/requested 🗳 labels Sep 27, 2021
@miniak miniak force-pushed the miniak/code-cache-path branch 3 times, most recently from a7e1b10 to 90ea368 Compare September 30, 2021 14:43
Copy link
Member

@zcbenz zcbenz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

API looks good to me.

docs/api/session.md Outdated Show resolved Hide resolved
@electron-cation electron-cation bot removed the new-pr 🌱 PR opened in the last 24 hours label Oct 4, 2021
docs/api/session.md Outdated Show resolved Hide resolved
docs/api/session.md Outdated Show resolved Hide resolved
docs/api/session.md Outdated Show resolved Hide resolved
@nornagon
Copy link
Member

nornagon commented Mar 9, 2022

API LGTM

Copy link
Member

@ckerr ckerr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like a good idea.

From an API WG pov, my two questions are

  1. Does it make sense to expose the size feature to end users? It looks like it would be easy to do, but IDK if there's benefit in it.

  2. The previous discussion about what to do with dead cache entries

shell/browser/api/electron_api_session.cc Outdated Show resolved Hide resolved
shell/browser/api/electron_api_session.cc Show resolved Hide resolved
docs/api/session.md Outdated Show resolved Hide resolved
deepak1556 and others added 2 commits March 14, 2022 20:12
Co-authored-by: Charles Kerr <charles@charleskerr.com>
Co-authored-by: Charles Kerr <charles@charleskerr.com>
@deepak1556 deepak1556 requested a review from ckerr March 15, 2022 03:24
Copy link
Member

@ckerr ckerr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

API LGTM

@ckerr
Copy link
Member

ckerr commented Mar 15, 2022

API Review In progress — Pending (0/2 LGTMs - ready on 2021-10-04)

There are three approvals, so this bot being broken is not a reason to block this PR

@ckerr ckerr merged commit 2657383 into main Mar 15, 2022
@ckerr ckerr deleted the miniak/code-cache-path branch March 15, 2022 20:34
@release-clerk
Copy link

release-clerk bot commented Mar 15, 2022

Release Notes Persisted

Added ses.setCodeCachePath() API for setting code cache directory.

@trop
Copy link
Contributor

trop bot commented Mar 15, 2022

I have automatically backported this PR to "17-x-y", please check out #33285

@trop
Copy link
Contributor

trop bot commented Mar 15, 2022

I have automatically backported this PR to "18-x-y", please check out #33286

bavulapati pushed a commit to bavulapati/electron that referenced this pull request Apr 29, 2022
* feat: allow setting code cache directory

* chore: address review feedback

* chore: update docs

Co-authored-by: Charles Kerr <charles@charleskerr.com>

* chore: rewrite with base::Contains

Co-authored-by: Charles Kerr <charles@charleskerr.com>

Co-authored-by: deepak1556 <hop2deep@gmail.com>
Co-authored-by: Charles Kerr <charles@charleskerr.com>
khalwa pushed a commit to solarwindscloud/electron that referenced this pull request Feb 22, 2023
* feat: allow setting code cache directory

* chore: address review feedback

* chore: update docs

Co-authored-by: Charles Kerr <charles@charleskerr.com>

* chore: rewrite with base::Contains

Co-authored-by: Charles Kerr <charles@charleskerr.com>

Co-authored-by: deepak1556 <hop2deep@gmail.com>
Co-authored-by: Charles Kerr <charles@charleskerr.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api-review/requested 🗳 semver/minor backwards-compatible functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants