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 in settings_overridden_params::GetSearchOverriddenParams #14218

Closed
iefremov opened this issue Feb 17, 2021 · 3 comments · Fixed by brave/brave-core#8626
Closed

Crash in settings_overridden_params::GetSearchOverriddenParams #14218

iefremov opened this issue Feb 17, 2021 · 3 comments · Fixed by brave/brave-core#8626

Comments

@iefremov
Copy link
Contributor

[ 00 ] settings_overridden_params::GetSearchOverriddenParams(Profile*)
[ 01 ] extensions::MaybeShowExtensionControlledSearchNotification(content::WebContents*, AutocompleteMatchType::Type)
[ 02 ] OmniboxEditModel::OpenMatch(AutocompleteMatch, WindowOpenDisposition, GURL const&, std::__1::basic_string<unsigned short, base::string16_internals::string16_char_traits, std::__1::allocator<unsigned short> > const&, unsigned long, base::TimeTicks)
[ 03 ] OmniboxView::OpenMatch(AutocompleteMatch const&, WindowOpenDisposition, GURL const&, std::__1::basic_string<unsigned short, base::string16_internals::string16_char_traits, std::__1::allocator<unsigned short> > const&, unsigned long, base::TimeTicks)
[ 04 ] OmniboxEditModel::AcceptInput(WindowOpenDisposition, base::TimeTicks)
[ 05 ] OmniboxViewViews::HandleKeyEvent(views::Textfield*, ui::KeyEvent const&)
[ 06 ] views::Textfield::OnKeyPressed(ui::KeyEvent const&)
[ 07 ] non-virtual thunk to views::View::OnKeyEvent(ui::KeyEvent*)
[ 08 ] ui::EventDispatcher::ProcessEvent(ui::EventTarget*, ui::Event*)
[ 09 ] ui::EventDispatcherDelegate::DispatchEvent(ui::EventTarget*, ui::Event*)
[ 10 ] ui::EventProcessor::OnEventFromSource(ui::Event*)
[ 11 ] ui::EventSource::SendEventToSinkFromRewriter(ui::Event const*, ui::EventRewriter const*)
[ 12 ] views::Widget::OnKeyEvent(ui::KeyEvent*)
[ 13 ] non-virtual thunk to views::NativeWidgetMac::DispatchKeyEventPostIME(ui::KeyEvent*)

https://brave.sp.backtrace.io/p/brave/triage?filters=((callstack.functions%2Ccontains%2CGetSearchOverriddenParams)%2Cptype%3Dbrowser)&aggregations=((channel%2Cdistribution%2C3)%2C(ptype%2Chead)%2C(uname.sysname%2Cdistribution%2C3))

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

iefremov commented Apr 6, 2021

maybe related to #15140

@simonhong simonhong self-assigned this Apr 7, 2021
simonhong added a commit to brave/brave-core that referenced this issue Apr 17, 2021
fix: brave/brave-browser#15224
fix: brave/brave-browser#10601
fix: brave/brave-browser#14218

Changed to use same TemplateURLService for normal & private profile.
simonhong added a commit to brave/brave-core that referenced this issue Apr 19, 2021
fix: brave/brave-browser#15224
fix: brave/brave-browser#10601
fix: brave/brave-browser#14218

Changed to use same TemplateURLService for normal & private profile.
simonhong added a commit to brave/brave-core that referenced this issue Apr 22, 2021
fix: brave/brave-browser#15224
fix: brave/brave-browser#10601
fix: brave/brave-browser#14218

Changed to use same TemplateURLService for normal & private profile.
simonhong added a commit to brave/brave-core that referenced this issue Apr 30, 2021
fix: brave/brave-browser#15224
fix: brave/brave-browser#10601
fix: brave/brave-browser#14218

Crash comes from absense of exension search provider data in private profile's
TemplateURLService because we are using different TemplateURLService for private profile.
When search provider extension is loaded/installed, browser doesn't notify about
this adding to private profile's TUS.
To fix this, each SearchEngineProviderService sets extension's template url data to
its service when normal profile's search provider comes from extension.
simonhong added a commit to brave/brave-core that referenced this issue May 7, 2021
fix: brave/brave-browser#15224
fix: brave/brave-browser#10601
fix: brave/brave-browser#14218

