75296 2011-12-28 00:54:08 -0800 JSString should not have JS_EXPORTCLASS annotation 2012-01-11 23:01:34 -0800 1 1 1 Unclassified WebKit JavaScriptCore 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED P2 Normal --- 1 morrita morrita ap aroben dbates ggaren kevino webkit.review.bot oldest_to_newest 527578 0 morrita 2011-12-28 00:54:08 -0800 JSString is annotated JS_EXPORTCLASS but we should remove it because it's no longer a part of public (exported) API. With this annotation, turning export macros on will result extra symbol exposure. 527583 1 120640 morrita 2011-12-28 01:02:44 -0800 Created attachment 120640 Patch 528207 2 ap 2011-12-30 12:28:52 -0800 JSString is used in WebCore (see e.g. DOMWrapperWorld.h). Why does it matter that it's not part of a public API? 528394 3 kevino 2012-01-01 18:58:05 -0800 (In reply to comment #2) > JSString is used in WebCore (see e.g. DOMWrapperWorld.h). Why does it matter that it's not part of a public API? It is WebKit policy that APIs that are not public should export only the methods needed by WebCore, etc., to discourage third party developers from coding against those APIs. Hajime's patch is not removing the JSString exports entirely, just decreasing what is exported to only what is needed. 529900 4 120640 kevino 2012-01-04 17:32:30 -0800 Comment on attachment 120640 Patch It looks like Windows needs the class declaration exported in order to provide access to JSString's vtable. I'd have to double-check, but I think the reason this patch works for other platforms is that gcc lets you add the class's vtable to the symbol exports file, but Windows does not. This is probably why we have JS_EXPORTCLASS and JS_EXPORTDATA macros even though we're using symbol export files. 529902 5 kevino 2012-01-04 17:34:29 -0800 (In reply to comment #4) > (From update of attachment 120640 [details]) > It looks like Windows needs the class declaration exported in order to provide access to JSString's vtable. I'd have to double-check, but I think the reason this patch works for other platforms is that gcc lets you add the class's vtable to the symbol exports file, but Windows does not. This is probably why we have JS_EXPORTCLASS and JS_EXPORTDATA macros even though we're using symbol export files. Sorry, by "Windows" I meant the Apple Windows port's compiler, that is, Visual C++. 530022 6 morrita 2012-01-04 23:11:19 -0800 Oliver, thanks for your explanation and investigation. > It looks like Windows needs the class declaration exported in order to provide access to JSString's vtable. I'd have to double-check, but I think the reason this patch works for other platforms is that gcc lets you add the class's vtable to the symbol exports file, but Windows does not. This is probably why we have JS_EXPORTCLASS and JS_EXPORTDATA macros even though we're using symbol export files Interesting. And I didn't know that Apple Windows port already relies on the macro. I'll try another patch to examine my understanding. If if doesn't work, I'll close this bug as invalid. 530976 7 121408 morrita 2012-01-06 00:51:03 -0800 Created attachment 121408 An attempt to fix windows build break 531526 8 121408 webkit.review.bot 2012-01-06 16:55:12 -0800 Comment on attachment 121408 An attempt to fix windows build break Clearing flags on attachment: 121408 Committed r104359: <http://trac.webkit.org/changeset/104359> 531527 9 webkit.review.bot 2012-01-06 16:55:17 -0800 All reviewed patches have been landed. Closing bug. 534239 10 aroben 2012-01-11 14:05:45 -0800 FYI, this caused bug 76101 534582 11 morrita 2012-01-11 23:01:34 -0800 (In reply to comment #10) > FYI, this caused bug 76101 Oops. Thanks for fixing that. 120640 2011-12-28 01:02:44 -0800 2012-01-06 00:50:56 -0800 Patch bug-75296-20111228180242.patch text/plain 1243 morrita U3VidmVyc2lvbiBSZXZpc2lvbjogMTAzNzU4CmRpZmYgLS1naXQgYS9Tb3VyY2UvSmF2YVNjcmlw dENvcmUvQ2hhbmdlTG9nIGIvU291cmNlL0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwppbmRleCBk N2JkNjE1OWVlMmEwZjBmODI3MTBhZDFhOTEzMWJlMzA3MDM3ODAxLi5mNDVjMmI5MmRhNmU3ZWY3 MDAxYzI3MTQyY2YxOThmYjZhMjQ4YjdhIDEwMDY0NAotLS0gYS9Tb3VyY2UvSmF2YVNjcmlwdENv cmUvQ2hhbmdlTG9nCisrKyBiL1NvdXJjZS9KYXZhU2NyaXB0Q29yZS9DaGFuZ2VMb2cKQEAgLTEs MyArMSwxMiBAQAorMjAxMS0xMi0yOCAgSGFqaW1lIE1vcnJpdGEgIDxtb3JyaXRhQGNocm9taXVt Lm9yZz4KKworICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9 NzUyOTYKKyAgICAgICAgSlNTdHJpbmcgc2hvdWxkIG5vdCBoYXZlIEpTX0VYUE9SVENMQVNTIGFu bm90YXRpb24KKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAg ICAqIHJ1bnRpbWUvSlNTdHJpbmcuaDogUmVtb3ZlZCBKU19FWFBPUlRDTEFTUyBhbm5vdGF0aW9u LgorCiAyMDExLTEyLTI3ICBTYW0gV2VpbmlnICA8c2FtQHdlYmtpdC5vcmc+CiAKICAgICAgICAg Q29udGludWUgbW92aW5nIGNvbXBpbGVyIGZlYXR1cmUgY2hlY2tzIHRvIHVzZSB0aGUgQ09NUElM RVJfU1VQUE9SVFMoKSBtYWNybwpkaWZmIC0tZ2l0IGEvU291cmNlL0phdmFTY3JpcHRDb3JlL3J1 bnRpbWUvSlNTdHJpbmcuaCBiL1NvdXJjZS9KYXZhU2NyaXB0Q29yZS9ydW50aW1lL0pTU3RyaW5n LmgKaW5kZXggNjcwMGUyZTQxMGM4ZjY1NzcwZTcwZjQ4N2VjMzZiNjhlNjU4YjU3My4uZjQwNDU1 NTcxNDdkYjBmMjliOGVlYjZjNzgxNDY5NDk2NzIwZmUyNCAxMDA2NDQKLS0tIGEvU291cmNlL0ph dmFTY3JpcHRDb3JlL3J1bnRpbWUvSlNTdHJpbmcuaAorKysgYi9Tb3VyY2UvSmF2YVNjcmlwdENv cmUvcnVudGltZS9KU1N0cmluZy5oCkBAIC01OSw3ICs1OSw3IEBAIG5hbWVzcGFjZSBKU0Mgewog CiAgICAgSlNTdHJpbmcqIGpzU3RyaW5nQnVpbGRlcihKU0dsb2JhbERhdGEqKTsKIAotICAgIGNs YXNzIEpTX0VYUE9SVENMQVNTIEpTU3RyaW5nIDogcHVibGljIEpTQ2VsbCB7CisgICAgY2xhc3Mg SlNTdHJpbmcgOiBwdWJsaWMgSlNDZWxsIHsKICAgICBwdWJsaWM6CiAgICAgICAgIGZyaWVuZCBj bGFzcyBKSVQ7CiAgICAgICAgIGZyaWVuZCBjbGFzcyBKU0dsb2JhbERhdGE7Cg== 121408 2012-01-06 00:51:03 -0800 2012-01-06 16:55:12 -0800 An attempt to fix windows build break bug-75296-20120106175101.patch text/plain 2612 morrita U3VidmVyc2lvbiBSZXZpc2lvbjogMTA0Mjc0CmRpZmYgLS1naXQgYS9Tb3VyY2UvSmF2YVNjcmlw dENvcmUvQ2hhbmdlTG9nIGIvU291cmNlL0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwppbmRleCA5 YzU2NjdkMjU4ODcyYjQ5MTgzOTFkZmExZDVlN2Y2MTljMDQ4MWY1Li4xM2FiYzEyOTM5YjU4ZDdh YTQ0ZDNhMGZmNTU3YjBlZDI4NmFiZDczIDEwMDY0NAotLS0gYS9Tb3VyY2UvSmF2YVNjcmlwdENv cmUvQ2hhbmdlTG9nCisrKyBiL1NvdXJjZS9KYXZhU2NyaXB0Q29yZS9DaGFuZ2VMb2cKQEAgLTEs MyArMSwxNCBAQAorMjAxMS0xMi0yOCAgSGFqaW1lIE1vcnJpdGEgIDxtb3JyaXRhQGNocm9taXVt Lm9yZz4KKworICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9 NzUyOTYKKyAgICAgICAgSlNTdHJpbmcgc2hvdWxkIG5vdCBoYXZlIEpTX0VYUE9SVENMQVNTIGFu bm90YXRpb24KKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAg ICAqIHJ1bnRpbWUvSlNTdHJpbmcuaDogUmVtb3ZlZCBKU19FWFBPUlRDTEFTUyBhbm5vdGF0aW9u LgorICAgICAgICAqIEphdmFTY3JpcHRDb3JlLnZjcHJvai9KYXZhU2NyaXB0Q29yZS9KYXZhU2Ny aXB0Q29yZS5kZWY6CisgICAgICAgIEFkZGVkIG1pc3Npbmcgc3ltYm9scyB3aGljaCB3ZXJlIGhp ZGRlbiBieSBKU19FWFBPUlRDTEFTUy4KKwogMjAxMi0wMS0wNiAgSGFqaW1lIE1vcnJpdGEgIDxt b3JyaXRhQGNocm9taXVtLm9yZz4KIAogICAgICAgICBXVEY6OlN0cmluZzogSW5saW5lIG1ldGhv ZCBzaG91bGRuJ3QgaGF2ZSBXVEZfRVhQT1JUX1BSSVZBVEUKZGlmZiAtLWdpdCBhL1NvdXJjZS9K YXZhU2NyaXB0Q29yZS9KYXZhU2NyaXB0Q29yZS52Y3Byb2ovSmF2YVNjcmlwdENvcmUvSmF2YVNj cmlwdENvcmUuZGVmIGIvU291cmNlL0phdmFTY3JpcHRDb3JlL0phdmFTY3JpcHRDb3JlLnZjcHJv ai9KYXZhU2NyaXB0Q29yZS9KYXZhU2NyaXB0Q29yZS5kZWYKaW5kZXggOWVlNjFlYWFiMTRiYjZh MzkwYmYxMTQ5MzcxMjdiYjc5MjhlMDkzOS4uNzFlZWQyNmIzNjk0ZDk0OGViYzg5OGRkZWUzMTVm MTFjMDZhNDYwZCAxMDA2NDQKLS0tIGEvU291cmNlL0phdmFTY3JpcHRDb3JlL0phdmFTY3JpcHRD b3JlLnZjcHJvai9KYXZhU2NyaXB0Q29yZS9KYXZhU2NyaXB0Q29yZS5kZWYKKysrIGIvU291cmNl L0phdmFTY3JpcHRDb3JlL0phdmFTY3JpcHRDb3JlLnZjcHJvai9KYXZhU2NyaXB0Q29yZS9KYXZh U2NyaXB0Q29yZS5kZWYKQEAgLTI5Niw2ICsyOTYsNyBAQCBFWFBPUlRTCiAgICAgP3JldHJpZXZl TGFzdENhbGxlckBJbnRlcnByZXRlckBKU0NAQFFCRVhQQVZFeGVjU3RhdGVAMkBBQUgxQUFWVVN0 cmluZ0AyQEFBVkpTVmFsdWVAMkBAWgogICAgID9zX2dsb2JhbE9iamVjdE1ldGhvZFRhYmxlQEpT R2xvYmFsT2JqZWN0QEpTQ0BAMVVHbG9iYWxPYmplY3RNZXRob2RUYWJsZUAyQEIKICAgICA/c19p bmZvQEV4ZWN1dGFibGVCYXNlQEpTQ0BAMlVDbGFzc0luZm9AMkBCCisgICAgP3NfaW5mb0BKU1N0 cmluZ0BKU0NAQDJVQ2xhc3NJbmZvQDJAQgogICAgID9zZXRDb25maWd1cmFibGVAUHJvcGVydHlE ZXNjcmlwdG9yQEpTQ0BAUUFFWF9OQFoKICAgICA/c2V0RGVzY3JpcHRvckBQcm9wZXJ0eURlc2Ny aXB0b3JASlNDQEBRQUVYVkpTVmFsdWVAMkBJQFoKICAgICA/c2V0RHVtcHNHZW5lcmF0ZWRDb2Rl QEJ5dGVjb2RlR2VuZXJhdG9yQEpTQ0BAU0FYX05AWgpAQCAtMzM2LDYgKzMzNyw3IEBAIEVYUE9S VFMKICAgICA/dGltZWRXYWl0QFRocmVhZENvbmRpdGlvbkBXVEZAQFFBRV9OQUFWTXV0ZXhAMkBO QFoKICAgICA/dGxzS2V5Q291bnRAV1RGQEBZQUFBSlhaCiAgICAgP3Rsc0tleXNAV1RGQEBZQVBB S1haCisgICAgP3RvQm9vbGVhbkBKU1N0cmluZ0BKU0NAQFFCRV9OUEFWRXhlY1N0YXRlQDJAQFoK ICAgICA/dG9JbnQzMkBKU0NAQFlBSE5AWgogICAgID90b0ludGVnZXJASlNWYWx1ZUBKU0NAQFFC RU5QQVZFeGVjU3RhdGVAMkBAWgogICAgID90b051bWJlclNsb3dDYXNlQEpTVmFsdWVASlNDQEBB QkVOUEFWRXhlY1N0YXRlQDJAQFoKZGlmZiAtLWdpdCBhL1NvdXJjZS9KYXZhU2NyaXB0Q29yZS9y dW50aW1lL0pTU3RyaW5nLmggYi9Tb3VyY2UvSmF2YVNjcmlwdENvcmUvcnVudGltZS9KU1N0cmlu Zy5oCmluZGV4IDY3MDBlMmU0MTBjOGY2NTc3MGU3MGY0ODdlYzM2YjY4ZTY1OGI1NzMuLmY0MDQ1 NTU3MTQ3ZGIwZjI5YjhlZWI2Yzc4MTQ2OTQ5NjcyMGZlMjQgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9K YXZhU2NyaXB0Q29yZS9ydW50aW1lL0pTU3RyaW5nLmgKKysrIGIvU291cmNlL0phdmFTY3JpcHRD b3JlL3J1bnRpbWUvSlNTdHJpbmcuaApAQCAtNTksNyArNTksNyBAQCBuYW1lc3BhY2UgSlNDIHsK IAogICAgIEpTU3RyaW5nKiBqc1N0cmluZ0J1aWxkZXIoSlNHbG9iYWxEYXRhKik7CiAKLSAgICBj bGFzcyBKU19FWFBPUlRDTEFTUyBKU1N0cmluZyA6IHB1YmxpYyBKU0NlbGwgeworICAgIGNsYXNz IEpTU3RyaW5nIDogcHVibGljIEpTQ2VsbCB7CiAgICAgcHVibGljOgogICAgICAgICBmcmllbmQg Y2xhc3MgSklUOwogICAgICAgICBmcmllbmQgY2xhc3MgSlNHbG9iYWxEYXRhOwo=