As per Bug 107657, we should start using the C API internally to avoid violating layering.
Created attachment 184475 [details] Patch
Created attachment 185406 [details] Patch Rebased on master.
Comment on attachment 185406 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=185406&action=review > Source/WebKit2/UIProcess/API/efl/ewk_file_chooser_request.cpp:128 > + String fileURL = ASCIILiteral("file://") + String::fromUTF8(url); According to Benjamin's comment, it seems to me String Operators is enough in this case. https://bugs.webkit.org/show_bug.cgi?id=107363#c13
(In reply to comment #3) > (From update of attachment 185406 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=185406&action=review > > > Source/WebKit2/UIProcess/API/efl/ewk_file_chooser_request.cpp:128 > > + String fileURL = ASCIILiteral("file://") + String::fromUTF8(url); > > According to Benjamin's comment, it seems to me String Operators is enough in this case. > https://bugs.webkit.org/show_bug.cgi?id=107363#c13 Gyuyoung, I do not understand your comment. I'm using operator+ from String. Benjamin's comment is about not using StringBuilder (which I'm not using).
> > (From update of attachment 185406 [details] [details]) > > View in context: https://bugs.webkit.org/attachment.cgi?id=185406&action=review > > > > > Source/WebKit2/UIProcess/API/efl/ewk_file_chooser_request.cpp:128 > > > + String fileURL = ASCIILiteral("file://") + String::fromUTF8(url); > > > > According to Benjamin's comment, it seems to me String Operators is enough in this case. > > https://bugs.webkit.org/show_bug.cgi?id=107363#c13 > > Gyuyoung, I do not understand your comment. I'm using operator+ from String. Benjamin's comment is about not using StringBuilder (which I'm not using). Quick comment before going to bed :) Gyuyoung is right String fileURL = ASCIILiteral("file://") + String::fromUTF8(url); is worse than String fileURL = "file://" + String::fromUTF8(url); but that's a bug and I am probably to blame for that. There is no StringTypeAdapter for ASCIILiteral. So your code will generate: String fileURL = StringAppend(String(ASCIILiteral("file://")), String::fromUTF8(url)); If you have 5 minutes, it would be great to make another patch adding a StringTypeAdapter for ASCIILiteral.
(In reply to comment #5) > > > (From update of attachment 185406 [details] [details] [details]) > > > View in context: https://bugs.webkit.org/attachment.cgi?id=185406&action=review > > > > > > > Source/WebKit2/UIProcess/API/efl/ewk_file_chooser_request.cpp:128 > > > > + String fileURL = ASCIILiteral("file://") + String::fromUTF8(url); > > > > > > According to Benjamin's comment, it seems to me String Operators is enough in this case. > > > https://bugs.webkit.org/show_bug.cgi?id=107363#c13 > > > > Gyuyoung, I do not understand your comment. I'm using operator+ from String. Benjamin's comment is about not using StringBuilder (which I'm not using). > > Quick comment before going to bed :) > > Gyuyoung is right > String fileURL = ASCIILiteral("file://") + String::fromUTF8(url); > is worse than > String fileURL = "file://" + String::fromUTF8(url); > but that's a bug and I am probably to blame for that. > > There is no StringTypeAdapter for ASCIILiteral. So your code will generate: > String fileURL = StringAppend(String(ASCIILiteral("file://")), String::fromUTF8(url)); > > If you have 5 minutes, it would be great to make another patch adding a StringTypeAdapter for ASCIILiteral. Yes. It sounds like a good idea to add a StringTypeAdapter for ASCIILiteral. I'll do that, thanks.
Created attachment 185446 [details] Patch Do not construct a ASCIILiteral anymore.
(In reply to comment #6) > Yes. It sounds like a good idea to add a StringTypeAdapter for ASCIILiteral. I'll do that, thanks. +1, Good idea to me as well.
Created attachment 185490 [details] Patch Use ASCIILiteral in String concatenations.
Created attachment 185491 [details] Patch Make use of toCopiedURLAPI().
Comment on attachment 185491 [details] Patch Attachment 185491 [details] did not pass win-ews (win): Output: http://queues.webkit.org/results/16217923
Comment on attachment 185491 [details] Patch win-ews seems to be in a bad state. False alarm.
Comment on attachment 185491 [details] Patch LGTM
Comment on attachment 185491 [details] Patch Clearing flags on attachment: 185491 Committed r141709: <http://trac.webkit.org/changeset/141709>
All reviewed patches have been landed. Closing bug.