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

fix: removeBrowserView draggable region removal #39387

Merged
merged 1 commit into from
Aug 8, 2023
Merged

Conversation

codebytere
Copy link
Member

Description of Change

Closes #39377.

This was happening because destroying the webContents in the BrowserView on removal by clicking triggered a hitTest, yet when our logic when to check against possible draggable regions, the browserView had not yet been removed from the set. This flips the order to ensure that there are no race conditions.

Cannot be tested (and was not caught in tests) because it specifically requires click or mouse interaction when removeBrowserView is called.

Stacktrace

Received signal 11 SEGV_ACCERR 0000000005a0
0   Electron Framework                  0x000000011dc86950 base::debug::CollectStackTrace(void**, unsigned long) + 28
1   Electron Framework                  0x000000011dc77060 base::debug::StackTrace::StackTrace() + 24
2   Electron Framework                  0x000000011dc868a0 base::debug::(anonymous namespace)::StackDumpSignalHandler(int, __siginfo*, void*) + 1204
3   libsystem_platform.dylib            0x0000000186c26a24 _sigtramp + 56
4   Electron Framework                  0x000000011911d108 non-virtual thunk to electron::api::BrowserView::NonClientHitTest(gfx::Point const&) + 36
5   Electron Framework                  0x0000000119214264 electron::NativeWindow::NonClientHitTest(gfx::Point const&) + 56
6   Electron Framework                  0x000000011d92e410 electron::NativeAppWindowFrameViewMac::NonClientHitTest(gfx::Point const&) + 84
7   Electron Framework                  0x000000012287c484 views::Widget::GetNonClientComponent(gfx::Point const&) + 36
8   Electron Framework                  0x000000012288d664 non-virtual thunk to views::NativeWidgetMacNSWindowHost::GetIsDraggableBackgroundAt(gfx::Point const&, bool*) + 52
9   Electron Framework                  0x000000011f61c6e8 -[BridgedContentView hitTest:] + 136
10  AppKit                              0x000000018a04fef0 -[NSView hitTest:] + 216
11  AppKit                              0x000000018a04fda4 -[NSThemeFrame _hitTest:ignoringResizeRegion:] + 544
12  AppKit                              0x000000018a4a64c8 -[_NSTrackingAreaAKManager setCursorForMouseLocation:] + 720
13  AppKit                              0x000000018a4a5d20 -[_NSTrackingAreaAKManager displayCycleUpdateStructuralRegions] + 284
14  AppKit                              0x0000000189fa1658 __NSWindowGetDisplayCycleObserverForUpdateStructuralRegions_block_invoke + 364
15  AppKit                              0x0000000189f9cc30 NSDisplayCycleObserverInvoke + 168
16  AppKit                              0x0000000189f9c88c NSDisplayCycleFlush + 644
17  QuartzCore                          0x000000018e1b9ce4 CA::Transaction::run_commit_handlers(CATransactionPhase) + 120
18  QuartzCore                          0x000000018e1b8aa0 CA::Transaction::commit() + 320
19  Electron Framework                  0x000000011d929604 electron::NativeWindowMac::RemoveBrowserView(electron::NativeBrowserView*) + 148
20  Electron Framework                  0x000000011910f208 electron::api::BaseWindow::RemoveBrowserView(gin::Handle<electron::api::BrowserView>) + 152
21  Electron Framework                  0x0000000119114884 base::RepeatingCallback<void (electron::api::BaseWindow*, gin::Handle<electron::api::View>)>::Run(electron::api::BaseWindow*, gin::Handle<electron::api::View>) const & + 88
22  Electron Framework                  0x0000000119119540 gin_helper::Dispatcher<void (electron::api::BaseWindow*, gin::Handle<electron::api::BrowserView>)>::DispatchToCallback(v8::FunctionCallbackInfo<v8::Value> const&) + 300
23  ???                                 0x000000015fdd2820 0x0 + 5903296544
24  ???                                 0x000000015fdd03fc 0x0 + 5903287292
25  ???                                 0x000000015fdd03fc 0x0 + 5903287292
26  ???                                 0x0000000158037aa4 0x0 + 5771590308
27  ???                                 0x000000015fdcdb88 0x0 + 5903276936
28  ???                                 0x000000015fdcd878 0x0 + 5903276152
29  Electron Framework                  0x000000011aabee38 v8::internal::(anonymous namespace)::Invoke(v8::internal::Isolate*, v8::internal::(anonymous namespace)::InvokeParams const&) + 4596
30  Electron Framework                  0x000000011aabdb14 v8::internal::Execution::Call(v8::internal::Isolate*, v8::internal::Handle<v8::internal::Object>, v8::internal::Handle<v8::internal::Object>, int, v8::internal::Handle<v8::internal::Object>*) + 248
31  Electron Framework                  0x000000011a7ac414 v8::Function::Call(v8::Local<v8::Context>, v8::Local<v8::Value>, int, v8::Local<v8::Value>*) + 708
32  Electron Framework                  0x000000012257af2c node::InternalMakeCallback(node::Environment*, v8::Local<v8::Object>, v8::Local<v8::Object>, v8::Local<v8::Function>, int, v8::Local<v8::Value>*, node::async_context) + 500
33  Electron Framework                  0x000000012257b29c node::MakeCallback(v8::Isolate*, v8::Local<v8::Object>, v8::Local<v8::Function>, int, v8::Local<v8::Value>*, node::async_context) + 260
34  Electron Framework                  0x00000001192961d8 gin_helper::internal::CallMethodWithArgs(v8::Isolate*, v8::Local<v8::Object>, char const*, std::__Cr::vector<v8::Local<v8::Value>, std::__Cr::allocator<v8::Local<v8::Value>>>*) + 116
35  Electron Framework                  0x00000001191b81d8 v8::Local<v8::Value> gin_helper::EmitEvent<std::__Cr::basic_string_view<char, std::__Cr::char_traits<char>>, gin::Handle<gin_helper::internal::Event>&, bool&, std::__Cr::basic_string<char, std::__Cr::char_traits<char>, std::__Cr::allocator<char>> const&, blink::CloneableMessage>(v8::Isolate*, v8::Local<v8::Object>, std::__Cr::basic_string_view<char, std::__Cr::char_traits<char>> const&, gin::Handle<gin_helper::internal::Event>&, bool&, std::__Cr::basic_string<char, std::__Cr::char_traits<char>, std::__Cr::allocator<char>> const&, blink::CloneableMessage&&) + 172
36  Electron Framework                  0x00000001191a14a0 bool electron::api::WebContents::EmitWithSender<bool&, std::__Cr::basic_string<char, std::__Cr::char_traits<char>, std::__Cr::allocator<char>> const&, blink::CloneableMessage>(std::__Cr::basic_string_view<char, std::__Cr::char_traits<char>>, content::RenderFrameHost*, base::OnceCallback<void (blink::CloneableMessage)>, bool&, std::__Cr::basic_string<char, std::__Cr::char_traits<char>, std::__Cr::allocator<char>> const&, blink::CloneableMessage&&) + 240
37  Electron Framework                  0x00000001191a15e0 electron::api::WebContents::Invoke(bool, std::__Cr::basic_string<char, std::__Cr::char_traits<char>, std::__Cr::allocator<char>> const&, blink::CloneableMessage, base::OnceCallback<void (blink::CloneableMessage)>, content::RenderFrameHost*) + 136
38  Electron Framework                  0x00000001191de4c8 electron::ElectronApiIPCHandlerImpl::Invoke(bool, std::__Cr::basic_string<char, std::__Cr::char_traits<char>, std::__Cr::allocator<char>> const&, blink::CloneableMessage, base::OnceCallback<void (blink::CloneableMessage)>) + 124
39  Electron Framework                  0x000000011d90dd98 electron::mojom::ElectronApiIPCStubDispatch::AcceptWithResponder(electron::mojom::ElectronApiIPC*, mojo::Message*, std::__Cr::unique_ptr<mojo::MessageReceiverWithStatus, std::__Cr::default_delete<mojo::MessageReceiverWithStatus>>) + 484
40  Electron Framework                  0x00000001191de7b8 electron::mojom::ElectronApiIPCStub<mojo::RawPtrImplRefTraits<electron::mojom::ElectronApiIPC>>::AcceptWithResponder(mojo::Message*, std::__Cr::unique_ptr<mojo::MessageReceiverWithStatus, std::__Cr::default_delete<mojo::MessageReceiverWithStatus>>) + 44
41  Electron Framework                  0x000000011df574cc mojo::InterfaceEndpointClient::HandleValidatedMessage(mojo::Message*) + 1112
42  Electron Framework                  0x000000011df5d780 mojo::MessageDispatcher::Accept(mojo::Message*) + 236
43  Electron Framework                  0x000000011df58f40 mojo::InterfaceEndpointClient::HandleIncomingMessage(mojo::Message*) + 100
44  Electron Framework                  0x000000011e35669c IPC::(anonymous namespace)::ChannelAssociatedGroupController::AcceptOnEndpointThread(mojo::Message) + 480
45  Electron Framework                  0x000000011df5b1c8 base::internal::Invoker<base::internal::BindState<void (mojo::(anonymous namespace)::ThreadSafeInterfaceEndpointClientProxy::*)(mojo::Message), scoped_refptr<mojo::(anonymous namespace)::ThreadSafeInterfaceEndpointClientProxy>, mojo::Message>, void ()>::RunOnce(base::internal::BindStateBase*) + 84
46  Electron Framework                  0x000000011dc0c6f8 base::TaskAnnotator::RunTaskImpl(base::PendingTask&) + 308
47  Electron Framework                  0x000000011dc341a8 base::sequence_manager::internal::ThreadControllerWithMessagePumpImpl::DoWorkImpl(base::LazyNow*) + 1164
48  Electron Framework                  0x000000011dc337dc base::sequence_manager::internal::ThreadControllerWithMessagePumpImpl::DoWork() + 104
49  Electron Framework                  0x000000011dc8eba0 base::MessagePumpCFRunLoopBase::RunWork() + 180
50  Electron Framework                  0x000000011dc8da48 base::mac::CallWithEHFrame(void () block_pointer) + 16
51  Electron Framework                  0x000000011dc8e0a8 base::MessagePumpCFRunLoopBase::RunWorkSource(void*) + 68
52  CoreFoundation                      0x0000000186cd663c __CFRUNLOOP_IS_CALLING_OUT_TO_A_SOURCE0_PERFORM_FUNCTION__ + 28
53  CoreFoundation                      0x0000000186cd65d0 __CFRunLoopDoSource0 + 176
54  CoreFoundation                      0x0000000186cd6340 __CFRunLoopDoSources0 + 244
55  CoreFoundation                      0x0000000186cd4f48 __CFRunLoopRun + 828
56  CoreFoundation                      0x0000000186cd44b8 CFRunLoopRunSpecific + 612
57  HIToolbox                           0x000000019051ec40 RunCurrentEventLoopInMode + 292
58  HIToolbox                           0x000000019051ea7c ReceiveNextEventCommon + 648
59  HIToolbox                           0x000000019051e7d4 _BlockUntilNextEventMatchingListInModeWithFilter + 76
60  AppKit                              0x0000000189ef5d44 _DPSNextEvent + 636
61  AppKit                              0x0000000189ef4ee0 -[NSApplication(NSEvent) _nextEventMatchingEventMask:untilDate:inMode:dequeue:] + 716
62  AppKit                              0x0000000189ee9344 -[NSApplication run] + 464
63  Electron Framework                  0x000000011dc8f624 base::MessagePumpNSApplication::DoRun(base::MessagePump::Delegate*) + 400
64  Electron Framework                  0x000000011dc8dae8 base::MessagePumpCFRunLoopBase::Run(base::MessagePump::Delegate*) + 144
65  Electron Framework                  0x000000011dc35008 base::sequence_manager::internal::ThreadControllerWithMessagePumpImpl::Run(bool, base::TimeDelta) + 472
66  Electron Framework                  0x000000011dbed4d4 base::RunLoop::Run(base::Location const&) + 396
67  Electron Framework                  0x000000011c909624 content::BrowserMainLoop::RunMainMessageLoop() + 144
68  Electron Framework                  0x000000011c90b3dc content::BrowserMainRunnerImpl::Run() + 44
69  Electron Framework                  0x000000011c9069b0 content::BrowserMain(content::MainFunctionParams) + 160
70  Electron Framework                  0x00000001194677c8 content::RunBrowserProcessMain(content::MainFunctionParams, content::ContentMainDelegate*) + 280
71  Electron Framework                  0x0000000119468c34 content::ContentMainRunnerImpl::RunBrowser(content::MainFunctionParams, bool) + 600
72  Electron Framework                  0x000000011946884c content::ContentMainRunnerImpl::Run() + 768
73  Electron Framework                  0x0000000119466dbc content::RunContentProcess(content::ContentMainParams, content::ContentMainRunner*) + 1228
74  Electron Framework                  0x0000000119467020 content::ContentMain(content::ContentMainParams) + 112
75  Electron Framework                  0x00000001190e6dc8 ElectronMain + 128
76  dyld                                0x000000018689ff28 start + 2236
[end of stack trace]

