feat: remove nativeWindowOpen option

[nornagon]

Description of Change

nativeWindowOpen: true was made the default in 15.x.y. Let's remove the option to set it to false.

Checklist

• PR description included and stakeholders cc'd
• npm test passes
• tests are changed or added
• relevant documentation is changed or added
• PR release notes describe the change in a way relevant to app developers, and are capitalized, punctuated, and past tense.

Release Notes

Notes: Removed the old BrowserWindowProxy-based implementation of window.open. This also removes the nativeWindowOpen option from webPreferences.

[nornagon]

Blocked on #29463

[miniak]

@nornagon you can also remove openerId from InternalWebPreferences in typings/internal-ambient.d.ts

[zcbenz]

The mac/linux CI are crashing:

ok 35 chromium feature navigator.geolocation returns position when permission is granted


#
# Fatal error in ../../v8/src/api/api-inl.h, line 183
# Debug check failed: microtask_queue->GetMicrotasksScopeDepth() || !microtask_queue->DebugMicrotasksScopeDepthIsZero().
#
#
#
#FailureMessage Object: 0x7ffee33942500   Electron Framework                  0x0000000112010a79 base::debug::CollectStackTrace(void**, unsigned long) + 9
1   Electron Framework                  0x0000000111f2c103 base::debug::StackTrace::StackTrace() + 19
2   Electron Framework                  0x00000001143aa24d gin::(anonymous namespace)::PrintStackTrace() + 45
3   Electron Framework                  0x0000000113d80d16 V8_Fatal(char const*, int, char const*, ...) + 326
4   Electron Framework                  0x0000000113d806f5 v8::base::(anonymous namespace)::DefaultDcheckHandler(char const*, int, char const*) + 21
5   Electron Framework                  0x000000010f23e9e1 v8::CallDepthScope<true>::~CallDepthScope() + 865
6   Electron Framework                  0x000000010f1fbd53 v8::Function::Call(v8::Local<v8::Context>, v8::Local<v8::Value>, int, v8::Local<v8::Value>*) + 835
7   Electron Framework                  0x00000001173b9d62 node::InternalMakeCallback(node::Environment*, v8::Local<v8::Object>, v8::Local<v8::Object>, v8::Local<v8::Function>, int, v8::Local<v8::Value>*, node::async_context) + 738
8   Electron Framework                  0x00000001173ba02b node::MakeCallback(v8::Isolate*, v8::Local<v8::Object>, v8::Local<v8::Function>, int, v8::Local<v8::Value>*, node::async_context) + 187
9   Electron Framework                  0x0000000117403175 node::Environment::CheckImmediate(uv_check_s*) + 181
10  Electron Framework                  0x000000010d7f352d uv__run_check + 157
11  Electron Framework                  0x000000010d7ed461 uv_run + 369
12  Electron Framework                  0x000000010d9b30bc electron::NodeBindings::UvRunOnce() + 348
13  Electron Framework                  0x000000010d9b4874 base::internal::Invoker<base::internal::BindState<void (electron::NodeBindings::*)(), base::WeakPtr<electron::NodeBindings> >, void ()>::RunOnce(base::internal::BindStateBase*) + 148
14  Electron Framework                  0x0000000111fa8050 base::TaskAnnotator::RunTask(char const*, base::PendingTask*) + 400
15  Electron Framework                  0x0000000111fcc152 base::sequence_manager::internal::ThreadControllerWithMessagePumpImpl::DoWorkImpl(base::sequence_manager::LazyNow*) + 994
16  Electron Framework                  0x0000000111fcb978 base::sequence_manager::internal::ThreadControllerWithMessagePumpImpl::DoWork() + 88
17  Electron Framework                  0x00000001120270b1 base::MessagePumpCFRunLoopBase::RunWork() + 65
18  Electron Framework                  0x0000000112020862 base::mac::CallWithEHFrame(void () block_pointer) + 10
19  Electron Framework                  0x0000000112026adf base::MessagePumpCFRunLoopBase::RunWorkSource(void*) + 63
20  CoreFoundation                      0x00007fff3384e8f4 __CFRUNLOOP_IS_CALLING_OUT_TO_A_SOURCE0_PERFORM_FUNCTION__ + 17
21  CoreFoundation                      0x00007fff3384e893 __CFRunLoopDoSource0 + 103
22  CoreFoundation                      0x00007fff3384e6ad __CFRunLoopDoSources0 + 209
23  CoreFoundation                      0x00007fff3384d3c9 __CFRunLoopRun + 937
24  CoreFoundation                      0x00007fff3384c9c3 CFRunLoopRunSpecific + 466
25  Foundation                          0x00007fff35f081c8 -[NSRunLoop(NSRunLoop) runMode:beforeDate:] + 212
26  Electron Framework                  0x00000001120276ee base::MessagePumpNSRunLoop::DoRun(base::MessagePump::Delegate*) + 126
27  Electron Framework                  0x000000011202648c base::MessagePumpCFRunLoopBase::Run(base::MessagePump::Delegate*) + 140
28  Electron Framework                  0x0000000111fccd1b base::sequence_manager::internal::ThreadControllerWithMessagePumpImpl::Run(bool, base::TimeDelta) + 651
29  Electron Framework                  0x0000000111f80c2a base::RunLoop::Run(base::Location const&) + 890
30  Electron Framework                  0x0000000117178efb content::RendererMain(content::MainFunctionParams const&) + 1611
31  Electron Framework                  0x000000010f17c6f7 content::ContentMainRunnerImpl::Run(bool) + 503
32  Electron Framework                  0x000000010f17b602 content::RunContentProcess(content::ContentMainParams const&, content::ContentMainRunner*) + 2658
33  Electron Framework                  0x000000010f17b6ec content::ContentMain(content::ContentMainParams const&) + 44
34  Electron Framework                  0x000000010d7fff17 ElectronMain + 135
35  Electron Helper (Renderer)          0x000000010c868eda main + 298
36  libdyld.dylib                       0x00007fff6d943cc9 start + 1
webContents 1 crashed: file:///Users/distiller/project/src/electron/spec/static/index.html?grep=&invert=&files=spec%2Fapi-clipboard-spec.js%2Cspec%2Fapi-process-spec.js%2Cspec%2Fapi-web-frame-spec.js%2Cspec%2Fchromium-spec.js%2Cspec%2Fwebview-spec.js (killed=false)
Renderer process crashed
Renderer process crashed - see https://www.electronjs.org/docs/tutorial/application-debugging for potential debugging information.
webContents 2 crashed:  (killed=false)
Renderer process crashed - see https://www.electronjs.org/docs/tutorial/application-debugging for potential debugging information.
[2832:0602/173246.272595:WARNING:discardable_shared_memory_manager.cc(432)] Some MojoDiscardableSharedMemoryManagerImpls are still alive. They will be leaked.
[2832:0602/173246.276213:ERROR:process_posix.cc(330)] Unable to terminate process 2835: No such process (3)
✗ Electron tests failed with code 1.

