168732 2017-02-22 10:42:24 -0800 Move instance members from WebSelectionRect.h to implementation file 2017-02-22 15:14:50 -0800 1 1 1 Unclassified WebKit WebKit Misc. Other Unspecified Unspecified RESOLVED FIXED P2 Normal --- 1 aakash_jain aakash_jain aakash_jain ap commit-queue juergen mitz thorton oldest_to_newest 1279958 0 aakash_jain 2017-02-22 10:42:24 -0800 There are few symbols in WebSelectionRect interface in WebKit/ios/WebCoreSupport/WebSelectionRect.h and are not exported. We should export these symbols so that the headers matches with the library. 1279959 1 302413 aakash_jain 2017-02-22 10:44:05 -0800 Created attachment 302413 Proposed patch 1279965 2 thorton 2017-02-22 10:56:06 -0800 If they're not exported, does that mean that nobody is using them, so maybe they shouldn't be in the header? 1279966 3 aakash_jain 2017-02-22 11:01:52 -0800 Yes, I believe that's another option. I'm not sure which option is better. 1279971 4 ap 2017-02-22 11:11:53 -0800 This seems like an issue for many classes. Even for those that hide instance variables in _private, we still have to export _private. I wonder if there is a better way to explain to the toolchain that private instance variables don't need symbols exported for them. 1279980 5 thorton 2017-02-22 11:53:03 -0800 (In reply to comment #4) > This seems like an issue for many classes. Even for those that hide instance > variables in _private, we still have to export _private. > > I wonder if there is a better way to explain to the toolchain that private > instance variables don't need symbols exported for them. Can we not move instance members to the implementation for iOS-only files (can't do it for things built on Mac because of the modern runtime), for some reason? Then, do we have to export anything? 1280006 6 302427 aakash_jain 2017-02-22 13:01:13 -0800 Created attachment 302427 Updated patch 1280007 7 aakash_jain 2017-02-22 13:02:23 -0800 (In reply to comment #5) > Can we not move instance members to the implementation for iOS-only files > (can't do it for things built on Mac because of the modern runtime), for > some reason? Then, do we have to export anything? Yes, I believe that would be clean approach. Patch updated. 1280018 8 302427 thorton 2017-02-22 13:26:40 -0800 Comment on attachment 302427 Updated patch View in context: https://bugs.webkit.org/attachment.cgi?id=302427&action=review > Source/WebKit/ios/WebCoreSupport/WebSelectionRect.h:32 > @interface WebSelectionRect : NSObject <NSCopying> { All of the comments from bug 168705 apply here too! 1280034 9 302437 aakash_jain 2017-02-22 13:51:41 -0800 Created attachment 302437 Updated patch remove extra {} and added ivars to @implementation. 1280072 10 302437 commit-queue 2017-02-22 15:14:46 -0800 Comment on attachment 302437 Updated patch Clearing flags on attachment: 302437 Committed r212856: <http://trac.webkit.org/changeset/212856> 1280073 11 commit-queue 2017-02-22 15:14:50 -0800 All reviewed patches have been landed. Closing bug. 302413 2017-02-22 10:44:05 -0800 2017-02-22 13:01:13 -0800 Proposed patch patch_WebSelectionRect text/plain 1350 aakash_jain SW5kZXg6IFNvdXJjZS9XZWJLaXQvaW9zL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2Uv V2ViS2l0L2lvcy9DaGFuZ2VMb2cJKHJldmlzaW9uIDIxMjg0MSkKKysrIFNvdXJjZS9XZWJLaXQv aW9zL0NoYW5nZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDEyIEBACisyMDE3LTAyLTIy ICBBYWthc2ggSmFpbiAgPGFha2FzaF9qYWluQGFwcGxlLmNvbT4KKworICAgICAgICBFeHBvcnQg dmFyaW91cyBzeW1ib2xzIGZyb20gV2ViU2VsZWN0aW9uUmVjdC5oCisgICAgICAgIGh0dHBzOi8v YnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD0xNjg3MzIKKworICAgICAgICBSZXZpZXdl ZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICAqIFdlYktpdC5pT1MuZXhwOgorCiAyMDE3 LTAyLTIyICBBYWthc2ggSmFpbiAgPGFha2FzaF9qYWluQGFwcGxlLmNvbT4KIAogICAgICAgICBN b3ZlIGlPUyBzcGVjaWZpYyBzeW1ib2xzIHRvIFdlYktpdC5pb3MuZXhwCkluZGV4OiBTb3VyY2Uv V2ViS2l0L2lvcy9XZWJLaXQuaU9TLmV4cAo9PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2ViS2l0L2lv cy9XZWJLaXQuaU9TLmV4cAkocmV2aXNpb24gMjEyODMzKQorKysgU291cmNlL1dlYktpdC9pb3Mv V2ViS2l0LmlPUy5leHAJKHdvcmtpbmcgY29weSkKQEAgLTQ3LDMgKzQ3LDEzIEBAIF9fV2ViVmll d1JlbW90ZUluc3BlY3Rvckhhc1Nlc3Npb25DaGFuZ2UKIAogIyBXZWJLaXQxIGludGVybmFsIHR5 cGVzIGV4cG9zZWQgZm9yIFdlYktpdDIuCiAub2JqY19jbGFzc19uYW1lX1dlYkdlb2xvY2F0aW9u Q29yZUxvY2F0aW9uUHJvdmlkZXIKKworX09CSkNfSVZBUl8kX1dlYlNlbGVjdGlvblJlY3QubV9j b250YWluc0VuZAorX09CSkNfSVZBUl8kX1dlYlNlbGVjdGlvblJlY3QubV9jb250YWluc1N0YXJ0 CitfT0JKQ19JVkFSXyRfV2ViU2VsZWN0aW9uUmVjdC5tX2lzRmlyc3RPbkxpbmUKK19PQkpDX0lW QVJfJF9XZWJTZWxlY3Rpb25SZWN0Lm1faXNIb3Jpem9udGFsCitfT0JKQ19JVkFSXyRfV2ViU2Vs ZWN0aW9uUmVjdC5tX2lzSW5GaXhlZFBvc2l0aW9uCitfT0JKQ19JVkFSXyRfV2ViU2VsZWN0aW9u UmVjdC5tX2lzTGFzdE9uTGluZQorX09CSkNfSVZBUl8kX1dlYlNlbGVjdGlvblJlY3QubV9pc0xp bmVCcmVhaworX09CSkNfSVZBUl8kX1dlYlNlbGVjdGlvblJlY3QubV9yZWN0CitfT0JKQ19JVkFS XyRfV2ViU2VsZWN0aW9uUmVjdC5tX3dyaXRpbmdEaXJlY3Rpb24K 302427 2017-02-22 13:01:13 -0800 2017-02-22 13:51:41 -0800 Updated patch patch_WebSelectionRect_v2 text/plain 2069 aakash_jain SW5kZXg6IFNvdXJjZS9XZWJLaXQvaW9zL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2Uv V2ViS2l0L2lvcy9DaGFuZ2VMb2cJKHJldmlzaW9uIDIxMjg0NykKKysrIFNvdXJjZS9XZWJLaXQv aW9zL0NoYW5nZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDEzIEBACisyMDE3LTAyLTIy ICBBYWthc2ggSmFpbiAgPGFha2FzaF9qYWluQGFwcGxlLmNvbT4KKworICAgICAgICBNb3ZlIGlu c3RhbmNlIG1lbWJlcnMgZnJvbSBXZWJTZWxlY3Rpb25SZWN0LmggdG8gaW1wbGVtZW50YXRpb24g ZmlsZQorICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9MTY4 NzMyCisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgKiBX ZWJDb3JlU3VwcG9ydC9XZWJTZWxlY3Rpb25SZWN0Lmg6IE1vdmUgaW5zdGFuY2UgbWVtYmVycyB0 byBpbXBsZW1lbnRhdGlvbiBmaWxlLgorICAgICAgICAqIFdlYkNvcmVTdXBwb3J0L1dlYlNlbGVj dGlvblJlY3QubTogRGl0dG8uCisKIDIwMTctMDItMjIgIEFha2FzaCBKYWluICA8YWFrYXNoX2ph aW5AYXBwbGUuY29tPgogCiAgICAgICAgIE1vdmUgaU9TIHNwZWNpZmljIHN5bWJvbHMgdG8gV2Vi S2l0Lmlvcy5leHAKSW5kZXg6IFNvdXJjZS9XZWJLaXQvaW9zL1dlYkNvcmVTdXBwb3J0L1dlYlNl bGVjdGlvblJlY3QuaAo9PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2ViS2l0L2lvcy9XZWJDb3JlU3Vw cG9ydC9XZWJTZWxlY3Rpb25SZWN0LmgJKHJldmlzaW9uIDIxMjg0NCkKKysrIFNvdXJjZS9XZWJL aXQvaW9zL1dlYkNvcmVTdXBwb3J0L1dlYlNlbGVjdGlvblJlY3QuaAkod29ya2luZyBjb3B5KQpA QCAtMzAsMTUgKzMwLDYgQEAKICNpbXBvcnQgPFdlYktpdExlZ2FjeS9XZWJGcmFtZUlPUy5oPgog CiBAaW50ZXJmYWNlIFdlYlNlbGVjdGlvblJlY3QgOiBOU09iamVjdCA8TlNDb3B5aW5nPiB7Ci0g ICAgQ0dSZWN0IG1fcmVjdDsKLSAgICBXS1dyaXRpbmdEaXJlY3Rpb24gbV93cml0aW5nRGlyZWN0 aW9uOwotICAgIEJPT0wgbV9pc0xpbmVCcmVhazsKLSAgICBCT09MIG1faXNGaXJzdE9uTGluZTsK LSAgICBCT09MIG1faXNMYXN0T25MaW5lOwotICAgIEJPT0wgbV9jb250YWluc1N0YXJ0OwotICAg IEJPT0wgbV9jb250YWluc0VuZDsKLSAgICBCT09MIG1faXNJbkZpeGVkUG9zaXRpb247Ci0gICAg Qk9PTCBtX2lzSG9yaXpvbnRhbDsKIH0KIAogQHByb3BlcnR5IChub25hdG9taWMsIGFzc2lnbikg Q0dSZWN0IHJlY3Q7CkluZGV4OiBTb3VyY2UvV2ViS2l0L2lvcy9XZWJDb3JlU3VwcG9ydC9XZWJT ZWxlY3Rpb25SZWN0Lm0KPT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gU291cmNlL1dlYktpdC9pb3MvV2ViQ29yZVN1 cHBvcnQvV2ViU2VsZWN0aW9uUmVjdC5tCShyZXZpc2lvbiAyMTI4NDQpCisrKyBTb3VyY2UvV2Vi S2l0L2lvcy9XZWJDb3JlU3VwcG9ydC9XZWJTZWxlY3Rpb25SZWN0Lm0JKHdvcmtpbmcgY29weSkK QEAgLTI3LDYgKzI3LDIxIEBACiAKICNpbXBvcnQgIldlYlNlbGVjdGlvblJlY3QuaCIKIAorQGlu dGVyZmFjZSBXZWJTZWxlY3Rpb25SZWN0ICgpCit7CisgICAgQ0dSZWN0IG1fcmVjdDsKKyAgICBX S1dyaXRpbmdEaXJlY3Rpb24gbV93cml0aW5nRGlyZWN0aW9uOworICAgIEJPT0wgbV9pc0xpbmVC cmVhazsKKyAgICBCT09MIG1faXNGaXJzdE9uTGluZTsKKyAgICBCT09MIG1faXNMYXN0T25MaW5l OworICAgIEJPT0wgbV9jb250YWluc1N0YXJ0OworICAgIEJPT0wgbV9jb250YWluc0VuZDsKKyAg ICBCT09MIG1faXNJbkZpeGVkUG9zaXRpb247CisgICAgQk9PTCBtX2lzSG9yaXpvbnRhbDsKK30K KworQGVuZAorCiBAaW1wbGVtZW50YXRpb24gV2ViU2VsZWN0aW9uUmVjdAogCiBAc3ludGhlc2l6 ZSByZWN0ID0gbV9yZWN0Owo= 302437 2017-02-22 13:51:41 -0800 2017-02-22 15:14:46 -0800 Updated patch patch_WebSelectionRect_v3 text/plain 2269 aakash_jain SW5kZXg6IFNvdXJjZS9XZWJLaXQvaW9zL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2Uv V2ViS2l0L2lvcy9DaGFuZ2VMb2cJKHJldmlzaW9uIDIxMjg0OSkKKysrIFNvdXJjZS9XZWJLaXQv aW9zL0NoYW5nZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDEzIEBACisyMDE3LTAyLTIy ICBBYWthc2ggSmFpbiAgPGFha2FzaF9qYWluQGFwcGxlLmNvbT4KKworICAgICAgICBNb3ZlIGlu c3RhbmNlIG1lbWJlcnMgZnJvbSBXZWJTZWxlY3Rpb25SZWN0LmggdG8gaW1wbGVtZW50YXRpb24g ZmlsZQorICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9MTY4 NzMyCisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgKiBX ZWJDb3JlU3VwcG9ydC9XZWJTZWxlY3Rpb25SZWN0Lmg6IE1vdmluZyBpbnN0YW5jZSBtZW1iZXJz IHRvIGltcGxlbWVudGF0aW9uIGZpbGUuCisgICAgICAgICogV2ViQ29yZVN1cHBvcnQvV2ViU2Vs ZWN0aW9uUmVjdC5tOiBEaXR0by4KKwogMjAxNy0wMi0yMiAgQWFrYXNoIEphaW4gIDxhYWthc2hf amFpbkBhcHBsZS5jb20+CiAKICAgICAgICAgTW92ZSBpT1Mgc3BlY2lmaWMgc3ltYm9scyB0byBX ZWJLaXQuaW9zLmV4cApJbmRleDogU291cmNlL1dlYktpdC9pb3MvV2ViQ29yZVN1cHBvcnQvV2Vi U2VsZWN0aW9uUmVjdC5oCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFNvdXJjZS9XZWJLaXQvaW9zL1dlYkNvcmVT dXBwb3J0L1dlYlNlbGVjdGlvblJlY3QuaAkocmV2aXNpb24gMjEyODQ0KQorKysgU291cmNlL1dl YktpdC9pb3MvV2ViQ29yZVN1cHBvcnQvV2ViU2VsZWN0aW9uUmVjdC5oCSh3b3JraW5nIGNvcHkp CkBAIC0yOSwxNyArMjksNyBAQAogI2ltcG9ydCA8Rm91bmRhdGlvbi9Gb3VuZGF0aW9uLmg+CiAj aW1wb3J0IDxXZWJLaXRMZWdhY3kvV2ViRnJhbWVJT1MuaD4KIAotQGludGVyZmFjZSBXZWJTZWxl Y3Rpb25SZWN0IDogTlNPYmplY3QgPE5TQ29weWluZz4gewotICAgIENHUmVjdCBtX3JlY3Q7Ci0g ICAgV0tXcml0aW5nRGlyZWN0aW9uIG1fd3JpdGluZ0RpcmVjdGlvbjsKLSAgICBCT09MIG1faXNM aW5lQnJlYWs7Ci0gICAgQk9PTCBtX2lzRmlyc3RPbkxpbmU7Ci0gICAgQk9PTCBtX2lzTGFzdE9u TGluZTsKLSAgICBCT09MIG1fY29udGFpbnNTdGFydDsKLSAgICBCT09MIG1fY29udGFpbnNFbmQ7 Ci0gICAgQk9PTCBtX2lzSW5GaXhlZFBvc2l0aW9uOwotICAgIEJPT0wgbV9pc0hvcml6b250YWw7 Ci19CitAaW50ZXJmYWNlIFdlYlNlbGVjdGlvblJlY3QgOiBOU09iamVjdCA8TlNDb3B5aW5nPgog CiBAcHJvcGVydHkgKG5vbmF0b21pYywgYXNzaWduKSBDR1JlY3QgcmVjdDsKIEBwcm9wZXJ0eSAo bm9uYXRvbWljLCBhc3NpZ24pIFdLV3JpdGluZ0RpcmVjdGlvbiB3cml0aW5nRGlyZWN0aW9uOwpJ bmRleDogU291cmNlL1dlYktpdC9pb3MvV2ViQ29yZVN1cHBvcnQvV2ViU2VsZWN0aW9uUmVjdC5t Cj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT0KLS0tIFNvdXJjZS9XZWJLaXQvaW9zL1dlYkNvcmVTdXBwb3J0L1dlYlNlbGVj dGlvblJlY3QubQkocmV2aXNpb24gMjEyODQ0KQorKysgU291cmNlL1dlYktpdC9pb3MvV2ViQ29y ZVN1cHBvcnQvV2ViU2VsZWN0aW9uUmVjdC5tCSh3b3JraW5nIGNvcHkpCkBAIC0yNyw3ICsyNywx NyBAQAogCiAjaW1wb3J0ICJXZWJTZWxlY3Rpb25SZWN0LmgiCiAKLUBpbXBsZW1lbnRhdGlvbiBX ZWJTZWxlY3Rpb25SZWN0CitAaW1wbGVtZW50YXRpb24gV2ViU2VsZWN0aW9uUmVjdCB7CisgICAg Q0dSZWN0IG1fcmVjdDsKKyAgICBXS1dyaXRpbmdEaXJlY3Rpb24gbV93cml0aW5nRGlyZWN0aW9u OworICAgIEJPT0wgbV9pc0xpbmVCcmVhazsKKyAgICBCT09MIG1faXNGaXJzdE9uTGluZTsKKyAg ICBCT09MIG1faXNMYXN0T25MaW5lOworICAgIEJPT0wgbV9jb250YWluc1N0YXJ0OworICAgIEJP T0wgbV9jb250YWluc0VuZDsKKyAgICBCT09MIG1faXNJbkZpeGVkUG9zaXRpb247CisgICAgQk9P TCBtX2lzSG9yaXpvbnRhbDsKK30KIAogQHN5bnRoZXNpemUgcmVjdCA9IG1fcmVjdDsKIEBzeW50 aGVzaXplIHdyaXRpbmdEaXJlY3Rpb24gPSBtX3dyaXRpbmdEaXJlY3Rpb247Cg==