183339 2018-03-05 11:17:07 -0800 Change the type of SVGToOTFFontConverter::m_weight to be not a char 2018-03-10 17:52:34 -0800 1 1 1 Unclassified WebKit SVG WebKit Nightly Build Unspecified Unspecified RESOLVED FIXED InRadar P2 Normal --- 1 sabouhallawa mmaxfield achristensen commit-queue darin mmaxfield webkit-bug-importer zimmermann oldest_to_newest 1403683 0 sabouhallawa 2018-03-05 11:17:07 -0800 This is a follow up for Darin's comment in https://bugs.webkit.org/show_bug.cgi?id=183165#c2. If m_weight is negative, clampTo<uint16_t> or static_cast<uint16_t> will give the wrong value. 1403880 1 mmaxfield 2018-03-05 20:03:21 -0800 m_result is a Vector<char> because that's what SharedBuffer::create() accepts. 1403881 2 335067 mmaxfield 2018-03-05 20:16:11 -0800 Created attachment 335067 Patch 1404054 3 335067 commit-queue 2018-03-06 10:35:54 -0800 Comment on attachment 335067 Patch Clearing flags on attachment: 335067 Committed r229328: <https://trac.webkit.org/changeset/229328> 1404055 4 commit-queue 2018-03-06 10:36:13 -0800 All reviewed patches have been landed. Closing bug. 1404056 5 webkit-bug-importer 2018-03-06 10:37:21 -0800 <rdar://problem/38185027> 1404258 6 darin 2018-03-06 20:36:46 -0800 (In reply to Myles C. Maxfield from comment #1) > m_result is a Vector<char> because that's what SharedBuffer::create() > accepts. At some point I intend to change Vector<char> to Vector<uint8_t> throughout WebKit for this sort of thing. 1404260 7 mmaxfield 2018-03-06 20:46:18 -0800 (In reply to Darin Adler from comment #6) > (In reply to Myles C. Maxfield from comment #1) > > m_result is a Vector<char> because that's what SharedBuffer::create() > > accepts. > > At some point I intend to change Vector<char> to Vector<uint8_t> throughout > WebKit for this sort of thing. Yes! 👍 1405523 8 335067 darin 2018-03-10 17:52:34 -0800 Comment on attachment 335067 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=335067&action=review > Source/WebCore/svg/SVGToOTFFontConversion.cpp:528 > + if (ok && value >= std::numeric_limits<uint8_t>::min() && value <= std::numeric_limits<uint8_t>::max()) If this pattern is common I think we should add a helper function. > Source/WebCore/svg/SVGToOTFFontConversion.cpp:1464 > + m_weight = std::max(std::min((value + 50) / 100, static_cast<int>(std::numeric_limits<uint8_t>::max())), static_cast<int>(std::numeric_limits<uint8_t>::min())); This is what clampTo is for, and it’s much easier to read: m_weight = clampTo<uint8_t>((value + 50) / 100); 335067 2018-03-05 20:16:11 -0800 2018-03-06 10:35:54 -0800 Patch bug-183339-20180305201610.patch text/plain 2685 mmaxfield U3VidmVyc2lvbiBSZXZpc2lvbjogMjI5MzAzCmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXggNjA3ZmE0M2I5YjA3MDI0 ZGQ5ZDM4NWFkMmRlY2FlZjI4OTE5OWEwZi4uNmZkZGY2YWRiMmI5M2I5MzgzMDVhMjJmOTEwMzMy ODQ4N2Q2ZDJkYyAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJDb3JlL0NoYW5nZUxvZwpAQCAtMSwzICsxLDE2IEBACisyMDE4LTAzLTA1ICBNeWxl cyBDLiBNYXhmaWVsZCAgPG1tYXhmaWVsZEBhcHBsZS5jb20+CisKKyAgICAgICAgQ2hhbmdlIHRo ZSB0eXBlIG9mIFNWR1RvT1RGRm9udENvbnZlcnRlcjo6bV93ZWlnaHQgdG8gYmUgbm90IGEgY2hh cgorICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9MTgzMzM5 CisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgTm8gbmV3 IHRlc3RzIGJlY2F1c2UgdGhlcmUgaXMgbm8gYmVoYXZpb3IgY2hhbmdlLgorCisgICAgICAgICog c3ZnL1NWR1RvT1RGRm9udENvbnZlcnNpb24uY3BwOgorICAgICAgICAoV2ViQ29yZTo6U1ZHVG9P VEZGb250Q29udmVydGVyOjphcHBlbmRPUzJUYWJsZSk6CisgICAgICAgIChXZWJDb3JlOjpTVkdU b09URkZvbnRDb252ZXJ0ZXI6OlNWR1RvT1RGRm9udENvbnZlcnRlcik6CisKIDIwMTgtMDMtMDUg IE15bGVzIEMuIE1heGZpZWxkICA8bW1heGZpZWxkQGFwcGxlLmNvbT4KIAogICAgICAgICBbQ29j b2FdIEFsbG93IHVzZXItaW5zdGFsbGVkIGZvbnRzIHRvIGJlIGRpc2FibGVkCmRpZmYgLS1naXQg YS9Tb3VyY2UvV2ViQ29yZS9zdmcvU1ZHVG9PVEZGb250Q29udmVyc2lvbi5jcHAgYi9Tb3VyY2Uv V2ViQ29yZS9zdmcvU1ZHVG9PVEZGb250Q29udmVyc2lvbi5jcHAKaW5kZXggY2Y2MmVlY2IzZWU5 MTZhOGMxYTJjNjFhYzM2NjE3NDQyNTY5NjIxYi4uNTBhYmNlNmExYmM4NWViZTM4NDY1ZDQyYmRl OWI1NmI0Mjc0NjNhNSAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvc3ZnL1NWR1RvT1RGRm9u dENvbnZlcnNpb24uY3BwCisrKyBiL1NvdXJjZS9XZWJDb3JlL3N2Zy9TVkdUb09URkZvbnRDb252 ZXJzaW9uLmNwcApAQCAtMjY0LDcgKzI2NCw3IEBAIHByaXZhdGU6CiAgICAgaW50IG1fZGVzY2Vu dDsKICAgICB1bnNpZ25lZCBtX2ZlYXR1cmVDb3VudEdTVUI7CiAgICAgdW5zaWduZWQgbV90YWJs ZXNBcHBlbmRlZENvdW50OwotICAgIGNoYXIgbV93ZWlnaHQ7CisgICAgdWludDhfdCBtX3dlaWdo dDsKICAgICBib29sIG1faXRhbGljOwogICAgIGJvb2wgbV9lcnJvciB7IGZhbHNlIH07CiB9OwpA QCAtNDk5LDcgKzQ5OSw3IEBAIHZvaWQgU1ZHVG9PVEZGb250Q29udmVydGVyOjphcHBlbmRPUzJU YWJsZSgpCiAKICAgICBhcHBlbmQxNigyKTsgLy8gVmVyc2lvbgogICAgIGFwcGVuZDE2KGNsYW1w VG88aW50MTZfdD4oYXZlcmFnZUFkdmFuY2UpKTsKLSAgICBhcHBlbmQxNihjbGFtcFRvPHVpbnQx Nl90PihtX3dlaWdodCkpOyAvLyBXZWlnaHQgY2xhc3MKKyAgICBhcHBlbmQxNihtX3dlaWdodCk7 IC8vIFdlaWdodCBjbGFzcwogICAgIGFwcGVuZDE2KDUpOyAvLyBXaWR0aCBjbGFzcwogICAgIGFw cGVuZDE2KDApOyAvLyBQcm90ZWN0ZWQgZm9udAogICAgIC8vIFdlYktpdCBoYW5kbGVzIHRoZXNl IHN1cGVyc2NyaXB0cyBhbmQgc3Vic2NyaXB0cwpAQCAtNTI1LDcgKzUyNSw3IEBAIHZvaWQgU1ZH VG9PVEZGb250Q29udmVydGVyOjphcHBlbmRPUzJUYWJsZSgpCiAgICAgICAgICAgICBmb3IgKGF1 dG8mIHNlZ21lbnQgOiBzZWdtZW50cykgewogICAgICAgICAgICAgICAgIGJvb2wgb2s7CiAgICAg ICAgICAgICAgICAgaW50IHZhbHVlID0gc2VnbWVudC50b0ludCgmb2spOwotICAgICAgICAgICAg ICAgIGlmIChvayAmJiB2YWx1ZSA+PSAwICYmIHZhbHVlIDw9IDB4RkYpCisgICAgICAgICAgICAg ICAgaWYgKG9rICYmIHZhbHVlID49IHN0ZDo6bnVtZXJpY19saW1pdHM8dWludDhfdD46Om1pbigp ICYmIHZhbHVlIDw9IHN0ZDo6bnVtZXJpY19saW1pdHM8dWludDhfdD46Om1heCgpKQogICAgICAg ICAgICAgICAgICAgICBwYW5vc2VCeXRlc1tudW1QYW5vc2VCeXRlcysrXSA9IHZhbHVlOwogICAg ICAgICAgICAgfQogICAgICAgICB9CkBAIC0xNDYxLDcgKzE0NjEsNyBAQCBTVkdUb09URkZvbnRD b252ZXJ0ZXI6OlNWR1RvT1RGRm9udENvbnZlcnRlcihjb25zdCBTVkdGb250RWxlbWVudCYgZm9u dEVsZW1lbnQpCiAgICAgICAgICAgICBib29sIG9rOwogICAgICAgICAgICAgaW50IHZhbHVlID0g c2VnbWVudC50b0ludChvayk7CiAgICAgICAgICAgICBpZiAob2sgJiYgdmFsdWUgPj0gMCAmJiB2 YWx1ZSA8IDEwMDApIHsKLSAgICAgICAgICAgICAgICBtX3dlaWdodCA9ICh2YWx1ZSArIDUwKSAv IDEwMDsKKyAgICAgICAgICAgICAgICBtX3dlaWdodCA9IHN0ZDo6bWF4KHN0ZDo6bWluKCh2YWx1 ZSArIDUwKSAvIDEwMCwgc3RhdGljX2Nhc3Q8aW50PihzdGQ6Om51bWVyaWNfbGltaXRzPHVpbnQ4 X3Q+OjptYXgoKSkpLCBzdGF0aWNfY2FzdDxpbnQ+KHN0ZDo6bnVtZXJpY19saW1pdHM8dWludDhf dD46Om1pbigpKSk7CiAgICAgICAgICAgICAgICAgYnJlYWs7CiAgICAgICAgICAgICB9CiAgICAg ICAgIH0K