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+

Carlos Garcia Campos
Reported 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.
Attachments
Patch (81.76 KB, patch)
2013-12-31 09:16 PST, Carlos Garcia Campos 		no flags
DetailsFormatted DiffDiff
Updated patch (91.32 KB, patch)
2014-01-03 02:52 PST, Carlos Garcia Campos 		no flags
DetailsFormatted DiffDiff
New patch (64.40 KB, patch)
2014-01-16 02:28 PST, Carlos Garcia Campos 		no flags
DetailsFormatted DiffDiff
Rebased patch (64.46 KB, patch)
2014-01-17 00:19 PST, Carlos Garcia Campos 		no flags
DetailsFormatted DiffDiff
Try to fix mac build (64.61 KB, patch)
2014-01-17 05:18 PST, Carlos Garcia Campos 		andersca: review+
DetailsFormatted DiffDiff
Show Obsolete (4) View All Add attachment proposed patch, testcase, etc.
Carlos Garcia Campos
Comment 1 2013-12-11 09:25:20 PST
I'm already working on this.
Carlos Garcia Campos
Comment 2 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
Sam Weinig
Comment 3 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.
Carlos Garcia Campos
Comment 4 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.
Carlos Garcia Campos
Comment 5 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.
Carlos Garcia Campos
Comment 6 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.
Carlos Garcia Campos
Comment 7 2014-01-17 00:19:16 PST
Created attachment 221449 [details] Rebased patch
Carlos Garcia Campos
Comment 8 2014-01-17 05:18:50 PST
Created attachment 221459 [details] Try to fix mac build
Carlos Garcia Campos
Comment 9 2014-01-21 10:26:06 PST
Committed r162449: <http://trac.webkit.org/changeset/162449>
Note You need to log in before you can comment on or make changes to this bug.