Bug 101640 – Have NetworkProcess do the actual loading of subresources

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit2

[Brady Eidson]

Have NetworkProcess do the actual loading of subresources

For this first run it just buffers up the entire resource until it's done loading and hands it to the WebProcess in one chunk.

We'll have to do many things such as notify the WebProcess progressively, handle authentication, etc etc.  Those are separable tasks on top of this first patch.

[Brady Eidson]

Created attachment 173114 [details]
Patch v1 - Do basic subresource loading

Laced with FIXMEs and obviously with much of the task ahead, this patch at least gets actual networking happening in the NetworkProcess.

[WebKit Review Bot]

Attachment 173114 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCor..." exit_code: 1
Source/WebKit2/NetworkProcess/NetworkRequest.cpp:35:  Alphabetical sorting problem.  [build/include_order] [4]
Source/WebKit2/Shared/ShareableResource.cpp:34:  Missing space inside { }.  [whitespace/braces] [5]
Total errors found: 2 in 21 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Alexey Proskuryakov]

Comment on attachment 173114 [details]
Patch v1 - Do basic subresource loading

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

> Source/WebKit2/ChangeLog:8
> +        Additional information of the change such as approach, rationale. Please add per-function descriptions below (OOPS!).

OOPS.

> Source/WebKit2/ChangeLog:65
> +        Add an implementation of WebCore::ResourceBuffer that wraps a ShareableResource instead of a SharedBuffer:

Hmm, it's very much not cool that we still have an m_sharedBuffer data member.

> Source/WebKit2/Shared/ShareableResource.cpp:34
> +ShareableResource::Handle::Handle()
> +{}

Braces go on separate lines.

> Source/WebKit2/Shared/ShareableResource.cpp:61
> +    // Create the shared memory.

Does this comment help? I'm not sure if it's accurate.

> Source/WebKit2/Shared/ShareableResource.cpp:75
> +    ASSERT(m_sharedMemory);
> +    ASSERT(m_offset + m_size <= m_sharedMemory->size());

Should these checks be made in production builds, too?

[Brady Eidson]

(In reply to comment #3)
> (From update of attachment 173114 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=173114&action=review
> 
> > Source/WebKit2/ChangeLog:8
> > +        Additional information of the change such as approach, rationale. Please add per-function descriptions below (OOPS!).
> 
> OOPS.
> 
> > Source/WebKit2/ChangeLog:65
> > +        Add an implementation of WebCore::ResourceBuffer that wraps a ShareableResource instead of a SharedBuffer:
> 
> Hmm, it's very much not cool that we still have an m_sharedBuffer data member.

Agreed.  Can be unraveled separately.
> 
> > Source/WebKit2/Shared/ShareableResource.cpp:34
> > +ShareableResource::Handle::Handle()
> > +{}
> 
> Braces go on separate lines.

You and Style-bot agree!

> 
> > Source/WebKit2/Shared/ShareableResource.cpp:61
> > +    // Create the shared memory.
> 
> Does this comment help? I'm not sure if it's accurate.

Leftovers.  Removed.

> > Source/WebKit2/Shared/ShareableResource.cpp:75
> > +    ASSERT(m_sharedMemory);
> > +    ASSERT(m_offset + m_size <= m_sharedMemory->size());
> 
> Should these checks be made in production builds, too?

Added a fixe with a reasonable course of action (assume the other process is compromised and kill it)

[Brady Eidson]

http://trac.webkit.org/changeset/133957