Bug 137876 - Avoid unnecessary NSURLResponse construction for QuickLook on iOS
Summary: Avoid unnecessary NSURLResponse construction for QuickLook on iOS
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Page Loading (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-10-20 04:14 PDT by Antti Koivisto
Modified: 2014-10-20 10:20 PDT (History)
3 users (show)

See Also:


Attachments
patch (7.43 KB, patch)
2014-10-20 04:22 PDT, Antti Koivisto
aestes: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Antti Koivisto 2014-10-20 04:14:55 PDT
QuickLook specific code path creates NSURLResponse in the web process for every response. It is rarely needed so this is unnecessary work.
Comment 1 Antti Koivisto 2014-10-20 04:22:47 PDT
Created attachment 240109 [details]
patch
Comment 2 Pratik Solanki 2014-10-20 08:53:52 PDT
Comment on attachment 240109 [details]
patch

Does this work in WebKit1?
Comment 3 Andy Estes 2014-10-20 09:19:36 PDT
Comment on attachment 240109 [details]
patch

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

> Source/WebCore/ChangeLog:21
> +            Switch inteface to take ResourceResponse.

interface
Comment 4 Antti Koivisto 2014-10-20 09:35:54 PDT
(In reply to comment #2)
> Comment on attachment 240109 [details]
> patch
> 
> Does this work in WebKit1?

WK1 shouldn't be affected as these code paths are WK2 only.
Comment 5 Darin Adler 2014-10-20 10:01:09 PDT
Comment on attachment 240109 [details]
patch

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

> Source/WebCore/platform/network/ios/QuickLook.mm:501
> +    std::unique_ptr<QuickLookHandle> quickLookHandle(new QuickLookHandle([loader.originalRequest().nsURLRequest(DoNotUpdateHTTPBody) URL], nil, response.nsURLResponse(), delegate.get()));

The correct style here is to use std::make_unique, not new. But I imagine if we tried that we’d have to change what’s public.

> Source/WebCore/platform/network/ios/QuickLook.mm:504
>      return WTF::move(quickLookHandle);

Normally we don’t need WTF::move for a return value in a case like this.
Comment 6 Antti Koivisto 2014-10-20 10:20:34 PDT
https://trac.webkit.org/r174889