Crash comes from absense of exension search provider data in private profile's
TemplateURLService because we are using different TemplateURLService for private profile.
When search provider extension is loaded/installed, browser doesn't notify about
this adding to private profile's TUS.
To fix this, each SearchEngineProviderService sets extension's template url data to
its service when normal profile's search provider comes from extension.
simonhong added a commit to brave/brave-core that referenced this issue May 10, 2021
fix: brave/brave-browser#15224
fix: brave/brave-browser#10601
fix: brave/brave-browser#14218

Crash comes from absense of exension search provider data in private profile's
TemplateURLService because we are using different TemplateURLService for private profile.
When search provider extension is loaded/installed, browser doesn't notify about
this adding to private profile's TUS.
To fix this, each SearchEngineProviderService sets extension's template url data to
its service when normal profile's search provider comes from extension.
@simonhong simonhong added this to the 1.26.x - Nightly milestone May 14, 2021
@stephendonner
Copy link

stephendonner commented May 18, 2021

Verified PASSED using the testplan from brave/brave-core#8626 with build

Brave 1.26.24 Chromium: 91.0.4472.57 (Official Build) nightly (x86_64)
Revision e3443317fa07f1e9997e4a9c738eddfefc3c0292-refs/branch-heads/4472_54@{#6}
OS macOS Version 11.3.1 (Build 20E241)

I confirmed both the functionality is correct, as well as -- more importantly for this issue -- that I didn't have any crashes 👍

Steps:

  1. new profile, launched Brave
  2. installed Ecosia extension from https://chrome.google.com/webstore/detail/ecosia-the-search-engine/eedlgdlajadkbbjoobobefphmfkcchfk/related
  3. confirmed Normal window's default search provider was Ecosia
  4. opened a New Private Window and confirmed Ecosia is the default search provider
  5. toggled DuckDuckGo button to On and confirmed search provider is still Ecosia
  6. toggled DuckDuckGo button to Off and confirmed search provider is still Ecosia
  7. went to brave://extensions, clicked on Details for Ecosia extension, and toggled Allow in private repeatedly
  8. confirmed changing Allow in private option doesn't affect private window's search provider - still used Ecosia
  9. disabled Ecosia extension via its Details page from brave://extensions and confirmed normal and private windows both use default search provider - Google, in my case
  10. toggled DuckDuckGo button to On in New Private Window tab and confirmed search provider is changed to DuckDuckGo
example example example example example example example example
Screen Shot 2021-05-18 at 1 19 30 PM Screen Shot 2021-05-18 at 1 19 45 PM Screen Shot 2021-05-18 at 1 20 05 PM Screen Shot 2021-05-18 at 1 20 52 PM Screen Shot 2021-05-18 at 1 21 14 PM Screen Shot 2021-05-18 at 1 21 32 PM Screen Shot 2021-05-18 at 1 21 48 PM Screen Shot 2021-05-18 at 1 21 58 PM

Verification passed on



<!--StartFragment-->
Brave | 1.26.47 Chromium: 91.0.4472.77&nbsp;(Official Build)&nbsp;beta&nbsp;(64-bit)
-- | --
Revision | 1cecd5c8a856bc2a5adda436e7b84d8d21b339b6-refs/branch-heads/4472@{#1246}
OS | Windows&nbsp;10 OS Version 2004 (Build 19041.985)

<!--EndFragment-->

example example example example example example example example example example example example example
image image image image image image image image image image image image image

Verification passed on

Brave 1.26.45 Chromium: 91.0.4472.77 (Official Build) beta (64-bit)
Revision 1cecd5c8a856bc2a5adda436e7b84d8d21b339b6-refs/branch-heads/4472@{#1246}
OS Ubuntu 18.04 LTS

Extension was unable to change default search engine in Normal Window nor Private Window. Logged #16227

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