Bug 125583 – [SOUP] Implement CUSTOM PROTOCOLS

• Component: WebKit2
• Status: RESOLVED FIXED

[Carlos Garcia Campos]

It will make easier to use it from the network process and we'll share more code with mac port.

[Carlos Garcia Campos]

I'm already working on this.

[Carlos Garcia Campos]

Created attachment 220164 [details]
Patch

$ WEBKIT_USE_NETWORK_PROCESS=1 Programs/WebKit2APITests/TestWebKitWebContext -p /webkit2/WebKitWebContext/uri-scheme
/webkit2/WebKitWebContext/uri-scheme: OK

[Sam Weinig]

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.

[Carlos Garcia Campos]

(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.

[Carlos Garcia Campos]

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.

[Carlos Garcia Campos]

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.

[Carlos Garcia Campos]

Created attachment 221449 [details]
Rebased patch

[Carlos Garcia Campos]

Created attachment 221459 [details]
Try to fix mac build

[Carlos Garcia Campos]

Committed r162449: <http://trac.webkit.org/changeset/162449>