Crash in BraveShieldsDataController::OnFaviconUpdated when toggling Brave Shields status between two same-site windows

[stephendonner]

Description

Crash in BraveShieldsDataController::OnFaviconUpdated when toggling Brave Shields status between two same-site windows

Steps to Reproduce

1. install 1.39.88
2. launch Brave
3. load a site -- like twitch.tv in two windows; Shields are default UP for both
4. in either window, toggle Shields to DOWN
5. in the other window, where Shields are still UP (whether or not the icon reflects that state accurately -- separate issue), trigger resources blocked by clicking on the < and > arrows in the middle content pane/carousel
6. wait for resources to load
7. now, toggle Brave Shields to DOWN
8. if you don't crash here, repeat steps 4-7

Actual result:

Crashes here:

[ 00 ] brave_shields::BraveShieldsDataController::OnFaviconUpdated(favicon::FaviconDriver*, favicon::FaviconDriverObserver::NotificationIconType, GURL const&, bool, gfx::Image const&)
[ 01 ] <name omitted>
[ 02 ] favicon::FaviconHandler::NotifyFaviconUpdated(GURL const&, favicon_base::IconType, gfx::Image const&)
[ 03 ] favicon::FaviconHandler::NotifyFaviconUpdated(std::__1::vector<favicon_base::FaviconRawBitmapResult, std::__1::allocator<favicon_base::FaviconRawBitmapResult> > const&)
[ 04 ] favicon::FaviconHandler::OnFaviconDataForInitialURLFromFaviconService(std::__1::vector<favicon_base::FaviconRawBitmapResult, std::__1::allocator<favicon_base::FaviconRawBitmapResult> > const&)
[ 05 ] void base::internal::ReplyAdapter<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&>(base::OnceCallback<void (std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&)>, std::__1::unique_ptr<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >, std::__1::default_delete<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > > >*)
[ 06 ] base::internal::Invoker<base::internal::BindState<void (*)(base::OnceCallback<std::__1::unique_ptr<net::SerialWorker::WorkItem, std::__1::default_delete<net::SerialWorker::WorkItem> > ()>, std::__1::unique_ptr<std::__1::unique_ptr<net::SerialWorker::WorkItem, std::__1::default_delete<net::SerialWorker::WorkItem> >, std::__1::default_delete<std::__1::unique_ptr<net::SerialWorker::WorkItem, std::__1::default_delete<net::SerialWorker::WorkItem> > > >*), base::OnceCallback<std::__1::unique_ptr<net::SerialWorker::WorkItem, std::__1::default_delete<net::SerialWorker::WorkItem> > ()>, base::internal::UnretainedWrapper<std::__1::unique_ptr<std::__1::unique_ptr<net::SerialWorker::WorkItem, std::__1::default_delete<net::SerialWorker::WorkItem> >, std::__1::default_delete<std::__1::unique_ptr<net::SerialWorker::WorkItem, std::__1::default_delete<net::SerialWorker::WorkItem> > > > > >, void ()>::RunOnce(base::internal::BindStateBase*)
[ 07 ] <name omitted>
[ 08 ] <name omitted>
[ 09 ] base::internal::Invoker<base::internal::BindState<void (*)(scoped_refptr<base::SequencedTaskRunner> const&, scoped_refptr<base::RefCountedData<base::AtomicFlag> > const&, base::OnceCallback<void ()>, base::OnceCallback<void ()>), scoped_refptr<base::SequencedTaskRunner>, scoped_refptr<base::RefCountedData<base::AtomicFlag> >, base::OnceCallback<void ()>, base::OnceCallback<void ()> >, void ()>::RunOnce(base::internal::BindStateBase*)
[ 10 ] base::internal::Invoker<base::internal::BindState<content::RenderFrameHostImpl::RunBeforeUnloadConfirm(bool, base::OnceCallback<void (bool)>)::$_10, base::OnceCallback<void (bool)> >, void (bool, std::__1::basic_string<char16_t, std::__1::char_traits<char16_t>, std::__1::allocator<char16_t> > const&)>::RunOnce(base::internal::BindStateBase*, bool, std::__1::basic_string<char16_t, std::__1::char_traits<char16_t>, std::__1::allocator<char16_t> > const&)
[ 11 ] base::internal::Invoker<base::internal::BindState<void (*)(base::(anonymous namespace)::PostTaskAndReplyRelay), base::(anonymous namespace)::PostTaskAndReplyRelay>, void ()>::RunOnce(base::internal::BindStateBase*)
[ 12 ] <name omitted>
[ 13 ] non-virtual thunk to base::sequence_manager::internal::ThreadControllerWithMessagePumpImpl::DoWork()
[ 14 ] invocation function for block in base::MessagePumpCFRunLoopBase::RunWorkSource(void*)
[ 15 ] base::mac::CallWithEHFrame(void () block_pointer)
[ 16 ] base::MessagePumpCFRunLoopBase::RunWorkSource(void*)
[ 17 ] 0x7fff2050537c
[ 18 ] 0x7fff205052e4
[ 19 ] 0x7fff20505064
[ 20 ] 0x7fff20503a8c
[ 21 ] 0x7fff2050304c
[ 22 ] 0x7fff2874ba83
[ 23 ] 0x7fff2874b7e5
[ 24 ] 0x7fff2874b583
[ 25 ] 0x7fff22d0bd72
[ 26 ] 0x7fff22d0a545
[ 27 ] __71-[BrowserCrApplication nextEventMatchingMask:untilDate:inMode:dequeue:]_block_invoke
[ 28 ] base::mac::CallWithEHFrame(void () block_pointer)
[ 29 ] -[BrowserCrApplication nextEventMatchingMask:untilDate:inMode:dequeue:]
[ 30 ] 0x7fff22cfc869
[ 31 ] base::MessagePumpNSApplication::DoRun(base::MessagePump::Delegate*)
[ 32 ] base::MessagePumpCFRunLoopBase::Run(base::MessagePump::Delegate*)
[ 33 ] <name omitted>
[ 34 ] <name omitted>
[ 35 ] <name omitted>
[ 36 ] content::BrowserMain(content::MainFunctionParams)
[ 37 ] <name omitted>
[ 38 ] content::ContentMain(content::ContentMainParams)
[ 39 ] ChromeMain
[ 40 ] main
[ 41 ] 0x7fff20428f3d

