99430 2012-10-16 00:50:14 -0700 hb_face_t instances should not depend on FontPlatformData 2012-10-16 02:28:12 -0700 1 1 1 Unclassified WebKit Platform 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED P2 Normal --- 1 bashi bashi tkent webkit.review.bot oldest_to_newest 742909 0 bashi 2012-10-16 00:50:14 -0700 The lifetime of hb_face_t instances should correspond with the lifetime of underlying font data(e.g. SkTypeface and CTFont). HarfBuzzNGFace has its cache mechanism to allow hb_face_t instances live as long as underlying font data live. Since the lifetime of underlying font data and FontPlatformData are different, hb_face_t instances should not depend on FontPlatformData. harfbuzzSkiaGetTable(), harfbuzzCoreTextGetTable() and harfbuzzCairoGetTable() violate this restriction. These functions uses FontPlatformData to get font tables. We should pass underlying font data (or handle of underlying font data) to these functions instead. Otherwise, these functions can access freed FontPlatformData objects (http://crbug.com/156015 is an instance). Note: we can use FontPlatformData in HarfBuzzNGFace::createFont(). This should be safe. Note: we need not add a reference to underlying font data because the cache mechanism takes care of it. 742938 1 168886 bashi 2012-10-16 01:19:09 -0700 Created attachment 168886 Patch 742946 2 bashi 2012-10-16 01:25:53 -0700 Kent-san, could you take a look? The change itself is trivial. I'm ccing you on crbug.com. I confirmed the fix on chromium linux, and compiled the patch for chromium mac. I'll wait and see whether efl port can compile the patch. (for changes of harfbuzzCairoGetTable()) 742948 3 168886 tkent 2012-10-16 01:30:47 -0700 Comment on attachment 168886 Patch rubber-stamped 742978 4 168886 bashi 2012-10-16 02:06:41 -0700 Comment on attachment 168886 Patch Thanks! 742991 5 168886 webkit.review.bot 2012-10-16 02:28:09 -0700 Comment on attachment 168886 Patch Clearing flags on attachment: 168886 Committed r131432: <http://trac.webkit.org/changeset/131432> 742992 6 webkit.review.bot 2012-10-16 02:28:12 -0700 All reviewed patches have been landed. Closing bug. 168886 2012-10-16 01:19:09 -0700 2012-10-16 02:28:08 -0700 Patch bug-99430-20121016171759.patch text/plain 5787 bashi U3VidmVyc2lvbiBSZXZpc2lvbjogMTMxNDIxCmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXggMjJjYzkxOWQ3M2M5ZDU3 YmExOGVkZmM2OWExNDMwYmQxZjJkYjgyYi4uYjk3NTJlZWI1N2FkNDNiNmJiN2U2YTdiYzUyYTg1 ZjU3MjllMWJhZCAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJDb3JlL0NoYW5nZUxvZwpAQCAtMSwzICsxLDI1IEBACisyMDEyLTEwLTE2ICBLZW5p Y2hpIElzaGliYXNoaSAgPGJhc2hpQGNocm9taXVtLm9yZz4KKworICAgICAgICBoYl9mYWNlX3Qg aW5zdGFuY2VzIHNob3VsZCBub3QgZGVwZW5kIG9uIEZvbnRQbGF0Zm9ybURhdGEKKyAgICAgICAg aHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTk5NDMwCisKKyAgICAgICAg UmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgVXNlIHBsYXRmb3JtIHNwZWNp ZmljIGZvbnQgZGF0YSAoZS5nLiBTa1R5cGVmYWNlKSB0byBnZXQKKyAgICAgICAgZm9udCB0YWJs ZXMgZm9yIGhhcmZidXp6LW5nLgorCisgICAgICAgIE5vIG5ldyB0ZXN0cy4gTm8gY2hhbmdlcyBp biBiZWhhdmlvci4KKworICAgICAgICAqIHBsYXRmb3JtL2dyYXBoaWNzL2hhcmZidXp6L25nL0hh cmZCdXp6TkdGYWNlQ2Fpcm8uY3BwOgorICAgICAgICAoV2ViQ29yZTo6aGFyZmJ1enpDYWlyb0dl dFRhYmxlKTogVGFrZXMgY2Fpcm9fc2NhbGVkX2ZvbnRfdCogYXMgdXNlckRhdGEuCisgICAgICAg IChXZWJDb3JlOjpIYXJmQnV6ek5HRmFjZTo6Y3JlYXRlRmFjZSk6CisgICAgICAgICogcGxhdGZv cm0vZ3JhcGhpY3MvaGFyZmJ1enovbmcvSGFyZkJ1enpOR0ZhY2VDb3JlVGV4dC5jcHA6CisgICAg ICAgIChXZWJDb3JlOjpoYXJmYnV6ekNvcmVUZXh0R2V0VGFibGUpOiBUYWtlcyBDR0ZvbnRSZWYg YXMgdXNlckRhdGEuCisgICAgICAgIChXZWJDb3JlOjpIYXJmQnV6ek5HRmFjZTo6Y3JlYXRlRmFj ZSk6CisgICAgICAgICogcGxhdGZvcm0vZ3JhcGhpY3MvaGFyZmJ1enovbmcvSGFyZkJ1enpOR0Zh Y2VTa2lhLmNwcDoKKyAgICAgICAgKFdlYkNvcmU6OmhhcmZidXp6U2tpYUdldFRhYmxlKTogVGFr ZXMgU2tGb250SUQgYXMgdXNlckRhdGEuCisgICAgICAgIChXZWJDb3JlOjpIYXJmQnV6ek5HRmFj ZTo6Y3JlYXRlRmFjZSk6CisKIDIwMTItMTAtMTUgIEtlbnQgVGFtdXJhICA8dGtlbnRAY2hyb21p dW0ub3JnPgogCiAgICAgICAgIEZpeCBzb21lIGFwcGVhcmFuY2UgZ2xpdGNoZXMgb2YgbXVsdGlw bGUgZmllbGRzIGlucHV0IGVsZW1lbnRzCmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9wbGF0 Zm9ybS9ncmFwaGljcy9oYXJmYnV6ei9uZy9IYXJmQnV6ek5HRmFjZUNhaXJvLmNwcCBiL1NvdXJj ZS9XZWJDb3JlL3BsYXRmb3JtL2dyYXBoaWNzL2hhcmZidXp6L25nL0hhcmZCdXp6TkdGYWNlQ2Fp cm8uY3BwCmluZGV4IDUyZmNhN2JhZmI5ZTQ2NjI0MTYzOGU0MzJjNzg2MDY4OTY4MWNhOGIuLjJk YjNkMjc4MDQxZWFhNTgxYmQ5MjA3ZWEzOGQ4ZGYyYmYyOGQ5ZGYgMTAwNjQ0Ci0tLSBhL1NvdXJj ZS9XZWJDb3JlL3BsYXRmb3JtL2dyYXBoaWNzL2hhcmZidXp6L25nL0hhcmZCdXp6TkdGYWNlQ2Fp cm8uY3BwCisrKyBiL1NvdXJjZS9XZWJDb3JlL3BsYXRmb3JtL2dyYXBoaWNzL2hhcmZidXp6L25n L0hhcmZCdXp6TkdGYWNlQ2Fpcm8uY3BwCkBAIC0xNTUsOSArMTU1LDcgQEAgc3RhdGljIGhiX2Zv bnRfZnVuY3NfdCogaGFyZmJ1enpDYWlyb1RleHRHZXRGb250RnVuY3MoKQogCiBzdGF0aWMgaGJf YmxvYl90KiBoYXJmYnV6ekNhaXJvR2V0VGFibGUoaGJfZmFjZV90KiwgaGJfdGFnX3QgdGFnLCB2 b2lkKiB1c2VyRGF0YSkKIHsKLSAgICBGb250UGxhdGZvcm1EYXRhKiBmb250ID0gcmVpbnRlcnBy ZXRfY2FzdDxGb250UGxhdGZvcm1EYXRhKj4odXNlckRhdGEpOwotCi0gICAgY2Fpcm9fc2NhbGVk X2ZvbnRfdCogc2NhbGVkRm9udCA9IGZvbnQtPnNjYWxlZEZvbnQoKTsKKyAgICBjYWlyb19zY2Fs ZWRfZm9udF90KiBzY2FsZWRGb250ID0gcmVpbnRlcnByZXRfY2FzdDxjYWlyb19zY2FsZWRfZm9u dF90Kj4odXNlckRhdGEpOwogICAgIGlmICghc2NhbGVkRm9udCkKICAgICAgICAgcmV0dXJuIDA7 CiAKQEAgLTE4Niw3ICsxODQsNyBAQCBzdGF0aWMgaGJfYmxvYl90KiBoYXJmYnV6ekNhaXJvR2V0 VGFibGUoaGJfZmFjZV90KiwgaGJfdGFnX3QgdGFnLCB2b2lkKiB1c2VyRGF0YQogCiBoYl9mYWNl X3QqIEhhcmZCdXp6TkdGYWNlOjpjcmVhdGVGYWNlKCkKIHsKLSAgICBoYl9mYWNlX3QqIGZhY2Ug PSBoYl9mYWNlX2NyZWF0ZV9mb3JfdGFibGVzKGhhcmZidXp6Q2Fpcm9HZXRUYWJsZSwgbV9wbGF0 Zm9ybURhdGEsIDApOworICAgIGhiX2ZhY2VfdCogZmFjZSA9IGhiX2ZhY2VfY3JlYXRlX2Zvcl90 YWJsZXMoaGFyZmJ1enpDYWlyb0dldFRhYmxlLCBtX3BsYXRmb3JtRGF0YS0+c2NhbGVkRm9udCgp LCAwKTsKICAgICBBU1NFUlQoZmFjZSk7CiAgICAgcmV0dXJuIGZhY2U7CiB9CmRpZmYgLS1naXQg YS9Tb3VyY2UvV2ViQ29yZS9wbGF0Zm9ybS9ncmFwaGljcy9oYXJmYnV6ei9uZy9IYXJmQnV6ek5H RmFjZUNvcmVUZXh0LmNwcCBiL1NvdXJjZS9XZWJDb3JlL3BsYXRmb3JtL2dyYXBoaWNzL2hhcmZi dXp6L25nL0hhcmZCdXp6TkdGYWNlQ29yZVRleHQuY3BwCmluZGV4IGYyYmFiYTczZjkwN2RmMzk4 NjU3NTE3ZTk4YzM4ZWUyZjE4OTFiOTcuLmU0YzY1Y2Q0MWUwZWMzMDQ0ZWI1OTliNWQ1NGUxNjc4 NDU2YzE0OTIgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJDb3JlL3BsYXRmb3JtL2dyYXBoaWNzL2hh cmZidXp6L25nL0hhcmZCdXp6TkdGYWNlQ29yZVRleHQuY3BwCisrKyBiL1NvdXJjZS9XZWJDb3Jl L3BsYXRmb3JtL2dyYXBoaWNzL2hhcmZidXp6L25nL0hhcmZCdXp6TkdGYWNlQ29yZVRleHQuY3Bw CkBAIC0xMDgsMTAgKzEwOCw3IEBAIHN0YXRpYyB2b2lkIHJlbGVhc2VUYWJsZURhdGEodm9pZCog dXNlckRhdGEpCiAKIHN0YXRpYyBoYl9ibG9iX3QqIGhhcmZidXp6Q29yZVRleHRHZXRUYWJsZSho Yl9mYWNlX3QqIGZhY2UsIGhiX3RhZ190IHRhZywgdm9pZCogdXNlckRhdGEpCiB7Ci0gICAgRm9u dFBsYXRmb3JtRGF0YSogcGxhdGZvcm1EYXRhID0gcmVpbnRlcnByZXRfY2FzdDxGb250UGxhdGZv cm1EYXRhKj4odXNlckRhdGEpOwotICAgIC8vIEl0IHNlZW1zIHRoYXQgQ1RGb250Q29weVRhYmxl IG9mIE1hY09TWDEwLjUgc2RrIGRvZXNuJ3Qgd29yayBmb3IKLSAgICAvLyBPcGVuVHlwZSBsYXlv dXQgdGFibGVzKEdERUYsIEdTVUIsIEdQT1MpLiBVc2UgQ0dGb250Q29weVRhYmxlRm9yVGFnIGlu c3RlYWQuCi0gICAgQ0dGb250UmVmIGNnRm9udCA9IHBsYXRmb3JtRGF0YS0+Y2dGb250KCk7Cisg ICAgQ0dGb250UmVmIGNnRm9udCA9IHJlaW50ZXJwcmV0X2Nhc3Q8Q0dGb250UmVmPih1c2VyRGF0 YSk7CiAgICAgQ0ZEYXRhUmVmIGNmRGF0YSA9IENHRm9udENvcHlUYWJsZUZvclRhZyhjZ0ZvbnQs IHRhZyk7CiAgICAgaWYgKCFjZkRhdGEpCiAgICAgICAgIHJldHVybiAwOwpAQCAtMTI1LDcgKzEy Miw5IEBAIHN0YXRpYyBoYl9ibG9iX3QqIGhhcmZidXp6Q29yZVRleHRHZXRUYWJsZShoYl9mYWNl X3QqIGZhY2UsIGhiX3RhZ190IHRhZywgdm9pZCoKIAogaGJfZmFjZV90KiBIYXJmQnV6ek5HRmFj ZTo6Y3JlYXRlRmFjZSgpCiB7Ci0gICAgaGJfZmFjZV90KiBmYWNlID0gaGJfZmFjZV9jcmVhdGVf Zm9yX3RhYmxlcyhoYXJmYnV6ekNvcmVUZXh0R2V0VGFibGUsIG1fcGxhdGZvcm1EYXRhLCAwKTsK KyAgICAvLyBJdCBzZWVtcyB0aGF0IENURm9udENvcHlUYWJsZSBvZiBNYWNPU1gxMC41IHNkayBk b2Vzbid0IHdvcmsgZm9yCisgICAgLy8gT3BlblR5cGUgbGF5b3V0IHRhYmxlcyhHREVGLCBHU1VC LCBHUE9TKS4gVXNlIENHRm9udENvcHlUYWJsZUZvclRhZyBpbnN0ZWFkLgorICAgIGhiX2ZhY2Vf dCogZmFjZSA9IGhiX2ZhY2VfY3JlYXRlX2Zvcl90YWJsZXMoaGFyZmJ1enpDb3JlVGV4dEdldFRh YmxlLCBtX3BsYXRmb3JtRGF0YS0+Y2dGb250KCksIDApOwogICAgIEFTU0VSVChmYWNlKTsKICAg ICByZXR1cm4gZmFjZTsKIH0KZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJDb3JlL3BsYXRmb3JtL2dy YXBoaWNzL2hhcmZidXp6L25nL0hhcmZCdXp6TkdGYWNlU2tpYS5jcHAgYi9Tb3VyY2UvV2ViQ29y ZS9wbGF0Zm9ybS9ncmFwaGljcy9oYXJmYnV6ei9uZy9IYXJmQnV6ek5HRmFjZVNraWEuY3BwCmlu ZGV4IDZkNTFmNDdiMDU5YTM5NTFiZWM2ZmZkZmM0ZGU2ZjkyNTJmODViOTYuLjVkMzllZmE3Mzcx M2I4ZGE1YzVmZGQ2MzVlYWUwODVmNzI5OTZlNjIgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJDb3Jl L3BsYXRmb3JtL2dyYXBoaWNzL2hhcmZidXp6L25nL0hhcmZCdXp6TkdGYWNlU2tpYS5jcHAKKysr IGIvU291cmNlL1dlYkNvcmUvcGxhdGZvcm0vZ3JhcGhpY3MvaGFyZmJ1enovbmcvSGFyZkJ1enpO R0ZhY2VTa2lhLmNwcApAQCAtMTQ0LDE2ICsxNDQsMTYgQEAgc3RhdGljIGhiX2ZvbnRfZnVuY3Nf dCogaGFyZmJ1enpTa2lhR2V0Rm9udEZ1bmNzKCkKIAogc3RhdGljIGhiX2Jsb2JfdCogaGFyZmJ1 enpTa2lhR2V0VGFibGUoaGJfZmFjZV90KiBmYWNlLCBoYl90YWdfdCB0YWcsIHZvaWQqIHVzZXJE YXRhKQogewotICAgIEZvbnRQbGF0Zm9ybURhdGEqIGZvbnQgPSByZWludGVycHJldF9jYXN0PEZv bnRQbGF0Zm9ybURhdGEqPih1c2VyRGF0YSk7CisgICAgU2tGb250SUQgdW5pcXVlSUQgPSBzdGF0 aWNfY2FzdDxTa0ZvbnRJRD4ocmVpbnRlcnByZXRfY2FzdDx1aW50NjRfdD4odXNlckRhdGEpKTsK IAotICAgIGNvbnN0IHNpemVfdCB0YWJsZVNpemUgPSBTa0ZvbnRIb3N0OjpHZXRUYWJsZVNpemUo Zm9udC0+dW5pcXVlSUQoKSwgdGFnKTsKKyAgICBjb25zdCBzaXplX3QgdGFibGVTaXplID0gU2tG b250SG9zdDo6R2V0VGFibGVTaXplKHVuaXF1ZUlELCB0YWcpOwogICAgIGlmICghdGFibGVTaXpl KQogICAgICAgICByZXR1cm4gMDsKIAogICAgIGNoYXIqIGJ1ZmZlciA9IHJlaW50ZXJwcmV0X2Nh c3Q8Y2hhcio+KGZhc3RNYWxsb2ModGFibGVTaXplKSk7CiAgICAgaWYgKCFidWZmZXIpCiAgICAg ICAgIHJldHVybiAwOwotICAgIHNpemVfdCBhY3R1YWxTaXplID0gU2tGb250SG9zdDo6R2V0VGFi bGVEYXRhKGZvbnQtPnVuaXF1ZUlEKCksIHRhZywgMCwgdGFibGVTaXplLCBidWZmZXIpOworICAg IHNpemVfdCBhY3R1YWxTaXplID0gU2tGb250SG9zdDo6R2V0VGFibGVEYXRhKHVuaXF1ZUlELCB0 YWcsIDAsIHRhYmxlU2l6ZSwgYnVmZmVyKTsKICAgICBpZiAodGFibGVTaXplICE9IGFjdHVhbFNp emUpIHsKICAgICAgICAgZmFzdEZyZWUoYnVmZmVyKTsKICAgICAgICAgcmV0dXJuIDA7CkBAIC0x NzEsNyArMTcxLDcgQEAgc3RhdGljIHZvaWQgZGVzdHJveUhhcmZCdXp6Rm9udERhdGEodm9pZCog dXNlckRhdGEpCiAKIGhiX2ZhY2VfdCogSGFyZkJ1enpOR0ZhY2U6OmNyZWF0ZUZhY2UoKQogewot ICAgIGhiX2ZhY2VfdCogZmFjZSA9IGhiX2ZhY2VfY3JlYXRlX2Zvcl90YWJsZXMoaGFyZmJ1enpT a2lhR2V0VGFibGUsIG1fcGxhdGZvcm1EYXRhLCAwKTsKKyAgICBoYl9mYWNlX3QqIGZhY2UgPSBo Yl9mYWNlX2NyZWF0ZV9mb3JfdGFibGVzKGhhcmZidXp6U2tpYUdldFRhYmxlLCByZWludGVycHJl dF9jYXN0PHZvaWQqPihtX3BsYXRmb3JtRGF0YS0+dW5pcXVlSUQoKSksIDApOwogICAgIEFTU0VS VChmYWNlKTsKICAgICByZXR1cm4gZmFjZTsKIH0K