Bug 166864 - [QuickLook] Do some cleanup in QuickLookHandle
Summary: [QuickLook] Do some cleanup in QuickLookHandle
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Andy Estes
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2017-01-09 15:03 PST by Andy Estes
Modified: 2017-01-16 15:06 PST (History)
9 users (show)

See Also:

Attachments
Patch (15.34 KB, patch)
2017-01-09 17:19 PST, Andy Estes 		no flags Details | Formatted Diff | Diff
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Andy Estes 2017-01-09 15:03:57 PST 
[QuickLook] Do some cleanup in QuickLookHandle
Comment 1 Andy Estes 2017-01-09 17:19:11 PST 
Created attachment 298423 [details]
Patch
Comment 2 WebKit Commit Bot 2017-01-09 17:20:04 PST 
Attachment 298423 [details] did not pass style-queue:


ERROR: Source/WebCore/loader/ios/QuickLook.mm:284:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Total errors found: 1 in 5 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 3 David Kilzer (:ddkilzer) 2017-01-10 11:41:23 PST 
Comment on attachment 298423 [details]
Patch

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

> Source/WebCore/ChangeLog:21
> +        (-[WebPreviewConverter _sendDidReceiveResponseIfNecessary]): Stoped checking for a nil

Typo:  Stoped => Stopped

> Source/WebCore/ChangeLog:24
> +        (-[WebResourceLoaderQuickLookDelegate connection:didReceiveDataArray:]): Deleted.
> +        QLPreviewConverter never calls this method.

Curious why this is never called (QuickLook.framework doesn't define the delegate itself?), but answering this is NOT a blocker to land the patch.
Comment 4 Andy Estes 2017-01-16 13:54:12 PST 
(In reply to comment #3)
> Comment on attachment 298423 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=298423&action=review
> 
> > Source/WebCore/ChangeLog:21
> > +        (-[WebPreviewConverter _sendDidReceiveResponseIfNecessary]): Stoped checking for a nil
> 
> Typo:  Stoped => Stopped

Oops, will fix.

> 
> > Source/WebCore/ChangeLog:24
> > +        (-[WebResourceLoaderQuickLookDelegate connection:didReceiveDataArray:]): Deleted.
> > +        QLPreviewConverter never calls this method.
> 
> Curious why this is never called (QuickLook.framework doesn't define the
> delegate itself?), but answering this is NOT a blocker to land the patch.

There is no formal protocol defined by QuickLook for the delegate to conform to. It's informally based on NSURLConnectionDelegate, but -connection:didReceiveDataArray: is not one of the methods QuickLook calls (I think we originally implemented this in QuickLook by copying one of our ResourceHandle delegates).
Comment 5 WebKit Commit Bot 2017-01-16 14:15:36 PST 
Comment on attachment 298423 [details]
Patch

Clearing flags on attachment: 298423

Committed r210789: <http://trac.webkit.org/changeset/210789>
Comment 6 WebKit Commit Bot 2017-01-16 14:15:41 PST 
All reviewed patches have been landed.  Closing bug.
Comment 7 Andy Estes 2017-01-16 15:06:11 PST 
Fixed the ChangeLog typo in r210791.