Bug 125583 - [SOUP] Implement CUSTOM PROTOCOLS
Summary: [SOUP] Implement CUSTOM PROTOCOLS
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit2 (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords: Soup
Depends on: 126343
Blocks: 108832 127091 136705
  Show dependency treegraph
 
Reported: 2013-12-11 09:24 PST by Carlos Garcia Campos
Modified: 2014-09-10 09:19 PDT (History)
12 users (show)

See Also:


Attachments
Patch (81.76 KB, patch)
2013-12-31 09:16 PST, Carlos Garcia Campos
no flags Details | Formatted Diff | Diff
Updated patch (91.32 KB, patch)
2014-01-03 02:52 PST, Carlos Garcia Campos
no flags Details | Formatted Diff | Diff
New patch (64.40 KB, patch)
2014-01-16 02:28 PST, Carlos Garcia Campos
no flags Details | Formatted Diff | Diff
Rebased patch (64.46 KB, patch)
2014-01-17 00:19 PST, Carlos Garcia Campos
no flags Details | Formatted Diff | Diff
Try to fix mac build (64.61 KB, patch)
2014-01-17 05:18 PST, Carlos Garcia Campos
andersca: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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>