[webkit-reviews] review granted: [Bug 211103] Improve SandboxExtension::HandleArray to reduce boilerplate : [Attachment 398014] Patch

[bugzilla-daemon at webkit.org]

Per Arne Vollan <pvollan at apple.com> has granted Brent Fulgham
<bfulgham at webkit.org>'s request for review:
Bug 211103: Improve SandboxExtension::HandleArray to reduce boilerplate
https://bugs.webkit.org/show_bug.cgi?id=211103

Attachment 398014: Patch

https://bugs.webkit.org/attachment.cgi?id=398014&action=review




--- Comment #8 from Per Arne Vollan <pvollan at apple.com> ---
Comment on attachment 398014
  --> https://bugs.webkit.org/attachment.cgi?id=398014
Patch

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

R=me.

> Source/WebKit/Shared/Cocoa/SandboxExtensionCocoa.mm:296
> +static SandboxExtension::HandleArray createHandlesForInput(const
Vector<String>& services, Function<bool(const String&,
SandboxExtension::Handle& handle)>&& createFunction)

The name of the first parameter does not seem to describe all variations of
possible input types, since it can be services, paths, and IOKit classes.
Perhaps this function could be renamed 'createHandlesForResources', and the
first parameter could be renamed 'resources', or something similar?

> Source/WebKit/Shared/Cocoa/SandboxExtensionCocoa.mm:300
> +    handleArray.allocate(services.size());

Is allocating with size 0 gracefully handled by HandleArray? On a related note,
would it be better to typedef HandleArray to Vector<Handle>? That's outside the
scope of this patch, though. In that case, allocating up front would not be
needed, we could just append in the loop.

> Source/WebKit/UIProcess/Cocoa/WebProcessPoolCocoa.mm:401
> +	   parameters.dynamicIOKitExtensionHandles =
createHandlesForIOKitClassExtensions(agxCompilerClasses(), WTF::nullopt);

I think this should be
SandboxExtension::createHandlesForIOKitClassExtensions(agxCompilerClasses(),
WTF::nullopt);