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 while using omnibox in Tor window with an enabled extension #15140

Closed
iefremov opened this issue Apr 6, 2021 · 11 comments · Fixed by brave/brave-core#8460
Closed

Crash while using omnibox in Tor window with an enabled extension #15140

iefremov opened this issue Apr 6, 2021 · 11 comments · Fixed by brave/brave-core#8460

Comments

@iefremov
Copy link
Contributor

iefremov commented Apr 6, 2021

STR:

  1. Install DDG privacy essentials https://chrome.google.com/webstore/detail/duckduckgo-privacy-essent/bkdgflcldnnnapblkhphbgpggdiikppg?hl=en
  2. on brave://extensions/?id=bkdgflcldnnnapblkhphbgpggdiikppg enable "Allow in private"
  3. Open TOR window
  4. focus omnibox and type
  5. crash

Mac OS:

[ 00 ] <name omitted>
[ 01 ] non-virtual thunk to TorWindowSearchEngineProviderService::OnTemplateURLServiceChanged()
[ 02 ] TemplateURLService::Scoper::~Scoper()
[ 03 ] TemplateURLService::OnWebDataServiceRequestDone(int, std::__1::unique_ptr<WDTypedResult, std::__1::default_delete<WDTypedResult> >)
[ 04 ] WebDataRequestManager::RequestCompletedOnThread(std::__1::unique_ptr<WebDataRequest, std::__1::default_delete<WebDataRequest> >, std::__1::unique_ptr<WDTypedResult, std::__1::default_delete<WDTypedResult> >)

Windows:

[ 00 ] PrefMember<int>::UpdatePref(int const &)
[ 01 ] TorWindowSearchEngineProviderService::OnTemplateURLServiceChanged()
[ 02 ] base::ObserverList<AccessContextAuditService::CookieAccessHelper,0,1,base::internal::CheckedObserverAdapter>::begin()
[ 03 ] base::internal::PartitionFree(base::allocator::AllocatorDispatch const *,void *,void *)
[ 04 ] TemplateURLService::Scoper::~Scoper()
[ 05 ] TemplateURLService::OnWebDataServiceRequestDone(int,std::__1::unique_ptr<WDTypedResult,std::__1::default_delete<WDTypedResult> >)
[ 06 ] WebDataRequestManager::RequestCompletedOnThread(std::__1::unique_ptr<WebDataRequest,std::__1::default_delete<WebDataRequest> >,std::__1::unique_ptr<WDTypedResult,std::__1::default_delete<WDTypedResult> >)
[ 07 ] _tailMerge_esent.dll
[ 08 ] _tailMerge_esent.dll
[ 09 ] base::internal::Invoker<base::internal::BindState<void (WebDataRequestManager::*)(std::unique_ptr<WebDataRequest,std::default_delete<WebDataRequest> >, std::unique_ptr<WDTypedResult,std::default_delete<WDTypedResult> >),scoped_refptr<WebDataRequestManager>,std::unique_ptr<WebDataRequest,std::default_delete<WebDataRequest> >,std::unique_ptr<WDTypedResult,std::default_delete<WDTypedResult> > >,void ()>::RunOnce
[ 10 ] base::ThreadLocalStorage::Slot::Get()
[ 11 ] base::TaskAnnotator::RunTask(char const *,base::PendingTask *)
[ 12 ] WebDataRequestManager::RequestCompleted(std::__1::unique_ptr<WebDataRequest,std::__1::default_delete<WebDataRequest> >,std::__1::unique_ptr<WDTypedResult,std::__1::default_delete<WDTypedResult> >)
[ 13 ] KeywordWebDataService::GetKeywords(WebDataServiceConsumer *)
[ 14 ] _tailMerge_esent.dll
[ 15 ] WebDataRequestManager::RequestCompleted(std::__1::unique_ptr<WebDataRequest,std::__1::default_delete<WebDataRequest> >,std::__1::unique_ptr<WDTypedResult,std::__1::default_delete<WDTypedResult> >)

