[webkit-reviews] review granted: [Bug 222145] Serialize NSURLRequest (rather than CFURLRequest) in IPC : [Attachment 420888] Patch

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Brent Fulgham <bfulgham at webkit.org>'s
request for review:
Bug 222145: Serialize NSURLRequest (rather than CFURLRequest) in IPC
https://bugs.webkit.org/show_bug.cgi?id=222145

Attachment 420888: Patch

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




--- Comment #4 from Darin Adler <darin at apple.com> ---
Comment on attachment 420888
  --> https://bugs.webkit.org/attachment.cgi?id=420888
Patch

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

Looks OK but I think we can do better.

> Source/WebKit/ChangeLog:16
> +	   * Shared/mac/WebCoreArgumentCodersMac.mm:

Shouldn't this be in the Cocoa.mm file, not the Mac.mm file? I assume we use it
on iOS, not just Mac.

> Source/WebKit/Shared/mac/WebCoreArgumentCodersMac.mm:219
> +    NSError *error = nil;
> +    auto *archivedData = [NSKeyedArchiver
archivedDataWithRootObject:requestToSerialize.get() requiringSecureCoding:YES
error:&error];
> +    if (error)
> +	   LOG_ERROR("Failed to encode NSURLRequest: %d", error.code);
> +    IPC::encode(encoder, (__bridge CFDataRef)archivedData);

This should use WebCoreArgumentCodersCocoa.h. I believe it can just be this:

    encode(encoder, requestToSerialize.get());

> Source/WebKit/Shared/mac/WebCoreArgumentCodersMac.mm:223
>      encoder <<
resourceRequest.responseContentDispositionEncodingFallbackArray();

Is this still correct with NSURLRequest serialization?

> Source/WebKit/Shared/mac/WebCoreArgumentCodersMac.mm:257
> +    RetainPtr<CFDataRef> data;
> +    if (!IPC::decode(decoder, data) || !data)
> +	   return false;
> +
> +    NSError *error = nil;
> +    auto nsURLRequest = retainPtr([NSKeyedUnarchiver
unarchivedObjectOfClass:[NSURLRequest class] fromData:(__bridge NSData
*)data.get() error:&error]);
> +    if (error) {
> +	   LOG_ERROR("Failed to decode NSURLRequest: %d", error.code);
> +	   return false;
> +    }

This would just be:

    auto request = decode(decoder, NSURLRequest.class);
    if (!request)
	return false;