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

When using vertical tabs, pinning a tab that is part of a group crashes the browser - follow up to #40201 #40365

Closed
1 of 6 tasks
LaurenWags opened this issue Aug 8, 2024 · 8 comments · Fixed by brave/brave-core#25983

Comments

@LaurenWags
Copy link
Member

Description

Found while testing #40201 for uplift

Crash when attempting to pin a tab in a tab group still happens sometimes. When a tab group has 1-2 tabs, I couldn't reproduce the crash with 1.70.57, but it easily reproduced with 1.70.56. However, once my tab group had more tabs (3-4), then the below STR relatively reliably reproduced a crash for me. Not 100%, but the crash was fairly consistent.

Steps to reproduce

  1. Clean profile, close and relaunch to pull griffin
  2. Enable vertical tabs
  3. Create a tab group with ~3 tabs
  4. Pin one of the tabs --> sometimes crashes, sometimes does not crash
  5. If no crash, unpin tab
  6. Re-add to same tab group
  7. Try to pin same tab again --> usually crashes
    Note, in step 7 I've also tried pinning a different tab in the group, sometimes that caused a crash, sometimes not

Actual result

browser crashes

Expected result

browser should not crash

Reproduces how often

Easily reproduced

Brave version (brave://version info)

Brave	1.70.57 Chromium: 127.0.6533.100 (Official Build) nightly (arm64) 
Revision	988908b19eb78f2e5fe4c65fa0955c5cc7e37086
OS	macOS Version 14.6.1 (Build 23G93)

Channel information

  • release (stable)
  • beta
  • nightly

Reproducibility

  • with Brave Shields disabled
  • with Brave Rewards disabled
  • in the latest version of Chrome

Miscellaneous information

Crash ID: ffc50300-15c1-690c-0000-000000000000

Call Stack
[ 00 ] base::ImmediateCrash() ( immediate_crash.h:176 )
[ 01 ] logging::NotReachedFailure() ( notreached.h:60 )
[ 02 ] content::PrerenderWebContentsDelegate::OpenURLFromTab(content::WebContents*, content::OpenURLParams const&, base::OnceCallback<void (content::NavigationHandle&)>) ( prerender_web_contents_delegate.cc:17 )
[ 03 ] TabStripLayoutHelper::SlotIsCollapsedTab(int) const ( tab_strip_layout_helper.cc:411 )
[ 04 ] TabStripLayoutHelper::CalculateIdealBounds(std::__Cr::optional<int>) ( tab_strip_layout_helper.cc:268 )
[ 05 ] TabStripLayoutHelper::UpdateIdealBounds(int) ( tab_strip_layout_helper.cc:206 )
[ 06 ] TabContainerImpl::UpdateIdealBounds() ( tab_container_impl.cc:1220 )
[ 07 ] TabContainerImpl::AnimateToIdealBounds() ( tab_container_impl.cc:621 )
[ 08 ] CompoundTabContainer::AnimateToIdealBounds() ( compound_tab_container.cc:566 )
[ 09 ] CompoundTabContainer::TransferTabBetweenContainers(int, int) ( compound_tab_container.cc:969 )
[ 10 ] BraveCompoundTabContainer::TransferTabBetweenContainers(int, int) ( brave_compound_tab_container.cc:193 )
[ 11 ] TabStrip::MoveTab(int, int, TabRendererData) ( tab_strip.cc:1156 )
[ 12 ] BrowserTabStripController::OnTabStripModelChanged(TabStripModel*, TabStripModelChange const&, TabStripSelectionChange const&) ( browser_tab_strip_controller.cc:677 )
[ 13 ] TabStripModel::OnChange(TabStripModelChange const&, TabStripSelectionChange const&) ( tab_strip_model.cc:393 )
[ 14 ] TabStripModel::OnChange(TabStripModelChange const&, TabStripSelectionChange const&) ( tab_strip_model.cc:393 )
[ 15 ] TabStripModel::SendMoveNotificationForWebContents(int, int, content::WebContents*, TabStripSelectionChange&) ( tab_strip_model.cc:2783 )
[ 16 ] TabStripModel::MoveTabToIndexImpl(int, int, std::__Cr::optional<tab_groups::TabGroupId>, bool, bool) ( tab_strip_model.cc:2638 )
[ 17 ] TabStripModel::SetTabsPinned(std::__Cr::vector<int, std::__Cr::allocator<int>>, bool) ( tab_strip_model.cc:2876 )
[ 18 ] TabStripModel::ExecuteContextMenuCommand(int, TabStripModel::ContextMenuCommand) ( tab_strip_model.cc:1476 )
[ 19 ] BraveTabStripModel::ExecuteContextMenuCommand(int, TabStripModel::ContextMenuCommand) ( brave_tab_strip_model.cc:70 )
[ 20 ] -[MenuControllerCocoa itemSelected:] ( menu_controller.mm:303 )
[ 21 ] -[MenuControllerCocoa itemSelected:] ( menu_controller.mm:303 )
[ 22 ] 0x191428ff8
[ 23 ] __43-[BrowserCrApplication sendAction:to:from:]_block_invoke ( chrome_browser_application_mac.mm:393 )
[ 24 ] base::apple::CallWithEHFrame(void () block_pointer)
[ 25 ] 0x1914f3084
[ 26 ] 0x1914f3084
[ 27 ] 0x191aa3488
[ 28 ] 0x19152b82c
[ 29 ] 0x19152b74c
[ 30 ] 0x191a99de8
[ 31 ] 0x1918c2c0c
[ 32 ] 0x1918c2980
[ 33 ] 0x191ff6c10
[ 34 ] 0x19187b844
[ 35 ] 0x191aa4c90
[ 36 ] 0x191aa9264
[ 37 ] 0x1915a96b8
[ 38 ] ui::ShowContextMenu(NSMenu*, NSEvent*, NSView*, bool, ui::ElementContext) ( menu_utils.mm:83 )
[ 39 ] views::internal::MenuRunnerImplCocoa::RunMenuAt(views::Widget*, views::MenuButtonController*, gfx::Rect const&, views::MenuAnchorPosition, int, gfx::NativeView, std::__Cr::optional<gfx::RoundedCornersF>, std::__Cr::optional<std::__Cr::basic_string<char, std::__Cr::char_traits<char>, std::__Cr::allocator<char>>>) ( menu_runner_impl_cocoa.mm:91 )
[ 40 ] views::internal::MenuRunnerImplMac::RunMenuAt(views::Widget*, views::MenuButtonController*, gfx::Rect const&, views::MenuAnchorPosition, int, gfx::NativeView, std::__Cr::optional<gfx::RoundedCornersF>, std::__Cr::optional<std::__Cr::basic_string<char, std::__Cr::char_traits<char>, std::__Cr::allocator<char>>>) ( menu_runner_impl_mac.mm:61 )
[ 41 ] views::MenuRunner::RunMenuAt(views::Widget*, views::MenuButtonController*, gfx::Rect const&, views::MenuAnchorPosition, ui::MenuSourceType, gfx::NativeView, std::__Cr::optional<gfx::RoundedCornersF>, std::__Cr::optional<std::__Cr::basic_string<char, std::__Cr::char_traits<char>, std::__Cr::allocator<char>>>) ( menu_runner.cc:91 )
[ 42 ] TabStrip::ShowContextMenuForTab(Tab*, gfx::Point const&, ui::MenuSourceType) ( tab_strip.cc:1687 )
[ 43 ] TabStrip::TabContextMenuController::ShowContextMenuForViewImpl(views::View*, gfx::Point const&, ui::MenuSourceType) ( tab_strip.cc:2313 )
[ 44 ] views::ContextMenuController::ShowContextMenuForView(views::View*, gfx::Point const&, ui::MenuSourceType) ( context_menu_controller.cc:29 )
[ 45 ] views::View::ProcessMousePressed(ui::MouseEvent const&) ( view.cc:3555 )
[ 46 ] views::View::ProcessMousePressed(ui::MouseEvent const&) ( view.cc:3555 )
[ 47 ] views::View::OnMouseEvent(ui::MouseEvent*) ( view.cc:1652 )
[ 48 ] ui::EventDispatcher::DispatchEvent(ui::EventHandler*, ui::Event*) ( event_dispatcher.cc:187 )
[ 49 ] ui::EventDispatcherDelegate::DispatchEventToTarget(ui::EventTarget*, ui::Event*) ( event_dispatcher.cc:82 )
[ 50 ] ui::EventDispatcherDelegate::DispatchEvent(ui::EventTarget*, ui::Event*) ( event_dispatcher.cc:54 )
[ 51 ] views::internal::RootView::OnMousePressed(ui::MouseEvent const&) ( root_view.cc:498 )
[ 52 ] views::Widget::OnMouseEvent(ui::MouseEvent*) ( widget.cc:1833 )
[ 53 ] views::Widget::OnMouseEvent(ui::MouseEvent*) ( widget.cc:1833 )
[ 54 ] views::NativeWidgetMacNSWindowHost::OnMouseEvent(std::__Cr::unique_ptr<ui::Event, std::__Cr::default_delete<ui::Event>>) ( native_widget_mac_ns_window_host.mm:984 )
[ 55 ] -[BridgedContentView mouseEvent:] ( bridged_content_view.mm:656 )
[ 56 ] 0x191dbfbc4
[ 57 ] 0x1913af524
[ 58 ] 0x1913af20c
[ 59 ] -[NativeWidgetMacNSWindow sendEvent:] ( native_widget_mac_nswindow.mm:473 )
[ 60 ] 0x191a784ec
[ 61 ] __34-[BrowserCrApplication sendEvent:]_block_invoke ( chrome_browser_application_mac.mm:441 )
[ 62 ] base::apple::CallWithEHFrame(void () block_pointer)
[ 63 ] 0x1916c5d98
[ 64 ] 0x1916c5d98
[ 65 ] 0x19127601c
[ 66 ] base::MessagePumpNSApplication::DoRun(base::MessagePump::Delegate*) ( message_pump_apple.mm:807 )
[ 67 ] base::MessagePumpCFRunLoopBase::Run(base::MessagePump::Delegate*) ( message_pump_apple.mm:161 )
[ 68 ] base::sequence_manager::internal::ThreadControllerWithMessagePumpImpl::Run(bool, base::TimeDelta) ( thread_controller_with_message_pump_impl.cc:654 )
[ 69 ] non-virtual thunk to base::sequence_manager::internal::ThreadControllerWithMessagePumpImpl::Run(bool, base::TimeDelta) ( run_loop.cc:0 )
[ 70 ] base::RunLoop::Run(base::Location const&) ( run_loop.cc:134 )
[ 71 ] content::BrowserMainLoop::RunMainMessageLoop() ( browser_main_loop.cc:1085 )
[ 72 ] content::BrowserMainRunnerImpl::Run() ( browser_main_runner_impl.cc:160 )
[ 73 ] content::BrowserMain(content::MainFunctionParams) ( browser_main.cc:34 )
[ 74 ] content::RunContentProcess(content::ContentMainParams, content::ContentMainRunner*) ( content_main.cc:332 )
[ 75 ] content::ContentMain(content::ContentMainParams) ( content_main.cc:345 )
[ 76 ] content::RunContentProcess(content::ContentMainParams, content::ContentMainRunner*) ( content_main.cc:332 )
[ 77 ] content::ContentMain(content::ContentMainParams) ( content_main.cc:345 )
[ 78 ] ChromeMain ( chrome_main.cc:192 )
[ 79 ] 0x18d5bb150
[ 80 ] 0x18d5bb150

Crash ID: b1c70300-15c1-690c-0000-000000000000

Call Stack
[ 00 ] base::ImmediateCrash() ( immediate_crash.h:176 )
[ 01 ] logging::NotReachedFailure() ( notreached.h:60 )
[ 02 ] content::PrerenderWebContentsDelegate::OpenURLFromTab(content::WebContents*, content::OpenURLParams const&, base::OnceCallback<void (content::NavigationHandle&)>) ( prerender_web_contents_delegate.cc:17 )
[ 03 ] TabStripLayoutHelper::SlotIsCollapsedTab(int) const ( tab_strip_layout_helper.cc:411 )
[ 04 ] TabStripLayoutHelper::CalculateIdealBounds(std::__Cr::optional<int>) ( tab_strip_layout_helper.cc:268 )
[ 05 ] TabStripLayoutHelper::UpdateIdealBounds(int) ( tab_strip_layout_helper.cc:206 )
[ 06 ] TabContainerImpl::UpdateIdealBounds() ( tab_container_impl.cc:1220 )
[ 07 ] TabContainerImpl::AnimateToIdealBounds() ( tab_container_impl.cc:621 )
[ 08 ] CompoundTabContainer::AnimateToIdealBounds() ( compound_tab_container.cc:566 )
[ 09 ] CompoundTabContainer::TransferTabBetweenContainers(int, int) ( compound_tab_container.cc:969 )
[ 10 ] BraveCompoundTabContainer::TransferTabBetweenContainers(int, int) ( brave_compound_tab_container.cc:193 )
[ 11 ] TabStrip::MoveTab(int, int, TabRendererData) ( tab_strip.cc:1156 )
[ 12 ] BrowserTabStripController::OnTabStripModelChanged(TabStripModel*, TabStripModelChange const&, TabStripSelectionChange const&) ( browser_tab_strip_controller.cc:677 )
[ 13 ] TabStripModel::OnChange(TabStripModelChange const&, TabStripSelectionChange const&) ( tab_strip_model.cc:393 )
[ 14 ] TabStripModel::OnChange(TabStripModelChange const&, TabStripSelectionChange const&) ( tab_strip_model.cc:393 )
[ 15 ] TabStripModel::SendMoveNotificationForWebContents(int, int, content::WebContents*, TabStripSelectionChange&) ( tab_strip_model.cc:2783 )
[ 16 ] TabStripModel::MoveTabToIndexImpl(int, int, std::__Cr::optional<tab_groups::TabGroupId>, bool, bool) ( tab_strip_model.cc:2638 )
[ 17 ] TabStripModel::SetTabsPinned(std::__Cr::vector<int, std::__Cr::allocator<int>>, bool) ( tab_strip_model.cc:2876 )
[ 18 ] TabStripModel::ExecuteContextMenuCommand(int, TabStripModel::ContextMenuCommand) ( tab_strip_model.cc:1476 )
[ 19 ] BraveTabStripModel::ExecuteContextMenuCommand(int, TabStripModel::ContextMenuCommand) ( brave_tab_strip_model.cc:70 )
[ 20 ] -[MenuControllerCocoa itemSelected:] ( menu_controller.mm:303 )
[ 21 ] -[MenuControllerCocoa itemSelected:] ( menu_controller.mm:303 )
[ 22 ] 0x191428ff8
[ 23 ] __43-[BrowserCrApplication sendAction:to:from:]_block_invoke ( chrome_browser_application_mac.mm:393 )
[ 24 ] base::apple::CallWithEHFrame(void () block_pointer)
[ 25 ] 0x1914f3084
[ 26 ] 0x1914f3084
[ 27 ] 0x191aa3488
[ 28 ] 0x19152b82c
[ 29 ] 0x19152b74c
[ 30 ] 0x191a99de8
[ 31 ] 0x1918c2c0c
[ 32 ] 0x1918c2980
[ 33 ] 0x191ff6c10
[ 34 ] 0x19187b844
[ 35 ] 0x191aa4c90
[ 36 ] 0x191aa9264
[ 37 ] 0x1915a96b8
[ 38 ] ui::ShowContextMenu(NSMenu*, NSEvent*, NSView*, bool, ui::ElementContext) ( menu_utils.mm:83 )
[ 39 ] views::internal::MenuRunnerImplCocoa::RunMenuAt(views::Widget*, views::MenuButtonController*, gfx::Rect const&, views::MenuAnchorPosition, int, gfx::NativeView, std::__Cr::optional<gfx::RoundedCornersF>, std::__Cr::optional<std::__Cr::basic_string<char, std::__Cr::char_traits<char>, std::__Cr::allocator<char>>>) ( menu_runner_impl_cocoa.mm:91 )
[ 40 ] views::internal::MenuRunnerImplMac::RunMenuAt(views::Widget*, views::MenuButtonController*, gfx::Rect const&, views::MenuAnchorPosition, int, gfx::NativeView, std::__Cr::optional<gfx::RoundedCornersF>, std::__Cr::optional<std::__Cr::basic_string<char, std::__Cr::char_traits<char>, std::__Cr::allocator<char>>>) ( menu_runner_impl_mac.mm:61 )
[ 41 ] views::MenuRunner::RunMenuAt(views::Widget*, views::MenuButtonController*, gfx::Rect const&, views::MenuAnchorPosition, ui::MenuSourceType, gfx::NativeView, std::__Cr::optional<gfx::RoundedCornersF>, std::__Cr::optional<std::__Cr::basic_string<char, std::__Cr::char_traits<char>, std::__Cr::allocator<char>>>) ( menu_runner.cc:91 )
[ 42 ] TabStrip::ShowContextMenuForTab(Tab*, gfx::Point const&, ui::MenuSourceType) ( tab_strip.cc:1687 )
[ 43 ] TabStrip::TabContextMenuController::ShowContextMenuForViewImpl(views::View*, gfx::Point const&, ui::MenuSourceType) ( tab_strip.cc:2313 )
[ 44 ] views::ContextMenuController::ShowContextMenuForView(views::View*, gfx::Point const&, ui::MenuSourceType) ( context_menu_controller.cc:29 )
[ 45 ] views::View::ProcessMousePressed(ui::MouseEvent const&) ( view.cc:3555 )
[ 46 ] views::View::ProcessMousePressed(ui::MouseEvent const&) ( view.cc:3555 )
[ 47 ] views::View::OnMouseEvent(ui::MouseEvent*) ( view.cc:1652 )
[ 48 ] ui::EventDispatcher::DispatchEvent(ui::EventHandler*, ui::Event*) ( event_dispatcher.cc:187 )
[ 49 ] ui::EventDispatcherDelegate::DispatchEventToTarget(ui::EventTarget*, ui::Event*) ( event_dispatcher.cc:82 )
[ 50 ] ui::EventDispatcherDelegate::DispatchEvent(ui::EventTarget*, ui::Event*) ( event_dispatcher.cc:54 )
[ 51 ] views::internal::RootView::OnMousePressed(ui::MouseEvent const&) ( root_view.cc:498 )
[ 52 ] views::Widget::OnMouseEvent(ui::MouseEvent*) ( widget.cc:1833 )
[ 53 ] views::Widget::OnMouseEvent(ui::MouseEvent*) ( widget.cc:1833 )
[ 54 ] views::NativeWidgetMacNSWindowHost::OnMouseEvent(std::__Cr::unique_ptr<ui::Event, std::__Cr::default_delete<ui::Event>>) ( native_widget_mac_ns_window_host.mm:984 )
[ 55 ] -[BridgedContentView mouseEvent:] ( bridged_content_view.mm:656 )
[ 56 ] 0x191dbfbc4
[ 57 ] 0x1913af524
[ 58 ] 0x1913af20c
[ 59 ] -[NativeWidgetMacNSWindow sendEvent:] ( native_widget_mac_nswindow.mm:473 )
[ 60 ] 0x191a784ec
[ 61 ] __34-[BrowserCrApplication sendEvent:]_block_invoke ( chrome_browser_application_mac.mm:441 )
[ 62 ] base::apple::CallWithEHFrame(void () block_pointer)
[ 63 ] 0x1916c5d98
[ 64 ] 0x1916c5d98
[ 65 ] 0x19127601c
[ 66 ] base::MessagePumpNSApplication::DoRun(base::MessagePump::Delegate*) ( message_pump_apple.mm:807 )
[ 67 ] base::MessagePumpCFRunLoopBase::Run(base::MessagePump::Delegate*) ( message_pump_apple.mm:161 )
[ 68 ] base::sequence_manager::internal::ThreadControllerWithMessagePumpImpl::Run(bool, base::TimeDelta) ( thread_controller_with_message_pump_impl.cc:654 )
[ 69 ] non-virtual thunk to base::sequence_manager::internal::ThreadControllerWithMessagePumpImpl::Run(bool, base::TimeDelta) ( run_loop.cc:0 )
[ 70 ] base::RunLoop::Run(base::Location const&) ( run_loop.cc:134 )
[ 71 ] content::BrowserMainLoop::RunMainMessageLoop() ( browser_main_loop.cc:1085 )
[ 72 ] content::BrowserMainRunnerImpl::Run() ( browser_main_runner_impl.cc:160 )
[ 73 ] content::BrowserMain(content::MainFunctionParams) ( browser_main.cc:34 )
[ 74 ] content::RunContentProcess(content::ContentMainParams, content::ContentMainRunner*) ( content_main.cc:332 )
[ 75 ] content::ContentMain(content::ContentMainParams) ( content_main.cc:345 )
[ 76 ] content::RunContentProcess(content::ContentMainParams, content::ContentMainRunner*) ( content_main.cc:332 )
[ 77 ] content::ContentMain(content::ContentMainParams) ( content_main.cc:345 )
[ 78 ] ChromeMain ( chrome_main.cc:192 )
[ 79 ] 0x18d5bb150
[ 80 ] 0x18d5bb150
@bsclifton
Copy link
Member

cc: @sangwoo108

@iefremov
Copy link
Contributor

@bsclifton we need someone to pick this one up

@simonhong
Copy link
Member

@bsclifton we need someone to pick this one up

Checking the PR brave/brave-core#25406

@simonhong
Copy link
Member

Found more concrete STR.
To make crash, create group with 3 tabs.
and try to pin second tab in the group. Always crashed.

@simonhong
Copy link
Member

This is upstream issue. Filed https://issues.chromium.org/issues/371112521.
This crash comes from using CompoundTabContainer what we use for vertical tab.
However, in the upstream, that class is only used under the SplitTabStrip that disabled by default.
So, maybe it's not the urgent issue for upstream. but we're using it in release.
I think we should upstream fix.

@simonhong simonhong self-assigned this Oct 4, 2024
@simonhong
Copy link
Member

@LaurenWags
Copy link
Member Author

Requires 1.71.113 or higher for verification 👍🏻 @brave/qa-team can see concrete STR via brave/brave-core#25983 (comment) and my verification notes from brave/brave-core#25983 (comment) for guidance when testing.

@GeetaSarvadnya GeetaSarvadnya added the QA/In-Progress Indicates that QA is currently in progress for that particular issue label Oct 16, 2024
@GeetaSarvadnya
Copy link

GeetaSarvadnya commented Oct 16, 2024

Verification INPROGRESS on

Brave | 1.71.113 Chromium: 130.0.6723.58 (Official Build) (64-bit)
-- | --
Revision | 3ee74228044546a84c661332ddb441a7a68108fa
OS | Windows 10 Version 22H2 (Build 19045.5011)

Reproduced the issue on 1.70.126
Image
Verified the test plan from brave/brave-core#25983 and ensured that there is no crash after pinning the 2nd tab in a tab group
Image

@GeetaSarvadnya GeetaSarvadnya added QA Pass-Win64 and removed QA/In-Progress Indicates that QA is currently in progress for that particular issue labels Oct 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment