Clicking on this link causes a crash on OS X, Windows 10

[bbondy]

https://www.eventbrite.com/d/ca--los-angeles/fashion-shows/
Click the list of the first box.

[luixxiul]

Confirmed on Windows 10.

[bridiver]

appears to be related to the notification bar. If you close it before clicking the link everything works fine

[luixxiul]

Not reproducible on 0.10.2 on Windows 10. Has this been fixed?

[bbondy]

This still reproduces. It has to do with navigating while the permissions bar is open. You probably have a saved permission so you don't reproduce.

[luixxiul]

Yes you are right. I cleared all permissions and clicked the link, and it caused a crash.

[bbondy]

another example link: https://www.homedepot.ca/en/home/categories/appliances.html

[diracdeltas]

i think this just happens if the webcontents are navigated while there is still a reference to the session.setPermissionRequestHandler callback. it goes away if cb is called immediately in https://github.com/brave/browser-laptop/blob/master/app/filtering.js#L316 instead of async.

this crash also happened with the certificate-error event callback at one point.

[bridiver]

there are actually two issues related to this issue:

1. Notifications appear on all windows and tabs, not just the tab they apply to
2. There is no good way to clear the notifications for a particular tab
   I'm working on fixing both

[diracdeltas]

1. Notifications appear on all windows and tabs, not just the tab they apply to

having notifications at the app level was intentional. there's a couple reasons:

• having notifications appear above the tab bar instead of under makes them harder to spoof by the page.
• sometimes a background page requests some permission, and having them be app-level lets you approve/deny them without having to find the tab.
• permissions are for a given origin for the entire app, not just for a tab.

open to changing this but i think this is not what is causing the crash

[bridiver]

they can still appear above the tab, but right now there is no good way to know which notifications can be dismissed because they are no longer relevant (navigated away or closed the tab). If a permission request comes from a tab it should only appear for that tab. If it comes from a background page then we can display it for the current tab as well, but I can't see any reason why it should appear on every window. It's not the direct cause of the crash, but the inability to automatically dismiss requests that are no longer relevant makes it difficult to fix the crash

[bridiver]

going to try a quick fix for the crash using WeakMap and maybe we can come back to the notification location issue

[bridiver]

no luck there unfortunately because the root of the issue is actually the reference in electron. The permission handler has several problems for us, but the main problem is that it expects one webcontents per process and also expects a new webcontents for each navigation. The problem could be fixed in Electron, but it would require a lot of refactoring and it will be a lot easier to just auto deny on tab navigation

[bridiver]

I'm also not sure what would happen right now if you had two webcontents in the same renderer process if one or both of them had a permission request

[bridiver]

I take that back, I might be able to fix Electron and keep things backwards compatible with upstream by replacing the map to the process id with a map to the render frame host id

[diracdeltas]

the workaround i used before for cert-error was always deny the permission request immediately unless the user has made a persistent choice, then reload the page after the user has accepted.

[bridiver]

turned out to be easier than I thought. Somehow I missed that the webcontents are available in the callback handler. I still think it's weird that the notifications show up on all windows and tabs, but I'll create a separate issue for that