Bug 17972 - [Curl] Rewrite of curl backend to run in event loop
Summary: [Curl] Rewrite of curl backend to run in event loop
Status: NEW
Alias: None
Product: WebKit
Classification: Unclassified
Component: Page Loading (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC Linux
: P2 Normal
Assignee: Nobody
URL:
Keywords: Curl
Depends on:
Blocks:
 
Reported: 2008-03-20 15:18 PDT by Michael Emmel
Modified: 2013-10-01 07:31 PDT (History)
11 users (show)

See Also:


Attachments
Curl with file handles watched by event loop (49.51 KB, patch)
2008-03-20 15:19 PDT, Michael Emmel
no flags Details | Formatted Diff | Diff
Update patch after testing acid2 (53.29 KB, patch)
2008-04-20 17:05 PDT, Michael Emmel
jmalonzo: review-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Michael Emmel 2008-03-20 15:18:13 PDT
This version of the curl backend can register file handles with the platform event loop for platforms that support this like gtk.
Comment 1 Michael Emmel 2008-03-20 15:19:06 PDT
Created attachment 19910 [details]
Curl with file handles watched by event loop
Comment 2 Mark Rowe (bdash) 2008-03-23 18:24:26 PDT
Why do we need a second curl backend rather than improving the existing backend?
Comment 3 Michael Emmel 2008-03-23 20:20:55 PDT
The biggest reason is Alp Toker said he wanted it this way to bring it in.

Primarily so the old backend can be used as a fallback during transition and give time for other ports using the curl backend to support the new interface.
Since it fallbacks to polling if no manager is registered we should be able to switch and remove the old one fairly quickly.
Comment 4 Michael Emmel 2008-03-26 14:19:40 PDT
Alp can you comment on this ?
Comment 5 Michael Emmel 2008-04-20 17:05:23 PDT
Created attachment 20710 [details]
Update patch after testing acid2


Add changelog several bug fixes to make patch work with acid2.
Works in polling and registered file handle mode.
Finally curl frees resources !
Comment 6 Eric Seidel (no email) 2008-08-25 20:58:10 PDT
Comment on attachment 20710 [details]
Update patch after testing acid2

Supposedly Alp is a person who knows about Curl...
Comment 7 Michael Emmel 2008-08-25 23:54:05 PDT
If someone is willing to review this I have one small change thats floating about.

--- WebKit-r34342-orig/WebCore/platform/network/ncurl/ResourceHandleInternalCurl.cpp    2008-07-02 16:26:30.000000000 +0000

+++ WebKit-r34342/WebCore/platform/network/ncurl/ResourceHandleInternalCurl.cpp 
@@ -659,7 +669,7 @@

         return;



     m_request.setHTTPHeaderField("PropagateHttpHeader", "true");

-    m_request.setHTTPContentType("Content-Type: application/x-www-form-urlencoded");

+    m_request.setHTTPContentType("application/x-www-form-urlencoded");

Comment 8 Holger Freyther 2008-11-03 07:27:38 PST
Yes, it is on my mind, but it will take some more time though. I want to get parts of the test suite running before adding and switching to a new backend.
Comment 9 Brent Fulgham 2009-06-15 21:58:39 PDT
I know this patch is ancient, but this might be usable on the Win/Cairo build using the CFRunLoop infrastructure.
Comment 10 Jan Alonzo 2009-07-31 21:04:51 PDT
Comment on attachment 20710 [details]
Update patch after testing acid2

Thanks for the patch. This patch needs to be updated and made standalone regardless of port by removing the Gtk related changes, as the Gtk port is now using libsoup as the sole backend. Other ports may benefit from these enhancements and would be nice if a port maintainer interested can follow it through.

Thanks again for the patch!
Comment 11 Michael Emmel 2009-07-31 21:59:04 PDT
(In reply to comment #10)
> (From update of attachment 20710 [details])
> Thanks for the patch. This patch needs to be updated and made standalone
> regardless of port by removing the Gtk related changes, as the Gtk port is now
> using libsoup as the sole backend. Other ports may benefit from these
> enhancements and would be nice if a port maintainer interested can follow it
> through.
> 
> Thanks again for the patch!

Wow this patch is over a year old now :)
I don't have any plans to rework it myself. There are new hooks for running in the main loop which should allow this to be done in a generic fashion.
If they don't work they should work.

I've since moved on to using neon as my core http lib so I'm no longer using curl.