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

Exiting brave://adblock crashes webview #13775

Closed
Maxime-J opened this issue Jan 26, 2021 · 7 comments · Fixed by brave/brave-core#7811
Closed

Exiting brave://adblock crashes webview #13775

Maxime-J opened this issue Jan 26, 2021 · 7 comments · Fixed by brave/brave-core#7811
Assignees
Labels
bug crash/webview Only tab webview crash. Browser doesn't crash crash OS/Desktop priority/P3 The next thing for us to work on. It'll ride the trains. QA Pass-Linux QA Pass-macOS QA Pass-Win64 QA/Yes release-notes/include

Comments

@Maxime-J
Copy link

Brave Version: 1.19.86 Chromium: 88.0.4324.96
Operating System: Windows NT 10.0.19042

URL (if applicable) where crash occurred:
brave://adblock
Can you reproduce this crash?
yes

What steps will reproduce this crash? (If it's not reproducible, what were you doing just before the crash?)

  1. Go to brave://adblock, either via URL bar or menu
  2. From that tab, go to any other website
    => RESULT_CODE_KILLED_BAD_MESSAGE crash

DO NOT CHANGE BELOW THIS LINE
Crash ID: crash/aa360100-604d-b505-0000-000000000000

@kjozwiak kjozwiak changed the title Exiting brave://adblock crash Exiting brave://adblock crashes webview Jan 26, 2021
@kjozwiak kjozwiak added bug crash QA/Yes crash/webview Only tab webview crash. Browser doesn't crash labels Jan 26, 2021
@kjozwiak
Copy link
Member

Thanks for the report @Maxime-J 👍 I managed to reproduce the issue using the following build:

Brave | 1.21.28 Chromium: 88.0.4324.96 (Official Build) nightly (64-bit)
-- | --
Revision | 68dba2d8a0b149a1d3afac56fa74648032bcf46b-refs/branch-heads/4324@{#1784}
OS | Windows 10 OS Version 2009 (Build 19042.746)

STR/Case

  • launch Brave and visit brave://adblock
  • in the same tab, visit another website (https://videocardz.com in this case)

Example:

image

@rebron rebron added the priority/P3 The next thing for us to work on. It'll ride the trains. label Jan 26, 2021
@ryanbr
Copy link

ryanbr commented Jan 28, 2021

cc: @antonok-edm

@antonok-edm antonok-edm self-assigned this Jan 28, 2021
@antonok-edm
Copy link
Collaborator

antonok-edm commented Jan 28, 2021

Also able to reproduce in Release and Nightly on Linux.

I see this error in the console when the crash occurs:

[ERROR:bad_message.cc(26)] Terminating renderer for bad IPC message, reason 193

Navigating to another brave:// internal page does not crash.

Navigating to other schemes (e.g. file://) still crashes.

The error appears to correspond to RVH_WEB_UI_BINDINGS_MISMATCH.

@antonok-edm antonok-edm removed their assignment Jan 28, 2021
@iefremov
Copy link
Contributor

iefremov commented Feb 2, 2021

Example dump (the browser process only does DumpWithoutCrashing, while renderer crashes, but there is no renderer dump)
https://brave.sp.backtrace.io/p/brave/debug?filters=(_rxid%3D%2282460000-9cc8-c705-0000-000000000000%22%2C(ver%2Cregex%2C%228%5B5%7C6%7C7%7C8%7C9%5D.*%22)%2Cptype%3Dbrowser)&fingerprint=d0ac1d11f0319c90f64afc2a70d9f4d939a8f626de16505956bda322e48e1878&debug=(c2b2c7,0,0)

[ 00 ] crash_reporter::DumpWithoutCrashing()
[ 01 ] base::debug::DumpWithoutCrashing()
[ 02 ] content::RenderProcessHostImpl::ShutdownForBadMessage(content::RenderProcessHost::CrashReportMode)
[ 03 ] BraveAdblockUI::CustomizeWebUIProperties(content::RenderFrameHost*)
[ 04 ] BraveAdblockUI::UpdateWebUIProperties()
[ 05 ] content::WebContentsImpl::RenderViewReady(content::RenderViewHost*)
[ 06 ] content::(anonymous namespace)::RenderProcessHostIsReadyObserver::CallTask()

@simonhong I think calling UpdateWebUIProperties() as we do now is not 100% correct. And given that the only implementaiton is BraveAdblockUI and it is buggy, maybe we can just simplify and remove UpdateWebUIProperties()?

@simonhong simonhong self-assigned this Feb 3, 2021
@simonhong
Copy link
Member

@rebron How about disable stats showing in adblock page?
This crash comes from updating blocked stats in adblock page. I think it's redundant because NTP shows same number.

simonhong added a commit to brave/brave-core that referenced this issue Feb 4, 2021
fix brave/brave-browser#13775

When navigting from adblock to other pages, renderer is crashed.
This crash comes from referring RFH to try update stats whlie
new naviagion occurs. As we don't need to show stats, stats updating
logic is removed.
@simonhong simonhong added this to the 1.22.x - Nightly milestone Feb 4, 2021
@iefremov
Copy link
Contributor

iefremov commented Feb 4, 2021

Thanks @simonhong :)

@btlechowski
Copy link

btlechowski commented Feb 9, 2021

Verification passed on

Brave 1.20.101 Chromium: 88.0.4324.152 (Official Build) (64-bit)
Revision 6579930fc53b4dc589c042bec9d0a3778326974d-refs/branch-heads/4324@{#2106}
OS Ubuntu 18.04 LTS

Verified test plan from brave/brave-core#7811

Reproduced on 1.19.92:
image

Verified no crash on 1.20.101
image
image


Verification passed on


Brave | 1.20.101 Chromium: 88.0.4324.152 (Official Build) (64-bit)
-- | --
Revision | 6579930fc53b4dc589c042bec9d0a3778326974d-refs/branch-heads/4324@{#2106}
OS | Windows 10 OS Version 2004 (Build 19041.746)



Verified passed with

Brave | 1.20.102 Chromium: 88.0.4324.152 (Official Build) (x86_64)
-- | --
Revision | 6579930fc53b4dc589c042bec9d0a3778326974d-refs/branch-heads/4324@{#2106}
OS | macOS Version 10.15.7 (Build 19H512)

Verified test plan from brave/brave-core#7811 (comment)
Verified STR from #13775 (comment)
Confirmed no crash when navigating away from brave://adblock

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug crash/webview Only tab webview crash. Browser doesn't crash crash OS/Desktop priority/P3 The next thing for us to work on. It'll ride the trains. QA Pass-Linux QA Pass-macOS QA Pass-Win64 QA/Yes release-notes/include
Projects
None yet
Development

Successfully merging a pull request may close this issue.

10 participants