Bug 167153 – [QuickLook] Support password-protected documents

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Andy Estes]

[QuickLook] Support password-protected documents

[Andy Estes]

Created attachment 299118 [details]
Patch

[Andy Estes]

rdar://problem/28544527

[Andy Estes]

Created attachment 299119 [details]
Patch

[Alex Christensen]

Comment on attachment 299119 [details]
Patch

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

> Source/WebCore/ChangeLog:17
> +        logic and adds support for testing.

:)

> Source/WebCore/loader/ios/QuickLook.mm:297
>  - (void)connection:(NSURLConnection *)connection didReceiveData:(NSData *)data lengthReceived:(long long)lengthReceived

NSURLConnection??? :(

> Source/WebCore/platform/spi/ios/QuickLookSPI.h:54
> +static_assert(kQLReturnPasswordProtected == 4, "kQLReturnPasswordProtected should equal 4.");

I think we should do more things like this in WebKit to make sure our SPI headers line up with the real headers.

> Source/WebCore/testing/Internals.cpp:436
> +    MockQuickLookHandleClient::singleton().setPassword("");

This will make it so the first time it runs, m_password will be a null string, but after that it will be an empty string.  Probably no big deal

> Source/WebCore/testing/MockQuickLookHandleClient.h:42
> +    void didRequestPassword(std::function<void(const String&)>&&) override;

It looks like we can use WTF::Function here instead because we don't need to be able to copy the function to pretend it's a block or anything.

[Andy Estes]

(In reply to comment #4)
> Comment on attachment 299119 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=299119&action=review
> 
> > Source/WebCore/ChangeLog:17
> > +        logic and adds support for testing.
> 
> :)
> 
> > Source/WebCore/loader/ios/QuickLook.mm:297
> >  - (void)connection:(NSURLConnection *)connection didReceiveData:(NSData *)data lengthReceived:(long long)lengthReceived
> 
> NSURLConnection??? :(

The QLPreviewConverter delegate was designed to interoperate with an NSURLConnectionDelegate, but these days we always pass a nil NSURLConnection to QLPreviewConverter's initializer (and now assert that it's nil in the delegate callbacks).

At this point it's purely vestigial; there's no actual NSURLConnection being used anywhere :)

> 
> > Source/WebCore/platform/spi/ios/QuickLookSPI.h:54
> > +static_assert(kQLReturnPasswordProtected == 4, "kQLReturnPasswordProtected should equal 4.");
> 
> I think we should do more things like this in WebKit to make sure our SPI
> headers line up with the real headers.
> 
> > Source/WebCore/testing/Internals.cpp:436
> > +    MockQuickLookHandleClient::singleton().setPassword("");
> 
> This will make it so the first time it runs, m_password will be a null
> string, but after that it will be an empty string.  Probably no big deal

I also don't think this is a big deal.

> 
> > Source/WebCore/testing/MockQuickLookHandleClient.h:42
> > +    void didRequestPassword(std::function<void(const String&)>&&) override;
> 
> It looks like we can use WTF::Function here instead because we don't need to
> be able to copy the function to pretend it's a block or anything.

Good idea, I'll use WTF::Function instead.

Thanks for the review!

[Andy Estes]

Created attachment 299162 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 299162 [details]
Patch

Clearing flags on attachment: 299162

Committed r210864: <http://trac.webkit.org/changeset/210864>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.