EFL DRT should translate file:///tmp/LayoutTests paths into a local URL that's derived from the path of the DRT application exec.
*** Bug 84336 has been marked as a duplicate of this bug. ***
Created attachment 145699 [details] Patch
(In reply to comment #2) > Created an attachment (id=145699) [details] Looks good to me, thanks.
Comment on attachment 145699 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=145699&action=review > Tools/DumpRenderTree/efl/LayoutTestControllerEfl.cpp:198 > + // If the url contains LayoutTests we need to remap that to %s/url/URL/g > Tools/DumpRenderTree/efl/LayoutTestControllerEfl.cpp:206 > + // If the url is a child of /tmp we need to convert it to be a child ditto.
Created attachment 145798 [details] Patch
Comment on attachment 145798 [details] Patch Looks fine.
Comment on attachment 145798 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=145798&action=review > Tools/DumpRenderTree/efl/LayoutTestControllerEfl.cpp:181 > +// Convert a JSStringRef to a WTF::String - converted from a method of the same > +// name in LayoutTestControllerMac.mm which returns a std::string > +static inline String stringFromJSString(JSStringRef jsString) Some nitpicking: have you tried operating on the UString returned by JSStringRef::ustring()? It has find() and substringSharingImpl(), so adding this method may be unnecessary. > Tools/DumpRenderTree/efl/LayoutTestControllerEfl.cpp:212 > + if (requestedUrl.find("LayoutTests") != notFound) { > + // If the URL contains LayoutTests we need to remap that to > + // LOCAL_RESOURCE_ROOT which is the path of the LayoutTests directory > + // within the WebKit source tree. > + resourceRoot = getenv("LOCAL_RESOURCE_ROOT"); > + String requestedRoot("/tmp/LayoutTests"); > + size_t indexOfRootStart = requestedUrl.reverseFind(requestedRoot); > + indexOfSeparatorAfterRoot = indexOfRootStart + requestedRoot.length(); > + } else if (requestedUrl.find("tmp") != notFound) { > + // If the URL is a child of /tmp we need to convert it to be a child > + // DUMPRENDERTREE_TEMP replace tmp with DUMPRENDERTREE_TEMP > + resourceRoot = getenv("DUMPRENDERTREE_TEMP"); > + String requestedRoot("/tmp"); > + size_t indexOfRootStart = requestedUrl.reverseFind(requestedRoot); > + indexOfSeparatorAfterRoot = indexOfRootStart + requestedRoot.length(); > + } The code to calculate the index of the separator seems to be duplicated in each case. What if you do something like if (requestedUrl.find("LayoutTests") != notFound) { requestedRoot = "/tmp/LayoutTests"; resourceRoot = getenv("LOCAL_RESOURCE_ROOT"); } else if (...) { requestedRoot = "/tmp"; resourceRoot = getenv("DUMPRENDERTREE_TEMP"); } else ASSERT_NOT_REACHED(); // via mac's LTC.mm // calculate separator index here
Comment on attachment 145798 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=145798&action=review >> Tools/DumpRenderTree/efl/LayoutTestControllerEfl.cpp:181 >> +static inline String stringFromJSString(JSStringRef jsString) > > Some nitpicking: have you tried operating on the UString returned by JSStringRef::ustring()? It has find() and substringSharingImpl(), so adding this method may be unnecessary. I have not, I looked to other LayoutController* for inspiration. Will have a go with ustring() now, I'd much prefer not to add extra methods - thanks for the hint. >> Tools/DumpRenderTree/efl/LayoutTestControllerEfl.cpp:212 >> + } > > The code to calculate the index of the separator seems to be duplicated in each case. What if you do something like > if (requestedUrl.find("LayoutTests") != notFound) { > requestedRoot = "/tmp/LayoutTests"; > resourceRoot = getenv("LOCAL_RESOURCE_ROOT"); > } else if (...) { > requestedRoot = "/tmp"; > resourceRoot = getenv("DUMPRENDERTREE_TEMP"); > } else > ASSERT_NOT_REACHED(); // via mac's LTC.mm > > // calculate separator index here Much nicer, will do.
Created attachment 146425 [details] Patch
Comment on attachment 145798 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=145798&action=review >>> Tools/DumpRenderTree/efl/LayoutTestControllerEfl.cpp:181 >>> +static inline String stringFromJSString(JSStringRef jsString) >> >> Some nitpicking: have you tried operating on the UString returned by JSStringRef::ustring()? It has find() and substringSharingImpl(), so adding this method may be unnecessary. > > I have not, I looked to other LayoutController* for inspiration. Will have a go with ustring() now, I'd much prefer not to add extra methods - thanks for the hint. I played around some with UString but instead I've opted to avoid the extra method by following a pattern I noticed in other parts of LayoutTestControllerEfl.cpp and construct a WTF::String from the characters of the JSStringRef like: String requestedUrl(url->characters()).
(In reply to comment #10) > I played around some with UString but instead I've opted to avoid the extra method by following a pattern I noticed in other parts of LayoutTestControllerEfl.cpp and construct a WTF::String from the characters of the JSStringRef like: String requestedUrl(url->characters()). Alright, it looks much better. Thank you!
Comment on attachment 146425 [details] Patch Clearing flags on attachment: 146425 Committed r119788: <http://trac.webkit.org/changeset/119788>
All reviewed patches have been landed. Closing bug.
(In reply to comment #10) > I played around some with UString but instead I've opted to avoid the extra method by following a pattern I noticed in other parts of LayoutTestControllerEfl.cpp and construct a WTF::String from the characters of the JSStringRef like: String requestedUrl(url->characters()). The following tests started failing with this patch. It seems the problem is due to characters() not being null-terminated. http://build.webkit.org/builders/EFL%20Linux%2064-bit%20Debug/builds/952/steps/layout-test/logs/stdio fast/dom/frame-loading-via-document-write.html = TEXT http/tests/security/frame-loading-via-document-write.html = TEXT http/tests/security/local-JavaScript-from-remote.html = TEXT http/tests/security/local-iFrame-from-remote.html = TEXT
(In reply to comment #14) > The following tests started failing with this patch. It seems the problem is due to characters() not being null-terminated. New bug created for this issue: https://bugs.webkit.org/show_bug.cgi?id=88661