[webkit-reviews] review granted: [Bug 101139] Expose security origin to BundleFrame : [Attachment 172235] Patch

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Jon Lee <jonlee at apple.com>'s request
for review:
Bug 101139: Expose security origin to BundleFrame
https://bugs.webkit.org/show_bug.cgi?id=101139

Attachment 172235: Patch
https://bugs.webkit.org/attachment.cgi?id=172235&action=review

------- Additional Comments from Darin Adler <darin at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=172235&action=review


> Source/WebKit2/Shared/API/c/WKSharedAPICast.h:207
> +    RefPtr<WebSecurityOrigin> webSecurityOrigin =
WebSecurityOrigin::create(origin);
> +    return toAPI(webSecurityOrigin.release().leakRef());

I think these two lines would read better as one, without the local variable.

> Source/WebKit2/Shared/WebSecurityOrigin.h:63
> +    static PassRefPtr<WebSecurityOrigin> create(WebCore::SecurityOrigin*
securityOrigin)

This could use PassRefPtr instead of a raw pointer as the argument. It would
compile down to the same code at call sites, since the function is inlined and
the pointer ends up inside a PassRefPtr eventually. And it would make it
possible to use this when the caller has a PassRefPtr. In fact, I think it
would be slightly better. If this took a PassRefPtr, you could rewrite all the
other create functions to call this one which would make them all into
one-liners:

    return create(WebCore::SecurityOrigin::createFromString(string));

    return
create(WebCore::SecurityOrigin::createFromDatabaseIdentifier(identifier));

    return create(WebCore::SecurityOrigin::create(protocol, host, port));