Bug 110097

Summary: [WK2] Port RemoteNetworkingContext for Soup
Product: WebKit Reporter: Balazs Kelemen <kbalazs>
Component: WebKit2Assignee: Kwang Yul Seo <skyul>
Status: RESOLVED FIXED    
Severity: Normal CC: andersca, ap, buildbot, cgarcia, commit-queue, darin, gustavo, gyuyoung.kim, jesus, kenneth, laszlo.gombos, ossy, rniwa, skyul, svillar
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 108832, 118231    
Attachments:
Description Flags
Patch
none
Patch
none
Patch
none
Patch
none
Patch
none
Patch
none
Patch none

Description Balazs Kelemen 2013-02-18 01:58:13 PST 
Make this interface portable and add an implementation for the Soup network backend which is used by several ports.
Comment 1 Sergio Villar Senin 2013-02-18 04:06:33 PST 
Being one of the active maintainers of the WK soup backend I'm interested in helping with this effort.
Comment 2 Balazs Kelemen 2013-02-18 06:12:42 PST 
(In reply to comment #1)
> Being one of the active maintainers of the WK soup backend I'm interested in helping with this effort.

Thanks. For now it will mostly be stubs and a bit of refactoring. Later we can implement private browsing sessions which probably requires more soup knowledge.
Comment 3 Balazs Kelemen 2013-02-18 06:21:27 PST 
Created attachment 188871 [details]
Patch
Comment 4 Kwang Yul Seo 2013-06-30 19:46:22 PDT 
Created attachment 205782 [details]
Patch
Comment 5 Kwang Yul Seo 2013-06-30 19:50:13 PDT 
(In reply to comment #4)
Build fix as RemoteNetworkingContext::privateBrowsingSession is changed to return a pointer in r150537.
Comment 6 Build Bot 2013-06-30 20:10:58 PDT 
Comment on attachment 205782 [details]
Patch

Attachment 205782 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/1001366
Comment 7 Build Bot 2013-06-30 20:25:00 PDT 
Comment on attachment 205782 [details]
Patch

Attachment 205782 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/943930
Comment 8 Kwang Yul Seo 2013-07-01 03:45:53 PDT 
Created attachment 205795 [details]
Patch
Comment 9 Kwang Yul Seo 2013-07-01 03:47:07 PDT 
(In reply to comment #8)
> Created an attachment (id=205795) [details]
> Patch

Mac build fix. RemoteNetworkingContext::storageSession() was mistakenly declared twice.
Comment 10 Brady Eidson 2013-07-04 18:27:05 PDT 
Comment on attachment 205795 [details]
Patch

Seems fine.  I'll let a Soup guy review, then will sign off.
Comment 11 Sergio Villar Senin 2013-07-05 01:29:44 PDT 
(In reply to comment #10)
> (From update of attachment 205795 [details])
> Seems fine.  I'll let a Soup guy review, then will sign off.

As this is mostly a refactoring and knowing that there is actually no specific soup code yet, I think you can safely sign off.
Comment 12 Kwang Yul Seo 2013-07-08 18:27:53 PDT 
CC'ing Gustavo Noronha Silva
Comment 13 Kwang Yul Seo 2013-07-25 02:17:23 PDT 
gns, would you review the patch please?
Comment 14 Csaba Osztrogonác 2013-08-26 06:59:35 PDT 
Created attachment 209650 [details]
Patch

Updated to ToT: solved conflicts in project.pbxproj, removed the change in the non-existent SyncNetworkResourceLoader.cpp
Comment 15 Csaba Osztrogonác 2013-09-26 07:57:53 PDT 
Comment on attachment 209650 [details]
Patch

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

> Source/WebKit2/ChangeLog:23
> +        * NetworkProcess/SyncNetworkResourceLoader.cpp:
> +        (WebKit::SyncNetworkResourceLoader::start):

It should be removed.
Comment 16 Csaba Osztrogonác 2013-09-26 07:59:44 PDT 
Created attachment 212710 [details]
Patch
Comment 17 Csaba Osztrogonác 2013-09-26 08:01:00 PDT 
Could you review this patch please?
Comment 18 Darin Adler 2013-09-26 09:28:56 PDT 
Comment on attachment 212710 [details]
Patch

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

> Source/WebKit2/NetworkProcess/RemoteNetworkingContext.h:34
> +class RemoteNetworkingContext : public WebCore::NetworkingContext {

As long as you are touching this, should mark this class FINAL.

> Source/WebKit2/NetworkProcess/RemoteNetworkingContext.h:54
> +    RemoteNetworkingContext(bool privateBrowsingEnabled, bool shouldClearReferrerOnHTTPSToHTTPRedirect)
> +        : m_privateBrowsingEnabled(privateBrowsingEnabled)
> +        , m_shouldClearReferrerOnHTTPSToHTTPRedirect(shouldClearReferrerOnHTTPSToHTTPRedirect)
> +    { }

This will leave m_needsSiteSpecificQuirks and m_localFileContentSniffingEnabled uninitialized on PLATFORM(MAC), which is not what we want.
Comment 19 Csaba Osztrogonác 2013-09-27 03:56:00 PDT 
Created attachment 212794 [details]
Patch

Thanks for the review, I added the FINAL and fixed the initialization.
Comment 20 Build Bot 2013-09-27 04:22:05 PDT 
Comment on attachment 212794 [details]
Patch

Attachment 212794 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/2659113
Comment 21 Csaba Osztrogonác 2013-09-27 04:31:40 PDT 
Created attachment 212796 [details]
Patch

typo fixed
Comment 22 Csaba Osztrogonác 2013-10-07 09:12:14 PDT 
(In reply to comment #19)
> Created an attachment (id=212794) [details]
> Patch
> 
> Thanks for the review, I added the FINAL and fixed the initialization.

Darin, could you review the updated patch please?
Comment 23 Csaba Osztrogonác 2013-10-10 06:06:21 PDT 
Could you review the fixed patch, please?
Comment 24 WebKit Commit Bot 2013-10-10 14:33:14 PDT 
Comment on attachment 212796 [details]
Patch

Clearing flags on attachment: 212796

Committed r157251: <http://trac.webkit.org/changeset/157251>
Comment 25 WebKit Commit Bot 2013-10-10 14:33:20 PDT 
All reviewed patches have been landed.  Closing bug.