rdar://problem/55547063 On macOS High Sierra, if Safari 13 was installed after Safari 12 had been installed, Safari may end up launching the 32-bit plug-in service left behind in the staged WebKit framework, and the service will crash immediately (because as of Safari 13, its dependencies don’t have a 32-bit slice). This can happen, for example, if there is still a 32-bit plug-in in /Library/Internet Plug-Ins, and the user chooses to manage website data. This is because even though the 32-bit plug-in service has been removed from WebKit, the code paths that launch it haven’t been removed. Patch forthcoming.
Created attachment 379323 [details] Remove 32-bit plug-in code paths
Comment on attachment 379323 [details] Remove 32-bit plug-in code paths View in context: https://bugs.webkit.org/attachment.cgi?id=379323&action=review > Source/WebKit/UIProcess/Launcher/mac/ProcessLauncherMac.mm:60 > case ProcessLauncher::ProcessType::Plugin32: What is the reason for keeping this enum case at all?
(In reply to Sam Weinig from comment #2) > Comment on attachment 379323 [details] > Remove 32-bit plug-in code paths > > View in context: > https://bugs.webkit.org/attachment.cgi?id=379323&action=review > > > Source/WebKit/UIProcess/Launcher/mac/ProcessLauncherMac.mm:60 > > case ProcessLauncher::ProcessType::Plugin32: > > What is the reason for keeping this enum case at all? ProcessType is a cross-platform type, and I didn’t want to expand the scope of this patch beyond Mac code.
Gotcha.
Comment on attachment 379323 [details] Remove 32-bit plug-in code paths Clearing flags on attachment: 379323 Committed r250186: <https://trac.webkit.org/changeset/250186>
All reviewed patches have been landed. Closing bug.
This is causing a lot of crashes on the bots (They exit early): https://build.webkit.org/results/Apple%20Mojave%20Debug%20WK2%20(Tests)/r250186%20(4826)/results.html
Comment on attachment 379323 [details] Remove 32-bit plug-in code paths View in context: https://bugs.webkit.org/attachment.cgi?id=379323&action=review > Source/WebKit/UIProcess/Plugins/mac/PluginProcessProxyMac.mm:63 > + ASSERT(pluginProcessAttributes.moduleInfo.pluginArchitecture == CPU_TYPE_X86); Looks like this assertion is hitting.
(In reply to Chris Dumez from comment #8) > Comment on attachment 379323 [details] > Remove 32-bit plug-in code paths > > View in context: > https://bugs.webkit.org/attachment.cgi?id=379323&action=review > > > Source/WebKit/UIProcess/Plugins/mac/PluginProcessProxyMac.mm:63 > > + ASSERT(pluginProcessAttributes.moduleInfo.pluginArchitecture == CPU_TYPE_X86); > > Looks like this assertion is hitting. I’ll see if I can reproduce this locally.
(In reply to mitz from comment #9) > (In reply to Chris Dumez from comment #8) > > Comment on attachment 379323 [details] > > Remove 32-bit plug-in code paths > > > > View in context: > > https://bugs.webkit.org/attachment.cgi?id=379323&action=review > > > > > Source/WebKit/UIProcess/Plugins/mac/PluginProcessProxyMac.mm:63 > > > + ASSERT(pluginProcessAttributes.moduleInfo.pluginArchitecture == CPU_TYPE_X86); Silly mistake, this should say CPU_TYPE_X86_64. I’ll fix it right away. Thanks for noticing this!
Comment on attachment 379323 [details] Remove 32-bit plug-in code paths View in context: https://bugs.webkit.org/attachment.cgi?id=379323&action=review >>> Source/WebKit/UIProcess/Plugins/mac/PluginProcessProxyMac.mm:63 >>> + ASSERT(pluginProcessAttributes.moduleInfo.pluginArchitecture == CPU_TYPE_X86); >> >> Looks like this assertion is hitting. > > I’ll see if I can reproduce this locally. Shouldn’t this be != instead of == ?
(In reply to mitz from comment #10) > (In reply to mitz from comment #9) > > (In reply to Chris Dumez from comment #8) > > > Comment on attachment 379323 [details] > > > Remove 32-bit plug-in code paths > > > > > > View in context: > > > https://bugs.webkit.org/attachment.cgi?id=379323&action=review > > > > > > > Source/WebKit/UIProcess/Plugins/mac/PluginProcessProxyMac.mm:63 > > > > + ASSERT(pluginProcessAttributes.moduleInfo.pluginArchitecture == CPU_TYPE_X86); > > Silly mistake, this should say CPU_TYPE_X86_64. I’ll fix it right away. > Thanks for noticing this! Oh just saw your comment, glad this is an easy fix.
Corrected the assertion in r250197.