Bug 18490

Summary: [SOUP] Local files are not handled
Product: WebKit Reporter: Marco Barisione <marco.barisione>
Component: WebKitGTKAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: alp, danw, elle.uca, lethalman88
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: OS X 10.5   
Attachments:
Description Flags
first draft of a patch for file: (and ftp:) support
none
revised soup file: support patch alp: review+

Description Marco Barisione 2008-04-14 07:42:12 PDT
Currently the soup backend is not able to load local files. Libsoup is just an HTTP library, so it doesn't handle other protocols (see http://bugzilla.gnome.org/show_bug.cgi?id=526755).

It would be nice to support this using gio so it will be possible to support also ftp and other protocols.

Note that WebKit depends on glib 2.0 and gio was added in glib 2.16 but this should not be a problem because libsoup is already depending on glib 2.16.
Comment 1 Luca Ferretti 2008-04-17 12:56:17 PDT
*** Bug 18545 has been marked as a duplicate of this bug. ***
Comment 2 Dan Winship 2008-04-20 09:30:32 PDT
Created attachment 20699 [details]
first draft of a patch for file: (and ftp:) support

OK, here is a first draft. It works for local files, mostly.

Among the problems with the patch are:

  1. It adds a bunch of crap to ResourceHandleInternal, although all of the
     other backends add lots of crap there too, so this is probably expected

  2. It doesn't handle any error cases at all. Even in the places where it
     looks like it handles errors, it doesn't really; you just get a blank
     page in GtkLauncher instead of an error message. (Maybe that's a
     GtkLauncher problem?) At any rate, there are also other places where
     it should return an error, but it doesn't. Also, I wasn't sure if you're
     allowed to call ->didFail() after having called didReceiveResponse(),
     etc.

  3. queryInfoCallback probably needs to be failing with ERROR_UNKNOWN_PROTOCOL
     in some situation, but I don't know what that is. (None of the GIOError
     values seem to correspond to that). Actually, we should probably just
     limit the gio usage to specific URI types. ("file" and "ftp"? What are
     we expected to support?)

  4. ftp support sort of works a little bit, but there's no code to deal with
     mounting remote filesystems currently, so it only works if you first
     "Connect to Server..." from nautilus, and then try to access the ftp:
     URL in GtkLauncher. To fix this, we should be looking for
     G_IO_ERROR_NOT_MOUNTED, and calling g_file_mount_enclosing_volume() in
     that case (and keeping a timeout and unmounting the volume later if we're
     done with it).

  5. What should happen if the URI is a directory rather than a file? We can
     use gio to iterate the directory and find out about its children, but
     how do we return that to the caller?
Comment 3 Luca Bruno 2008-04-20 10:06:49 PDT
I'd say to put everything into another file, without SOUP references (like macros).

(In reply to comment #2)
>   1. It adds a bunch of crap to ResourceHandleInternal, although all of the
>      other backends add lots of crap there too, so this is probably expected
 
I didn't see in other ports, but are you sure this job is not done by their http stack?

>   2. It doesn't handle any error cases at all. Even in the places where it
>      looks like it handles errors, it doesn't really; you just get a blank
>      page in GtkLauncher instead of an error message. (Maybe that's a
>      GtkLauncher problem?) At any rate, there are also other places where
>      it should return an error, but it doesn't. Also, I wasn't sure if you're
>      allowed to call ->didFail() after having called didReceiveResponse(),
>      etc.

It's a FrameLoaderClient missing implementation, I created a brief patch for that. It's not a problem of the network backend.

Comment 4 Dan Winship 2008-04-20 10:26:53 PDT
(In reply to comment #3)
> I'd say to put everything into another file, without SOUP references (like
> macros).

It would be nice to be able to have ResourceHandleSoup, ResourceHandleData, and ResourceHandleGio, and then have ResourceHandle::create create an object of the right subclass for the given URL. ResourceHandleData could even be shared between curl and soup (and other backends?)...

(I also don't really get what the point of ResourceHandleInternal is... couldn't it all just be done as private parts of ResourceHandle? Or alternatively, couldn't it be internal to the network backend, such that I could then have separate kinds of ResourceHandleInternal for each of the three request types?)
Comment 5 Alp Toker 2008-04-29 15:57:45 PDT
Dan,

ResourceHandleInternal is the way private data is done right now in the http backends so I wouldn't worry too much about this.

Could you tidy this up and submit it for review? I think it's worth getting this in with FIXMEs for any major issues since it brings the soup backend up to parity with the curl backend (which has even less error reporting, and no consideration for directory listing or FTP).

(Incidentally, I'm observing a crash on Acid3 with the soup backend somewhere in parseDataUrl(). Did anyone else notice this?)
Comment 6 Dan Winship 2008-04-30 12:07:42 PDT
Created attachment 20903 [details]
revised soup file: support patch

Updated. Mostly I just added some more FIXME comments, but also I made it so the gio branch only gets called for file:, ftp:, and ftpd: URIs, not other unrecognized URI types.
Comment 7 Alp Toker 2008-05-01 09:45:15 PDT
Comment on attachment 20903 [details]
revised soup file: support patch

r=me

I'll move the start*() functions to the private section of the class.
Comment 8 Alp Toker 2008-05-01 09:46:51 PDT
I'm also seeing occasional GIO warnings with this:

 Programs/GtkLauncher file:/home/alp/Projects/webkit/ng/webkit/LayoutTests/fast/reflections/reflection-direction.html 

(lt-GtkLauncher:29946): GLib-GIO-CRITICAL **: g_input_stream_close_finish: assertion `G_IS_INPUT_STREAM (stream)' failed

(lt-GtkLauncher:29946): GLib-GIO-CRITICAL **: g_input_stream_close_finish: assertion `G_IS_INPUT_STREAM (stream)' failed

Can you reproduce that?
Comment 9 Alp Toker 2008-05-01 09:55:00 PDT
Landed in r32761. We'll need to track the gio warnings and data URL crashers another bug report.