Expected result:

No crash; Brave Shields icon/state should update accurately without crashing, among tabs/windows, etc.

Reproduces how often:

100%

Brave version (brave://version info)

Brave	1.39.45 Chromium: 100.0.4896.79 (Official Build) nightly (x86_64)
Revision	8fb749dcab8700c24213791969e59deb72fee36f-refs/branch-heads/4896@{#1015}
OS	macOS Version 11.6.5 (Build 20G527)

Version/Channel Information:

• Can you reproduce this issue with the current release? no
• Can you reproduce this issue with the beta channel? not sure
• Can you reproduce this issue with the nightly channel? yes

cc @nullhook @rebron @bridiver @MadhaviSeelam @GeetaSarvadnya @LaurenWags @kjozwiak

[stephendonner]

@simonhong Tested https://brave-jenkins-build-artifacts.s3.amazonaws.com/brave-browser-build-pr/crash_shields_v2/dad21c0df534f4d292e68c3d8c8b65b4216f5f14-4658a1046d7223cef1321727284cd6f44021d28d/macos/unsigned/Brave%20Browser%20Nightly.dmg and verified that it no longer crashes for me with my use-cases and exploratory testing 👍

[kjozwiak]

Above requires 1.38.102 or higher for 1.38.x verification.

[stephendonner]

Verified PASSED using

Brave	1.38.103 Chromium: 100.0.4896.127 (Official Build) beta (x86_64)
Revision	ff0d0695743e65305d7194f9bd309e5e1c824aa0-refs/branch-heads/4896_88@{#4}
OS	macOS Version 11.6.5 (Build 20G527)

Followed my original steps to reproduce, as well as exploratory testing, and didn't crash.

[btlechowski]

Verification passed on

Brave	1.38.105 Chromium: 101.0.4951.41 (Official Build) (64-bit)
Revision	93c720db8323b3ec10d056025ab95c23a31997c9-refs/branch-heads/4951@{#904}
OS	Ubuntu 18.04 LTS

Followed the original steps to reproduce, as well as exploratory testing, and didn't crash.

[MadhaviSeelam]

Verification PASSED on

Brave	1.38.107 Chromium: 101.0.4951.41 (Official Build) (64-bit)
Revision	93c720db8323b3ec10d056025ab95c23a31997c9-refs/branch-heads/4951@{#904}
OS	Windows 11 Version 21H2 (Build 22000.613)

Followed the original steps to reproduce, as well as exploratory testing, and didn't crash.