Bug 134441

Summary: Create NSURLRequest lazily when USE(CFNETWORK) is enabled
Product: WebKit Reporter: Pratik Solanki <psolanki>
Component: PlatformAssignee: Pratik Solanki <psolanki>
Status: RESOLVED FIXED    
Severity: Normal CC: bunhere, cdumez, commit-queue, gyuyoung.kim, psolanki, sergio
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 134462    
Bug Blocks:    
Attachments:
Description Flags
Patch
none
Patch with assertion fix kling: review+

Description Pratik Solanki 2014-06-29 22:16:44 PDT
No need to create NSURLRequest until we actually need it.
Comment 1 Pratik Solanki 2014-06-29 22:19:11 PDT
Created attachment 234067 [details]
Patch
Comment 2 Sam Weinig 2014-06-29 22:20:41 PDT
Comment on attachment 234067 [details]
Patch

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

> Source/WebCore/platform/network/cf/ResourceRequestCFNet.cpp:199
> +    m_nsRequest = 0;

nullptr?

> Source/WebCore/platform/network/cf/ResourceRequestCFNet.cpp:234
> +    m_nsRequest = 0;

nullptr?

> Source/WebCore/platform/network/cf/ResourceRequestCFNet.cpp:346
> +    m_nsRequest = 0;

nullptr?
Comment 3 Pratik Solanki 2014-06-29 23:36:51 PDT
Committed r170578: <http://trac.webkit.org/changeset/170578>
Comment 4 Pratik Solanki 2014-06-30 15:28:52 PDT
Rolling this out since it caused assertions in debug builds.
Comment 5 WebKit Commit Bot 2014-06-30 15:29:37 PDT
Re-opened since this is blocked by bug 134462
Comment 6 Pratik Solanki 2014-06-30 15:59:48 PDT
Ah, the issue was

        bool encodingRequiresPlatformData() const { return m_httpBody || m_nsRequest; }

Since m_nsRequest is now null until needed, encodingRequiresPlatformData() returned false and this triggered the assert. good catch. Fix will need more ifdefs in ResourceRequest.h... sigh...
Comment 7 Pratik Solanki 2014-06-30 22:47:26 PDT
Created attachment 234145 [details]
Patch with assertion fix
Comment 8 Pratik Solanki 2014-07-01 09:45:41 PDT
Committed r170642: <http://trac.webkit.org/changeset/170642>