Bug 134441 - Create NSURLRequest lazily when USE(CFNETWORK) is enabled
Summary: Create NSURLRequest lazily when USE(CFNETWORK) is enabled
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Platform (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Pratik Solanki
URL:
Keywords:
Depends on: 134462
Blocks:
  Show dependency tree / graph
 
Reported: 2014-06-29 22:16 PDT by Pratik Solanki
Modified: 2014-07-01 09:45 PDT (History)
6 users (show)

See Also:

Attachments
Patch (3.37 KB, patch)
2014-06-29 22:19 PDT, Pratik Solanki 		no flags Details | Formatted Diff | Diff
Patch with assertion fix (3.82 KB, patch)
2014-06-30 22:47 PDT, Pratik Solanki 		kling: review+
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 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>