115448 2013-04-30 15:06:29 -0700 ipc-posix-shm backwards compatibility 2013-05-01 21:49:07 -0700 1 1 1 Unclassified WebKit WebKit2 528+ (Nightly build) Mac OS X 10.7 RESOLVED FIXED InRadar P2 Major --- 0 scooper webkit-unassigned ap commit-queue mrowe scooper oldest_to_newest 883241 0 scooper 2013-04-30 15:06:29 -0700 Need backwards compatibility for older ipc-posix-shm operations. 883357 1 200194 scooper 2013-04-30 17:35:28 -0700 Created attachment 200194 Patch 883387 2 200194 ap 2013-04-30 22:26:46 -0700 Comment on attachment 200194 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=200194&action=review r=me > Source/WebKit2/Resources/PlugInSandboxProfiles/com.apple.WebKit.plugin-common.sb:250 > +(if (not (defined? 'ipc-posix-shm*)) > + (define ipc-posix-shm* ipc-posix-shm)) I wish we pre-processed plug-in sandbox profile files, so that we could do conditional compilation. With an #if check, removing obsolete code is much more straightforward. Also, we wouldn't have to merge parts at runtime, as we could simply #include "com.apple.WebKit.plugin-common.sb", and let preprocessor build a complete profile. 883389 3 200194 commit-queue 2013-04-30 23:07:57 -0700 Comment on attachment 200194 Patch Clearing flags on attachment: 200194 Committed r149427: <http://trac.webkit.org/changeset/149427> 883390 4 commit-queue 2013-04-30 23:08:00 -0700 All reviewed patches have been landed. Closing bug. 883541 5 scooper 2013-05-01 14:48:20 -0700 (In reply to comment #2) > (From update of attachment 200194 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=200194&action=review > > r=me > > > Source/WebKit2/Resources/PlugInSandboxProfiles/com.apple.WebKit.plugin-common.sb:250 > > +(if (not (defined? 'ipc-posix-shm*)) > > + (define ipc-posix-shm* ipc-posix-shm)) > > I wish we pre-processed plug-in sandbox profile files, so that we could do conditional compilation. With an #if check, removing obsolete code is much more straightforward. This isn't obsolete code -- it provides backwards compatibility. > Also, we wouldn't have to merge parts at runtime, as we could simply #include "com.apple.WebKit.plugin-common.sb", and let preprocessor build a complete profile. This is not a good idea. - This will not work if the AppleConnect profile is moved to a different project. - Forcing cpp preprocessing means that some sort of "compilation step" has to occur before passing to sandbox which makes it harder to develop and debug profiles. 883551 6 ap 2013-05-01 15:02:34 -0700 > This isn't obsolete code -- it provides backwards compatibility. It will be obsolete relatively soon, at which point we'll certainly forget to remove it, because there is no indication in code about when it can be removed. We know this currently, and being good forward looking engineers, we can pave the path now. Conditional compilation macros is how we do this in other code, so this would be the most straightforward way for profiles too. > - This will not work if the AppleConnect profile is moved to a different project. I can't agree with that. In fact, it's the opposite - having half of the profile in WebKit and another half in a different project is not workable long term. > - Forcing cpp preprocessing means that some sort of "compilation step" has to occur before passing to sandbox which makes it harder to develop and debug profiles. We perform preprocessing for some of the profiles, and haven't had any issues with that yet. 883699 7 scooper 2013-05-01 18:20:04 -0700 (In reply to comment #6) > > This isn't obsolete code -- it provides backwards compatibility. > > It will be obsolete relatively soon, at which point we'll certainly forget to remove it, because there is no indication in code about when it can be removed. We know this currently, and being good forward looking engineers, we can pave the path now. If that is the issue, it can certainly be addressed. What specifically do you need to see? 883719 8 ap 2013-05-01 21:49:07 -0700 CC'ing Mark, as I think that he is the one who most frequently does such cleanup. I _think_ that preprocessor checks like "#if __MAC_OS_X_VERSION_MIN_REQUIRED == 1070" would make it easiest. Alternatively, a FIXME comment explaining when the code can be removed would help us do that eventually, even if with a bit more manual work. 200194 2013-04-30 17:35:28 -0700 2013-04-30 23:07:57 -0700 Patch bug-115448-20130430173442.patch text/plain 1854 scooper U3VidmVyc2lvbiBSZXZpc2lvbjogMTQ5MjE3CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViS2l0Mi9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViS2l0Mi9DaGFuZ2VMb2cKaW5kZXggYTQ3MDE4ZjVkYzcwY2M0 ZTVhZmNjYjZjM2ZmY2FjOWQ2ZjE3OWU3Ny4uYjViNGYxZWY4YTY2ZWM2Njc5MzQzMzZjMTZiNjg5 ZmIwODcwMDUxZSAxMDA2NDQKLS0tIGEvU291cmNlL1dlYktpdDIvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJLaXQyL0NoYW5nZUxvZwpAQCAtMSwzICsxLDE2IEBACisyMDEzLTA0LTMwICBTaW1v biBDb29wZXIgIDxzY29vcGVyQGFwcGxlLmNvbT4KKworICAgICAgICBpcGMtcG9zaXgtc2htIGJh Y2t3YXJkcyBjb21wYXRpYmlsaXR5IAorICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9z aG93X2J1Zy5jZ2k/aWQ9MTE1NDQ4CisgICAgICAgIDxyZGFyOi8vcHJvYmxlbS8xMzc0MzAzNz4K KworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICBBZGQgY29t cGF0aWJpbGl0eSBmb3Igb2xkZXIgaXBjLXBvc2l4LXNobSBvcGVyYXRpb25zLCBidXQgcHJlZmVy IHRoZQorICAgICAgICBuZXdlciBuYW1lcyB3aGVuIHdyaXRpbmcgcHJvZmlsZXMuCisKKyAgICAg ICAgKiBSZXNvdXJjZXMvUGx1Z0luU2FuZGJveFByb2ZpbGVzL2NvbS5hcHBsZS5XZWJLaXQucGx1 Z2luLWNvbW1vbi5zYjoKKwogMjAxMy0wNC0yNiAgU2ltb24gQ29vcGVyICA8c2Nvb3BlckBhcHBs ZS5jb20+CiAKICAgICAgICAgUGVybWl0IH4vTGlicmFyeSBvciB+L0xpYnJhcnkvUHJlZmVyZW5j ZXMgdG8gYmUgc3ltbGlua3MKZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJLaXQyL1Jlc291cmNlcy9Q bHVnSW5TYW5kYm94UHJvZmlsZXMvY29tLmFwcGxlLldlYktpdC5wbHVnaW4tY29tbW9uLnNiIGIv U291cmNlL1dlYktpdDIvUmVzb3VyY2VzL1BsdWdJblNhbmRib3hQcm9maWxlcy9jb20uYXBwbGUu V2ViS2l0LnBsdWdpbi1jb21tb24uc2IKaW5kZXggZGM0MjU0M2M5Yzc1MjRmOTg2NWJkZDI1MDEw OGM5YTg1ZDBkZWZiNS4uZjZhNDIyZTFhM2JkMDFlYWU3OWNhNjBiY2ZiNjZhMDczMzg5OTY1YyAx MDA2NDQKLS0tIGEvU291cmNlL1dlYktpdDIvUmVzb3VyY2VzL1BsdWdJblNhbmRib3hQcm9maWxl cy9jb20uYXBwbGUuV2ViS2l0LnBsdWdpbi1jb21tb24uc2IKKysrIGIvU291cmNlL1dlYktpdDIv UmVzb3VyY2VzL1BsdWdJblNhbmRib3hQcm9maWxlcy9jb20uYXBwbGUuV2ViS2l0LnBsdWdpbi1j b21tb24uc2IKQEAgLTI0NiwxMiArMjQ2LDE4IEBACiAoZGVmaW5lICh3ZWJraXQtaW1hZ2VjYXB0 dXJlKQogICAgIChhbGxvdyBhcHBsZWV2ZW50LXNlbmQgKGFwcGxlZXZlbnQtZGVzdGluYXRpb24g ImNvbS5hcHBsZS5pbWFnZWNhcHR1cmVleHRlbnNpb24yIikpKQogCisoaWYgKG5vdCAoZGVmaW5l ZD8gJ2lwYy1wb3NpeC1zaG0qKSkKKyAgICAoZGVmaW5lIGlwYy1wb3NpeC1zaG0qIGlwYy1wb3Np eC1zaG0pKQorCiAoYWxsb3cgaXBjLXBvc2l4LXNobSoKICAgICAoaXBjLXBvc2l4LW5hbWUtcmVn ZXggIyJeQXVkaW9JTyIpCiAgICAgKGlwYy1wb3NpeC1uYW1lLXJlZ2V4ICMiXkNGUEJTOiIpCiAg ICAgKGlwYy1wb3NpeC1uYW1lICJfQ1NfR1NITUVNTE9DSyIpCiAgICAgKGlwYy1wb3NpeC1uYW1l ICJfQ1NfRFNITUVNTE9DSyIpKQogCisoaWYgKG5vdCAoZGVmaW5lZD8gJ2lwYy1wb3NpeC1zaG0t cmVhZCopKQorICAgIChkZWZpbmUgaXBjLXBvc2l4LXNobS1yZWFkKiBpcGMtcG9zaXgtc2htKSkK KwogKGFsbG93IGlwYy1wb3NpeC1zaG0tcmVhZCoKICAgICAoaXBjLXBvc2l4LW5hbWUgImFwcGxl LnNobS5ub3RpZmljYXRpb25fY2VudGVyIikpCiAK