Bug 125583

Summary: [SOUP] Implement CUSTOM PROTOCOLS
Product: WebKit Reporter: Carlos Garcia Campos <cgarcia>
Component: WebKit2Assignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: andersca, ap, beidson, bunhere, cdumez, commit-queue, danw, gustavo, gyuyoung.kim, rakuco, svillar, zan
Priority: P2 Keywords: Soup
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 126343    
Bug Blocks: 108832, 127091, 136705    
Attachments:
Description Flags
Patch
none
Updated patch
none
New patch
none
Rebased patch
none
Try to fix mac build andersca: review+

Description Carlos Garcia Campos 2013-12-11 09:24:56 PST
It will make easier to use it from the network process and we'll share more code with mac port.
Comment 1 Carlos Garcia Campos 2013-12-11 09:25:20 PST
I'm already working on this.
Comment 2 Carlos Garcia Campos 2013-12-31 09:16:12 PST
Created attachment 220164 [details]
Patch

$ WEBKIT_USE_NETWORK_PROCESS=1 Programs/WebKit2APITests/TestWebKitWebContext -p /webkit2/WebKitWebContext/uri-scheme
/webkit2/WebKitWebContext/uri-scheme: OK
Comment 3 Sam Weinig 2013-12-31 22:09:34 PST
Comment on attachment 220164 [details]
Patch

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

> Source/WebKit2/Shared/Network/CustomProtocols/CustomProtocolManager.h:91
> +#if USE(SOUP)
> +    // SoupRequest implementation.
> +    void send(GTask*);
> +    GInputStream* finish(GTask*, GError**);
> +#endif

Can you find a way to do this without putting SOUP specific functions in this file.  If you need to refactor, that is fine, but please don't add to the #ifdef hell.
Comment 4 Carlos Garcia Campos 2014-01-01 02:41:41 PST
(In reply to comment #3)
> (From update of attachment 220164 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=220164&action=review
> 
> > Source/WebKit2/Shared/Network/CustomProtocols/CustomProtocolManager.h:91
> > +#if USE(SOUP)
> > +    // SoupRequest implementation.
> > +    void send(GTask*);
> > +    GInputStream* finish(GTask*, GError**);
> > +#endif
> 
> Can you find a way to do this without putting SOUP specific functions in this file.  If you need to refactor, that is fine, but please don't add to the #ifdef hell.

Thanks for looking at the patch. We need a way to make soup notify the custom protocol manager to start the load, passing the GTask associated, and that's very specific to soup. The only think I can think of is having an intermediate object entirely defined in platform specific code, so that we can also remove the mac ifdefs. Something like CustomProtocolManagerImpl or CustomProtocolManagerPrivate. However, since the header already contains mac #ifdefs, maybe it would be better to do the refactoring in a follow up patch to get rid of both mac and soup ifdefs there.
Comment 5 Carlos Garcia Campos 2014-01-03 02:52:16 PST
Created attachment 220296 [details]
Updated patch

Patch updated to not use any soup #ifdef in CustomProtocolManager header file. In the end I've added a CustomProtocolManagerImpl with all the soup specific implementation. This leaves CustomProtocolManagerSoup.cpp mostly cross-platform, so if mac switches to use their own CustomProtocolManagerImpl to remove also the mac #ifdefs, we could remove CustomProtocolManagerSoup.cpp and CustomProtocolManagerMac.cpp an use a common implementation for the cross-platform bits. I've also changed some uses of OwnPtr to use std::unique_ptr instead.
Comment 6 Carlos Garcia Campos 2014-01-16 02:28:18 PST
Created attachment 221351 [details]
New patch

I've removed the initiating page support for now until I figure out how to bring it back. And I've also removed the GTK specific part to try to make the patch easier to review.
Comment 7 Carlos Garcia Campos 2014-01-17 00:19:16 PST
Created attachment 221449 [details]
Rebased patch
Comment 8 Carlos Garcia Campos 2014-01-17 05:18:50 PST
Created attachment 221459 [details]
Try to fix mac build
Comment 9 Carlos Garcia Campos 2014-01-21 10:26:06 PST
Committed r162449: <http://trac.webkit.org/changeset/162449>