feat: flesh out the api for //extensions#21587
Conversation
|
(cc @samuelmaddock fyi!) |
|
This looks to be going in a good direction to me! 👍 The only opinion I have is about the |
|
Makes sense to me! Also updated the sketch over in #19447. |
jkleinsc
left a comment
There was a problem hiding this comment.
LGTM, but I think the API WG should weigh in on this. Also, would it make more sense to wait to add the unimplemented functions when they are actually implemented?
| extension_system->RemoveExtension(extension_id); | ||
| } | ||
|
|
||
| void Session::EnableExtension(const std::string& extension_id) { |
There was a problem hiding this comment.
Should we wait until these are implemented to add them?
There was a problem hiding this comment.
Sure, seems reasonable. I added them here because it was easy to do them all together but no particular need to keep them in this PR.
FWIW, none of these functions are available in any released version of Electron, because these are behind a build flag that is off by default. I agree that we should do API review of the proposed APIs for extensions, over in #19447. I don't think API review should block this PR though as it does not change any exposed APIs of Electron. |
zcbenz
left a comment
There was a problem hiding this comment.
I agree this PR does not need API-wg's approval since there is no change to public APIs.
shell/browser/api/atom_api_session.h
Outdated
|
|
||
| #if BUILDFLAG(ENABLE_ELECTRON_EXTENSIONS) | ||
| void LoadChromeExtension(const base::FilePath extension_path); | ||
| v8::Local<v8::Promise> LoadExtension(const base::FilePath extension_path); |
There was a problem hiding this comment.
Should be const base::FilePath&.
|
No Release Notes |
Description of Change
This adds a few more APIs to the //extensions system, which is still behind the
enable_electron_extensionsbuild flag.No release notes because none of these features are available in released versions of Electron yet.
Ref #19447
Checklist
npm testpassesRelease Notes
Notes: none