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+

Pratik Solanki
Reported 2014-06-29 22:16:44 PDT
No need to create NSURLRequest until we actually need it.
Attachments
Patch (3.37 KB, patch)
2014-06-29 22:19 PDT, Pratik Solanki 		no flags
DetailsFormatted DiffDiff
Patch with assertion fix (3.82 KB, patch)
2014-06-30 22:47 PDT, Pratik Solanki 		kling: review+
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Pratik Solanki
Comment 1 2014-06-29 22:19:11 PDT
Created attachment 234067 [details] Patch
Sam Weinig
Comment 2 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?
Pratik Solanki
Comment 3 2014-06-29 23:36:51 PDT
Committed r170578: <http://trac.webkit.org/changeset/170578>
Pratik Solanki
Comment 4 2014-06-30 15:28:52 PDT
Rolling this out since it caused assertions in debug builds.
WebKit Commit Bot
Comment 5 2014-06-30 15:29:37 PDT
Re-opened since this is blocked by bug 134462
Pratik Solanki
Comment 6 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...
Pratik Solanki
Comment 7 2014-06-30 22:47:26 PDT
Created attachment 234145 [details] Patch with assertion fix
Pratik Solanki
Comment 8 2014-07-01 09:45:41 PDT
Committed r170642: <http://trac.webkit.org/changeset/170642>
Note You need to log in before you can comment on or make changes to this bug.