Implement setDomainRelaxationForbiddenForURLScheme method in EFL's LayoutTestController.
Created attachment 138340 [details] Patch Implement setDomainRelaxationForbiddenForURLScheme
Comment on attachment 138340 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=138340&action=review > Tools/DumpRenderTree/efl/LayoutTestControllerEfl.cpp:611 > + DumpRenderTreeSupportEfl::setDomainRelaxationForbiddenForURLScheme(forbidden, scheme->ustring().utf8().data()); Please pass a WTF::String directly in the second argument to avoid unneeded const char* conversions. You can create a WTF::String very easily via String(scheme->ustring().impl()).
(In reply to comment #2) > (From update of attachment 138340 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=138340&action=review > > > Tools/DumpRenderTree/efl/LayoutTestControllerEfl.cpp:611 > > + DumpRenderTreeSupportEfl::setDomainRelaxationForbiddenForURLScheme(forbidden, scheme->ustring().utf8().data()); > > Please pass a WTF::String directly in the second argument to avoid unneeded const char* conversions. You can create a WTF::String very easily via String(scheme->ustring().impl()). Thanks for the review Raphael.
Created attachment 138542 [details] patch fixed comment #2
Comment on attachment 138542 [details] patch Looks good.
Comment on attachment 138542 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=138542&action=review > Tools/DumpRenderTree/efl/LayoutTestControllerEfl.cpp:612 > + DumpRenderTreeSupportEfl::setDomainRelaxationForbiddenForURLScheme(forbidden, String(scheme->ustring().impl())); Is it common to reach into the guts of a JSStringRef. From what I've seen of the DRT, typically you use the JavaScriptCore API to convert into a char* or a String.
Comment on attachment 138542 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=138542&action=review > Tools/DumpRenderTree/efl/LayoutTestControllerEfl.cpp:612 > + DumpRenderTreeSupportEfl::setDomainRelaxationForbiddenForURLScheme(forbidden, String(scheme->ustring().impl())); Is it common to reach into the guts of a JSStringRef? From what I've seen of the DRT, typically you use the JavaScriptCore API to convert into a char* or a String.
(In reply to comment #7) > (From update of attachment 138542 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=138542&action=review > > > Tools/DumpRenderTree/efl/LayoutTestControllerEfl.cpp:612 > > + DumpRenderTreeSupportEfl::setDomainRelaxationForbiddenForURLScheme(forbidden, String(scheme->ustring().impl())); > > Is it common to reach into the guts of a JSStringRef? From what I've seen of the DRT, typically you use the JavaScriptCore API to convert into a char* or a String. Thanks for the review. Looked around the EFL's DRT code and this is how we have been doing the convertion everywhere. For example we are converting JSStringRef into a char* like 'jsstringref'->ustring().utf8().data(). So, I thought it's Ok to construct a String directly without using JavaScriptCore API.
Created attachment 140907 [details] Patch rebased
Martin, could you please have a look at this bug again?
Comment on attachment 140907 [details] Patch Needs rebasing.
Comment on attachment 140907 [details] Patch Looks fine.
Comment on attachment 140907 [details] Patch R117428 moves setDomainRelaxationForbiddenForURLScheme to InternalSettings. http://trac.webkit.org/changeset/117428 So, this patch just is enough to touches LayoutTests/platform/efl/Skipped file. Could you please modify this patch ?
(In reply to comment #13) > (From update of attachment 140907 [details]) > R117428 moves setDomainRelaxationForbiddenForURLScheme to InternalSettings. > http://trac.webkit.org/changeset/117428 > > So, this patch just is enough to touches LayoutTests/platform/efl/Skipped file. Could you please modify this patch ? Thanks for notifying me. I will update the patch now.
Created attachment 142457 [details] Patch Unskip passing test case.
Comment on attachment 142457 [details] Patch It is possible that r117428 will be reverted because it may cause build break on Safari. Let's wait for final decision
Hi Gyuyoung, Do you think that the previous Sudarsana Nagineni's patch with setDomainRelaxationForbiddenForURLScheme implementation should be restored? It looks like there are some discussion about 86704 so we cannot just unskip http/tests/security/setDomainRelaxationForbiddenForURLScheme.html.
(In reply to comment #17) > Hi Gyuyoung, > Do you think that the previous Sudarsana Nagineni's patch with setDomainRelaxationForbiddenForURLScheme implementation should be restored? It looks like there are some discussion about 86704 so we cannot just unskip http/tests/security/setDomainRelaxationForbiddenForURLScheme.html. Yes, please restore this patch. My original patch was reverted because of Safari build.
Created attachment 144537 [details] patch r117428 from 86704 was rolled out, so restoring the original patch.
Comment on attachment 144537 [details] patch Looks good to me.
Comment on attachment 144537 [details] patch Clearing flags on attachment: 144537 Committed r120294: <http://trac.webkit.org/changeset/120294>
All reviewed patches have been landed. Closing bug.