[MarshallOfSound]

nativeWindowOpen being made the default doesn't allow us to just remove it. It needs to go through a deprecation cycle just like everything else.

As far as I can see the Electron 14 PR to enable nativeWindowOpen by default also failed to go through a proper deprecation cycle #28552

The deprecation cycle for a default change should be as follows:

• Version N - Initial state
• Version N + 1 - Deprecation warning about initial state changing
• Version N + 2 - Change initial state
• Version N + 3 - Add deprecation warning that the flag will be removed
• Version N + 4 - Remove the flag

We skipped over step N + 1 and N + 3 Electron 14 hasn't really existed for long though, so we can still fix this.

A good example of a deprecation timeline that worked really well IMO is the context aware module deprecation --> #18397

See #29463

[nornagon]

The deprecation cycle for a default change should be as follows:

• Version N - Initial state
• Version N + 1 - Deprecation warning about initial state changing
• Version N + 2 - Change initial state
• Version N + 3 - Add deprecation warning that the flag will be removed
• Version N + 4 - Remove the flag

We skipped over step N + 1 and N + 3 Electron 14 hasn't really existed for long though, so we can still fix this.

We chose N=13. So:

• 14 - leaving nativeWindowOpen unspecified warns that the default will change.
• 15 - nativeWindowOpen: true becomes the default.
• 16 - specifying nativeWindowOpen: false will result in a deprecation warning.
• 17 - the nativeWindowOpen option will be removed.

Should we consider lengthening this timeline on account of the new release cadence? Perhaps we should target the final removal for 18 instead of 17?

[miniak]

@zcbenz I am still seeing those crashes when running tests, I will probably need some help with that if possible.

[zcbenz]

The node::FreeEnvironment implicitly executes callbacks, adding a microtasks scope fixes the crash.

[miniak]

@zcbenz there is still one test failing:

not ok 39 chromium feature window.open accepts "nodeIntegration" as feature
  AssertionError: expected false to be true
      at Context.<anonymous> (/Users/distiller/project/src/electron/spec/chromium-spec.js:71:61)

It's caused by this bug #31949

[zcbenz]

After looking into related code, I found that the referenced code path is still used by the new-window event. I have update the comments instead.

Deprecation cycle now respected

[miniak]

@nornagon #32116 is merged, this should be finally ready for merging

[miniak]

API LGTM

[nornagon]

API LGTM

[release-clerk]

Release Notes Persisted

Removed the old BrowserWindowProxy-based implementation of window.open. This also removes the nativeWindowOpen option from webPreferences.