Electron exited with signal SIGSEGV.

Checklist

Release Notes

Notes: Fixed an issue where browserView.removeBrowserView could cause a crash in some cases.

@codebytere codebytere added semver/patch backwards-compatible bug fixes target/24-x-y PR should also be added to the "24-x-y" branch. target/25-x-y PR should also be added to the "25-x-y" branch. target/26-x-y PR should also be added to the "26-x-y" branch. labels Aug 7, 2023
@electron-cation electron-cation bot added the new-pr 🌱 PR opened in the last 24 hours label Aug 7, 2023
@electron-cation electron-cation bot removed the new-pr 🌱 PR opened in the last 24 hours label Aug 8, 2023
@codebytere codebytere merged commit 8f4f826 into main Aug 8, 2023
18 checks passed
@codebytere codebytere deleted the fix-bv-remove-crash branch August 8, 2023 08:23
@release-clerk
Copy link

release-clerk bot commented Aug 8, 2023

Release Notes Persisted

Fixed an issue where browserView.removeBrowserView could cause a crash in some cases.

@trop
Copy link
Contributor

trop bot commented Aug 8, 2023

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

@trop trop bot added the in-flight/26-x-y label Aug 8, 2023
@trop
Copy link
Contributor

trop bot commented Aug 8, 2023

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

