130492 2014-03-19 18:49:47 -0700 [GTK] Running minibrowser with url crashes in debug build 2014-04-01 05:37:20 -0700 1 1 1 Unclassified WebKit WebKitGTK 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED P2 Normal --- 1 jaepark webkit-unassigned berto cgarcia commit-queue fvallee gustavo mario mrobinson svillar oldest_to_newest 992355 0 jaepark 2014-03-19 18:49:47 -0700 #0 0x00007ffff2549fdb in WTFCrash () at ../../Source/WTF/wtf/Assertions.cpp:333 #1 0x00007ffff2f7f508 in WebCore::URL::URL (this=0x7fffffffcb30, url=...) at ../../Source/WebCore/platform/URL.cpp:331 #2 0x00007ffff247329b in WebCore::ResourceRequest::ResourceRequest (this=0x7fffffffcbc0, url=...) at ../../Source/WebCore/platform/network/soup/ResourceRequest.h:42 #3 0x00007ffff2482337 in webkit_web_view_load_uri (webView=0x6cfd30, uri=0x704730 "http://www.naver.com") at ../../Source/WebKit2/UIProcess/API/gtk/WebKitWebView.cpp:2002 #4 0x000000000040ef63 in browser_window_load_uri (window=0x70c2a0, uri=0x6beec0 "http://www.naver.com") at ../../Tools/MiniBrowser/gtk/BrowserWindow.c:801 #5 0x000000000040f09e in createBrowserWindow (uri=0x6c0e50 "http://www.naver.com", webkitSettings=0x681310) at ../../Tools/MiniBrowser/gtk/main.c:74 #6 0x000000000040fa25 in main (argc=1, argv=0x7fffffffce88) at ../../Tools/MiniBrowser/gtk/main.c:286 This crash appeared as of r163037. Test Scenario. 1. Run Debug MiniBrowser > Tools/Script/run-launcher --debug --gtk -2 http://www.google.com 2. Crashes at WebCore::URL::URL() because "url = http://www.google.com" but "m_string = http://www.google.com/" If we always put "/" at the end of URL, there won't be assertion failure at URL::URL() 995325 1 228051 mario 2014-03-28 08:13:09 -0700 Created attachment 228051 Patch proposal This patch fixes the issue by using SoupURI API to create a URL object to be passed to loadRequest() Please review 995327 2 mario 2014-03-28 08:13:49 -0700 *** Bug 129606 has been marked as a duplicate of this bug. *** 995328 3 commit-queue 2014-03-28 08:15:56 -0700 Thanks for the patch. If this patch contains new public API please make sure it follows the guidelines for new WebKit2 GTK+ API. See http://trac.webkit.org/wiki/WebKitGTK/AddingNewWebKit2API 995341 4 228051 mario 2014-03-28 09:11:29 -0700 Comment on attachment 228051 Patch proposal Thanks! 995352 5 228051 commit-queue 2014-03-28 09:42:29 -0700 Comment on attachment 228051 Patch proposal Clearing flags on attachment: 228051 Committed r166410: <http://trac.webkit.org/changeset/166410> 995353 6 commit-queue 2014-03-28 09:42:32 -0700 All reviewed patches have been landed. Closing bug. 995791 7 cgarcia 2014-03-31 01:30:00 -0700 This broke /webkit2/WebKitWebContext/uri-scheme /webkit2/WebKitWebContext/uri-scheme: ** ERROR:../../Tools/TestWebKitAPI/gtk/WebKit2Gtk/LoadTrackingTest.cpp:30:void loadChangedCallback(WebKitWebView*, WebKitLoadEvent, LoadTrackingTest*): assertion failed (test->m_activeURI.data() == webkit_web_view_get_uri(webView)): ("echo:hello world" == "echo:hello%20world") 995848 8 mario 2014-03-31 06:39:12 -0700 (In reply to comment #7) > This broke /webkit2/WebKitWebContext/uri-scheme > > /webkit2/WebKitWebContext/uri-scheme: ** > ERROR:../../Tools/TestWebKitAPI/gtk/WebKit2Gtk/LoadTrackingTest.cpp:30:void loadChangedCallback(WebKitWebView*, WebKitLoadEvent, LoadTrackingTest*): assertion failed (test->m_activeURI.data() == webkit_web_view_get_uri(webView)): ("echo:hello world" == "echo:hello%20world") I think the problem here is in the test, not in the code, since "hello world" is not a valid URI (it should be normalized to "hello%20world" to become one) and so that's why the WebView, after this patch landed, is returning a mismatch in comparison to what the test class has stored. So, unless there's a good reason to support URIs such as "hello world" or "foo bar" in this test, I'd suggest to just update the test to use "hello-world" or "foo-bar" in these two cases, so that we do not need to either artificially unnormalize whatever valid URI comes from webkit_web_view_get_uri() nor to normalize the URIs used in these test cases. 995849 9 cgarcia 2014-03-31 06:41:25 -0700 (In reply to comment #8) > (In reply to comment #7) > > This broke /webkit2/WebKitWebContext/uri-scheme > > > > /webkit2/WebKitWebContext/uri-scheme: ** > > ERROR:../../Tools/TestWebKitAPI/gtk/WebKit2Gtk/LoadTrackingTest.cpp:30:void loadChangedCallback(WebKitWebView*, WebKitLoadEvent, LoadTrackingTest*): assertion failed (test->m_activeURI.data() == webkit_web_view_get_uri(webView)): ("echo:hello world" == "echo:hello%20world") > > I think the problem here is in the test, not in the code, since "hello world" is not a valid URI (it should be normalized to "hello%20world" to become one) and so that's why the WebView, after this patch landed, is returning a mismatch in comparison to what the test class has stored. Yes, I think you are right. > So, unless there's a good reason to support URIs such as "hello world" or "foo bar" in this test, I'd suggest to just update the test to use "hello-world" or "foo-bar" in these two cases, so that we do not need to either artificially unnormalize whatever valid URI comes from webkit_web_view_get_uri() nor to normalize the URIs used in these test cases. Sounds good to me 995850 10 228177 mario 2014-03-31 06:43:05 -0700 Created attachment 228177 Patch proposal Patch updating the test case as mentioned in the previous comment. Please review 995881 11 228177 mario 2014-03-31 09:04:00 -0700 Comment on attachment 228177 Patch proposal Thanks for the review. Now setting the cq+ flag 996234 12 228177 mario 2014-04-01 05:36:08 -0700 Comment on attachment 228177 Patch proposal This cq+ did not work. Applying manually... 996235 13 mario 2014-04-01 05:37:20 -0700 Committed r166577: <http://trac.webkit.org/changeset/166577> 228051 2014-03-28 08:13:09 -0700 2014-03-28 09:42:28 -0700 Patch proposal 0001-2014-03-28-Mario-Sanchez-Prada-mario.prada-samsung.c.patch text/plain 2514 mario RnJvbSAyNzc5OGJlMjFjOWZkMTc3OWU5MGI1ZGVhZmM3ZGY5NmQzNzk4ZDI5IE1vbiBTZXAgMTcg MDA6MDA6MDAgMjAwMQpGcm9tOiBNYXJpbyBTYW5jaGV6IFByYWRhIDxtYXJpby5wcmFkYUBzYW1z dW5nLmNvbT4KRGF0ZTogRnJpLCAyOCBNYXIgMjAxNCAxNToxMToxNCArMDAwMApTdWJqZWN0OiBb UEFUQ0hdIDIwMTQtMDMtMjggIE1hcmlvIFNhbmNoZXogUHJhZGEgIDxtYXJpby5wcmFkYUBzYW1z dW5nLmNvbT4KCiAgICAgICAgW0dUS10gUnVubmluZyBtaW5pYnJvd3NlciB3aXRoIHVybCBjcmFz aGVzIGluIGRlYnVnIGJ1aWxkCiAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19i dWcuY2dpP2lkPTEzMDQ5MgoKICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KCiAg ICAgICAgVXNlIHRoZSBSZXNvdXJjZVJlcXVlc3QoY29uc3QgVVJMJiB1cmwpIGNvbnN0cnVjdG9y LCBpbnN0ZWFkIG9mIHRoZQogICAgICAgIG9uZSB0YWtpbmcgYSBTdHJpbmcsIHRvIGNhbGwgV2Vi UGFnZVByb3h5Ojpsb2FkUmVxdWVzdCgpLCBzbyB3ZQogICAgICAgIGVuc3VyZSB0aGF0IHRoZSBw YXNzZWQgVVJJIGlzIGNvcnJlY3RseSBjb25zdHJ1Y3RlZC4KCiAgICAgICAgKiBVSVByb2Nlc3Mv QVBJL2d0ay9XZWJLaXRXZWJWaWV3LmNwcDoKICAgICAgICAod2Via2l0X3dlYl92aWV3X2xvYWRf dXJpKTogQ3JlYXRlIGFuIFVSTCBvdXQgb2YgYSBTb3VwVVJJIGFuZAogICAgICAgIHVzZSB0aGF0 IHRvIGNyZWF0ZSBhIFJlc291cmNlUmVxdWVzdCB0byBwYXNzIHRvIGxvYWRSZXF1ZXN0KCkuCi0t LQogU291cmNlL1dlYktpdDIvQ2hhbmdlTG9nICAgICAgICAgICAgICAgICAgICAgICAgICAgfCAx NSArKysrKysrKysrKysrKysKIFNvdXJjZS9XZWJLaXQyL1VJUHJvY2Vzcy9BUEkvZ3RrL1dlYktp dFdlYlZpZXcuY3BwIHwgIDMgKystCiAyIGZpbGVzIGNoYW5nZWQsIDE3IGluc2VydGlvbnMoKyks IDEgZGVsZXRpb24oLSkKCmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViS2l0Mi9DaGFuZ2VMb2cgYi9T b3VyY2UvV2ViS2l0Mi9DaGFuZ2VMb2cKaW5kZXggMzZkOGE1ZC4uMTNkMDliMSAxMDA2NDQKLS0t IGEvU291cmNlL1dlYktpdDIvQ2hhbmdlTG9nCisrKyBiL1NvdXJjZS9XZWJLaXQyL0NoYW5nZUxv ZwpAQCAtMSwzICsxLDE4IEBACisyMDE0LTAzLTI4ICBNYXJpbyBTYW5jaGV6IFByYWRhICA8bWFy aW8ucHJhZGFAc2Ftc3VuZy5jb20+CisKKyAgICAgICAgW0dUS10gUnVubmluZyBtaW5pYnJvd3Nl ciB3aXRoIHVybCBjcmFzaGVzIGluIGRlYnVnIGJ1aWxkCisgICAgICAgIGh0dHBzOi8vYnVncy53 ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD0xMzA0OTIKKworICAgICAgICBSZXZpZXdlZCBieSBO T0JPRFkgKE9PUFMhKS4KKworICAgICAgICBVc2UgdGhlIFJlc291cmNlUmVxdWVzdChjb25zdCBV UkwmIHVybCkgY29uc3RydWN0b3IsIGluc3RlYWQgb2YgdGhlCisgICAgICAgIG9uZSB0YWtpbmcg YSBTdHJpbmcsIHRvIGNhbGwgV2ViUGFnZVByb3h5Ojpsb2FkUmVxdWVzdCgpLCBzbyB3ZQorICAg ICAgICBlbnN1cmUgdGhhdCB0aGUgcGFzc2VkIFVSSSBpcyBjb3JyZWN0bHkgY29uc3RydWN0ZWQu CisKKyAgICAgICAgKiBVSVByb2Nlc3MvQVBJL2d0ay9XZWJLaXRXZWJWaWV3LmNwcDoKKyAgICAg ICAgKHdlYmtpdF93ZWJfdmlld19sb2FkX3VyaSk6IENyZWF0ZSBhbiBVUkwgb3V0IG9mIGEgU291 cFVSSSBhbmQKKyAgICAgICAgdXNlIHRoYXQgdG8gY3JlYXRlIGEgUmVzb3VyY2VSZXF1ZXN0IHRv IHBhc3MgdG8gbG9hZFJlcXVlc3QoKS4KKwogMjAxNC0wMy0yNyAgSmFlIEh5dW4gUGFyayAgPGph ZXBhcmtAd2Via2l0Lm9yZz4KIAogICAgICAgICBSZW1vdmUgTGF5ZXJUcmVlQ29udGV4dENhaXJv LmNwcApkaWZmIC0tZ2l0IGEvU291cmNlL1dlYktpdDIvVUlQcm9jZXNzL0FQSS9ndGsvV2ViS2l0 V2ViVmlldy5jcHAgYi9Tb3VyY2UvV2ViS2l0Mi9VSVByb2Nlc3MvQVBJL2d0ay9XZWJLaXRXZWJW aWV3LmNwcAppbmRleCAxMjJhZDZiLi4xMjRjNWNiIDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViS2l0 Mi9VSVByb2Nlc3MvQVBJL2d0ay9XZWJLaXRXZWJWaWV3LmNwcAorKysgYi9Tb3VyY2UvV2ViS2l0 Mi9VSVByb2Nlc3MvQVBJL2d0ay9XZWJLaXRXZWJWaWV3LmNwcApAQCAtMjAwNCw3ICsyMDA0LDgg QEAgdm9pZCB3ZWJraXRfd2ViX3ZpZXdfbG9hZF91cmkoV2ViS2l0V2ViVmlldyogd2ViVmlldywg Y29uc3QgZ2NoYXIqIHVyaSkKICAgICBnX3JldHVybl9pZl9mYWlsKFdFQktJVF9JU19XRUJfVklF Vyh3ZWJWaWV3KSk7CiAgICAgZ19yZXR1cm5faWZfZmFpbCh1cmkpOwogCi0gICAgZ2V0UGFnZSh3 ZWJWaWV3KS0+bG9hZFJlcXVlc3QoU3RyaW5nOjpmcm9tVVRGOCh1cmkpKTsKKyAgICBHVW5pcXVl UHRyPFNvdXBVUkk+IHNvdXBVUkkoc291cF91cmlfbmV3KHVyaSkpOworICAgIGdldFBhZ2Uod2Vi VmlldyktPmxvYWRSZXF1ZXN0KFVSTChzb3VwVVJJLmdldCgpKSk7CiB9CiAKIC8qKgotLSAKMS44 LjUuMwoK 228177 2014-03-31 06:43:05 -0700 2014-04-01 05:36:08 -0700 Patch proposal 0001-2014-03-31-Mario-Sanchez-Prada-mario.prada-samsung.c.patch text/plain 2998 mario RnJvbSA1N2RjMGNmZTk0YmMxZDlmOTZlMjA3YWVjMGU2OThhY2QyYWI4YWY2IE1vbiBTZXAgMTcg MDA6MDA6MDAgMjAwMQpGcm9tOiBNYXJpbyBTYW5jaGV6IFByYWRhIDxtYXJpby5wcmFkYUBzYW1z dW5nLmNvbT4KRGF0ZTogTW9uLCAzMSBNYXIgMjAxNCAxNDoyNTozNiArMDEwMApTdWJqZWN0OiBb UEFUQ0hdIDIwMTQtMDMtMzEgIE1hcmlvIFNhbmNoZXogUHJhZGEgIDxtYXJpby5wcmFkYUBzYW1z dW5nLmNvbT4KCiAgICAgICAgW0dUS10gUnVubmluZyBtaW5pYnJvd3NlciB3aXRoIHVybCBjcmFz aGVzIGluIGRlYnVnIGJ1aWxkCiAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19i dWcuY2dpP2lkPTEzMDQ5MgoKICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KCiAg ICAgICAgVXBkYXRlICJ1cmktc2NoZW1lIiB0ZXN0IG5vdCB0byB1c2UgaW52YWxpZCBVUklzIGlu IGl0LCBzbyB3ZQogICAgICAgIGRvbid0IGdldCBhIG1pc3RtYXRjaCB3aGVuIGNvbXBhcmluZyB0 aGUgb3JpZ2luYWwgVVJJIHN0b3JlZCBpbgogICAgICAgIHRoZSB0ZXN0IGNsYXNzIHdpdGggdGhl IG9uZSByZXR1cm5lZCBieSB0aGUgV2ViS2l0V2ViVmlldy4KCiAgICAgICAgKiBUZXN0V2ViS2l0 QVBJL1Rlc3RzL1dlYktpdDJHdGsvVGVzdFdlYktpdFdlYkNvbnRleHQuY3BwOgogICAgICAgICh0 ZXN0V2ViQ29udGV4dFVSSVNjaGVtZSk6IFVwZGF0ZSB0aGUgdGVzdC4KLS0tCiBUb29scy9DaGFu Z2VMb2cgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgIHwgMTQgKysr KysrKysrKysrKysKIC4uLi9UZXN0cy9XZWJLaXQyR3RrL1Rlc3RXZWJLaXRXZWJDb250ZXh0LmNw cCAgICAgICAgICAgICAgfCAgNCArKy0tCiAyIGZpbGVzIGNoYW5nZWQsIDE2IGluc2VydGlvbnMo KyksIDIgZGVsZXRpb25zKC0pCgpkaWZmIC0tZ2l0IGEvVG9vbHMvQ2hhbmdlTG9nIGIvVG9vbHMv Q2hhbmdlTG9nCmluZGV4IDY0NWYzNTMuLjMxNDQ3N2EgMTAwNjQ0Ci0tLSBhL1Rvb2xzL0NoYW5n ZUxvZworKysgYi9Ub29scy9DaGFuZ2VMb2cKQEAgLTEsMyArMSwxNyBAQAorMjAxNC0wMy0zMSAg TWFyaW8gU2FuY2hleiBQcmFkYSAgPG1hcmlvLnByYWRhQHNhbXN1bmcuY29tPgorCisgICAgICAg IFtHVEtdIFJ1bm5pbmcgbWluaWJyb3dzZXIgd2l0aCB1cmwgY3Jhc2hlcyBpbiBkZWJ1ZyBidWls ZAorICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9MTMwNDky CisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgVXBkYXRl ICJ1cmktc2NoZW1lIiB0ZXN0IG5vdCB0byB1c2UgaW52YWxpZCBVUklzIGluIGl0LCBzbyB3ZQor ICAgICAgICBkb24ndCBnZXQgYSBtaXN0bWF0Y2ggd2hlbiBjb21wYXJpbmcgdGhlIG9yaWdpbmFs IFVSSSBzdG9yZWQgaW4KKyAgICAgICAgdGhlIHRlc3QgY2xhc3Mgd2l0aCB0aGUgb25lIHJldHVy bmVkIGJ5IHRoZSBXZWJLaXRXZWJWaWV3LgorCisgICAgICAgICogVGVzdFdlYktpdEFQSS9UZXN0 cy9XZWJLaXQyR3RrL1Rlc3RXZWJLaXRXZWJDb250ZXh0LmNwcDoKKyAgICAgICAgKHRlc3RXZWJD b250ZXh0VVJJU2NoZW1lKTogVXBkYXRlIHRoZSB0ZXN0LgorCiAyMDE0LTAzLTI2ICBBbmR5IEVz dGVzICA8YWVzdGVzQGFwcGxlLmNvbT4KIAogICAgICAgICBbQ29jb2FdIEV4cG9zZSBEb3dubG9h ZENsaWVudDo6ZGlkRmFpbCgpIGFuZCBEb3dubG9hZENsaWVudDo6ZGlkQ2FuY2VsKCkKZGlmZiAt LWdpdCBhL1Rvb2xzL1Rlc3RXZWJLaXRBUEkvVGVzdHMvV2ViS2l0Mkd0ay9UZXN0V2ViS2l0V2Vi Q29udGV4dC5jcHAgYi9Ub29scy9UZXN0V2ViS2l0QVBJL1Rlc3RzL1dlYktpdDJHdGsvVGVzdFdl YktpdFdlYkNvbnRleHQuY3BwCmluZGV4IDMxOWVlYzQuLjc0ZThkNjggMTAwNjQ0Ci0tLSBhL1Rv b2xzL1Rlc3RXZWJLaXRBUEkvVGVzdHMvV2ViS2l0Mkd0ay9UZXN0V2ViS2l0V2ViQ29udGV4dC5j cHAKKysrIGIvVG9vbHMvVGVzdFdlYktpdEFQSS9UZXN0cy9XZWJLaXQyR3RrL1Rlc3RXZWJLaXRX ZWJDb250ZXh0LmNwcApAQCAtMTk1LDcgKzE5NSw3IEBAIHN0YXRpYyB2b2lkIHRlc3RXZWJDb250 ZXh0VVJJU2NoZW1lKFVSSVNjaGVtZVRlc3QqIHRlc3QsIGdjb25zdHBvaW50ZXIpCiAgICAgZ19h c3NlcnQoIXN0cm5jbXAobWFpblJlc291cmNlRGF0YSwga0JhckhUTUwsIG1haW5SZXNvdXJjZURh dGFTaXplKSk7CiAKICAgICB0ZXN0LT5yZWdpc3RlclVSSVNjaGVtZUhhbmRsZXIoImVjaG8iLCBr RWNob0hUTUxGb3JtYXQsIC0xLCAidGV4dC9odG1sIik7Ci0gICAgdGVzdC0+bG9hZFVSSSgiZWNo bzpoZWxsbyB3b3JsZCIpOworICAgIHRlc3QtPmxvYWRVUkkoImVjaG86aGVsbG8td29ybGQiKTsK ICAgICB0ZXN0LT53YWl0VW50aWxMb2FkRmluaXNoZWQoKTsKICAgICBHVW5pcXVlUHRyPGNoYXI+ IGVjaG9IVE1MKGdfc3RyZHVwX3ByaW50ZihrRWNob0hUTUxGb3JtYXQsIHdlYmtpdF91cmlfc2No ZW1lX3JlcXVlc3RfZ2V0X3BhdGgodGVzdC0+bV91cmlTY2hlbWVSZXF1ZXN0LmdldCgpKSkpOwog ICAgIG1haW5SZXNvdXJjZURhdGFTaXplID0gMDsKQEAgLTIwNSw3ICsyMDUsNyBAQCBzdGF0aWMg dm9pZCB0ZXN0V2ViQ29udGV4dFVSSVNjaGVtZShVUklTY2hlbWVUZXN0KiB0ZXN0LCBnY29uc3Rw b2ludGVyKQogCiAgICAgdGVzdC0+cmVnaXN0ZXJVUklTY2hlbWVIYW5kbGVyKCJub21pbWUiLCBr QmFySFRNTCwgLTEsIDApOwogICAgIHRlc3QtPm1fbG9hZEV2ZW50cy5jbGVhcigpOwotICAgIHRl c3QtPmxvYWRVUkkoIm5vbWltZTpmb28gYmFyIik7CisgICAgdGVzdC0+bG9hZFVSSSgibm9taW1l OmZvby1iYXIiKTsKICAgICB0ZXN0LT53YWl0VW50aWxMb2FkRmluaXNoZWQoKTsKICAgICBnX2Fz c2VydCh0ZXN0LT5tX2xvYWRFdmVudHMuY29udGFpbnMoTG9hZFRyYWNraW5nVGVzdDo6UHJvdmlz aW9uYWxMb2FkRmFpbGVkKSk7CiAKLS0gCjEuOC41LjMKCg==