82793 2012-03-30 15:42:59 -0700 Add a compile assert for the size of BidiContext 2012-03-30 21:43:49 -0700 1 1 1 Unclassified WebKit New Bugs 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED P2 Normal --- 1 rniwa rniwa darin eric krit leviw mitz oldest_to_newest 592574 0 rniwa 2012-03-30 15:42:59 -0700 Add a compile assert for the size of BidiContext 592580 1 134903 rniwa 2012-03-30 15:47:22 -0700 Created attachment 134903 Patch 592581 2 rniwa 2012-03-30 15:48:13 -0700 Unfortunately, very few people can review changes to this file :( 592589 3 134903 eric 2012-03-30 15:55:26 -0700 Comment on attachment 134903 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=134903&action=review > Source/WebCore/platform/text/BidiContext.h:62 > - unsigned char m_level; > + unsigned m_level : 6; You should add a comment which explains that we only need to handle 64 bidi-levels. Do we have tests for overflow of this value? 592593 4 rniwa 2012-03-30 15:56:30 -0700 (In reply to comment #3) > (From update of attachment 134903 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=134903&action=review > > > Source/WebCore/platform/text/BidiContext.h:62 > > - unsigned char m_level; > > + unsigned m_level : 6; > > You should add a comment which explains that we only need to handle 64 bidi-levels. Do we have tests for overflow of this value? Maybe we should define numBidiLevelBits? 592627 5 134910 rniwa 2012-03-30 16:11:34 -0700 Created attachment 134910 Patch 592629 6 134910 eric 2012-03-30 16:17:44 -0700 Comment on attachment 134910 Patch OK. LGTM. 592640 7 rniwa 2012-03-30 16:27:56 -0700 Committed r112742: <http://trac.webkit.org/changeset/112742> 592711 8 134910 darin 2012-03-30 18:25:18 -0700 Comment on attachment 134910 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=134910&action=review > Source/WebCore/platform/text/BidiContext.cpp:31 > + uint32_t bitfields : 16; Tiny style question. Why use uint32_t here instead of unsigned? 592793 9 rniwa 2012-03-30 21:43:49 -0700 (In reply to comment #8) > (From update of attachment 134910 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=134910&action=review > > > Source/WebCore/platform/text/BidiContext.cpp:31 > > + uint32_t bitfields : 16; > > Tiny style question. Why use uint32_t here instead of unsigned? I was just following the pattern used elsewhere. We can change all of them to use unsigned instead. 134903 2012-03-30 15:47:22 -0700 2012-03-30 16:11:31 -0700 Patch bug-82793-20120330154721.patch text/plain 2338 rniwa SW5kZXg6IFNvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2Vi Q29yZS9DaGFuZ2VMb2cJKHJldmlzaW9uIDExMjczNikKKysrIFNvdXJjZS9XZWJDb3JlL0NoYW5n ZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDE5IEBACisyMDEyLTAzLTMwICBSeW9zdWtl IE5pd2EgIDxybml3YUB3ZWJraXQub3JnPgorCisgICAgICAgIEFkZCBhIGNvbXBpbGUgYXNzZXJ0 IGZvciB0aGUgc2l6ZSBvZiBCaWRpQ29udGV4dAorICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0 Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9ODI3OTMKKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkg KE9PUFMhKS4KKworICAgICAgICBBZGRlZCB0aGUgYXNzZXJ0aW9uLiBBbHNvIHJlZHVjZWQgdGhl IG51bWJlciBvZiBiaXRzIHVzZWQgZm9yIGJpZGkgbGV2ZWxzIGZyb20KKyAgICAgICAgOCB0byA2 IGFzIGRvbmUgaW4gSW5saW5lQm94IHNpbmNlIGJpZGkgbGV2ZWxzIHJlcXVpcmUgZXhhY3RseSA2 IGJpdHMuCisKKyAgICAgICAgKiBwbGF0Zm9ybS90ZXh0L0JpZGlDb250ZXh0LmNwcDoKKyAgICAg ICAgKFNhbWVTaXplQXNCaWRpQ29udGV4dCk6CisgICAgICAgIChXZWJDb3JlKToKKyAgICAgICAg KiBwbGF0Zm9ybS90ZXh0L0JpZGlDb250ZXh0Lmg6CisgICAgICAgIChCaWRpQ29udGV4dCk6CisK IDIwMTItMDMtMzAgIFJ5b3N1a2UgTml3YSAgPHJuaXdhQHdlYmtpdC5vcmc+CiAKICAgICAgICAg QWRkIGEgY29tcGlsZSBhc3NlcnQgZm9yIHRoZSBzaXplIG9mIEZvbnREZXNjcmlwdGlvbgpJbmRl eDogU291cmNlL1dlYkNvcmUvcGxhdGZvcm0vdGV4dC9CaWRpQ29udGV4dC5jcHAKPT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PQotLS0gU291cmNlL1dlYkNvcmUvcGxhdGZvcm0vdGV4dC9CaWRpQ29udGV4dC5jcHAJKHJldmlz aW9uIDExMjcyMykKKysrIFNvdXJjZS9XZWJDb3JlL3BsYXRmb3JtL3RleHQvQmlkaUNvbnRleHQu Y3BwCSh3b3JraW5nIGNvcHkpCkBAIC0yNyw2ICsyNywxMyBAQCBuYW1lc3BhY2UgV2ViQ29yZSB7 CiAKIHVzaW5nIG5hbWVzcGFjZSBXVEY6OlVuaWNvZGU7CiAKK3N0cnVjdCBTYW1lU2l6ZUFzQmlk aUNvbnRleHQgOiBwdWJsaWMgUmVmQ291bnRlZDxTYW1lU2l6ZUFzQmlkaUNvbnRleHQ+IHsKKyAg ICB1aW50MzJfdCBiaXRmaWVsZHMgOiAxNjsKKyAgICB2b2lkKiBwYXJlbnQ7Cit9OworCitDT01Q SUxFX0FTU0VSVChzaXplb2YoQmlkaUNvbnRleHQpID09IHNpemVvZihTYW1lU2l6ZUFzQmlkaUNv bnRleHQpLCBCaWRpQ29udGV4dF9zaG91bGRfc3RheV9zbWFsbCk7CisKIGlubGluZSBQYXNzUmVm UHRyPEJpZGlDb250ZXh0PiBCaWRpQ29udGV4dDo6Y3JlYXRlVW5jYWNoZWQodW5zaWduZWQgY2hh ciBsZXZlbCwgRGlyZWN0aW9uIGRpcmVjdGlvbiwgYm9vbCBvdmVycmlkZSwgQmlkaUVtYmVkZGlu Z1NvdXJjZSBzb3VyY2UsIEJpZGlDb250ZXh0KiBwYXJlbnQpCiB7CiAgICAgcmV0dXJuIGFkb3B0 UmVmKG5ldyBCaWRpQ29udGV4dChsZXZlbCwgZGlyZWN0aW9uLCBvdmVycmlkZSwgc291cmNlLCBw YXJlbnQpKTsKSW5kZXg6IFNvdXJjZS9XZWJDb3JlL3BsYXRmb3JtL3RleHQvQmlkaUNvbnRleHQu aAo9PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2ViQ29yZS9wbGF0Zm9ybS90ZXh0L0JpZGlDb250ZXh0 LmgJKHJldmlzaW9uIDExMjcyMykKKysrIFNvdXJjZS9XZWJDb3JlL3BsYXRmb3JtL3RleHQvQmlk aUNvbnRleHQuaAkod29ya2luZyBjb3B5KQpAQCAtNTksOSArNTksOSBAQCBwcml2YXRlOgogCiAg ICAgc3RhdGljIFBhc3NSZWZQdHI8QmlkaUNvbnRleHQ+IGNyZWF0ZVVuY2FjaGVkKHVuc2lnbmVk IGNoYXIgbGV2ZWwsIFdURjo6VW5pY29kZTo6RGlyZWN0aW9uLCBib29sIG92ZXJyaWRlLCBCaWRp RW1iZWRkaW5nU291cmNlLCBCaWRpQ29udGV4dCogcGFyZW50KTsKIAotICAgIHVuc2lnbmVkIGNo YXIgbV9sZXZlbDsKKyAgICB1bnNpZ25lZCBtX2xldmVsIDogNjsKICAgICB1bnNpZ25lZCBtX2Rp cmVjdGlvbiA6IDU7IC8vIERpcmVjdGlvbgotICAgIGJvb2wgbV9vdmVycmlkZSA6IDE7CisgICAg dW5zaWduZWQgbV9vdmVycmlkZSA6IDE7CiAgICAgdW5zaWduZWQgbV9zb3VyY2UgOiAxOyAvLyBC aWRpRW1iZWRkaW5nU291cmNlCiAgICAgUmVmUHRyPEJpZGlDb250ZXh0PiBtX3BhcmVudDsKIH07 Cg== 134910 2012-03-30 16:11:34 -0700 2012-03-30 18:25:18 -0700 Patch bug-82793-20120330161133.patch text/plain 3139 rniwa SW5kZXg6IFNvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2Vi Q29yZS9DaGFuZ2VMb2cJKHJldmlzaW9uIDExMjczNikKKysrIFNvdXJjZS9XZWJDb3JlL0NoYW5n ZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDIwIEBACisyMDEyLTAzLTMwICBSeW9zdWtl IE5pd2EgIDxybml3YUB3ZWJraXQub3JnPgorCisgICAgICAgIEFkZCBhIGNvbXBpbGUgYXNzZXJ0 IGZvciB0aGUgc2l6ZSBvZiBCaWRpQ29udGV4dAorICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0 Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9ODI3OTMKKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkg KE9PUFMhKS4KKworICAgICAgICBBZGRlZCB0aGUgYXNzZXJ0aW9uLiBBbHNvIHJlZHVjZWQgdGhl IG51bWJlciBvZiBiaXRzIHVzZWQgZm9yIGJpZGkgbGV2ZWxzIGZyb20KKyAgICAgICAgOCB0byA2 IGFzIGRvbmUgaW4gSW5saW5lQm94IHNpbmNlIGJpZGkgbGV2ZWxzIHJlcXVpcmUgZXhhY3RseSA2 IGJpdHMuCisKKyAgICAgICAgKiByZW5kZXJpbmcvSW5saW5lQm94Lmg6IEFkZGVkIGEgY29tbWVu dCBhYm91dCB3aHkgYmlkaSBsZXZlbCBuZWVkcyBleGFjdGx5IDYgYml0cy4KKyAgICAgICAgKiBw bGF0Zm9ybS90ZXh0L0JpZGlDb250ZXh0LmNwcDoKKyAgICAgICAgKFNhbWVTaXplQXNCaWRpQ29u dGV4dCk6CisgICAgICAgIChXZWJDb3JlKToKKyAgICAgICAgKiBwbGF0Zm9ybS90ZXh0L0JpZGlD b250ZXh0Lmg6CisgICAgICAgIChCaWRpQ29udGV4dCk6CisKIDIwMTItMDMtMzAgIFJ5b3N1a2Ug Tml3YSAgPHJuaXdhQHdlYmtpdC5vcmc+CiAKICAgICAgICAgQWRkIGEgY29tcGlsZSBhc3NlcnQg Zm9yIHRoZSBzaXplIG9mIEZvbnREZXNjcmlwdGlvbgpJbmRleDogU291cmNlL1dlYkNvcmUvcGxh dGZvcm0vdGV4dC9CaWRpQ29udGV4dC5jcHAKPT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gU291cmNlL1dlYkNvcmUv cGxhdGZvcm0vdGV4dC9CaWRpQ29udGV4dC5jcHAJKHJldmlzaW9uIDExMjcyMykKKysrIFNvdXJj ZS9XZWJDb3JlL3BsYXRmb3JtL3RleHQvQmlkaUNvbnRleHQuY3BwCSh3b3JraW5nIGNvcHkpCkBA IC0yNyw2ICsyNywxMyBAQCBuYW1lc3BhY2UgV2ViQ29yZSB7CiAKIHVzaW5nIG5hbWVzcGFjZSBX VEY6OlVuaWNvZGU7CiAKK3N0cnVjdCBTYW1lU2l6ZUFzQmlkaUNvbnRleHQgOiBwdWJsaWMgUmVm Q291bnRlZDxTYW1lU2l6ZUFzQmlkaUNvbnRleHQ+IHsKKyAgICB1aW50MzJfdCBiaXRmaWVsZHMg OiAxNjsKKyAgICB2b2lkKiBwYXJlbnQ7Cit9OworCitDT01QSUxFX0FTU0VSVChzaXplb2YoQmlk aUNvbnRleHQpID09IHNpemVvZihTYW1lU2l6ZUFzQmlkaUNvbnRleHQpLCBCaWRpQ29udGV4dF9z aG91bGRfc3RheV9zbWFsbCk7CisKIGlubGluZSBQYXNzUmVmUHRyPEJpZGlDb250ZXh0PiBCaWRp Q29udGV4dDo6Y3JlYXRlVW5jYWNoZWQodW5zaWduZWQgY2hhciBsZXZlbCwgRGlyZWN0aW9uIGRp cmVjdGlvbiwgYm9vbCBvdmVycmlkZSwgQmlkaUVtYmVkZGluZ1NvdXJjZSBzb3VyY2UsIEJpZGlD b250ZXh0KiBwYXJlbnQpCiB7CiAgICAgcmV0dXJuIGFkb3B0UmVmKG5ldyBCaWRpQ29udGV4dChs ZXZlbCwgZGlyZWN0aW9uLCBvdmVycmlkZSwgc291cmNlLCBwYXJlbnQpKTsKSW5kZXg6IFNvdXJj ZS9XZWJDb3JlL3BsYXRmb3JtL3RleHQvQmlkaUNvbnRleHQuaAo9PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3Vy Y2UvV2ViQ29yZS9wbGF0Zm9ybS90ZXh0L0JpZGlDb250ZXh0LmgJKHJldmlzaW9uIDExMjcyMykK KysrIFNvdXJjZS9XZWJDb3JlL3BsYXRmb3JtL3RleHQvQmlkaUNvbnRleHQuaAkod29ya2luZyBj b3B5KQpAQCAtNTksOSArNTksOSBAQCBwcml2YXRlOgogCiAgICAgc3RhdGljIFBhc3NSZWZQdHI8 QmlkaUNvbnRleHQ+IGNyZWF0ZVVuY2FjaGVkKHVuc2lnbmVkIGNoYXIgbGV2ZWwsIFdURjo6VW5p Y29kZTo6RGlyZWN0aW9uLCBib29sIG92ZXJyaWRlLCBCaWRpRW1iZWRkaW5nU291cmNlLCBCaWRp Q29udGV4dCogcGFyZW50KTsKIAotICAgIHVuc2lnbmVkIGNoYXIgbV9sZXZlbDsKKyAgICB1bnNp Z25lZCBtX2xldmVsIDogNjsgLy8gVGhlIG1heGltaXVtIGJpZGkgbGV2ZWwgaXMgNjI6IGh0dHA6 Ly91bmljb2RlLm9yZy9yZXBvcnRzL3RyOS8jRXhwbGljaXRfTGV2ZWxzX2FuZF9EaXJlY3Rpb25z CiAgICAgdW5zaWduZWQgbV9kaXJlY3Rpb24gOiA1OyAvLyBEaXJlY3Rpb24KLSAgICBib29sIG1f b3ZlcnJpZGUgOiAxOworICAgIHVuc2lnbmVkIG1fb3ZlcnJpZGUgOiAxOwogICAgIHVuc2lnbmVk IG1fc291cmNlIDogMTsgLy8gQmlkaUVtYmVkZGluZ1NvdXJjZQogICAgIFJlZlB0cjxCaWRpQ29u dGV4dD4gbV9wYXJlbnQ7CiB9OwpJbmRleDogU291cmNlL1dlYkNvcmUvcmVuZGVyaW5nL0lubGlu ZUJveC5oCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT0KLS0tIFNvdXJjZS9XZWJDb3JlL3JlbmRlcmluZy9JbmxpbmVCb3gu aAkocmV2aXNpb24gMTEyNzIzKQorKysgU291cmNlL1dlYkNvcmUvcmVuZGVyaW5nL0lubGluZUJv eC5oCSh3b3JraW5nIGNvcHkpCkBAIC0zNDIsNyArMzQyLDcgQEAgcHVibGljOgogICAgICAgICBB RERfQk9PTEVBTl9CSVRGSUVMRChjb25zdHJ1Y3RlZCwgQ29uc3RydWN0ZWQpOwogCiAgICAgcHJp dmF0ZToKLSAgICAgICAgdW5zaWduZWQgbV9iaWRpRW1iZWRkaW5nTGV2ZWwgOiA2OworICAgICAg ICB1bnNpZ25lZCBtX2JpZGlFbWJlZGRpbmdMZXZlbCA6IDY7IC8vIFRoZSBtYXhpbWl1bSBiaWRp IGxldmVsIGlzIDYyOiBodHRwOi8vdW5pY29kZS5vcmcvcmVwb3J0cy90cjkvI0V4cGxpY2l0X0xl dmVsc19hbmRfRGlyZWN0aW9ucwogCiAgICAgcHVibGljOgogICAgICAgICB1bnNpZ25lZCBjaGFy IGJpZGlFbWJlZGRpbmdMZXZlbCgpIGNvbnN0IHsgcmV0dXJuIG1fYmlkaUVtYmVkZGluZ0xldmVs OyB9Cg==