We are still seeing some accesses from the WebContent process to a small set of services. Since we do not currently have backtraces for these accesses, make a speculative patch, where we issue these extension for all apps except Safari.
Created attachment 391559 [details] Patch
Comment on attachment 391559 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=391559&action=review This patch seems to be having build issues. Is this the patch you meant to upload? > Source/WebKit/Shared/WebProcessCreationParameters.cpp:464 > + parameters.frontboardServicesExtensionHandle = WTFMove(*frontboardServicesExtensionHandle); I think your life would be easier if you switched to a SandboxExtension::HandleArray for these items. > Source/WebKit/Shared/WebProcessCreationParameters.h:212 > + Optional<SandboxExtension::Handle> frontboardServicesExtensionHandle; I think it makes sense to have individual extension handles for things we turn on and off dynamically (like camera/microphone, perhaps), but for things that are based on the linked application we should probably just add them as strings in the WebProcessProxy, and serialize them all in a single SandboxExtension::HandleArray. > Source/WebKit/UIProcess/Cocoa/WebProcessPoolCocoa.mm:365 > + SandboxExtension::createHandleForMachLookup("com.apple.AGXCompilerService", WTF::nullopt, parameters.compilerServiceExtensionHandle); Much tidier! :-) > Source/WebKit/UIProcess/Cocoa/WebProcessPoolCocoa.mm:374 > + SandboxExtension::createHandleForMachLookup("com.apple.frontboard.systemappservices", WTF::nullopt, parameters.frontboardServicesExtensionHandle); I think these could make sense as a single SandboxExtension::HandleArray.
(In reply to Brent Fulgham from comment #2) > Comment on attachment 391559 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=391559&action=review > > This patch seems to be having build issues. Is this the patch you meant to > upload? > > > Source/WebKit/Shared/WebProcessCreationParameters.cpp:464 > > + parameters.frontboardServicesExtensionHandle = WTFMove(*frontboardServicesExtensionHandle); > > I think your life would be easier if you switched to a > SandboxExtension::HandleArray for these items. > > > Source/WebKit/Shared/WebProcessCreationParameters.h:212 > > + Optional<SandboxExtension::Handle> frontboardServicesExtensionHandle; > > I think it makes sense to have individual extension handles for things we > turn on and off dynamically (like camera/microphone, perhaps), but for > things that are based on the linked application we should probably just add > them as strings in the WebProcessProxy, and serialize them all in a single > SandboxExtension::HandleArray. > > > Source/WebKit/UIProcess/Cocoa/WebProcessPoolCocoa.mm:365 > > + SandboxExtension::createHandleForMachLookup("com.apple.AGXCompilerService", WTF::nullopt, parameters.compilerServiceExtensionHandle); > > Much tidier! :-) > > > Source/WebKit/UIProcess/Cocoa/WebProcessPoolCocoa.mm:374 > > + SandboxExtension::createHandleForMachLookup("com.apple.frontboard.systemappservices", WTF::nullopt, parameters.frontboardServicesExtensionHandle); > > I think these could make sense as a single SandboxExtension::HandleArray. I will change the patch to use HandleArray. Thanks for reviewing!
Created attachment 391587 [details] Patch
Comment on attachment 391587 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=391587&action=review Very nice! r=me. > Source/WebKit/WebProcess/cocoa/WebProcessCocoa.mm:264 > + SandboxExtension::consumePermanently(parameters.dynamicMachExtensionHandles[i]); Someday we should make a proper iterator interface for SandboxExtension::HandleArray so we can use modern looping.
Comment on attachment 391587 [details] Patch Thanks for reviewing!
Comment on attachment 391587 [details] Patch Clearing flags on attachment: 391587 Committed r257508: <https://trac.webkit.org/changeset/257508>
All reviewed patches have been landed. Closing bug.
<rdar://problem/59819320>
Reverted r257508 for reason: This commit broke the watchos build Committed r257534: <https://trac.webkit.org/changeset/257534>
Created attachment 391884 [details] Patch
Comment on attachment 391884 [details] Patch Rejecting attachment 391884 [details] from commit-queue. Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.webkit.org', '--bot-id=webkit-cq-01', 'validate-changelog', '--check-oops', '--non-interactive', 391884, '--port=mac']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit ChangeLog entry in LayoutTests/ChangeLog contains OOPS!. Full output: https://webkit-queues.webkit.org/results/13329981
Created attachment 391885 [details] Patch
Comment on attachment 391885 [details] Patch Clearing flags on attachment: 391885 Committed r257575: <https://trac.webkit.org/changeset/257575>
Reverted r257575 for reason: Broke the watchOS build. Committed r257593: <https://trac.webkit.org/changeset/257593>
Created attachment 391952 [details] Patch
Comment on attachment 391952 [details] Patch Clearing flags on attachment: 391952 Committed r257611: <https://trac.webkit.org/changeset/257611>