69380 2011-10-04 15:05:49 -0700 toURLRef() in WKSharedAPICast.h should return null for a null string 2011-10-14 17:14:37 -0700 1 1 1 Unclassified WebKit WebKit2 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED P2 Normal --- 1 adachan adachan aroben webkit.review.bot oldest_to_newest 477987 0 adachan 2011-10-04 15:05:49 -0700 toURLRef() in WKSharedAPICast.h should return null for a null string. Looks like it's just missing a "return" here: if (!string) ProxyingRefPtr<WebURL>(0); 478004 1 109705 adachan 2011-10-04 15:29:52 -0700 Created attachment 109705 Patch: Add the missing return for the null string case. 478017 2 adachan 2011-10-04 15:46:41 -0700 Committed: http://trac.webkit.org/changeset/96656 478263 3 aroben 2011-10-05 04:02:02 -0700 What's the user-visible or developer-visible effect of this bug? In general I think it's a lot more useful to have bugs describe symptoms rather than describing changes we'd like to make to the code. Is it possible to write a regression test for this? 478489 4 adachan 2011-10-05 10:39:52 -0700 (In reply to comment #3) > What's the user-visible or developer-visible effect of this bug? In general I think it's a lot more useful to have bugs describe symptoms rather than describing changes we'd like to make to the code. If WKURLCopyCFURL() is called on the resulting WKURLRef later, it'll assert in ASSERT(!toImpl(URLRef)->string().isNull()); because the string is null. > Is it possible to write a regression test for this? I'll look into writing a TestWebKitAPI test for this. Thanks for the suggestion! 484422 5 adachan 2011-10-14 17:14:37 -0700 (In reply to comment #3) > What's the user-visible or developer-visible effect of this bug? In general I think it's a lot more useful to have bugs describe symptoms rather than describing changes we'd like to make to the code. > > Is it possible to write a regression test for this? Coming up with a test for this is not as straightforward as I imagined, since we can't call toURLRef() directly from a TestWebKitAPI test. I've filed https://bugs.webkit.org/show_bug.cgi?id=70159 to track coming up with a test for this. 109705 2011-10-04 15:29:52 -0700 2011-10-04 15:38:57 -0700 Patch: Add the missing return for the null string case. 69380.patch.txt text/plain 1253 adachan SW5kZXg6IFNvdXJjZS9XZWJLaXQyL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2Vi S2l0Mi9DaGFuZ2VMb2cJKHJldmlzaW9uIDk2NjUyKQorKysgU291cmNlL1dlYktpdDIvQ2hhbmdl TG9nCSh3b3JraW5nIGNvcHkpCkBAIC0xLDMgKzEsMTMgQEAKKzIwMTEtMTAtMDQgIEFkYSBDaGFu ICA8YWRhY2hhbkBhcHBsZS5jb20+CisKKyAgICAgICAgdG9VUkxSZWYoKSBpbiBXS1NoYXJlZEFQ SUNhc3QuaCBzaG91bGQgcmV0dXJuIG51bGwgZm9yIGEgbnVsbCBzdHJpbmcKKyAgICAgICAgaHR0 cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTY5MzgwCisKKyAgICAgICAgUmV2 aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgKiBTaGFyZWQvQVBJL2MvV0tTaGFy ZWRBUElDYXN0Lmg6CisgICAgICAgIChXZWJLaXQ6OnRvVVJMUmVmKTogQWRkIHRoZSBtaXNzaW5n ICJyZXR1cm4iIGZvciB0aGUgbnVsbCBzdHJpbmcgY2FzZS4KKwogMjAxMS0xMC0wNCAgSm9uIExl ZSAgPGpvbmxlZUBhcHBsZS5jb20+CiAKICAgICAgICAgUkVHUkVTU0lPTiAoV0syKTogKFNoaWZ0 LSlvcHRpb24tdGFiYmluZyBza2lwcyBvdmVyIGVsZW1lbnRzIHdoZW4gdHJhbnNpdGlvbmluZyBm cm9tIGNocm9tZSB0byB3ZWJ2aWV3CkluZGV4OiBTb3VyY2UvV2ViS2l0Mi9TaGFyZWQvQVBJL2Mv V0tTaGFyZWRBUElDYXN0LmgKPT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gU291cmNlL1dlYktpdDIvU2hhcmVkL0FQ SS9jL1dLU2hhcmVkQVBJQ2FzdC5oCShyZXZpc2lvbiA5NjY1MSkKKysrIFNvdXJjZS9XZWJLaXQy L1NoYXJlZC9BUEkvYy9XS1NoYXJlZEFQSUNhc3QuaAkod29ya2luZyBjb3B5KQpAQCAtMTUxLDcg KzE1MSw3IEBAIGlubGluZSBXS1N0cmluZ1JlZiB0b0NvcGllZEFQSShjb25zdCBTdHIKIGlubGlu ZSBQcm94eWluZ1JlZlB0cjxXZWJVUkw+IHRvVVJMUmVmKFN0cmluZ0ltcGwqIHN0cmluZykKIHsK ICAgICBpZiAoIXN0cmluZykKLSAgICAgICAgUHJveHlpbmdSZWZQdHI8V2ViVVJMPigwKTsKKyAg ICAgICAgcmV0dXJuIFByb3h5aW5nUmVmUHRyPFdlYlVSTD4oMCk7CiAgICAgcmV0dXJuIFByb3h5 aW5nUmVmUHRyPFdlYlVSTD4oV2ViVVJMOjpjcmVhdGUoU3RyaW5nKHN0cmluZykpKTsKIH0KIAo=