Insecure download crashes Brave when resuming after warning icon/flow

[stephendonner]

Description

Insecure download crashes Brave when resuming after warning icon/flow

Steps to Reproduce

1. install 1.53.67
2. launch Brave
3. load https://www.leaseweb.com/platform/network
4. context-click on any file-download link
5. choose Save Link As...
6. click Save
7. you'll get the orange warning triangle icon, instead of the normal Downloads icon
8. click it

Actual result:

💥 Crashes in main - literally says nothing more than that in Backtrace.io, other than it hit a breakpoint.

Expected result:

No crash

Reproduces how often:

100%

Brave version (brave://version info)

Brave	1.53.67 Chromium: 114.0.5735.53 (Official Build) beta (x86_64)
Revision	c499d7ea22c8b2dba278465a5df7b86a8efa4e64-refs/branch-heads/5735@{#970}
OS	macOS Version 11.7.7 (Build 20G1345)

Version/Channel Information:

• Can you reproduce this issue with the current release?
• Can you reproduce this issue with the beta channel?
• Can you reproduce this issue with the nightly channel?

Other Additional Information:

• Does the issue resolve itself when disabling Brave Shields?
• Does the issue resolve itself when disabling Brave Rewards?
• Is the issue reproducible on the latest version of Chrome?

Miscellaneous Information:

cc @spylogsster @sangwoo108 @simonhong @rebron @brave/qa-team

[stephendonner]

New stacktrace (same crash steps) from 1.53.74 Chromium: 114.0.5735.53 is:

[ 00 ] base::ImmediateCrash
[ 01 ] logging::CheckFailure
[ 02 ] views::DialogDelegate::GetInitiallyFocusedView
[ 03 ] views::BubbleDialogDelegate::GetAccessibleWindowRole
[ 04 ] views::BubbleDialogDelegate::OnBubbleWidgetVisibilityChanged
[ 05 ] views::BubbleDialogDelegate::BubbleWidgetObserver::OnWidgetVisibilityChanged
[ 06 ] views::Widget::OnNativeWidgetVisibilityChanged
[ 07 ] -[NativeWidgetMacNSWindow orderWindow:relativeTo:]
[ 08 ] -[NativeWidgetMacNSWindow addChildWindow:ordered:]
[ 09 ] remote_cocoa::NativeWidgetNSWindowBridge::OrderChildren
[ 10 ] remote_cocoa::NativeWidgetNSWindowBridge::SetVisibilityState
[ 11 ] base::sequence_manager::internal::ThreadControllerWithMessagePumpImpl::DoWorkImpl
[ 12 ] base::sequence_manager::internal::ThreadControllerWithMessagePumpImpl::DoWork
[ 13 ] non-virtual thunk to base::sequence_manager::internal::ThreadControllerWithMessagePumpImpl::DoWork
[ 14 ] base::MessagePumpCFRunLoopBase::RunWork
[ 15 ] base::MessagePumpCFRunLoopBase::RunWorkSource
[ 16 ] base::mac::CallWithEHFrame(void
[ 17 ] -[BrowserCrApplication nextEventMatchingMask:untilDate:inMode:dequeue:]
[ 18 ] base::MessagePumpNSApplication::DoRun
[ 19 ] base::MessagePumpCFRunLoopBase::Run
[ 20 ] base::sequence_manager::internal::ThreadControllerWithMessagePumpImpl::Run
[ 21 ] non-virtual thunk to base::sequence_manager::internal::ThreadControllerWithMessagePumpImpl::Run
[ 22 ] base::RunLoop::Run
[ 23 ] content::BrowserMainLoop::RunMainMessageLoop
[ 24 ] content::BrowserMainRunnerImpl::Run
[ 25 ] content::BrowserMain
[ 26 ] content::RunBrowserProcessMain
[ 27 ] content::ContentMainRunnerImpl::RunBrowser
[ 28 ] content::ContentMainRunnerImpl::Run
[ 29 ] content::RunContentProcess
[ 30 ] content::ContentMain
[ 31 ] ChromeMain
[ 32 ] main

[simonhong]

Looking now

[simonhong]

In local build, I got different dcheck failure first.

[16648:259:0601/092439.328107:FATAL:ax_node_data.cc(621)] Check failed: name.empty() || (role != ax::mojom::Role::kNone && role != ax::mojom::Role::kUnknown). Cannot set name to 'This file may have been read or edited because this site isn't using a secure connection' on class: '' because a valid role is needed to set the default NameFrom attribute. Set the role first.
0   libbase.dylib                       0x0000000105d151a2 base::debug::CollectStackTrace(void**, unsigned long) + 18
1   libbase.dylib                       0x0000000105cf8b83 base::debug::StackTrace::StackTrace() + 19
2   libbase.dylib                       0x0000000105bc27d1 logging::LogMessage::~LogMessage() + 177
3   libbase.dylib                       0x0000000105ba32f0 logging::(anonymous namespace)::DCheckLogMessage::~DCheckLogMessage() + 48
4   libbase.dylib                       0x0000000105ba2ec7 logging::CheckError::~CheckError() + 23
5   libbase.dylib                       0x0000000105ba2ee9 logging::CheckError::~CheckError() + 9
6   libui_accessibility_ax_base.dylib   0x000000010933d825 ui::AXNodeData::SetName(std::Cr::basic_string<char, std::Cr::char_traits<char>, std::Cr::allocator<char>> const&) + 245
7   libui_accessibility_ax_base.dylib   0x000000010933daf8 ui::AXNodeData::SetNameChecked(std::Cr::basic_string<char, std::Cr::char_traits<char>, std::Cr::allocator<char>> const&) + 24
8   libui_views.dylib                   0x000000011ebf2bb3 views::ViewAccessibility::OverrideName(std::Cr::basic_string<char, std::Cr::char_traits<char>, std::Cr::allocator<char>> const&, ax::mojom::NameFrom) + 243
9   libui_views.dylib                   0x000000011ebf2c5a views::ViewAccessibility::OverrideName(std::Cr::basic_string<char16_t, std::Cr::char_traits<char16_t>, std::Cr::allocator<char16_t>> const&, ax::mojom::NameFrom) + 58
10  libchrome_dll.dylib                 0x0000000114116e2c DownloadBubbleSecurityView::UpdateAccessibilityTextAndFocus() + 44
11  libchrome_dll.dylib                 0x000000011411a5e7 non-virtual thunk to DownloadToolbarButtonViewChromium::OpenSecurityDialog(DownloadBubbleRowView*) + 167
12  libui_views.dylib                   0x000000011ebf52b3 base::RepeatingCallback<void ()>::Run() const & + 67
13  libui_views.dylib                   0x000000011ec2c40a _ZN4base8internal7InvokerINS0_9BindStateIZN5views6Button15PressedCallbackC1ENS_17RepeatingCallbackIFvvEEEE3$_0JS8_EEEFvRKN2ui5EventEEE3RunEPNS0_13BindStateBaseESE_ + 42
14  libui_views.dylib                   0x000000011ec2c07b base::RepeatingCallback<void (ui::Event const&)>::Run(ui::Event const&) const & + 75
15  libui_views.dylib                   0x000000011ec2b313 views::Button::NotifyClick(ui::Event const&) + 243
16  libui_views.dylib                   0x000000011ec29331 views::Button::DefaultButtonControllerDelegate::NotifyClick(ui::Event const&) + 17
17  libui_views.dylib                   0x000000011ec2cdff views::ButtonController::OnMouseReleased(ui::MouseEvent const&) + 159
18  libui_events.dylib                  0x00000001092fcd7b ui::ScopedTargetHandler::OnEvent(ui::Event*) + 59
19  libui_events.dylib                  0x00000001092f5bb3 ui::EventDispatcher::ProcessEvent(ui::EventTarget*, ui::Event*) + 323
20  libui_events.dylib                  0x00000001092f598d ui::EventDispatcherDelegate::DispatchEventToTarget(ui::EventTarget*, ui::Event*) + 93
21  libui_events.dylib                  0x00000001092f58a7 ui::EventDispatcherDelegate::DispatchEvent(ui::EventTarget*, ui::Event*) + 119
22  libui_views.dylib                   0x000000011ed0cffd views::internal::RootView::OnMouseReleased(ui::MouseEvent const&) + 141

And, above seems fixed by calling GetViewAccessibility().OverrideRole(ax::mojom::Role::kAlert); from DownloadBubbleSecurityView::UpdateAccessibilityTextAndFocus().

And then I can get this crash log and it's same crash with the description.

[19158:259:0601/102915.121635:FATAL:dialog_delegate.cc(184)] Check failed: default_button & GetDialogButtons().
0   libbase.dylib                       0x00000001077191a2 base::debug::CollectStackTrace(void**, unsigned long) + 18
1   libbase.dylib                       0x00000001076fcb83 base::debug::StackTrace::StackTrace() + 19
2   libbase.dylib                       0x00000001075c67d1 logging::LogMessage::~LogMessage() + 177
3   libbase.dylib                       0x00000001075c769e logging::LogMessage::~LogMessage() + 14
4   libbase.dylib                       0x00000001075a6ec7 logging::CheckError::~CheckError() + 23
5   libbase.dylib                       0x00000001075a6ee9 logging::CheckError::~CheckError() + 9
6   libui_views.dylib                   0x000000012072e852 views::DialogDelegate::GetInitiallyFocusedView() + 578
7   libui_views.dylib                   0x000000012061af04 views::BubbleDialogDelegate::BubbleWidgetObserver::OnWidgetVisibilityChanged(views::Widget*, bool) + 68
8   libui_views.dylib                   0x000000012071f268 views::Widget::OnNativeWidgetVisibilityChanged(bool) + 632
9   libcomponents_remote_cocoa_app_shim 0x0000000120e1f470 -[NativeWidgetMacNSWindow orderWindow:relativeTo:] + 80
10  AppKit                              0x00007ff81e8dd53e _NSWindowRebuildOrderingGroupInternal + 838
11  AppKit                              0x00007ff81e70e2c7 NSPerformVisuallyAtomicChange + 132
12  AppKit                              0x00007ff81e8dcb9b -[NSWindow addChildWindow:ordered:] + 640
13  libcomponents_remote_cocoa_app_shim 0x0000000120e1def6 -[NativeWidgetMacNSWindow addChildWindow:ordered:] + 86
14  libcomponents_remote_cocoa_app_shim 0x0000000120e22ca5 remote_cocoa::NativeWidgetNSWindowBridge::OrderChildren() + 149
15  libcomponents_remote_cocoa_app_shim 0x0000000120e245f2 remote_cocoa::NativeWidgetNSWindowBridge::SetVisibilityState(remote_cocoa::mojom::WindowVisibilityState) + 626
16  libui_views.dylib                   0x000000012071b484 views::Widget::Activate() + 84
17  libchrome_dll.dylib                 0x00000001130ad489 base::internal::Invoker<base::internal::BindState<void (views::Widget::*)(), base::WeakPtr<views::Widget>>, void ()>::RunOnce(base::internal::BindStateBase*) + 89
18  libbase.dylib                       0x000000010764d02c base::TaskAnnotator::RunTaskImpl(base::PendingTask&) + 300
19  libbase.dylib                       0x000000010767f000 base::sequence_manager::internal::ThreadControllerWithMessagePumpImpl::DoWorkImpl(base::LazyNow*) + 1616
20  libbase.dylib                       0x000000010767e3f4 base::sequence_manager::internal::ThreadControllerWithMessagePumpImpl::DoWork() + 132
21  libbase.dylib                       0x000000010767fbe5 non-virtual thunk to base::sequence_manager::internal::ThreadControllerWithMessagePumpImpl::DoWork() + 21
22  libbase.dylib                       0x0000000107746d3f base::MessagePumpCFRunLoopBase::RunWork() + 95
23  libbase.dylib                       0x000000010773f452 base::mac::CallWithEHFrame(void () block_pointer) + 10
24  libbase.dylib                       0x00000001077462cf base::MessagePumpCFRunLoopBase::RunWorkSource(void*) + 63

[simonhong]

Ok, I found the issue. It's upstream bug and chrome stable also has this crash.
and fix was merged yesterday. https://chromium-review.googlesource.com/c/chromium/src/+/4574027
Let's use it in advance.

[simonhong]

BTW, I found another issue while working on this issue - #30736. Looking.

[kjozwiak]

The above requires 1.52.122 or higher for 1.52.x verification 👍

[LaurenWags]

Verified with

Brave | 1.52.122 Chromium: 114.0.5735.110 (Official Build) (x86_64)
-- | --
Revision | 1c828682b85bbc70230a48f5e345489ec447373e-refs/branch-heads/5735_90@{#13}
OS | macOS Version 13.4 (Build 22F66)

Reproduced the crash using STR from description and 1.52.117 Chromium: 114.0.5735.90.

Confirmed when using 1.52.122 Chromium: 114.0.5735.110 no crash occurs with STR from description.

[MadhaviSeelam]

Verification PASSED using

Brave | 1.52.122 Chromium: 114.0.5735.110 (Official Build) (64-bit)
-- | --
Revision | 1c828682b85bbc70230a48f5e345489ec447373e-refs/branch-heads/5735_90@{#13}
OS | Windows 11 Version 22H2 (Build 22621.1702)

Verified using original STR from the description.
Confirmed no crash occurred when clicked on the orange warning triangle icon.

ex	ex