https://brave.sp.backtrace.io/p/brave/explore?time=month&filters=((callstack.functions%2Ccontains%2COnTemplateURLServiceChanged)%2C_deleted%3D0%2C(ver%2Cregex%2C%2289%7C90.*%22)%2Cptype%3Dbrowser)&aggregations=((uname.sysname%2Cdistribution%2C3)%2C(ver%2Cmax)%2C(callstack%2Chead))&

@iefremov iefremov added crash feature/tor priority/P2 A bad problem. We might uplift this to the next planned release. OS/Desktop labels Apr 6, 2021
@iefremov
Copy link
Contributor Author

iefremov commented Apr 6, 2021

cc @darkdh

@darkdh
Copy link
Member

darkdh commented Apr 6, 2021

cc @simonhong

@simonhong simonhong self-assigned this Apr 6, 2021
@simonhong
Copy link
Member

simonhong commented Apr 7, 2021

Got earlier DCHECK failure with above repro step.
Also got same failure witn private window.

[15610:775:0407/102627.994637:FATAL:template_url_service.cc(1797)] Check failed: default_search_provider_.
0   libbase.dylib                       0x0000000118b8aa49 base::debug::CollectStackTrace(void**, unsigned long) + 9
1   libbase.dylib                       0x0000000118a63b73 base::debug::StackTrace::StackTrace() + 19
2   libbase.dylib                       0x0000000118a887df logging::LogMessage::~LogMessage() + 175
3   libbase.dylib                       0x0000000118a896de logging::LogMessage::~LogMessage() + 14
4   libchrome_dll.dylib                 0x000000010f5e21bc TemplateURLService::ApplyDefaultSearchChangeNoMetrics(TemplateURLData const*, DefaultSearchManager::Source) + 300
5   libchrome_dll.dylib                 0x000000010f5dc6e9 TemplateURLService::ChangeToLoadedState() + 121
6   libchrome_dll.dylib                 0x000000010f5dc9fd TemplateURLService::OnWebDataServiceRequestDone(int, std::__Cr::unique_ptr<WDTypedResult, std::__Cr::default_delete<WDTypedResult> >) + 301
7   libwebdata_common.dylib             0x0000000124678472 WebDataRequestManager::RequestCompletedOnThread(std::__Cr::unique_ptr<WebDataRequest, std::__Cr::default_delete<WebDataRequest> >, std::__Cr::unique_ptr<WDTypedResult, std::__Cr::default_delete<WDTypedResult> >) + 50
8   libwebdata_common.dylib             0x0000000124679149 base::internal::Invoker<base::internal::BindState<void (WebDataRequestManager::*)(std::__Cr::unique_ptr<WebDataRequest, std::__Cr::default_delete<WebDataRequest> >, std::__Cr::unique_ptr<WDTypedResult, std::__Cr::default_delete<WDTypedResult> >), scoped_refptr<WebDataRequestManager>, std::__Cr::unique_ptr<WebDataRequest, std::__Cr::default_delete<WebDataRequest> >, std::__Cr::unique_ptr<WDTypedResult, std::__Cr::default_delete<WDTypedResult> > >, void ()>::RunOnce(base::internal::BindStateBase*) + 121
9   libbase.dylib                       0x0000000118b0fb23 base::TaskAnnotator::RunTask(char const*, base::PendingTask*) + 387
10  libbase.dylib                       0x0000000118b36bf7 base::sequence_manager::internal::ThreadControllerWithMessagePumpImpl::DoWorkImpl(base::sequence_manager::LazyNow*) + 1015
11  libbase.dylib                       0x0000000118b3640d base::sequence_manager::internal::ThreadControllerWithMessagePumpImpl::DoWork() + 93
12  libbase.dylib                       0x0000000118bafe9f base::MessagePumpCFRunLoopBase::RunWork() + 79
13  libbase.dylib                       0x0000000118ba3162 base::mac::CallWithEHFrame(void () block_pointer) + 10
14  libbase.dylib                       0x0000000118baf87f base::MessagePumpCFRunLoopBase::RunWorkSource(void*) + 63
15  CoreFoundation                      0x00007fff2053ca0c __CFRUNLOOP_IS_CALLING_OUT_TO_A_SOURCE0_PERFORM_FUNCTION__ + 17
16  CoreFoundation                      0x00007fff2053c974 __CFRunLoopDoSource0 + 180
17  CoreFoundation                      0x00007fff2053c6ef __CFRunLoopDoSources0 + 248
18  CoreFoundation                      0x00007fff2053b121 __CFRunLoopRun + 890
19  CoreFoundation                      0x00007fff2053a6ce CFRunLoopRunSpecific + 563
20  HIToolbox                           0x00007fff287c2630 RunCurrentEventLoopInMode + 292
21  HIToolbox                           0x00007fff287c2282 ReceiveNextEventCommon + 283
22  HIToolbox                           0x00007fff287c214f _BlockUntilNextEventMatchingListInModeWithFilter + 64
23  AppKit                              0x00007fff22d5a9b1 _DPSNextEvent + 883
24  AppKit                              0x00007fff22d59177 -[NSApplication(NSEvent) _nextEventMatchingEventMask:untilDate:inMode:dequeue:] + 1366
25  libchrome_dll.dylib                 0x000000010edda7b0 __71-[BrowserCrApplication nextEventMatchingMask:untilDate:inMode:dequeue:]_block_invoke + 64
26  libbase.dylib                       0x0000000118ba3162 base::mac::CallWithEHFrame(void () block_pointer) + 10
27  libchrome_dll.dylib                 0x000000010edda6e9 -[BrowserCrApplication nextEventMatchingMask:untilDate:inMode:dequeue:] + 153
28  AppKit                              0x00007fff22d4b68a -[NSApplication run] + 586
29  libbase.dylib                       0x0000000118bb0dac base::MessagePumpNSApplication::DoRun(base::MessagePump::Delegate*) + 348
30  libbase.dylib                       0x0000000118baf30c base::MessagePumpCFRunLoopBase::Run(base::MessagePump::Delegate*) + 140
31  libbase.dylib                       0x0000000118b37756 base::sequence_manager::internal::ThreadControllerWithMessagePumpImpl::Run(bool, base::TimeDelta) + 646
32  libbase.dylib                       0x0000000118adcbc5 base::RunLoop::Run(base::Location const&) + 885
33  libchrome_dll.dylib                 0x000000010e8e7f7b ChromeBrowserMainParts::MainMessageLoopRun(int*) + 203
34  libcontent.dylib                    0x000000011d5fb766 content::BrowserMainLoop::RunMainMessageLoopParts() + 54
35  libcontent.dylib                    0x000000011d5fd792 content::BrowserMainRunnerImpl::Run() + 82
36  libcontent.dylib                    0x000000011d5f8b16 content::BrowserMain(content::MainFunctionParams const&) + 230
37  libcontent.dylib                    0x000000011e27157d content::ContentMainRunnerImpl::RunBrowser(content::MainFunctionParams&, bool) + 1181
38  libcontent.dylib                    0x000000011e271072 content::ContentMainRunnerImpl::Run(bool) + 514
39  libcontent.dylib                    0x000000011e26fd6f content::RunContentProcess(content::ContentMainParams const&, content::ContentMainRunner*) + 2639
40  libcontent.dylib                    0x000000011e26fe4d content::ContentMain(content::ContentMainParams const&) + 29
41  libchrome_dll.dylib                 0x000000010d4cf720 ChromeMain + 288
42  Brave Browser Development           0x000000010d300caf main + 287
43  libdyld.dylib                       0x00007fff2045f621 start + 1
Task trace:

