108103 2013-01-28 12:41:40 -0800 String constructed from Literals should be non-empty 2013-01-29 10:37:51 -0800 1 1 1 Unclassified WebKit Web Template Framework 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED P2 Normal --- 1 benjamin benjamin andersca darin eric.carlson feature-media-reviews ojan.autocc webkit.review.bot oldest_to_newest 818003 0 benjamin 2013-01-28 12:41:40 -0800 Empty strings can be evil and should be marked as such. 818011 1 185038 benjamin 2013-01-28 12:47:46 -0800 Created attachment 185038 Patch 818306 2 185038 benjamin 2013-01-28 17:30:34 -0800 Comment on attachment 185038 Patch Clearing flags on attachment: 185038 Committed r141030: <http://trac.webkit.org/changeset/141030> 818307 3 benjamin 2013-01-28 17:30:36 -0800 All reviewed patches have been landed. Closing bug. 819030 4 185038 darin 2013-01-29 09:50:46 -0800 Comment on attachment 185038 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=185038&action=review This patch disagrees with itself. > Source/WTF/wtf/text/StringImpl.cpp:154 > + ASSERT_WITH_MESSAGE(length, "Use StringImpl::empty() to create an empty string"); This says you should use StringImpl::empty().T > Source/WebCore/html/HTMLMediaElement.cpp:691 > + canPlay = emptyString(); This uses emptyString(). 819060 5 benjamin 2013-01-29 10:16:00 -0800 My rationale: > > Source/WTF/wtf/text/StringImpl.cpp:154 > > + ASSERT_WITH_MESSAGE(length, "Use StringImpl::empty() to create an empty string"); > > This says you should use StringImpl::empty().T The assertion is in StringImpl. You can get here from multiple path. From StringImpl, StringImpl::empty() is the empty String. > > Source/WebCore/html/HTMLMediaElement.cpp:691 > > + canPlay = emptyString(); > > This uses emptyString(). The variable "canPlay" is a WTFString, so here I used the empty string constructor for WTFString. If you think that can cause confusion, I can add similar assertions in WTFString, AtomicString and maybe JSString. 819086 6 darin 2013-01-29 10:37:51 -0800 No, probably not really confusing in practice. 185038 2013-01-28 12:47:46 -0800 2013-01-29 09:50:46 -0800 Patch bug-108103-20130128124433.patch text/plain 3533 benjamin U3VidmVyc2lvbiBSZXZpc2lvbjogMTQwOTc4CmRpZmYgLS1naXQgYS9Tb3VyY2UvV1RGL0NoYW5n ZUxvZyBiL1NvdXJjZS9XVEYvQ2hhbmdlTG9nCmluZGV4IDE3ZWUzNjhiMGJjM2E4NDQ3Y2ZkNmVh OGRiOTFlYTFkZWRiNjM5YjIuLmMzMmE3ZmYyOGU4ZjU3ZWJiZGE2N2Y1ZGQ5N2QxY2YxNmRlNDU5 MjYgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XVEYvQ2hhbmdlTG9nCisrKyBiL1NvdXJjZS9XVEYvQ2hh bmdlTG9nCkBAIC0xLDMgKzEsMTggQEAKKzIwMTMtMDEtMjggIEJlbmphbWluIFBvdWxhaW4gIDxi ZW5qYW1pbkB3ZWJraXQub3JnPgorCisgICAgICAgIFN0cmluZyBjb25zdHJ1Y3RlZCBmcm9tIExp dGVyYWxzIHNob3VsZCBiZSBub24tZW1wdHkKKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5v cmcvc2hvd19idWcuY2dpP2lkPTEwODEwMworCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAo T09QUyEpLgorCisgICAgICAgIEZvciBlZmZpY2llbmN5LCB0aGUgY29uc3RydWN0aW9uIGZyb20g bGl0ZXJhbCBvbmx5IHdvcmtzIHdpdGggdmFsaWQgbm9uLWVtcHR5IHN0cmluZ3MuCisgICAgICAg IE9uZSBvZiB0aGUgY29uc3RydWN0b3Igd2FzIGNoYW5nZWQgdG8gZml4IGFuIGVycm9yIGZyb20g SFRNTE1lZGlhRWxlbWVudC4KKworICAgICAgICBUaGlzIHBhdGNoIHJlcGxhY2VzIHRoZSBicmFu Y2ggd2l0aCBhbiBhc3NlcnRpb25zLgorCisgICAgICAgICogd3RmL3RleHQvU3RyaW5nSW1wbC5j cHA6CisgICAgICAgIChXVEY6OlN0cmluZ0ltcGw6OmNyZWF0ZUZyb21MaXRlcmFsKToKKwogMjAx My0wMS0yNyAgTGFzemxvIEdvbWJvcyAgPGwuZ29tYm9zQHNhbXN1bmcuY29tPgogCiAgICAgICAg IFtBTkRST0lEXSBGaXggYSB0eXBvCmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VM b2cgYi9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXggNjE3YTM0ZjZlODhkMmNjOWM5MTY5 MjgzZDU4NDM5OTQxOWViZDhhOC4uZGIyYWJlM2IwMjMyMTdmYzU5MDQ5OWU4OTkzYzg1MjhkZTU5 NWZjMiAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCisrKyBiL1NvdXJjZS9X ZWJDb3JlL0NoYW5nZUxvZwpAQCAtMSwzICsxLDE2IEBACisyMDEzLTAxLTI4ICBCZW5qYW1pbiBQ b3VsYWluICA8YmVuamFtaW5Ad2Via2l0Lm9yZz4KKworICAgICAgICBTdHJpbmcgY29uc3RydWN0 ZWQgZnJvbSBMaXRlcmFscyBzaG91bGQgYmUgbm9uLWVtcHR5CisgICAgICAgIGh0dHBzOi8vYnVn cy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD0xMDgxMDMKKworICAgICAgICBSZXZpZXdlZCBi eSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICBTdHJpbmdzIGZyb20gbGl0ZXJhbCBzaG91bGQg bm90IGJlIGNvbnN0cnVjdGVkIGZyb20gZW1wdHkgc3RyaW5ncy4gVXNlIGVtcHR5U3RyaW5nKCkK KyAgICAgICAgaW5zdGVhZC4KKworICAgICAgICAqIGh0bWwvSFRNTE1lZGlhRWxlbWVudC5jcHA6 CisgICAgICAgIChXZWJDb3JlOjpIVE1MTWVkaWFFbGVtZW50OjpjYW5QbGF5VHlwZSk6CisKIDIw MTMtMDEtMjggIEJlYXIgVHJhdmlzICA8YmV0cmF2aXNAYWRvYmUuY29tPgogCiAgICAgICAgIFtD U1MgRXhjbHVzaW9uc10gUmVmYWN0b3IgRXhjbHVzaW9uU2hhcGVJbnNpZGVJbmZvIHRvIG1vcmUg Z2VuZXJhbCBFeGNsdXNpb25TaGFwZUluZm8KZGlmZiAtLWdpdCBhL1NvdXJjZS9XVEYvd3RmL3Rl eHQvU3RyaW5nSW1wbC5jcHAgYi9Tb3VyY2UvV1RGL3d0Zi90ZXh0L1N0cmluZ0ltcGwuY3BwCmlu ZGV4IDlhNGUwOTUwMWQzNTg3Njk5NzMzY2NmYzM2OTU1ODcyZGFmZTBhOTMuLmZmMmFkZDU5N2Ix MGQyNWYxNGYyMzYyNDlhNTkxY2Y5YzYwYTQ1MDUgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XVEYvd3Rm L3RleHQvU3RyaW5nSW1wbC5jcHAKKysrIGIvU291cmNlL1dURi93dGYvdGV4dC9TdHJpbmdJbXBs LmNwcApAQCAtMTUxLDYgKzE1MSw3IEBAIFN0cmluZ0ltcGw6On5TdHJpbmdJbXBsKCkKIAogUGFz c1JlZlB0cjxTdHJpbmdJbXBsPiBTdHJpbmdJbXBsOjpjcmVhdGVGcm9tTGl0ZXJhbChjb25zdCBj aGFyKiBjaGFyYWN0ZXJzLCB1bnNpZ25lZCBsZW5ndGgpCiB7CisgICAgQVNTRVJUX1dJVEhfTUVT U0FHRShsZW5ndGgsICJVc2UgU3RyaW5nSW1wbDo6ZW1wdHkoKSB0byBjcmVhdGUgYW4gZW1wdHkg c3RyaW5nIik7CiAgICAgQVNTRVJUKGNoYXJhY3RlcnNBcmVBbGxBU0NJSTxMQ2hhcj4ocmVpbnRl cnByZXRfY2FzdDxjb25zdCBMQ2hhcio+KGNoYXJhY3RlcnMpLCBsZW5ndGgpKTsKICAgICByZXR1 cm4gYWRvcHRSZWYobmV3IFN0cmluZ0ltcGwoY2hhcmFjdGVycywgbGVuZ3RoLCBDb25zdHJ1Y3RG cm9tTGl0ZXJhbCkpOwogfQpAQCAtMTU4LDggKzE1OSw3IEBAIFBhc3NSZWZQdHI8U3RyaW5nSW1w bD4gU3RyaW5nSW1wbDo6Y3JlYXRlRnJvbUxpdGVyYWwoY29uc3QgY2hhciogY2hhcmFjdGVycywg dW5zCiBQYXNzUmVmUHRyPFN0cmluZ0ltcGw+IFN0cmluZ0ltcGw6OmNyZWF0ZUZyb21MaXRlcmFs KGNvbnN0IGNoYXIqIGNoYXJhY3RlcnMpCiB7CiAgICAgc2l6ZV90IGxlbmd0aCA9IHN0cmxlbihj aGFyYWN0ZXJzKTsKLSAgICBpZiAoIWxlbmd0aCkKLSAgICAgICAgcmV0dXJuIGVtcHR5KCk7Cisg ICAgQVNTRVJUX1dJVEhfTUVTU0FHRShsZW5ndGgsICJVc2UgU3RyaW5nSW1wbDo6ZW1wdHkoKSB0 byBjcmVhdGUgYW4gZW1wdHkgc3RyaW5nIik7CiAgICAgQVNTRVJUKGNoYXJhY3RlcnNBcmVBbGxB U0NJSTxMQ2hhcj4ocmVpbnRlcnByZXRfY2FzdDxjb25zdCBMQ2hhcio+KGNoYXJhY3RlcnMpLCBs ZW5ndGgpKTsKICAgICByZXR1cm4gYWRvcHRSZWYobmV3IFN0cmluZ0ltcGwoY2hhcmFjdGVycywg bGVuZ3RoLCBDb25zdHJ1Y3RGcm9tTGl0ZXJhbCkpOwogfQpkaWZmIC0tZ2l0IGEvU291cmNlL1dl YkNvcmUvaHRtbC9IVE1MTWVkaWFFbGVtZW50LmNwcCBiL1NvdXJjZS9XZWJDb3JlL2h0bWwvSFRN TE1lZGlhRWxlbWVudC5jcHAKaW5kZXggMDliMjdkMDM2MjMwZTRkMmRlYmE5MGU0MjljNjU4N2E5 OThmNGNmZS4uMzJjZmE1MDNlMzFkYzhmNWU0YjExZDcyZmYxOTk0YzRiNmI3ZDE2ZiAxMDA2NDQK LS0tIGEvU291cmNlL1dlYkNvcmUvaHRtbC9IVE1MTWVkaWFFbGVtZW50LmNwcAorKysgYi9Tb3Vy Y2UvV2ViQ29yZS9odG1sL0hUTUxNZWRpYUVsZW1lbnQuY3BwCkBAIC02ODgsNyArNjg4LDcgQEAg U3RyaW5nIEhUTUxNZWRpYUVsZW1lbnQ6OmNhblBsYXlUeXBlKGNvbnN0IFN0cmluZyYgbWltZVR5 cGUsIGNvbnN0IFN0cmluZyYga2V5U3kKICAgICBzd2l0Y2ggKHN1cHBvcnQpCiAgICAgewogICAg ICAgICBjYXNlIE1lZGlhUGxheWVyOjpJc05vdFN1cHBvcnRlZDoKLSAgICAgICAgICAgIGNhblBs YXkgPSBBU0NJSUxpdGVyYWwoIiIpOworICAgICAgICAgICAgY2FuUGxheSA9IGVtcHR5U3RyaW5n KCk7CiAgICAgICAgICAgICBicmVhazsKICAgICAgICAgY2FzZSBNZWRpYVBsYXllcjo6TWF5QmVT dXBwb3J0ZWQ6CiAgICAgICAgICAgICBjYW5QbGF5ID0gQVNDSUlMaXRlcmFsKCJtYXliZSIpOwo=