Create sandbox extensions for GPU process access to cache and temp directory.
<rdar://problem/72450307>
Created attachment 417544 [details] Patch
Comment on attachment 417544 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=417544&action=review > Source/WebKit/GPUProcess/GPUProcessCreationParameters.cpp:57 > +#if PLATFORM(IOS_FAMILY) > + encoder << containerCachesDirectoryExtensionHandle; > + encoder << containerTemporaryDirectoryExtensionHandle; > +#endif Why is this iOS only?
(In reply to Simon Fraser (smfr) from comment #3) > Comment on attachment 417544 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=417544&action=review > > > Source/WebKit/GPUProcess/GPUProcessCreationParameters.cpp:57 > > +#if PLATFORM(IOS_FAMILY) > > + encoder << containerCachesDirectoryExtensionHandle; > > + encoder << containerTemporaryDirectoryExtensionHandle; > > +#endif > > Why is this iOS only? When GPUProcess is disabled and we pass those to the WebProcess, the code is for IOS_FAMILY only too, so at least it is consistent. The patch does not build on iOS EWS though.
Created attachment 417547 [details] Patch
(In reply to Simon Fraser (smfr) from comment #3) > Comment on attachment 417544 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=417544&action=review > > > Source/WebKit/GPUProcess/GPUProcessCreationParameters.cpp:57 > > +#if PLATFORM(IOS_FAMILY) > > + encoder << containerCachesDirectoryExtensionHandle; > > + encoder << containerTemporaryDirectoryExtensionHandle; > > +#endif > > Why is this iOS only? This works a little different on macOS, where confstr is used in the UI process to determine the cache and temp directory. The cache and temp directories are then passed as sandbox parameters, and sandbox rules will allow access to these directories. Thanks for reviewing!
(In reply to Chris Dumez from comment #4) > (In reply to Simon Fraser (smfr) from comment #3) > > Comment on attachment 417544 [details] > > Patch > > > > View in context: > > https://bugs.webkit.org/attachment.cgi?id=417544&action=review > > > > > Source/WebKit/GPUProcess/GPUProcessCreationParameters.cpp:57 > > > +#if PLATFORM(IOS_FAMILY) > > > + encoder << containerCachesDirectoryExtensionHandle; > > > + encoder << containerTemporaryDirectoryExtensionHandle; > > > +#endif > > > > Why is this iOS only? > > When GPUProcess is disabled and we pass those to the WebProcess, the code is > for IOS_FAMILY only too, so at least it is consistent. > > The patch does not build on iOS EWS though. Should be fixed in latest patch. Thanks for reviewing!
(In reply to Per Arne Vollan from comment #6) > (In reply to Simon Fraser (smfr) from comment #3) > > Comment on attachment 417544 [details] > > Patch > > > > View in context: > > https://bugs.webkit.org/attachment.cgi?id=417544&action=review > > > > > Source/WebKit/GPUProcess/GPUProcessCreationParameters.cpp:57 > > > +#if PLATFORM(IOS_FAMILY) > > > + encoder << containerCachesDirectoryExtensionHandle; > > > + encoder << containerTemporaryDirectoryExtensionHandle; > > > +#endif > > > > Why is this iOS only? > > This works a little different on macOS, where confstr is used in the UI > process to determine the cache and temp directory. The cache and temp > directories are then passed as sandbox parameters, and sandbox rules will > allow access to these directories. It would be nicer to use a HAVE_FOO or USE_FOO macro then. We should avoid sprinkling platform #ifdefs around.
(In reply to Simon Fraser (smfr) from comment #8) > (In reply to Per Arne Vollan from comment #6) > > (In reply to Simon Fraser (smfr) from comment #3) > > > Comment on attachment 417544 [details] > > > Patch > > > > > > View in context: > > > https://bugs.webkit.org/attachment.cgi?id=417544&action=review > > > > > > > Source/WebKit/GPUProcess/GPUProcessCreationParameters.cpp:57 > > > > +#if PLATFORM(IOS_FAMILY) > > > > + encoder << containerCachesDirectoryExtensionHandle; > > > > + encoder << containerTemporaryDirectoryExtensionHandle; > > > > +#endif > > > > > > Why is this iOS only? > > > > This works a little different on macOS, where confstr is used in the UI > > process to determine the cache and temp directory. The cache and temp > > directories are then passed as sandbox parameters, and sandbox rules will > > allow access to these directories. > > It would be nicer to use a HAVE_FOO or USE_FOO macro then. We should avoid > sprinkling platform #ifdefs around. Sounds good, I will use a USE macro!
Created attachment 417564 [details] Patch
Created attachment 417612 [details] Patch
Created attachment 417614 [details] Patch
Committed r271482: <https://trac.webkit.org/changeset/271482> All reviewed patches have been landed. Closing bug and clearing flags on attachment 417614 [details].