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

Browser crash when trying to close tab with sidebar open #41124

Closed
srirambv opened this issue Sep 18, 2024 · 6 comments
Closed

Browser crash when trying to close tab with sidebar open #41124

srirambv opened this issue Sep 18, 2024 · 6 comments
Labels
crash feature/sidebar Relating to Brave's Sidebar feature OS/Desktop priority/P2 A bad problem. We might uplift this to the next planned release. QA Pass-Win64 QA/Yes release-notes/exclude

Comments

@srirambv
Copy link
Contributor

Description

IMPORTANT: Your crash has already been automatically reported to our crash system. Please file this bug only if you can provide more information about it.

Brave Version: 1.70.115 Chromium: 129.0.6668.59
Operating System: Windows NT 10.0.22631

URL (if applicable) where crash occurred:

Can you reproduce this crash?
Yes

What steps will reproduce this crash? (If it's not reproducible, what were you doing just before the crash?)

  1. Launch browser with Leo pointing to staging
  2. Connect to staging account and load Leo credentials
  3. Open a new tab and load Leo in sidebar
  4. Navigate back and forth between two tabs where Leo is opened in both
  5. Close one of the tab
  6. Browser crashes

DO NOT CHANGE BELOW THIS LINE
Crash ID: crash/ead61900-9ce4-970c-0000-000000000000
Crash ID: crash/e7d61900-9ce4-970c-0000-000000000000

cc: @brave/qa-team @iefremov @rebron

@rebron rebron added priority/P2 A bad problem. We might uplift this to the next planned release. OS/Desktop feature/sidebar Relating to Brave's Sidebar feature labels Sep 19, 2024
@mkarolin
Copy link
Contributor

cc: @simonhong @petemill

The crash stack is not symbolized and I found it a bit difficult to reproduce. The one way I was able to reproduce (assuming it's the same crash) was under a debugger by:

  1. Start browser
  2. In tab A open Chat UI
  3. Open a new tab (B) and open Reading list
  4. Open a new tab (C) (Reading list should be showin)
  5. Close tab B
  6. Activate tab A
  7. Close the browser
  8. Crash with ACCESS_VIOLATION:
ntdll.dll!00007ffddcbf2650()
base.dll!base::internal::LockImpl::Try() Line 103
	at C:\bb2\brave-browser\src\base\synchronization\lock_impl.h(103)
base.dll!base::internal::LockImpl::Lock() Line 94
	at C:\bb2\brave-browser\src\base\synchronization\lock_impl.h(94)
base.dll!base::Lock::Acquire(base::subtle::LockTracking tracking) Line 52
	at C:\bb2\brave-browser\src\base\synchronization\lock.cc(52)
base.dll!base::internal::BasicAutoLock<base::Lock>::BasicAutoLock(base::Lock & lock, base::subtle::LockTracking tracking) Line 147
	at C:\bb2\brave-browser\src\base\synchronization\lock_impl.h(147)
base.dll!base::SequenceCheckerImpl::CalledOnValidSequence(std::__Cr::unique_ptr<base::debug::StackTrace,std::__Cr::default_delete<base::debug::StackTrace>> * out_bound_at) Line 89
	at C:\bb2\brave-browser\src\base\sequence_checker_impl.cc(89)
base.dll!base::ScopedValidateSequenceChecker::ScopedValidateSequenceChecker(const base::SequenceCheckerImpl & checker) Line 21
	at C:\bb2\brave-browser\src\base\sequence_checker.cc(21)
components_prefs.dll!PrefService::GetValue(std::__Cr::basic_string_view<char,std::__Cr::char_traits<char>> path) Line 348
	at C:\bb2\brave-browser\src\components\prefs\pref_service.cc(348)
components_prefs.dll!PrefService::GetTime(std::__Cr::basic_string_view<char,std::__Cr::char_traits<char>> path) Line 519
	at C:\bb2\brave-browser\src\components\prefs\pref_service.cc(519)
chrome.dll!ai_chat::HasUserOptedIn(PrefService * prefs) Line 71
	at C:\bb2\brave-browser\src\brave\components\ai_chat\core\browser\utils.cc(71)
chrome.dll!ai_chat::ConversationDriver::HasUserOptedIn() Line 583
	at C:\bb2\brave-browser\src\brave\components\ai_chat\core\browser\conversation_driver.cc(583)
chrome.dll!ai_chat::ConversationDriver::ShouldFetchSearchQuerySummary() Line 368
	at C:\bb2\brave-browser\src\brave\components\ai_chat\core\browser\conversation_driver.cc(368)
chrome.dll!ai_chat::ConversationDriver::MaybeFetchOrClearSearchQuerySummary(base::OnceCallback<void (const std::__Cr::optional<std::__Cr::vector<ai_chat::SearchQuerySummary,std::__Cr::allocator<ai_chat::SearchQuerySummary>>> &)> callback) Line 379
	at C:\bb2\brave-browser\src\brave\components\ai_chat\core\browser\conversation_driver.cc(379)
chrome.dll!ai_chat::ConversationDriver::OnConversationActiveChanged(bool is_conversation_active) Line 351
	at C:\bb2\brave-browser\src\brave\components\ai_chat\core\browser\conversation_driver.cc(351)
chrome.dll!ai_chat::AIChatUIPageHandler::OnVisibilityChanged(content::Visibility visibility) Line 470
	at C:\bb2\brave-browser\src\brave\browser\ui\webui\ai_chat\ai_chat_ui_page_handler.cc(470)
content.dll!content::WebContentsImpl::WebContentsObserverList::NotifyObservers<void (content::WebContentsObserver::*)(content::Visibility),content::Visibility &>(void(content::WebContentsObserver::*)(content::Visibility) func, content::Visibility & args) Line 1686
	at C:\bb2\brave-browser\src\content\browser\web_contents\web_contents_impl.h(1686)
content.dll!content::WebContentsImpl::SetVisibilityAndNotifyObservers(content::Visibility visibility) Line 6157
	at C:\bb2\brave-browser\src\content\browser\web_contents\web_contents_impl.cc(6157)
content.dll!content::WebContentsImpl::UpdateVisibilityAndNotifyPageAndView(content::Visibility new_visibility, bool is_activity) Line 4403
	at C:\bb2\brave-browser\src\content\browser\web_contents\web_contents_impl.cc(4403)
content.dll!content::WebContentsImpl::UpdateWebContentsVisibility(content::Visibility visibility) Line 10534
	at C:\bb2\brave-browser\src\content\browser\web_contents\web_contents_impl.cc(10534)
content.dll!content::WebContentsViewAura::UpdateWebContentsVisibility() Line 975
	at C:\bb2\brave-browser\src\content\browser\web_contents\web_contents_view_aura.cc(975)
content.dll!content::WebContentsViewAura::OnWindowOcclusionChanged(aura::Window::OcclusionState old_occlusion_state, aura::Window::OcclusionState new_occlusion_state) Line 1305
	at C:\bb2\brave-browser\src\content\browser\web_contents\web_contents_view_aura.cc(1305)
ui_aura.dll!aura::Window::SetOcclusionInfo(aura::Window::OcclusionState occlusion_state, const SkRegion & occluded_region) Line 1104
	at C:\bb2\brave-browser\src\ui\aura\window.cc(1104)
ui_aura.dll!aura::DefaultWindowOcclusionChangeBuilder::~DefaultWindowOcclusionChangeBuilder() Line 30
	at C:\bb2\brave-browser\src\ui\aura\window_occlusion_change_builder.cc(30)
ui_aura.dll!aura::DefaultWindowOcclusionChangeBuilder::~DefaultWindowOcclusionChangeBuilder() Line 26
	at C:\bb2\brave-browser\src\ui\aura\window_occlusion_change_builder.cc(26)
ui_aura.dll!std::__Cr::default_delete<aura::WindowOcclusionChangeBuilder>::operator()(aura::WindowOcclusionChangeBuilder * __ptr) Line 69
	at C:\bb2\brave-browser\src\third_party\libc++\src\include\__memory\unique_ptr.h(69)
ui_aura.dll!std::__Cr::unique_ptr<aura::WindowOcclusionChangeBuilder,std::__Cr::default_delete<aura::WindowOcclusionChangeBuilder>>::reset(aura::WindowOcclusionChangeBuilder * __p) Line 281
	at C:\bb2\brave-browser\src\third_party\libc++\src\include\__memory\unique_ptr.h(281)
ui_aura.dll!std::__Cr::unique_ptr<aura::WindowOcclusionChangeBuilder,std::__Cr::default_delete<aura::WindowOcclusionChangeBuilder>>::~unique_ptr() Line 249
	at C:\bb2\brave-browser\src\third_party\libc++\src\include\__memory\unique_ptr.h(249)
ui_aura.dll!aura::WindowOcclusionTracker::MaybeComputeOcclusion() Line 330
	at C:\bb2\brave-browser\src\ui\aura\window_occlusion_tracker.cc(330)
ui_aura.dll!aura::WindowOcclusionTracker::Unpause() Line 814
	at C:\bb2\brave-browser\src\ui\aura\window_occlusion_tracker.cc(814)
ui_aura.dll!aura::WindowOcclusionTracker::ScopedPause::~ScopedPause() Line 200
	at C:\bb2\brave-browser\src\ui\aura\window_occlusion_tracker.cc(200)
ui_aura.dll!aura::Window::SetVisibleInternal(bool visible) Line 1090
	at C:\bb2\brave-browser\src\ui\aura\window.cc(1090)
ui_aura.dll!aura::Window::Hide() Line 372
	at C:\bb2\brave-browser\src\ui\aura\window.cc(372)
ui_views.dll!views::DesktopWindowTreeHostWin::Close() Line 248
	at C:\bb2\brave-browser\src\ui\views\widget\desktop_aura\desktop_window_tree_host_win.cc(248)
ui_views.dll!views::DesktopNativeWidgetAura::Close() Line 913
	at C:\bb2\brave-browser\src\ui\views\widget\desktop_aura\desktop_native_widget_aura.cc(913)
ui_views.dll!views::Widget::CloseWithReason(views::Widget::ClosedReason closed_reason) Line 838
	at C:\bb2\brave-browser\src\ui\views\widget\widget.cc(838)
ui_views.dll!views::Widget::Close() Line 842
	at C:\bb2\brave-browser\src\ui\views\widget\widget.cc(842)
chrome.dll!BrowserView::Close() Line 1443
	at C:\bb2\brave-browser\src\chrome\browser\ui\views\frame\browser_view.cc(1443)
chrome.dll!Browser::TabStripEmpty() Line 1562
	at C:\bb2\brave-browser\src\chrome\browser\ui\browser.cc(1562)
chrome.dll!BraveBrowser::TabStripEmpty() Line 132
	at C:\bb2\brave-browser\src\brave\browser\ui\brave_browser.cc(132)
chrome.dll!TabStripModel::SendDetachWebContentsNotifications(TabStripModel::DetachNotifications * notifications) Line 483
	at C:\bb2\brave-browser\src\chrome\browser\ui\tabs\tab_strip_model.cc(483)
chrome.dll!TabStripModel::CloseTabs(base::span<content::WebContents *const,18446744073709551615,content::WebContents *const *> items, unsigned int close_types) Line 2085
	at C:\bb2\brave-browser\src\chrome\browser\ui\tabs\tab_strip_model.cc(2085)
chrome.dll!TabStripModel::CloseAllTabs() Line 676
	at C:\bb2\brave-browser\src\chrome\browser\ui\tabs\tab_strip_model.cc(676)
chrome.dll!Browser::OnWindowClosing() Line 1241
	at C:\bb2\brave-browser\src\chrome\browser\ui\browser.cc(1241)
chrome.dll!UnloadController::ProcessPendingTabs(bool skip_beforeunload) Line 356
	at C:\bb2\brave-browser\src\chrome\browser\ui\unload_controller.cc(356)
chrome.dll!UnloadController::ClearUnloadState(content::WebContents * web_contents, bool process_now) Line 442
	at C:\bb2\brave-browser\src\chrome\browser\ui\unload_controller.cc(442)
chrome.dll!UnloadController::CanCloseContents(content::WebContents * contents) Line 54
	at C:\bb2\brave-browser\src\chrome\browser\ui\unload_controller.cc(54)
chrome.dll!Browser::CloseContents(content::WebContents * source) Line 2012
	at C:\bb2\brave-browser\src\chrome\browser\ui\browser.cc(2012)
content.dll!content::WebContentsImpl::Close() Line 8536
	at C:\bb2\brave-browser\src\content\browser\web_contents\web_contents_impl.cc(8536)
content.dll!content::RenderFrameHostImpl::ClosePageIgnoringUnloadEvents(content::RenderFrameHostImpl::ClosePageSource source) Line 6554
	at C:\bb2\brave-browser\src\content\browser\renderer_host\render_frame_host_impl.cc(6554)

However, after brave/brave-core#24921 was merged I can't repro with those steps any more.
Don't think it matters, but I was testing with brave/brave-core#25679 changes in the tree.

@GeetaSarvadnya
Copy link

Reproduced the issue on Windows 10 x64 - 1.70.117 Chromium: 129.0.6668.59

Uploaded Crash Report ID: | a4351900-162b-a10c-0000-000000000000
Upload Time: | Tuesday, September 24, 2024 at 2:23:07 PM

@darkdh
Copy link
Member

darkdh commented Sep 26, 2024

I did hit this DCHECK on master when following the STR

[52213:259:0926/140531.595013:FATAL:side_panel_coordinator.cc(1076)] Check failed: content_wrapper->children().size() == 1.
0   libbase.dylib                       0x0000000106173e92 base::debug::CollectStackTrace(base::span<void const*, 18446744073709551615ul, void const**>) + 18
1   libbase.dylib                       0x00000001061548aa base::debug::StackTrace::StackTrace(unsigned long) + 106
2   libbase.dylib                       0x000000010602d69e logging::LogMessage::Flush() + 190
3   libbase.dylib                       0x000000010602d56d logging::LogMessage::~LogMessage() + 29
4   libbase.dylib                       0x0000000106007aff logging::(anonymous namespace)::DCheckLogMessage::~DCheckLogMessage() + 111
5   libbase.dylib                       0x0000000106007b5e logging::(anonymous namespace)::DCheckLogMessage::~DCheckLogMessage() + 14
6   libbase.dylib                       0x0000000106007463 logging::CheckError::~CheckError() + 35
7   libbase.dylib                       0x00000001060074b9 logging::CheckError::~CheckError() + 9
8   libchrome_dll.dylib                 0x0000000113eef551 SidePanelCoordinator::OnTabStripModelChanged(TabStripModel*, TabStripModelChange const&, TabStripSelectionChange const&) + 673
9   libchrome_dll.dylib                 0x00000001141c36a6 non-virtual thunk to BraveSidePanelCoordinator::OnTabStripModelChanged(TabStripModel*, TabStripModelChange const&, TabStripSelectionChange const&) + 70
10  libchrome_dll.dylib                 0x000000011687cf29 TabStripModel::OnChange(TabStripModelChange const&, TabStripSelectionChange const&) + 505
11  libchrome_dll.dylib                 0x000000011688b7a7 TabStripModel::InsertTabAtIndexImpl(std::__Cr::unique_ptr<tabs::TabModel, std::__Cr::default_delete<tabs::TabModel>>, int, std::__Cr::optional<tab_groups::TabGroupId>, bool, bool) + 839
12  libchrome_dll.dylib                 0x000000011687c82c TabStripModel::InsertTabAtImpl(int, std::__Cr::unique_ptr<tabs::TabModel, std::__Cr::default_delete<tabs::TabModel>>, int, std::__Cr::optional<tab_groups::TabGroupId>) + 492
13  libchrome_dll.dylib                 0x0000000116884577 TabStripModel::AddTab(std::__Cr::unique_ptr<tabs::TabModel, std::__Cr::default_delete<tabs::TabModel>>, int, ui::PageTransition, int, std::__Cr::optional<tab_groups::TabGroupId>) + 1383
14  libchrome_dll.dylib                 0x00000001167cdcc6 Navigate(NavigateParams*) + 5686
15  libchrome_dll.dylib                 0x00000001167d22ff chrome::AddAndReturnTabAt(Browser*, GURL const&, int, bool, std::__Cr::optional<tab_groups::TabGroupId>) + 255
16  libchrome_dll.dylib                 0x00000001167d23ce chrome::AddTabAt(Browser*, GURL const&, int, bool, std::__Cr::optional<tab_groups::TabGroupId>) + 78
17  libchrome_dll.dylib                 0x00000001167cff3f chrome::BrowserTabStripModelDelegate::AddTabAt(GURL const&, int, bool, std::__Cr::optional<tab_groups::TabGroupId>) + 79
18  libchrome_dll.dylib                 0x0000000116e7e9b7 BrowserTabStripController::CreateNewTab() + 71
19  libchrome_dll.dylib                 0x0000000116ed0d7f TabStrip::NewTabButtonPressed(ui::Event const&) + 367
20  libui_views.dylib                   0x0000000122f1d945 base::RepeatingCallback<void (ui::Event const&)>::Run(ui::Event const&) const & + 85
21  libui_views.dylib                   0x0000000122f1bfc5 views::Button::NotifyClick(ui::Event const&) + 245
22  libchrome_dll.dylib                 0x0000000116e8b3ae NewTabButton::NotifyClick(ui::Event const&) + 14
23  libui_views.dylib                   0x0000000122f1e6a6 views::ButtonController::OnMouseReleased(ui::MouseEvent const&) + 150
24  libui_events.dylib                  0x000000010ad89768 ui::ScopedTargetHandler::OnEvent(ui::Event*) + 56
25  libui_events.dylib                  0x000000010ad8155e ui::EventDispatcher::ProcessEvent(ui::EventTarget*, ui::Event*) + 366
26  libui_events.dylib                  0x000000010ad81241 ui::EventDispatcherDelegate::DispatchEvent(ui::EventTarget*, ui::Event*) + 161
27  libui_views.dylib                   0x000000012301255a views::internal::RootView::OnMouseReleased(ui::MouseEvent const&) + 138
28  libui_views.dylib                   0x0000000123022e2e views::Widget::OnMouseEvent(ui::MouseEvent*) + 814
29  libui_views.dylib                   0x0000000123043d50 non-virtual thunk to views::NativeWidgetMacNSWindowHost::OnMouseEvent(std::__Cr::unique_ptr<ui::Event, std::__Cr::default_delete<ui::Event>>) + 96

@iefremov
Copy link
Contributor

@srirambv @kjozwiak
The issue is easy to reproduce on stable and beta. On nightly this crash is slightly different and harder to reproduce (probably, this is what Max is seeing in #41124 (comment)) after this big refactoring brave/brave-core#24921
So there are two PRs to beta and stable. I'll make a separate PR for nightly to fix potentially crashing code.

@LaurenWags
Copy link
Member

Requires 1.70.122 or higher for verification 👍🏻

@srirambv
Copy link
Contributor Author

srirambv commented Oct 2, 2024

Verification passed on

Brave 1.70.123 Chromium: 129.0.6668.89 (Official Build) (64-bit)
Revision 7c1bbf03546a64d969d010808e4f5c99fdf343bb
OS Windows 11 Version 23H2 (Build 22631.4169)
  • Verified steps from issue description and #41124 (comment)
  • Verified no crash when closing tab with sidebar open and Leo set to focus
  • Encountered #41320
41124.mp4

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
crash feature/sidebar Relating to Brave's Sidebar feature OS/Desktop priority/P2 A bad problem. We might uplift this to the next planned release. QA Pass-Win64 QA/Yes release-notes/exclude
Projects
Status: Done
Development

No branches or pull requests

8 participants