@trop trop bot added in-flight/24-x-y and removed target/26-x-y PR should also be added to the "26-x-y" branch. target/24-x-y PR should also be added to the "24-x-y" branch. labels Aug 8, 2023
@trop
Copy link
Contributor

trop bot commented Aug 8, 2023

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

@trop trop bot added in-flight/25-x-y merged/24-x-y PR was merged to the "24-x-y" branch merged/26-x-y PR was merged to the "26-x-y" branch. and removed target/25-x-y PR should also be added to the "25-x-y" branch. in-flight/24-x-y labels Aug 8, 2023
@trop trop bot added merged/25-x-y PR was merged to the "25-x-y" branch. and removed in-flight/26-x-y in-flight/25-x-y labels Aug 8, 2023
win32ss pushed a commit to win32ss/supermium-electron that referenced this pull request Sep 24, 2023
fix: removeBrowserView draggable region removal

Closes electron#39377.
MrHuangJser pushed a commit to MrHuangJser/electron that referenced this pull request Dec 11, 2023
fix: removeBrowserView draggable region removal

Closes electron#39377.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merged/24-x-y PR was merged to the "24-x-y" branch merged/25-x-y PR was merged to the "25-x-y" branch. merged/26-x-y PR was merged to the "26-x-y" branch. semver/patch backwards-compatible bug fixes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]: removeBrowserView() crashes if view.webContents.close() is called beforehand.
3 participants