Bug 102049 - Split NetworkRequest up into a request class and a loading class
Summary: Split NetworkRequest up into a request class and a loading class
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit2 (show other bugs)
Version: 528+ (Nightly build)
Hardware: Mac All
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks: 98537
  Show dependency tree / graph
 
Reported: 2012-11-12 22:46 PST by Brady Eidson
Modified: 2012-11-13 20:36 PST (History)
4 users (show)

See Also:

Attachments
Patch v1 (58.75 KB, patch)
2012-11-12 22:52 PST, Brady Eidson 		ap: review+
Details | Formatted Diff | Diff
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 Brady Eidson 2012-11-12 22:46:57 PST 
Split NetworkRequest up into a request class and a loading class

Sam and I discussed this today how NetworkRequest is a terrible class to do loading.  It's better to have one class represent "all the bits of data needed to start a network load", and "the owner of the network load itself"
Comment 1 Brady Eidson 2012-11-12 22:52:20 PST 
Created attachment 173827 [details]
Patch v1
Comment 2 WebKit Review Bot 2012-11-12 22:55:26 PST 
Attachment 173827 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebKit2/ChangeLog', u'Source/WebKit..." exit_code: 1
Source/WebKit2/NetworkProcess/HostRecord.cpp:32:  Alphabetical sorting problem.  [build/include_order] [4]
Source/WebKit2/NetworkProcess/NetworkResourceLoadScheduler.cpp:8:  Alphabetical sorting problem.  [build/include_order] [4]
Source/WebKit2/Shared/Network/NetworkResourceLoadParameters.h:35:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Total errors found: 3 in 14 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 3 Brady Eidson 2012-11-13 10:49:04 PST 
Fixed style errors locally
Comment 4 Alexey Proskuryakov 2012-11-13 11:00:18 PST 
Comment on attachment 173827 [details]
Patch v1

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

Great idea!

> Source/WebKit2/ChangeLog:10
> +        This patch splits it into "NetworkResourceLoadParameters" for all request-related data

Can the former keep the NetworkRequest name?
Comment 5 Brady Eidson 2012-11-13 14:01:26 PST 
(In reply to comment #4)
> (From update of attachment 173827 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=173827&action=review
> 
> Great idea!
> 
> > Source/WebKit2/ChangeLog:10
> > +        This patch splits it into "NetworkResourceLoadParameters" for all request-related data
> 
> Can the former keep the NetworkRequest name?

I started out keeping that name, but the naming started to fall apart as the "request" contained a "request" and it also contained datum not related directly to the request.

I also noticed that - from the perspective of WebCore in the WebProcess - this data represents a "ResourceLoader" that has already been created.

I'm definitely open to discussion and suggestions on this naming, but I'm going to land as-is since any changes are an easy substitution followup.
Comment 6 Brady Eidson 2012-11-13 14:06:15 PST 
http://trac.webkit.org/changeset/134481