@simonhong
Copy link
Member

Search settings from extension is not set for private window(tor)

simonhong added a commit to brave/brave-core that referenced this issue Apr 7, 2021
fix brave/brave-browser#15140

When OnTemplateURLServiceChanged() is called, it tries to access default
search engine provider but it's null when search engine extension is installed
and that extension enabled to private window.
Those code is not valid now so just deleted because we can't change tor window's
search engine during the run time. It was possible ago but not now.
simonhong added a commit to brave/brave-core that referenced this issue Apr 8, 2021
fix brave/brave-browser#15140

When OnTemplateURLServiceChanged() is called, it tries to access default
search engine provider but it's null when search engine extension is installed
and that extension enabled to private window.
Those code is not valid now so just deleted because we can't change tor window's
search engine during the run time. It was possible ago but not now.
@simonhong simonhong added this to the 1.25.x - Nightly milestone Apr 9, 2021
@kjozwiak
Copy link
Member

@brave/legacy_qa we'll probably want to wait till we get #15224 fixed as you'll still run into crashes when searching in Private & Tor windows while having the DDG extension installed. This basically fixes the issue of crashing when focusing on the omnibox and typing something into the box.

@stephendonner
Copy link

@kjozwiak should this be QA/Test-All-Platforms or just the macOS and Windows ones?

