chore: remove app.allowRendererProcessReuse and BrowserWindow affinity options
#26874
Conversation
MarshallOfSound
force-pushed
the
remove-allow-process-reuse
branch
from
0004215
to
5eb9685
Compare
|
Would be good to note in the release notes that |
There was a problem hiding this comment.
Can we do the reverse order of removing the patches first and then flipping the public api bits.
- https://github.com/electron/electron/blob/master/patches/chromium/frame_host_manager.patch
- https://github.com/electron/electron/blob/master/patches/chromium/revert_remove_contentrendererclient_shouldfork.patch
- Navigation controller
Our custom navigation controller should not be used for process reuse mode and also the fork logic in the renderer client. This would ensure the right tests pass when the bits are flipped.
|
@deepak1556 Unfortunately not, there's a lot of logic that needs to be unwound here and the first thing we can (and need to) do is remove this public API. I'll write up an issue shortly outlining all the work that still needs to happen, but this is a very tangled mess and untangling it going to take a lot of time, refactoring and pull requests. Unfortunately the final step will be removing the patches. |
MarshallOfSound
force-pushed
the
remove-allow-process-reuse
branch
2 times, most recently
from
1b832ad
to
56143f8
Compare
deepak1556
dismissed
their stale review
discussed offline about required api changes before patch removal
MarshallOfSound
force-pushed
the
remove-allow-process-reuse
branch
from
56143f8
to
139cf1a
Compare
|
Does it also mean that Electron will use Chromium's NavigationController instead of its own? |
Long term, yes |
MarshallOfSound
force-pushed
the
remove-allow-process-reuse
branch
from
139cf1a
to
3311550
Compare
MarshallOfSound
force-pushed
the
remove-allow-process-reuse
branch
3 times, most recently
from
bccc17f
to
4b44acc
Compare
|
@MarshallOfSound 13.x is already in beta, are we sure we want to backport major breaking changes? Can this target 14.x instead? |
Eh sure. Not super attached to when it goes out, just want to get it into master so I can start destroying our frame host manager patch 😄 |
MarshallOfSound
force-pushed
the
remove-allow-process-reuse
branch
from
4b44acc
to
5fa625e
Compare
| @@ -1426,19 +1426,6 @@ This is the user agent that will be used when no user agent is set at the | |||
| app has the same user agent. Set to a custom value as early as possible | |||
| in your app's initialization to ensure that your overridden value is used. | |||
|
|
|||
| ### `app.allowRendererProcessReuse` | |||
There was a problem hiding this comment.
we should mark this as deprecated in 13.x. did it already make it into docs/breaking-changes as a deprecation?
There was a problem hiding this comment.
It's definitely been deprecated since like, Electron 11. There's been a warning. I'm not sure if we put deprecations in breaking-changes. The default flip back in Electron 9 is in the breaking-changes list though.
There was a problem hiding this comment.
We generally mark deprecations in the docs for a feature when we deprecate it, and for major things we put them in breaking changes too.
There was a problem hiding this comment.
Hm, I'll put a PR back into 13-x-y to add the @depreacted tag
|
Release Notes Persisted
|
As in title and notes, this is going to be a multiple step process and it'll take a decent amount of refactoring to remove the patch files that we actually want to remove. But this is step 1 "remove the public API".
Notes: Removed the deprecated
app.allowRendererProcessReuseand BrowserWindowaffinityoptions