Bug 85736 – Cleanup: FileSystem API's Entry.toURL() impl should return KURL instead of String

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit Misc.

[Kinuko Yasuda]

Entry.toURL() implementation should return KURL instead of String.  This would not have any visible impact in the app-facing javascript layer but must make the code clearer and more readable.

[Kinuko Yasuda]

Created attachment 140423 [details]
Patch

[Kinuko Yasuda]

This is one of the patches separated from the bigger one for http://webkit.org/b/84135 (for cross-filesystem support).

[Kinuko Yasuda]

Created attachment 140428 [details]
Patch

[David Levin]

Comment on attachment 140428 [details]
Patch

I don't understand why I don't see any changes to whoever calls toURL (maybe there was an implicit conversion?). Anyway, as long as this compiles it seems fine.

[Kinuko Yasuda]

(In reply to comment #4)
> (From update of attachment 140428 [details])
> I don't understand why I don't see any changes to whoever calls toURL (maybe there was an implicit conversion?). Anyway, as long as this compiles it seems fine.

Yes it is converted back to String by implicit conversion in the binding layer. I found that some other IDL implementation is doing this and thought it'd be nice to make it clearer what type of object we are dealing with in C++ world. I could add an explicit conversion code before at the boundary between the binding code if it makes things even clearer.

[Kinuko Yasuda]

Comment on attachment 140428 [details]
Patch

Committed r116273 <http://trac.webkit.org/changeset/116273>.

[Darin Adler]

Comment on attachment 140428 [details]
Patch

This seems like a bad change. Why parse the URL and then convert it back to a string each time? We should not use KURL for anything just because it happens to be a URL. This is adding extra work for no benefit.

[Kinuko Yasuda]

(In reply to comment #7)
> (From update of attachment 140428 [details])
> This seems like a bad change. Why parse the URL and then convert it back to a string each time? We should not use KURL for anything just because it happens to be a URL. This is adding extra work for no benefit.

Ok, on the second thought I wondered the same.  I'm going to make another change to revert it.  Thanks,

[Kinuko Yasuda]

(In reply to comment #8)
> (In reply to comment #7)
> > (From update of attachment 140428 [details] [details])
> > This seems like a bad change. Why parse the URL and then convert it back to a string each time? We should not use KURL for anything just because it happens to be a URL. This is adding extra work for no benefit.
> 
> Ok, on the second thought I wondered the same.  I'm going to make another change to revert it.  Thanks,

The patch is: https://bugs.webkit.org/show_bug.cgi?id=85858