Changeset: http://trac.webkit.org/changeset/45615 Failing test: fast/loader/subframe-navigate-during-main-frame-load.html Test diff: ============== Back Forward List ============== - (file test):fast/loader/subframe-navigate-during-main-frame-load.html **nav target** -curr-> (file test):fast/loader/resources/subframe-navigate-during-main-frame-load2.html **nav target** + file:///home/jmalonzo/OpenSource/WebKit/LayoutTests/fast/loader/subframe-navigate-during-main-frame-load.html **nav target** +curr-> file:///home/jmalonzo/OpenSource/WebKit/LayoutTests/fast/loader/resources/subframe-navigate-during-main-frame-load2.html **nav target** data:text/html,%3Cbody%20onload=%22layoutTestController.notifyDone();%22%3E%3C/body%3E (in frame "subframe") =============================================== Doing a similar fix as the one in r45615 in Gtk's DRT should fix this.
Created attachment 32651 [details] patch
Comment on attachment 32651 [details] patch > + const gchar* uri = webkit_web_history_item_get_uri(item); > + if (g_strcmp0(g_uri_parse_scheme(uri), "file") == 0) { You are leaking the return of g_uri_parse_scheme here. > + static gchar* layoutTestString = g_strdup("/LayoutTests/"); > + static gchar* fileTestString = g_strdup("(file test):"); These are also leaked. There should be no need to g_strdup here. Just use the static strings instead? > + result = g_string_append(result, pos + strlen(layoutTestString)); I'd suggest using sizeof("/LayoutTests/") instead of strlen.
Created attachment 32782 [details] updated patch
(In reply to comment #3) > Created an attachment (id=32782) [details] > updated patch I made the updates except for the strlen -> sizeof change. May I ask why? Thanks.
Comment on attachment 32782 [details] updated patch > + gchar* resultStr = g_string_free(result, FALSE); > + printf("%s", resultStr); > + g_free(resultStr); I would prefer the following: printf("%s", result->str); g_string_free(result, TRUE); But I am fine either way. If you feel like changing it, please do. r=me
(In reply to comment #4) > (In reply to comment #3) > > Created an attachment (id=32782) [details] [details] > > updated patch > > I made the updates except for the strlen -> sizeof change. May I ask why? > Thanks. Err... forgot to answer this one. This is more of a nit-pick. A strlen will waste runtime - we already know the size of the string in build time, since it is static and won't change. sizeof is compile-time, so no time wasted counting chars. Notice that we would have to use sizeof on the actual static string, not on the variable. But, as I said, this is a nitpick, if you feel like using strlen, there should be no problem, since this is not performance-critical code.
Comment on attachment 32782 [details] updated patch Please post an updated patch with the nits addressed. Removing from commit queue.
Created attachment 32995 [details] Updated to use result->str instead of a temp var
Comment on attachment 32995 [details] Updated to use result->str instead of a temp var r=me
(In reply to comment #9) > (From update of attachment 32995 [details]) > r=me Landed as http://trac.webkit.org/changeset/46096. Thanks.