Bug 166713 - [QuickLook] Consolidate usage of QuickLookHandle into SubresourceLoader
Summary: [QuickLook] Consolidate usage of QuickLookHandle into SubresourceLoader
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-04 20:40 PST by Andy Estes
Modified: 2017-01-07 13:01 PST (History)
9 users (show)

See Also:

Attachments
Patch (40.51 KB, patch)
2017-01-04 20:41 PST, Andy Estes 		no flags Details | Formatted Diff | Diff
Patch (42.54 KB, patch)
2017-01-07 05:00 PST, Andy Estes 		no flags Details | Formatted Diff | Diff
Show Obsolete (1) 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-04 20:40:31 PST 
[QuickLook] Consolidate usage of QuickLookHandle into SubresourceLoader
Comment 1 Andy Estes 2017-01-04 20:41:36 PST 
Created attachment 298065 [details]
Patch
Comment 2 WebKit Commit Bot 2017-01-04 20:43:12 PST 
Attachment 298065 [details] did not pass style-queue:


ERROR: Source/WebCore/ChangeLog:8:  You should remove the 'No new tests' and either add and list tests, or explain why no new tests were possible.  [changelog/nonewtests] [5]
Total errors found: 1 in 16 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 3 Andy Estes 2017-01-07 05:00:09 PST 
Created attachment 298272 [details]
Patch
Comment 4 Alex Christensen 2017-01-07 11:44:39 PST 
Comment on attachment 298272 [details]
Patch

ahhhhh, much better.
I guess we're ok with appcache not using quicklook for now?  See ApplicationCacheGroup, which we ought to also fix so it's not loading in the WebProcess.  It uses ResourceHandle directly right now.
Comment 5 Andy Estes 2017-01-07 12:35:28 PST 
(In reply to comment #4)
> Comment on attachment 298272 [details]
> Patch
> 
> ahhhhh, much better.
> I guess we're ok with appcache not using quicklook for now?  See
> ApplicationCacheGroup, which we ought to also fix so it's not loading in the
> WebProcess.  It uses ResourceHandle directly right now.

Yeah, we're ok with this. Documents converted by QuickLook can't have a cache manifest (they aren't HTML), and can't be specified in a cache manifest (they're only loaded as main resources).

The old ResourceHandle code would've never even created a QuickLookHandle for ApplicationCacheGroup loads, since it checked that firstRequest's requester was Requester::Main, which is only set by DocumentLoader.

Thanks for the review!
Comment 6 WebKit Commit Bot 2017-01-07 13:01:30 PST 
Comment on attachment 298272 [details]
Patch

Clearing flags on attachment: 298272

Committed r210480: <http://trac.webkit.org/changeset/210480>
Comment 7 WebKit Commit Bot 2017-01-07 13:01:35 PST 
All reviewed patches have been landed.  Closing bug.