55189 2011-02-24 15:36:16 -0800 Add a USE() macro to control use of the built-in UTF8 codec 2011-03-16 17:32:19 -0700 1 1 1 Unclassified WebKit New Bugs 528+ (Nightly build) Other OS X 10.5 RESOLVED FIXED P2 Normal --- 1 jamesr jamesr abarth ap darin dglazkov fishd oldest_to_newest 357458 0 jamesr 2011-02-24 15:36:16 -0800 Add a USE() macro to control use of the built-in UTF8 codec 357463 1 83733 jamesr 2011-02-24 15:38:08 -0800 Created attachment 83733 Patch 357466 2 jamesr 2011-02-24 15:39:22 -0800 The new builtin UTF8 codec should default to on for trunk, but it's still too crashy to use in releases. This patch adds a USE() macro to control it so we don't have to go through crazy contortions on release branches. Once the new code is stabilized and has sufficient test coverage we can remove this. 357468 3 83733 darin 2011-02-24 15:39:23 -0800 Comment on attachment 83733 Patch What’s the motivation here? Could you explain why Chrome doesn’t want to use the codec. This seems like a terrible idea to me, and not just because I wrote the new codec. 357469 4 jamesr 2011-02-24 15:39:40 -0800 (In reply to comment #3) > (From update of attachment 83733 [details]) > What’s the motivation here? Could you explain why Chrome doesn’t want to use the codec. This seems like a terrible idea to me, and not just because I wrote the new codec. We do, but not yet. It's still too crashy. 357472 5 darin 2011-02-24 15:40:00 -0800 (In reply to comment #2) > The new builtin UTF8 codec should default to on for trunk, but it's still too crashy to use in releases. What are the bug numbers for the crashes? I don’t know of any right now. 357480 6 jamesr 2011-02-24 15:43:38 -0800 (In reply to comment #5) > (In reply to comment #2) > > The new builtin UTF8 codec should default to on for trunk, but it's still too crashy to use in releases. > > What are the bug numbers for the crashes? I don’t know of any right now. https://bugs.webkit.org/show_bug.cgi?id=54862#c19 says that the latest patch is still crashing on some platforms, and as far as I can tell no regression tests have been added for the crashes found so far. I understand that you are still working on this code, but want to make sure that your bugfixes aren't tied to any port's release schedule as that's awkward and can lead to bad decisions. There should be no change in behavior for any port (assuming no bugs in ICU or the builtin code) so from a platform perspective this should be a neutral change. 357489 7 darin 2011-02-24 15:48:01 -0800 (In reply to comment #6) > (In reply to comment #5) > > What are the bug numbers for the crashes? I don’t know of any right now. > > https://bugs.webkit.org/show_bug.cgi?id=54862#c19 says that the latest patch is still crashing on some platforms That confusing comment is about an assertion failure, covered by bug 55135. I fixed it this morning by removing the assertion but it’s still waiting in the commit queue. > and as far as I can tell no regression tests have been added for the crashes found so far. Yes, we should add the tests, but I don’t think the lack of regression tests is a significant factor in this decision. > I understand that you are still working on this code I’m not. I consider it done. This patch is an overreaction. Would you be willing to wait a couple days to see if we find any crashes? 357501 8 jamesr 2011-02-24 15:55:03 -0800 (In reply to comment #7) > (In reply to comment #6) > > (In reply to comment #5) > > > What are the bug numbers for the crashes? I don’t know of any right now. > > > > https://bugs.webkit.org/show_bug.cgi?id=54862#c19 says that the latest patch is still crashing on some platforms > > That confusing comment is about an assertion failure, covered by bug 55135. I fixed it this morning by removing the assertion but it’s still waiting in the commit queue. > > > and as far as I can tell no regression tests have been added for the crashes found so far. > > Yes, we should add the tests, but I don’t think the lack of regression tests is a significant factor in this decision. > > > I understand that you are still working on this code > > I’m not. I consider it done. > > This patch is an overreaction. Would you be willing to wait a couple days to see if we find any crashes? I intend to leave the builtin UTF8 codec on by default in trunk. For release branches, though, it needs a little bake time before shipping and it's too difficult to disable it on a branch right now. Normally new code is developed behind a macro and then turned on when it seems to me that this patch is moving closer to the usual WebKit development pattern. 357504 9 darin 2011-02-24 15:58:11 -0800 (In reply to comment #8) > Normally new code is developed behind a macro There’s a lot of new code that is not done behind a macro, but I agree that major efforts usually do use one. If you really want this, it’s OK with me. But I think you’re wrong about the stage this code is at. I think it’s highly likely we are done with the new UTF-8 codec and there won’t be any further changes. Please do remove this conditional soon once you see that we aren’t going to need it. 357519 10 jamesr 2011-02-24 16:00:56 -0800 Thanks. I hope we don't need this flexibility either but would rather have the option and not need it than need the option and not have it :) 357523 11 jamesr 2011-02-24 16:04:20 -0800 Committed r79639: <http://trac.webkit.org/changeset/79639> 367200 12 darin 2011-03-14 13:58:09 -0700 It’s three weeks later and there have been no bug reports related to the UTF-8 decoder. Is it time to take this out now? 368644 13 abarth 2011-03-16 16:50:38 -0700 (In reply to comment #12) > It’s three weeks later and there have been no bug reports related to the UTF-8 decoder. Is it time to take this out now? Hearing no objections, I'll post a patch removing the flag today. 368645 14 jamesr 2011-03-16 16:51:47 -0700 Sounds good to me! 368675 15 abarth 2011-03-16 17:32:19 -0700 Removal in https://bugs.webkit.org/show_bug.cgi?id=56508 83733 2011-02-24 15:38:08 -0800 2011-02-24 15:41:16 -0800 Patch bug-55189-20110224153807.patch text/plain 5272 jamesr U3VidmVyc2lvbiBSZXZpc2lvbjogNzk2MjMKZGlmZiAtLWdpdCBhL1NvdXJjZS9KYXZhU2NyaXB0 Q29yZS9DaGFuZ2VMb2cgYi9Tb3VyY2UvSmF2YVNjcmlwdENvcmUvQ2hhbmdlTG9nCmluZGV4IGY5 MzMyZjRjYjhlYjkxOWNjOWJlNWQ0M2RlNDEyMjI4NzViYTE1ZjAuLjY2NDZjZTg2MDQyOTg3NWZl N2E2NzU5ZGVkMzkyYzNhZjhkZDY1NjIgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9KYXZhU2NyaXB0Q29y ZS9DaGFuZ2VMb2cKKysrIGIvU291cmNlL0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwpAQCAtMSwz ICsxLDE0IEBACisyMDExLTAyLTI0ICBKYW1lcyBSb2JpbnNvbiAgPGphbWVzckBjaHJvbWl1bS5v cmc+CisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgQWRk IGEgVVNFKCkgbWFjcm8gdG8gY29udHJvbCB1c2Ugb2YgdGhlIGJ1aWx0LWluIFVURjggY29kZWMK KyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTU1MTg5CisK KyAgICAgICAgRGVmYXVsdHMgVVNFKEJVSUxUSU5fVVRGOF9DT0RFQykgdG8gdHJ1ZSBmb3IgYWxs IHBsYXRmb3JtcyBleGNlcHQgY2hyb21pdW0sIHdoaWNoIGNvbnRyb2xzIHRoZSBmbGFnIHZpYSBm ZWF0dXJlcy5neXBpLgorCisgICAgICAgICogd3RmL1BsYXRmb3JtLmg6CisKIDIwMTEtMDItMjMg IE9saXZlciBIdW50ICA8b2xpdmVyQGFwcGxlLmNvbT4KIAogICAgICAgICBSZXZpZXdlZCBieSBH ZW9mZnJleSBHYXJlbi4KZGlmZiAtLWdpdCBhL1NvdXJjZS9KYXZhU2NyaXB0Q29yZS93dGYvUGxh dGZvcm0uaCBiL1NvdXJjZS9KYXZhU2NyaXB0Q29yZS93dGYvUGxhdGZvcm0uaAppbmRleCA1MGVm NzNiOTdmNDY5NGI4MmIwNTkyMDY3ZTEwNTgwYTJlMjllY2M1Li4zNDI2MDA0YjM3Y2QzODMwMWVj OTRjOTU5MDVkNGE0MTU2YmU5ODM4IDEwMDY0NAotLS0gYS9Tb3VyY2UvSmF2YVNjcmlwdENvcmUv d3RmL1BsYXRmb3JtLmgKKysrIGIvU291cmNlL0phdmFTY3JpcHRDb3JlL3d0Zi9QbGF0Zm9ybS5o CkBAIC01OTYsNiArNTk2LDEwIEBACiAjZGVmaW5lIFdURl9VU0VfSUNVX1VOSUNPREUgMQogI2Vu ZGlmCiAKKyNpZiAhUExBVEZPUk0oQ0hST01JVU0pIC8qIENocm9taXVtIGNvbnRyb2xzIHRoaXMg bWFjcm8gd2l0aCBhIGd5cCBkZWZpbmUgKi8KKyNkZWZpbmUgV1RGX1VTRV9CVUlMVElOX1VURjhf Q09ERUMgMQorI2VuZGlmCisKICNpZiBQTEFURk9STShNQUMpICYmICFQTEFURk9STShJT1MpCiAj aWYgIWRlZmluZWQoQlVJTERJTkdfT05fTEVPUEFSRCkgJiYgIWRlZmluZWQoQlVJTERJTkdfT05f VElHRVIpICYmIENQVShYODZfNjQpCiAjZGVmaW5lIFdURl9VU0VfUExVR0lOX0hPU1RfUFJPQ0VT UyAxCmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cgYi9Tb3VyY2UvV2ViQ29y ZS9DaGFuZ2VMb2cKaW5kZXggYTEyM2E5N2E3YTNiMWFkNjc2ZjU4OGEwMTMzYTFjNmJlMTg1ODY1 Ni4uMGFlYTFiMjdkMDZjNTI0ZGUxOWYzOTEzOTNkYWU2YmU4ODE1YmMwNSAxMDA2NDQKLS0tIGEv U291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCisrKyBiL1NvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZwpA QCAtMSw1ICsxLDE5IEBACiAyMDExLTAyLTI0ICBKYW1lcyBSb2JpbnNvbiAgPGphbWVzckBjaHJv bWl1bS5vcmc+CiAKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAg ICAgQWRkIGEgVVNFKCkgbWFjcm8gdG8gY29udHJvbCB1c2Ugb2YgdGhlIGJ1aWx0LWluIFVURjgg Y29kZWMKKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTU1 MTg5CisKKyAgICAgICAgR3VhcmRzIHRoZSBidWlsdCBpbiBVVEY4IGNvZGVjIHJlZ2lzdHJhdGlv biB3aXRoIFVTRShCVUlMVElOX1VURjhfQ09ERUMpLiBJQ1UgaXMgdXNlZCBpZiB0aGUgVVNFKCkg aXMgbm90IHNldC4KKworICAgICAgICAqIHBsYXRmb3JtL3RleHQvVGV4dENvZGVjSUNVLmNwcDoK KyAgICAgICAgKFdlYkNvcmU6OlRleHRDb2RlY0lDVTo6cmVnaXN0ZXJFbmNvZGluZ05hbWVzKToK KyAgICAgICAgKiBwbGF0Zm9ybS90ZXh0L1RleHRFbmNvZGluZ1JlZ2lzdHJ5LmNwcDoKKyAgICAg ICAgKFdlYkNvcmU6OmJ1aWxkQmFzZVRleHRDb2RlY01hcHMpOgorCisyMDExLTAyLTI0ICBKYW1l cyBSb2JpbnNvbiAgPGphbWVzckBjaHJvbWl1bS5vcmc+CisKICAgICAgICAgVW5yZXZpZXdlZCwg cm9sbGluZyBvdXQgcjc5NjA0LgogICAgICAgICBodHRwOi8vdHJhYy53ZWJraXQub3JnL2NoYW5n ZXNldC83OTYwNAogICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/ aWQ9NTUwMTcKZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJDb3JlL3BsYXRmb3JtL3RleHQvVGV4dENv ZGVjSUNVLmNwcCBiL1NvdXJjZS9XZWJDb3JlL3BsYXRmb3JtL3RleHQvVGV4dENvZGVjSUNVLmNw cAppbmRleCAwZDk5MTk2ZDJjYWQ3YWI0ZWM0MTM5YjcyNjlhYTFlNmFjOGQwODczLi5kMDdiNTBl ZDk2ZjU1NDlkZWEwNDgzM2JiMmY2NTNhZjMwMjNmNzgzIDEwMDY0NAotLS0gYS9Tb3VyY2UvV2Vi Q29yZS9wbGF0Zm9ybS90ZXh0L1RleHRDb2RlY0lDVS5jcHAKKysrIGIvU291cmNlL1dlYkNvcmUv cGxhdGZvcm0vdGV4dC9UZXh0Q29kZWNJQ1UuY3BwCkBAIC0xNDksNiArMTQ5LDEzIEBAIHZvaWQg VGV4dENvZGVjSUNVOjpyZWdpc3RlckVuY29kaW5nTmFtZXMoRW5jb2RpbmdOYW1lUmVnaXN0cmFy IHJlZ2lzdHJhcikKICAgICByZWdpc3RyYXIoIngtd2luZG93cy05NDkiLCAid2luZG93cy05NDki KTsKICAgICByZWdpc3RyYXIoIngtdWhjIiwgIndpbmRvd3MtOTQ5Iik7CiAgICAgcmVnaXN0cmFy KCJzaGlmdC1qaXMiLCAiU2hpZnRfSklTIik7CisjaWYgIVVTRShCVUlMVElOX1VURjhfQ09ERUMp CisgICAgcmVnaXN0cmFyKCJ1bmljb2RlMTF1dGY4IiwgIlVURi04Iik7CisgICAgcmVnaXN0cmFy KCJ1bmljb2RlMjB1dGY4IiwgIlVURi04Iik7CisgICAgcmVnaXN0cmFyKCJ4LXVuaWNvZGUyMHV0 ZjgiLCAiVVRGLTgiKTsKKyAgICByZWdpc3RyYXIoInV0ZjgiLCAiVVRGLTgiKTsKKyAgICByZWdp c3RyYXIoIlVURi04IiwgIlVURi04Iik7CisjZW5kaWYKIAogICAgIC8vIFRoZXNlIGFsaWFzZXMg YXJlIHByZXNlbnQgaW4gbW9kZXJuIHZlcnNpb25zIG9mIElDVSwgYnV0IHVzZSBkaWZmZXJlbnQg Y29kZWNzLCBhbmQgaGF2ZSBubyBzdGFuZGFyZCBuYW1lcy4KICAgICAvLyBUaGV5IGFyZSBub3Qg cHJlc2VudCBpbiBJQ1UgMy4yLgpkaWZmIC0tZ2l0IGEvU291cmNlL1dlYkNvcmUvcGxhdGZvcm0v dGV4dC9UZXh0RW5jb2RpbmdSZWdpc3RyeS5jcHAgYi9Tb3VyY2UvV2ViQ29yZS9wbGF0Zm9ybS90 ZXh0L1RleHRFbmNvZGluZ1JlZ2lzdHJ5LmNwcAppbmRleCAwOWUwNWJmZTI3MWRhOGVjYjA2ZWQz NjNiNjYzMDIzMzVkZTliZDI4Li4xZDM4YzcxZmYzMmI4YmJlZDdmNjdiM2Y1YTQ2Yzg5YmY0Mzcz ZTc5IDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViQ29yZS9wbGF0Zm9ybS90ZXh0L1RleHRFbmNvZGlu Z1JlZ2lzdHJ5LmNwcAorKysgYi9Tb3VyY2UvV2ViQ29yZS9wbGF0Zm9ybS90ZXh0L1RleHRFbmNv ZGluZ1JlZ2lzdHJ5LmNwcApAQCAtMjIzLDggKzIyMywxMCBAQCBzdGF0aWMgdm9pZCBidWlsZEJh c2VUZXh0Q29kZWNNYXBzKCkKICAgICBUZXh0Q29kZWNMYXRpbjE6OnJlZ2lzdGVyRW5jb2RpbmdO YW1lcyhhZGRUb1RleHRFbmNvZGluZ05hbWVNYXApOwogICAgIFRleHRDb2RlY0xhdGluMTo6cmVn aXN0ZXJDb2RlY3MoYWRkVG9UZXh0Q29kZWNNYXApOwogCisjaWYgVVNFKEJVSUxUSU5fVVRGOF9D T0RFQykKICAgICBUZXh0Q29kZWNVVEY4OjpyZWdpc3RlckVuY29kaW5nTmFtZXMoYWRkVG9UZXh0 RW5jb2RpbmdOYW1lTWFwKTsKICAgICBUZXh0Q29kZWNVVEY4OjpyZWdpc3RlckNvZGVjcyhhZGRU b1RleHRDb2RlY01hcCk7CisjZW5kaWYKIAogICAgIFRleHRDb2RlY1VURjE2OjpyZWdpc3RlckVu Y29kaW5nTmFtZXMoYWRkVG9UZXh0RW5jb2RpbmdOYW1lTWFwKTsKICAgICBUZXh0Q29kZWNVVEYx Njo6cmVnaXN0ZXJDb2RlY3MoYWRkVG9UZXh0Q29kZWNNYXApOwpkaWZmIC0tZ2l0IGEvU291cmNl L1dlYktpdC9jaHJvbWl1bS9DaGFuZ2VMb2cgYi9Tb3VyY2UvV2ViS2l0L2Nocm9taXVtL0NoYW5n ZUxvZwppbmRleCBhZTU3MjIwZjliYjg5MGU0ODQzMGQzZTNlOTEyNjFhOWRhZTFkOWY2Li45MThj OTNkMWI3MTUzNWQyYTYwZmFkMzZhODgzNzUzODgzNjM0MDk2IDEwMDY0NAotLS0gYS9Tb3VyY2Uv V2ViS2l0L2Nocm9taXVtL0NoYW5nZUxvZworKysgYi9Tb3VyY2UvV2ViS2l0L2Nocm9taXVtL0No YW5nZUxvZwpAQCAtMSwzICsxLDE0IEBACisyMDExLTAyLTI0ICBKYW1lcyBSb2JpbnNvbiAgPGph bWVzckBjaHJvbWl1bS5vcmc+CisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISku CisKKyAgICAgICAgQWRkIGEgVVNFKCkgbWFjcm8gdG8gY29udHJvbCB1c2Ugb2YgdGhlIGJ1aWx0 LWluIFVURjggY29kZWMKKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcu Y2dpP2lkPTU1MTg5CisKKyAgICAgICAgRGVmYXVsdHMgVVNFKEJVSUxUSU5fVVRGOF9DT0RFQykg dG8gdHJ1ZSBmb3IgdGhlIENocm9taXVtIHBvcnQuCisKKyAgICAgICAgKiBmZWF0dXJlcy5neXBp OgorCiAyMDExLTAyLTI0ICBKYW1lcyBLb3ppYW5za2kgIDxrb3pAY2hyb21pdW0ub3JnPgogCiAg ICAgICAgIFJldmlld2VkIGJ5IE9qYW4gVmFmYWkuCmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViS2l0 L2Nocm9taXVtL2ZlYXR1cmVzLmd5cGkgYi9Tb3VyY2UvV2ViS2l0L2Nocm9taXVtL2ZlYXR1cmVz Lmd5cGkKaW5kZXggODk0NGZkZTNiZGIyYmQwOTJjNGYwMTA2NDE3YjgyZjY4YTVjOTlkMy4uZDQ2 MDE3MmVjNTkxYTA2NWYxZTgxYjdiMjhmNGI0MWY0ODBiNGVhNyAxMDA2NDQKLS0tIGEvU291cmNl L1dlYktpdC9jaHJvbWl1bS9mZWF0dXJlcy5neXBpCisrKyBiL1NvdXJjZS9XZWJLaXQvY2hyb21p dW0vZmVhdHVyZXMuZ3lwaQpAQCAtOTAsNiArOTAsNyBAQAogICAgICAgICAnRU5BQkxFX1hQQVRI PTEnLAogICAgICAgICAnRU5BQkxFX1hTTFQ9MScsCiAgICAgICAgICdXVEZfVVNFX0FDQ0VMRVJB VEVEX0NPTVBPU0lUSU5HPTEnLAorICAgICAgICAnV1RGX1VTRV9CVUlMVElOX1VURjhfQ09ERUM9 MScsCiAgICAgICAgICdXVEZfVVNFX1dFQlA9MScsCiAgICAgICAgICdXVEZfVVNFX1dFQktJVF9J TUFHRV9ERUNPREVSUz0xJywKICAgICAgIF0sCg==