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

Crash on Chromium 94-based Brave when downloading large files. #17386

Closed
mariospr opened this issue Aug 6, 2021 · 1 comment · Fixed by brave/brave-core#9688
Closed

Crash on Chromium 94-based Brave when downloading large files. #17386

mariospr opened this issue Aug 6, 2021 · 1 comment · Fixed by brave/brave-core#9688

Comments

@mariospr
Copy link
Contributor

mariospr commented Aug 6, 2021

Description

When downloading a large file the DownloadItemView widget is supposed to show certain information about the item being downloaded, such as the name and the current status. Additionally, it's also meant to show the URL but, due to changes done recently in PR 9521 (see issue 1638), Brave contains changes to only show the URL when hovering over the download item view, to save space.

Turns out that these changes were causing a crash after CL 3044607 upstream whenever you attempted to download a file large enough that the DownloadItemView widget would start showing progress:

 base::debug::CollectStackTrace()
 base::debug::StackTrace::StackTrace()
 content::(anonymous namespace)::DumpStackTraceSignalHandler()
 (/usr/lib/x86_64-linux-gnu/libc-2.33.so+0x4103f)
 gfx::ImageSkia::width()
 views::ImageButton::ComputeImagePaintPosition()
 views::ImageButton::PaintButtonContents()
 views::Button::OnPaint()
 views::View::Paint()
 views::View::RecursivePaintHelper()
 views::View::Paint()
 views::View::RecursivePaintHelper()
 views::View::Paint()
 views::View::RecursivePaintHelper()
 BrowserView::PaintChildren()
 views::View::Paint()
 views::View::RecursivePaintHelper()
 views::View::Paint()
 views::View::RecursivePaintHelper()
 views::View::Paint()
 views::View::RecursivePaintHelper()
 BrowserRootView::PaintChildren()
 views::View::Paint()
 views::View::PaintFromPaintRoot()
 ui::Layer::PaintContentsToDisplayList()
 cc::PictureLayer::Update()
 cc::LayerTreeHost::DoUpdateLayers()
 cc::LayerTreeHost::UpdateLayers()
 cc::SingleThreadProxy::DoPainting()
 cc::SingleThreadProxy::BeginMainFrame()
 base::internal::Invoker<>::RunOnce()
 base::TaskAnnotator::RunTask()
 base::sequence_manager::internal::ThreadControllerWithMessagePumpImpl::DoWorkImpl()
 base::sequence_manager::internal::ThreadControllerWithMessagePumpImpl::DoWork()
 base::sequence_manager::internal::ThreadControllerWithMessagePumpImpl::DoWork()
 base::MessagePumpGlib::Run()
 base::sequence_manager::internal::ThreadControllerWithMessagePumpImpl::Run()
 base::RunLoop::Run()
 [...]

Such change also manifests when running the following 2 browser tests:

  BraveCheckClientDownloadRequestBaseBrowserTest.FilterRequest
  SavePackageFinishedObserverBrowserTest.Success

Steps to Reproduce

Using a build of Brave based at least on the 94.0.4584.0 release:

  1. Attempt to download a large enough file (e.g. an Ubuntu installation ISO)

Actual result:

Browser crashes.

Expected result:

Browser should not crash and behaviour implemented to satisfy issue 1638 should be preserved.

Reproduces how often:

Always.

Brave version (brave://version info)

Build of Brave based at least on the 94.0.4584.0 (sorry you'd have to compile it at this point)

Miscellaneous Information:

Since this is only affecting the cr94 branch for now, it might be considered as not urgent strictly speaking. However, the potential for causing issues of a redefinition leaking outside of its chromium_src file is not something minor, so I think it would be better to get this fixed on master ASAP.

@stephendonner
Copy link

https://www.leaseweb.com/platform/network is also very handy for testing file downloads of a few sizes, from various locations.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants