Bug 232460

Summary: Layout test is crashing inside WebChromeClient::createWindow
Product: WebKit Reporter: Per Arne Vollan <pvollan>
Component: WebKit Misc.Assignee: Per Arne Vollan <pvollan>
Status: RESOLVED FIXED    
Severity: Normal CC: bfulgham, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
bfulgham: review+
Patch
none
Patch none

Description Per Arne Vollan 2021-10-28 15:35:58 PDT
The test fast/frames/sandboxed-iframe-close-top-noclose.html is crashing under WebChromeClient::createWindow.
Comment 1 Per Arne Vollan 2021-10-28 15:36:22 PDT
<rdar://81344393>
Comment 2 Per Arne Vollan 2021-10-28 15:41:34 PDT
Created attachment 442755 [details]
Patch
Comment 3 Brent Fulgham 2021-10-28 15:47:08 PDT
Comment on attachment 442755 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=442755&action=review

r=me

> Source/WebKitLegacy/mac/Plugins/Hosted/NetscapePluginHostManager.mm:315
> +        if (hostProxy && !hostProxy->isMenuBarVisible()) {

Should we log something when we have a set of plugin hosts with no host?

Should we remove the m_pluginHosts entry if the value is nil?
Comment 4 Per Arne Vollan 2021-10-28 16:13:17 PDT
Created attachment 442761 [details]
Patch
Comment 5 Per Arne Vollan 2021-10-28 16:19:46 PDT
(In reply to Brent Fulgham from comment #3)
> Comment on attachment 442755 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=442755&action=review
> 
> r=me
> 
> > Source/WebKitLegacy/mac/Plugins/Hosted/NetscapePluginHostManager.mm:315
> > +        if (hostProxy && !hostProxy->isMenuBarVisible()) {
> 
> Should we log something when we have a set of plugin hosts with no host?
> 
> Should we remove the m_pluginHosts entry if the value is nil?

That is a good point. I have added logging in the latest patch. It would also be good to remove the items with nil value, although I did not include that in the latest patch, since there could possibly be side effects from doing that.

Thanks for reviewing!
Comment 6 Per Arne Vollan 2021-10-28 20:23:03 PDT
Created attachment 442775 [details]
Patch
Comment 7 EWS 2021-10-28 21:19:07 PDT
Committed r285018 (243662@main): <https://commits.webkit.org/243662@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 442775 [details].