@kjozwiak
Copy link
Member

Lets check this on all the platforms. Crashes are never good so we should make sure it's been fixed on Win, macOS & Linux.

@LaurenWags
Copy link
Member

Adding QA/Blocked until a new RC is built

@stephendonner
Copy link

Sorry, jumped the gun a bit and did actually crash, as per #15140 (comment) - restoring all the correct labels, and removing my erroneous "verification" (boo).

@kjozwiak
Copy link
Member

kjozwiak commented Apr 14, 2021

Sorry, jumped the gun a bit and did actually crash, as per #15140 (comment) - restoring all the correct labels, and removing my erroneous "verification" (boo).

@StephenBass we never ended up getting #15224 into 1.23.x as @simonhong is still investigating/working on it. He mentioned that the fix wouldn't be easy and would most likely not make it into 1.23.x. That was when we were supposed to release tomorrow till C89 threw a ranch into the release schedule. Plus, #15224 is also reproducible on 1.22.x as per #15224 (comment).

So we can QA what we have so far re: not crashing when focusing/typing into the omnibox. You can re-add your verification 👍 Apologies, should have update the issue earlier.

@kjozwiak
Copy link
Member

kjozwiak commented Apr 14, 2021

Verification PASSED on Ubuntu 20.04 x64 using the following build:

Brave | 1.23.70 Chromium: 90.0.4430.70 (Official Build) (64-bit)
--- | ---
Revision | 3954de7175366f3b7edca576f140dfa273e6b5ae-refs/branch-heads/4430@{#1210}
OS | Linux

I believe this only affects Win & macOS as I couldn't reproduce #15224. Either way, verified that the STR/Cases outlined via #15140 (comment) are working as per the following:

  • ensured that focusing & type via Private, Tor & Guest windows didn't crash the browser

Verified PASSED using

Brave 1.23.70 Chromium: 90.0.4430.70 (Official Build) (x86_64)
Revision 3954de7175366f3b7edca576f140dfa273e6b5ae-refs/branch-heads/4430@{#1210}
OS macOS Version 11.2.3 (Build 20D91)

Ensured that focusing & typing via Private, Tor & Guest windows didn't crash the browser.

Private Tor Guest
Screen Shot 2021-04-14 at 8 59 36 AM Screen Shot 2021-04-14 at 8 59 55 AM Screen Shot 2021-04-14 at 8 59 45 AM

Verification passed on

Brave | 1.23.70 Chromium: 90.0.4430.70 (Official Build) (64-bit)
-- | --
Revision | 3954de7175366f3b7edca576f140dfa273e6b5ae-refs/branch-heads/4430@{#1210}
OS | Windows 10 OS Version 2004 (Build 19041.867)

@stephendonner stephendonner added QA/In-Progress Indicates that QA is currently in progress for that particular issue QA Pass-macOS and removed QA/In-Progress Indicates that QA is currently in progress for that particular issue labels Apr 14, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment