62975 2011-06-20 03:18:33 -0700 Refactor local variable usage in CSSFontFaceSource::getFontData 2011-06-20 23:57:12 -0700 1 1 1 Unclassified WebKit CSS 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED P2 Minor --- 1 yuzo yuzo webkit.review.bot oldest_to_newest 423582 0 yuzo 2011-06-20 03:18:33 -0700 There are two issues in the local variable usages in CSSFontFaceSource::getFontData - There are two different local variables named "fontData". - One is of type OwnPtr<SimpleFontData> where SimpleFontData* would suffice. 423585 1 97775 yuzo 2011-06-20 03:25:09 -0700 Created attachment 97775 Patch 424185 2 97775 tkent 2011-06-20 19:16:39 -0700 Comment on attachment 97775 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=97775&action=review > Source/WebCore/css/CSSFontFaceSource.cpp:121 > - OwnPtr<SimpleFontData> fontData; > + SimpleFontData* fontData = 0; I don't think this change is reasonable. fontData.leakPtr() shows us that this 'fontData' object is not deleted by anyone. Your change will remove this intention. 424276 3 97933 yuzo 2011-06-20 23:02:55 -0700 Created attachment 97933 Patch 424277 4 yuzo 2011-06-20 23:04:10 -0700 Thank you for the review. I've reverted the second part. Can you take another look? 424285 5 97933 tkent 2011-06-20 23:06:39 -0700 Comment on attachment 97933 Patch ok 424306 6 97933 webkit.review.bot 2011-06-20 23:57:08 -0700 Comment on attachment 97933 Patch Clearing flags on attachment: 97933 Committed r89340: <http://trac.webkit.org/changeset/89340> 424307 7 webkit.review.bot 2011-06-20 23:57:12 -0700 All reviewed patches have been landed. Closing bug. 97775 2011-06-20 03:25:09 -0700 2011-06-20 23:02:50 -0700 Patch bug-62975-20110620032507.patch text/plain 4667 yuzo U3VidmVyc2lvbiBSZXZpc2lvbjogODkyMjgKZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJDb3JlL0No YW5nZUxvZyBiL1NvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZwppbmRleCAzYzZhOTQzYzdiZGQxMTJh YmU0M2U4ZTBiOTdiMTZhYjhjMjM5ZTFlLi5mNTA5NGY3YmMyODVjZDgzNTAzMWVmMzdlNzU0MmRi NDlhODEzMmM5IDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKKysrIGIvU291 cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCkBAIC0xLDMgKzEsMTUgQEAKKzIwMTEtMDYtMjAgIFl1em8g RnVqaXNoaW1hICA8eXV6b0Bnb29nbGUuY29tPgorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9E WSAoT09QUyEpLgorCisgICAgICAgIEZpeCBmb3IgQnVnIDYyOTc1IC0gUmVmYWN0b3IgbG9jYWwg dmFyaWFibGUgdXNhZ2VzIGluIENTU0ZvbnRGYWNlU291cmNlOjpnZXRGb250RGF0YQorICAgICAg ICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9NjI5NzUKKworICAgICAg ICBObyBuZXcgdGVzdHMgYmVjYXVzZSBubyBiZWhhdmlvcmFsIGNoYW5nZXMuCisKKyAgICAgICAg KiBjc3MvQ1NTRm9udEZhY2VTb3VyY2UuY3BwOgorICAgICAgICAoV2ViQ29yZTo6Q1NTRm9udEZh Y2VTb3VyY2U6OmdldEZvbnREYXRhKTogUmVtb3ZlIG9uZSBvZiBmb250RGF0YSB2YXJpYWJsZS4g Q2hhbmdlIHRoZSB0eXBlIG9mIHRoZSBvdGhlciBmb250RGF0YSB2YXJpYWJsZSBmcm9tIE93bmVk UHRyPFNpbXBsZUZvbnREYXRhPiB0byBTaW1wbGVGb250RGF0YSouCisKIDIwMTEtMDYtMTkgIEFk YW0gQmVyZ2t2aXN0ICA8YWRhbS5iZXJna3Zpc3RAZXJpY3Nzb24uY29tPgogCiAgICAgICAgIFJl dmlld2VkIGJ5IE1hcnRpbiBSb2JpbnNvbi4KZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJDb3JlL2Nz cy9DU1NGb250RmFjZVNvdXJjZS5jcHAgYi9Tb3VyY2UvV2ViQ29yZS9jc3MvQ1NTRm9udEZhY2VT b3VyY2UuY3BwCmluZGV4IDYyZmY1NmY1MDY3YmY1NGJkZjIyMjM1NTc5YjMyMDE5ZjI4YmRhYjMu LmNiNjhiNTZiZGE0NGFhN2IyY2I1YmViNTk1ZmJlZTYxMjBjNTg0MWYgMTAwNjQ0Ci0tLSBhL1Nv dXJjZS9XZWJDb3JlL2Nzcy9DU1NGb250RmFjZVNvdXJjZS5jcHAKKysrIGIvU291cmNlL1dlYkNv cmUvY3NzL0NTU0ZvbnRGYWNlU291cmNlLmNwcApAQCAtMTA4LDEwICsxMDgsOCBAQCBTaW1wbGVG b250RGF0YSogQ1NTRm9udEZhY2VTb3VyY2U6OmdldEZvbnREYXRhKGNvbnN0IEZvbnREZXNjcmlw dGlvbiYgZm9udERlc2NyaQogI2Vsc2UKICAgICBpZiAoIW1fZm9udCkgewogI2VuZGlmCi0gICAg ICAgIFNpbXBsZUZvbnREYXRhKiBmb250RGF0YSA9IGZvbnRDYWNoZSgpLT5nZXRDYWNoZWRGb250 RGF0YShmb250RGVzY3JpcHRpb24sIG1fc3RyaW5nKTsKLQogICAgICAgICAvLyBXZSdyZSBsb2Nh bC4gSnVzdCByZXR1cm4gYSBTaW1wbGVGb250RGF0YSBmcm9tIHRoZSBub3JtYWwgY2FjaGUuCi0g ICAgICAgIHJldHVybiBmb250RGF0YTsKKyAgICAgICAgcmV0dXJuIGZvbnRDYWNoZSgpLT5nZXRD YWNoZWRGb250RGF0YShmb250RGVzY3JpcHRpb24sIG1fc3RyaW5nKTsKICAgICB9CiAKICAgICAv LyBTZWUgaWYgd2UgaGF2ZSBhIG1hcHBpbmcgaW4gb3VyIEZvbnREYXRhIGNhY2hlLgpAQCAtMTIw LDcgKzExOCw3IEBAIFNpbXBsZUZvbnREYXRhKiBDU1NGb250RmFjZVNvdXJjZTo6Z2V0Rm9udERh dGEoY29uc3QgRm9udERlc2NyaXB0aW9uJiBmb250RGVzY3JpCiAgICAgaWYgKFNpbXBsZUZvbnRE YXRhKiBjYWNoZWREYXRhID0gbV9mb250RGF0YVRhYmxlLmdldChoYXNoS2V5KSkKICAgICAgICAg cmV0dXJuIGNhY2hlZERhdGE7CiAKLSAgICBPd25QdHI8U2ltcGxlRm9udERhdGE+IGZvbnREYXRh OworICAgIFNpbXBsZUZvbnREYXRhKiBmb250RGF0YSA9IDA7CiAKICAgICAvLyBJZiB3ZSBhcmUg c3RpbGwgbG9hZGluZywgdGhlbiB3ZSBsZXQgdGhlIHN5c3RlbSBwaWNrIGEgZm9udC4KICAgICBp ZiAoaXNMb2FkZWQoKSkgewpAQCAtMTU0LDcgKzE1Miw3IEBAIFNpbXBsZUZvbnREYXRhKiBDU1NG b250RmFjZVNvdXJjZTo6Z2V0Rm9udERhdGEoY29uc3QgRm9udERlc2NyaXB0aW9uJiBmb250RGVz Y3JpCiAgICAgICAgICAgICAgICAgICAgICAgICBtX3N2Z0ZvbnRGYWNlRWxlbWVudCA9IGZvbnRG YWNlRWxlbWVudDsKICAgICAgICAgICAgICAgICAgICAgfQogCi0gICAgICAgICAgICAgICAgICAg IGZvbnREYXRhID0gYWRvcHRQdHIobmV3IFNpbXBsZUZvbnREYXRhKFNWR0ZvbnREYXRhOjpjcmVh dGUoZm9udEZhY2VFbGVtZW50KSwgZm9udERlc2NyaXB0aW9uLmNvbXB1dGVkUGl4ZWxTaXplKCks IHN5bnRoZXRpY0JvbGQsIHN5bnRoZXRpY0l0YWxpYykpOworICAgICAgICAgICAgICAgICAgICBm b250RGF0YSA9IG5ldyBTaW1wbGVGb250RGF0YShTVkdGb250RGF0YTo6Y3JlYXRlKGZvbnRGYWNl RWxlbWVudCksIGZvbnREZXNjcmlwdGlvbi5jb21wdXRlZFBpeGVsU2l6ZSgpLCBzeW50aGV0aWNC b2xkLCBzeW50aGV0aWNJdGFsaWMpOwogICAgICAgICAgICAgICAgIH0KICAgICAgICAgICAgIH0g ZWxzZQogI2VuZGlmCkBAIC0xNjMsMTQgKzE2MSwxNCBAQCBTaW1wbGVGb250RGF0YSogQ1NTRm9u dEZhY2VTb3VyY2U6OmdldEZvbnREYXRhKGNvbnN0IEZvbnREZXNjcmlwdGlvbiYgZm9udERlc2Ny aQogICAgICAgICAgICAgICAgIGlmICghbV9mb250LT5lbnN1cmVDdXN0b21Gb250RGF0YSgpKQog ICAgICAgICAgICAgICAgICAgICByZXR1cm4gMDsKIAotICAgICAgICAgICAgICAgIGZvbnREYXRh ID0gYWRvcHRQdHIobmV3IFNpbXBsZUZvbnREYXRhKG1fZm9udC0+cGxhdGZvcm1EYXRhRnJvbUN1 c3RvbURhdGEoZm9udERlc2NyaXB0aW9uLmNvbXB1dGVkUGl4ZWxTaXplKCksIHN5bnRoZXRpY0Jv bGQsIHN5bnRoZXRpY0l0YWxpYywgZm9udERlc2NyaXB0aW9uLm9yaWVudGF0aW9uKCksCi0gICAg ICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAg ICAgICAgICAgICAgICAgICAgICAgIGZvbnREZXNjcmlwdGlvbi50ZXh0T3JpZW50YXRpb24oKSwg Zm9udERlc2NyaXB0aW9uLndpZHRoVmFyaWFudCgpLCBmb250RGVzY3JpcHRpb24ucmVuZGVyaW5n TW9kZSgpKSwgdHJ1ZSwgZmFsc2UpKTsKKyAgICAgICAgICAgICAgICBmb250RGF0YSA9IG5ldyBT aW1wbGVGb250RGF0YShtX2ZvbnQtPnBsYXRmb3JtRGF0YUZyb21DdXN0b21EYXRhKGZvbnREZXNj cmlwdGlvbi5jb21wdXRlZFBpeGVsU2l6ZSgpLCBzeW50aGV0aWNCb2xkLCBzeW50aGV0aWNJdGFs aWMsIGZvbnREZXNjcmlwdGlvbi5vcmllbnRhdGlvbigpLAorICAgICAgICAgICAgICAgICAgICAg ICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAg ICAgICBmb250RGVzY3JpcHRpb24udGV4dE9yaWVudGF0aW9uKCksIGZvbnREZXNjcmlwdGlvbi53 aWR0aFZhcmlhbnQoKSwgZm9udERlc2NyaXB0aW9uLnJlbmRlcmluZ01vZGUoKSksIHRydWUsIGZh bHNlKTsKICAgICAgICAgICAgIH0KICAgICAgICAgfSBlbHNlIHsKICNpZiBFTkFCTEUoU1ZHX0ZP TlRTKQogICAgICAgICAgICAgLy8gSW4tRG9jdW1lbnQgU1ZHIEZvbnRzCiAgICAgICAgICAgICBp ZiAobV9zdmdGb250RmFjZUVsZW1lbnQpCi0gICAgICAgICAgICAgICAgZm9udERhdGEgPSBhZG9w dFB0cihuZXcgU2ltcGxlRm9udERhdGEoU1ZHRm9udERhdGE6OmNyZWF0ZShtX3N2Z0ZvbnRGYWNl RWxlbWVudC5nZXQoKSksIGZvbnREZXNjcmlwdGlvbi5jb21wdXRlZFBpeGVsU2l6ZSgpLCBzeW50 aGV0aWNCb2xkLCBzeW50aGV0aWNJdGFsaWMpKTsKKyAgICAgICAgICAgICAgICBmb250RGF0YSA9 IG5ldyBTaW1wbGVGb250RGF0YShTVkdGb250RGF0YTo6Y3JlYXRlKG1fc3ZnRm9udEZhY2VFbGVt ZW50LmdldCgpKSwgZm9udERlc2NyaXB0aW9uLmNvbXB1dGVkUGl4ZWxTaXplKCksIHN5bnRoZXRp Y0JvbGQsIHN5bnRoZXRpY0l0YWxpYyk7CiAjZW5kaWYKICAgICAgICAgfQogICAgIH0gZWxzZSB7 CkBAIC0xODIsMTMgKzE4MCwxMiBAQCBTaW1wbGVGb250RGF0YSogQ1NTRm9udEZhY2VTb3VyY2U6 OmdldEZvbnREYXRhKGNvbnN0IEZvbnREZXNjcmlwdGlvbiYgZm9udERlc2NyaQogICAgICAgICBp ZiAoIXRlbXBEYXRhKQogICAgICAgICAgICAgdGVtcERhdGEgPSBmb250Q2FjaGUoKS0+Z2V0TGFz dFJlc29ydEZhbGxiYWNrRm9udChmb250RGVzY3JpcHRpb24pOwogCi0gICAgICAgIGZvbnREYXRh ID0gYWRvcHRQdHIobmV3IFNpbXBsZUZvbnREYXRhKHRlbXBEYXRhLT5wbGF0Zm9ybURhdGEoKSwg dHJ1ZSwgdHJ1ZSkpOworICAgICAgICBmb250RGF0YSA9IG5ldyBTaW1wbGVGb250RGF0YSh0ZW1w RGF0YS0+cGxhdGZvcm1EYXRhKCksIHRydWUsIHRydWUpOwogICAgIH0KIAotICAgIFNpbXBsZUZv bnREYXRhKiBmb250RGF0YVJhd1B0ciA9IGZvbnREYXRhLmxlYWtQdHIoKTsKLSAgICBtX2ZvbnRE YXRhVGFibGUuc2V0KGhhc2hLZXksIGZvbnREYXRhUmF3UHRyKTsKKyAgICBtX2ZvbnREYXRhVGFi bGUuc2V0KGhhc2hLZXksIGZvbnREYXRhKTsKIAotICAgIHJldHVybiBmb250RGF0YVJhd1B0cjsK KyAgICByZXR1cm4gZm9udERhdGE7CiB9CiAKICNpZiBFTkFCTEUoU1ZHX0ZPTlRTKQo= 97933 2011-06-20 23:02:55 -0700 2011-06-20 23:57:08 -0700 Patch bug-62975-20110620230253.patch text/plain 1548 yuzo U3VidmVyc2lvbiBSZXZpc2lvbjogODkyMjgKZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJDb3JlL0No YW5nZUxvZyBiL1NvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZwppbmRleCAzYzZhOTQzYzdiZGQxMTJh YmU0M2U4ZTBiOTdiMTZhYjhjMjM5ZTFlLi5kOTBmNjcxMmQzMjQ1MzkyM2U1ZGZiMWM3NmRjNjQz YzkwNGIwZjdkIDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKKysrIGIvU291 cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCkBAIC0xLDMgKzEsMTUgQEAKKzIwMTEtMDYtMjAgIFl1em8g RnVqaXNoaW1hICA8eXV6b0Bnb29nbGUuY29tPgorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9E WSAoT09QUyEpLgorCisgICAgICAgIEZpeCBmb3IgQnVnIDYyOTc1IC0gUmVmYWN0b3IgbG9jYWwg dmFyaWFibGUgdXNhZ2UgaW4gQ1NTRm9udEZhY2VTb3VyY2U6OmdldEZvbnREYXRhCisgICAgICAg IGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD02Mjk3NQorCisgICAgICAg IE5vIG5ldyB0ZXN0cyBiZWNhdXNlIG5vIGJlaGF2aW9yYWwgY2hhbmdlcy4KKworICAgICAgICAq IGNzcy9DU1NGb250RmFjZVNvdXJjZS5jcHA6CisgICAgICAgIChXZWJDb3JlOjpDU1NGb250RmFj ZVNvdXJjZTo6Z2V0Rm9udERhdGEpOiBSZW1vdmUgb25lIG9mIGZvbnREYXRhIHZhcmlhYmxlLgor CiAyMDExLTA2LTE5ICBBZGFtIEJlcmdrdmlzdCAgPGFkYW0uYmVyZ2t2aXN0QGVyaWNzc29uLmNv bT4KIAogICAgICAgICBSZXZpZXdlZCBieSBNYXJ0aW4gUm9iaW5zb24uCmRpZmYgLS1naXQgYS9T b3VyY2UvV2ViQ29yZS9jc3MvQ1NTRm9udEZhY2VTb3VyY2UuY3BwIGIvU291cmNlL1dlYkNvcmUv Y3NzL0NTU0ZvbnRGYWNlU291cmNlLmNwcAppbmRleCA2MmZmNTZmNTA2N2JmNTRiZGYyMjIzNTU3 OWIzMjAxOWYyOGJkYWIzLi40YTg1MWFjMzViYzY0M2I3NzllZDgwYmM3MWVjMjFkNzYzMTU0OTdm IDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViQ29yZS9jc3MvQ1NTRm9udEZhY2VTb3VyY2UuY3BwCisr KyBiL1NvdXJjZS9XZWJDb3JlL2Nzcy9DU1NGb250RmFjZVNvdXJjZS5jcHAKQEAgLTEwOCwxMCAr MTA4LDggQEAgU2ltcGxlRm9udERhdGEqIENTU0ZvbnRGYWNlU291cmNlOjpnZXRGb250RGF0YShj b25zdCBGb250RGVzY3JpcHRpb24mIGZvbnREZXNjcmkKICNlbHNlCiAgICAgaWYgKCFtX2ZvbnQp IHsKICNlbmRpZgotICAgICAgICBTaW1wbGVGb250RGF0YSogZm9udERhdGEgPSBmb250Q2FjaGUo KS0+Z2V0Q2FjaGVkRm9udERhdGEoZm9udERlc2NyaXB0aW9uLCBtX3N0cmluZyk7Ci0KICAgICAg ICAgLy8gV2UncmUgbG9jYWwuIEp1c3QgcmV0dXJuIGEgU2ltcGxlRm9udERhdGEgZnJvbSB0aGUg bm9ybWFsIGNhY2hlLgotICAgICAgICByZXR1cm4gZm9udERhdGE7CisgICAgICAgIHJldHVybiBm b250Q2FjaGUoKS0+Z2V0Q2FjaGVkRm9udERhdGEoZm9udERlc2NyaXB0aW9uLCBtX3N0cmluZyk7 CiAgICAgfQogCiAgICAgLy8gU2VlIGlmIHdlIGhhdmUgYSBtYXBwaW5nIGluIG91ciBGb250RGF0 YSBjYWNoZS4K