136473 2014-09-02 18:34:11 -0700 Avoid warning if a process does not have access to com.apple.webinspector 2014-09-03 15:50:29 -0700 1 1 1 Unclassified WebKit Web Inspector 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED InRadar P2 Normal --- 1 joepeck webkit-unassigned ap commit-queue graouts joepeck timothy webkit-bug-importer oldest_to_newest 1032813 0 joepeck 2014-09-02 18:34:11 -0700 Some sandboxed processes will warn that they do not have access to the com.apple.webinspector mach port. Applications will automatically get the JavaScriptCore framework sandbox extensions to access the port but it seems some processes don't. In such cases the sandbox warning in syslog can be avoided because it is just noise. <rdar://problem/16647344> 1032814 1 237539 joepeck 2014-09-02 18:35:57 -0700 Created attachment 237539 [PATCH] Proposed Fix Still don't know how I feel about this but here is the patch. 1032815 2 commit-queue 2014-09-02 18:37:29 -0700 Attachment 237539 did not pass style-queue: ERROR: Source/JavaScriptCore/inspector/remote/RemoteInspector.mm:69: Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons. [readability/comparison_to_zero] [5] Total errors found: 1 in 2 files If any of these errors are false positives, please file a bug against check-webkit-style. 1032820 3 237539 ap 2014-09-02 18:54:43 -0700 Comment on attachment 237539 [PATCH] Proposed Fix View in context: https://bugs.webkit.org/attachment.cgi?id=237539&action=review > Source/JavaScriptCore/inspector/remote/RemoteInspector.mm:59 > +#if __has_include(<sandbox/private.h>) > +#import <sandbox/private.h> > +#else > +enum sandbox_filter_type { > + SANDBOX_FILTER_GLOBAL_NAME, > +}; > +extern "C" { > +int sandbox_check(pid_t, const char *operation, enum sandbox_filter_type, ...); > +} > +#endif Sandbox SPIs seem like a prime target for the new technique Maciej is suggesting, as we use them in many places. Instead of declaring the SPIs in each .mm file, they would go into SandboxSPI.h. In any case, sandbox_check declaration should not be inside the #else. 1032941 4 joepeck 2014-09-03 11:16:48 -0700 (In reply to comment #3) > (From update of attachment 237539 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=237539&action=review > > > Source/JavaScriptCore/inspector/remote/RemoteInspector.mm:59 > > +#if __has_include(<sandbox/private.h>) > > +#import <sandbox/private.h> > > +#else > > +enum sandbox_filter_type { > > + SANDBOX_FILTER_GLOBAL_NAME, > > +}; > > +extern "C" { > > +int sandbox_check(pid_t, const char *operation, enum sandbox_filter_type, ...); > > +} > > +#endif > > Sandbox SPIs seem like a prime target for the new technique Maciej is suggesting, as we use them in many places. Instead of declaring the SPIs in each .mm file, they would go into SandboxSPI.h. I suppose I could. This would be the first in JavaScriptCore so I opted for the easier path. > In any case, sandbox_check declaration should not be inside the #else. I don't follow. Why not? 1032944 5 ap 2014-09-03 11:27:00 -0700 > > In any case, sandbox_check declaration should not be inside the #else. > > I don't follow. Why not? This way, we'll get a compile error internally if the function signature changes in the future. That's much better than having to chase a mysterious behavior difference between internal and open source builds. 1032948 6 joepeck 2014-09-03 11:43:28 -0700 (In reply to comment #5) > > > In any case, sandbox_check declaration should not be inside the #else. > > > > I don't follow. Why not? > > This way, we'll get a compile error internally if the function signature changes in the future. That's much better than having to chase a mysterious behavior difference between internal and open source builds. Internally we should just get the include, so if the signature changes we would fail immediately. If the file changes, then we'd be out of luck. #if __has_include(<sandbox/private.h>) #import <sandbox/private.h> ... I think we have to put some signature in the else, so that open source developers can compile the file. This pattern is repeated in a couple places (WebKit2). 1032964 7 ap 2014-09-03 12:05:22 -0700 > if the signature changes we would fail immediately This depends on how the signature changes - it can be source compatible, but not binary compatible. Having an unconditional declaration after the conditional include would catch that. 1033048 8 237587 joepeck 2014-09-03 15:08:27 -0700 Created attachment 237587 [PATCH] For Landing 1033066 9 237587 commit-queue 2014-09-03 15:50:26 -0700 Comment on attachment 237587 [PATCH] For Landing Clearing flags on attachment: 237587 Committed r173238: <http://trac.webkit.org/changeset/173238> 1033067 10 commit-queue 2014-09-03 15:50:29 -0700 All reviewed patches have been landed. Closing bug. 237539 2014-09-02 18:35:57 -0700 2014-09-03 15:08:27 -0700 [PATCH] Proposed Fix sandbox_check.patch text/plain 2207 joepeck ZGlmZiAtLWdpdCBhL1NvdXJjZS9KYXZhU2NyaXB0Q29yZS9DaGFuZ2VMb2cgYi9Tb3VyY2UvSmF2 YVNjcmlwdENvcmUvQ2hhbmdlTG9nCmluZGV4IGU5MmMxNWUuLmQyODg2MTggMTAwNjQ0Ci0tLSBh L1NvdXJjZS9KYXZhU2NyaXB0Q29yZS9DaGFuZ2VMb2cKKysrIGIvU291cmNlL0phdmFTY3JpcHRD b3JlL0NoYW5nZUxvZwpAQCAtMSwzICsxLDE3IEBACisyMDE0LTA5LTAyICBKb3NlcGggUGVjb3Jh cm8gIDxwZWNvcmFyb0BhcHBsZS5jb20+CisKKyAgICAgICAgQXZvaWQgd2FybmluZyBpZiBhIHBy b2Nlc3MgZG9lcyBub3QgaGF2ZSBhY2Nlc3MgdG8gY29tLmFwcGxlLndlYmluc3BlY3RvcgorICAg ICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9MTM2NDczCisKKyAg ICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgUHJlLWNoZWNrIGZv ciBhY2Nlc3MgdG8gdGhlIG1hY2ggcG9ydCB0byBhdm9pZCBlbWl0dGluZyB3YXJuaW5ncworICAg ICAgICBpbiBzeXNsb2cgZm9yIHByb2Nlc3NlcyB0aGF0IGRvIG5vdCBoYXZlIGFjY2Vzcy4KKwor ICAgICAgICAqIGluc3BlY3Rvci9yZW1vdGUvUmVtb3RlSW5zcGVjdG9yLm1tOgorICAgICAgICAo SW5zcGVjdG9yOjpjYW5BY2Nlc3NXZWJJbnNwZWN0b3JNYWNoUG9ydCk6CisgICAgICAgIChJbnNw ZWN0b3I6OlJlbW90ZUluc3BlY3Rvcjo6c2hhcmVkKToKKwogMjAxNC0wOC0yOCAgQW5kcmVhcyBL bGluZyAgPGFrbGluZ0BhcHBsZS5jb20+CiAKICAgICAgICAgVXNlIEpTU3RyaW5nOjp0b0lkZW50 aWZpZXIoKSBpbiBtb3JlIHBsYWNlcy4KZGlmZiAtLWdpdCBhL1NvdXJjZS9KYXZhU2NyaXB0Q29y ZS9pbnNwZWN0b3IvcmVtb3RlL1JlbW90ZUluc3BlY3Rvci5tbSBiL1NvdXJjZS9KYXZhU2NyaXB0 Q29yZS9pbnNwZWN0b3IvcmVtb3RlL1JlbW90ZUluc3BlY3Rvci5tbQppbmRleCA0MGQ2ZjU0Li41 NThjMGNhIDEwMDY0NAotLS0gYS9Tb3VyY2UvSmF2YVNjcmlwdENvcmUvaW5zcGVjdG9yL3JlbW90 ZS9SZW1vdGVJbnNwZWN0b3IubW0KKysrIGIvU291cmNlL0phdmFTY3JpcHRDb3JlL2luc3BlY3Rv ci9yZW1vdGUvUmVtb3RlSW5zcGVjdG9yLm1tCkBAIC00NywxMiArNDcsMjggQEAgZXh0ZXJuICJD IiB7CiB9CiAjZW5kaWYKIAorI2lmIF9faGFzX2luY2x1ZGUoPHNhbmRib3gvcHJpdmF0ZS5oPikK KyNpbXBvcnQgPHNhbmRib3gvcHJpdmF0ZS5oPgorI2Vsc2UKK2VudW0gc2FuZGJveF9maWx0ZXJf dHlwZSB7CisgICAgU0FOREJPWF9GSUxURVJfR0xPQkFMX05BTUUsCit9OworZXh0ZXJuICJDIiB7 CitpbnQgc2FuZGJveF9jaGVjayhwaWRfdCwgY29uc3QgY2hhciAqb3BlcmF0aW9uLCBlbnVtIHNh bmRib3hfZmlsdGVyX3R5cGUsIC4uLik7Cit9CisjZW5kaWYKKwogI2lmIFBMQVRGT1JNKElPUykK ICNpbXBvcnQgPHd0Zi9pb3MvV2ViQ29yZVRocmVhZC5oPgogI2VuZGlmCiAKIG5hbWVzcGFjZSBJ bnNwZWN0b3IgewogCitzdGF0aWMgYm9vbCBjYW5BY2Nlc3NXZWJJbnNwZWN0b3JNYWNoUG9ydCgp Cit7CisgICAgcmV0dXJuIHNhbmRib3hfY2hlY2soZ2V0cGlkKCksICJtYWNoLWxvb2t1cCIsIFNB TkRCT1hfRklMVEVSX0dMT0JBTF9OQU1FLCBXSVJYUENNYWNoUG9ydE5hbWUpID09IDA7Cit9CisK IHN0YXRpYyB2b2lkIGRpc3BhdGNoQXN5bmNPblF1ZXVlU2FmZUZvckFueURlYnVnZ2FibGUodm9p ZCAoXmJsb2NrKSgpKQogewogI2lmIFBMQVRGT1JNKElPUykKQEAgLTc4LDkgKzk0LDExIEBAIFJl bW90ZUluc3BlY3RvciYgUmVtb3RlSW5zcGVjdG9yOjpzaGFyZWQoKQogCiAgICAgc3RhdGljIGRp c3BhdGNoX29uY2VfdCBvbmNlOwogICAgIGRpc3BhdGNoX29uY2UoJm9uY2UsIF57Ci0gICAgICAg IEpTQzo6aW5pdGlhbGl6ZVRocmVhZGluZygpOwotICAgICAgICBpZiAoUmVtb3RlSW5zcGVjdG9y OjpzdGFydEVuYWJsZWQpCi0gICAgICAgICAgICBzaGFyZWQuZ2V0KCkuc3RhcnQoKTsKKyAgICAg ICAgaWYgKGNhbkFjY2Vzc1dlYkluc3BlY3Rvck1hY2hQb3J0KCkpIHsKKyAgICAgICAgICAgIEpT Qzo6aW5pdGlhbGl6ZVRocmVhZGluZygpOworICAgICAgICAgICAgaWYgKFJlbW90ZUluc3BlY3Rv cjo6c3RhcnRFbmFibGVkKQorICAgICAgICAgICAgICAgIHNoYXJlZC5nZXQoKS5zdGFydCgpOwor ICAgICAgICB9CiAgICAgfSk7CiAKICAgICByZXR1cm4gc2hhcmVkOwo= 237587 2014-09-03 15:08:27 -0700 2014-09-03 15:50:26 -0700 [PATCH] For Landing sandbox-check-fix.patch text/plain 2440 joepeck ZGlmZiAtLWdpdCBhL1NvdXJjZS9KYXZhU2NyaXB0Q29yZS9DaGFuZ2VMb2cgYi9Tb3VyY2UvSmF2 YVNjcmlwdENvcmUvQ2hhbmdlTG9nCmluZGV4IGVjYTJlNDguLmJhNWZiNjIgMTAwNjQ0Ci0tLSBh L1NvdXJjZS9KYXZhU2NyaXB0Q29yZS9DaGFuZ2VMb2cKKysrIGIvU291cmNlL0phdmFTY3JpcHRD b3JlL0NoYW5nZUxvZwpAQCAtMSwzICsxLDE3IEBACisyMDE0LTA5LTAzICBKb3NlcGggUGVjb3Jh cm8gIDxwZWNvcmFyb0BhcHBsZS5jb20+CisKKyAgICAgICAgQXZvaWQgd2FybmluZyBpZiBhIHBy b2Nlc3MgZG9lcyBub3QgaGF2ZSBhY2Nlc3MgdG8gY29tLmFwcGxlLndlYmluc3BlY3RvcgorICAg ICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9MTM2NDczCisKKyAg ICAgICAgUmV2aWV3ZWQgYnkgQWxleGV5IFByb3NrdXJ5YWtvdi4KKworICAgICAgICBQcmUtY2hl Y2sgZm9yIGFjY2VzcyB0byB0aGUgbWFjaCBwb3J0IHRvIGF2b2lkIGVtaXR0aW5nIHdhcm5pbmdz CisgICAgICAgIGluIHN5c2xvZyBmb3IgcHJvY2Vzc2VzIHRoYXQgZG8gbm90IGhhdmUgYWNjZXNz LgorCisgICAgICAgICogaW5zcGVjdG9yL3JlbW90ZS9SZW1vdGVJbnNwZWN0b3IubW06CisgICAg ICAgIChJbnNwZWN0b3I6OmNhbkFjY2Vzc1dlYkluc3BlY3Rvck1hY2hQb3J0KToKKyAgICAgICAg KEluc3BlY3Rvcjo6UmVtb3RlSW5zcGVjdG9yOjpzaGFyZWQpOgorCiAyMDE0LTA5LTAzICBTYWFt IEJhcmF0aSAgPHNhYW1iYXJhdGkxQGdtYWlsLmNvbT4KIAogICAgICAgICBDcmVhdGUgdGVzdHMg Zm9yIHR5cGUgcHJvZmlsaW5nCmRpZmYgLS1naXQgYS9Tb3VyY2UvSmF2YVNjcmlwdENvcmUvaW5z cGVjdG9yL3JlbW90ZS9SZW1vdGVJbnNwZWN0b3IubW0gYi9Tb3VyY2UvSmF2YVNjcmlwdENvcmUv aW5zcGVjdG9yL3JlbW90ZS9SZW1vdGVJbnNwZWN0b3IubW0KaW5kZXggNDBkNmY1NC4uNjhmYWE4 MiAxMDA2NDQKLS0tIGEvU291cmNlL0phdmFTY3JpcHRDb3JlL2luc3BlY3Rvci9yZW1vdGUvUmVt b3RlSW5zcGVjdG9yLm1tCisrKyBiL1NvdXJjZS9KYXZhU2NyaXB0Q29yZS9pbnNwZWN0b3IvcmVt b3RlL1JlbW90ZUluc3BlY3Rvci5tbQpAQCAtNDAsMTIgKzQwLDIyIEBACiAKICNpZiBfX2hhc19p bmNsdWRlKDx4cGMveHBjLmg+KQogI2ltcG9ydCA8eHBjL3hwYy5oPgotI2Vsc2UKKyNlbmRpZgog ZXh0ZXJuICJDIiB7CiAgICAgeHBjX2Nvbm5lY3Rpb25fdCB4cGNfY29ubmVjdGlvbl9jcmVhdGVf bWFjaF9zZXJ2aWNlKGNvbnN0IGNoYXIqIG5hbWUsIGRpc3BhdGNoX3F1ZXVlX3QsIHVpbnQ2NF90 IGZsYWdzKTsKICAgICB2b2lkIHhwY19yZWxlYXNlKHhwY19vYmplY3RfdCk7CiB9CisKKyNpZiBf X2hhc19pbmNsdWRlKDxzYW5kYm94L3ByaXZhdGUuaD4pCisjaW1wb3J0IDxzYW5kYm94L3ByaXZh dGUuaD4KKyNlbHNlCitlbnVtIHNhbmRib3hfZmlsdGVyX3R5cGUgeworICAgIFNBTkRCT1hfRklM VEVSX0dMT0JBTF9OQU1FLAorfTsKICNlbmRpZgorZXh0ZXJuICJDIiB7CisgICAgaW50IHNhbmRi b3hfY2hlY2socGlkX3QsIGNvbnN0IGNoYXIgKm9wZXJhdGlvbiwgZW51bSBzYW5kYm94X2ZpbHRl cl90eXBlLCAuLi4pOworfQogCiAjaWYgUExBVEZPUk0oSU9TKQogI2ltcG9ydCA8d3RmL2lvcy9X ZWJDb3JlVGhyZWFkLmg+CkBAIC01Myw2ICs2MywxMSBAQCBleHRlcm4gIkMiIHsKIAogbmFtZXNw YWNlIEluc3BlY3RvciB7CiAKK3N0YXRpYyBib29sIGNhbkFjY2Vzc1dlYkluc3BlY3Rvck1hY2hQ b3J0KCkKK3sKKyAgICByZXR1cm4gc2FuZGJveF9jaGVjayhnZXRwaWQoKSwgIm1hY2gtbG9va3Vw IiwgU0FOREJPWF9GSUxURVJfR0xPQkFMX05BTUUsIFdJUlhQQ01hY2hQb3J0TmFtZSkgPT0gMDsK K30KKwogc3RhdGljIHZvaWQgZGlzcGF0Y2hBc3luY09uUXVldWVTYWZlRm9yQW55RGVidWdnYWJs ZSh2b2lkICheYmxvY2spKCkpCiB7CiAjaWYgUExBVEZPUk0oSU9TKQpAQCAtNzgsOSArOTMsMTEg QEAgUmVtb3RlSW5zcGVjdG9yJiBSZW1vdGVJbnNwZWN0b3I6OnNoYXJlZCgpCiAKICAgICBzdGF0 aWMgZGlzcGF0Y2hfb25jZV90IG9uY2U7CiAgICAgZGlzcGF0Y2hfb25jZSgmb25jZSwgXnsKLSAg ICAgICAgSlNDOjppbml0aWFsaXplVGhyZWFkaW5nKCk7Ci0gICAgICAgIGlmIChSZW1vdGVJbnNw ZWN0b3I6OnN0YXJ0RW5hYmxlZCkKLSAgICAgICAgICAgIHNoYXJlZC5nZXQoKS5zdGFydCgpOwor ICAgICAgICBpZiAoY2FuQWNjZXNzV2ViSW5zcGVjdG9yTWFjaFBvcnQoKSkgeworICAgICAgICAg ICAgSlNDOjppbml0aWFsaXplVGhyZWFkaW5nKCk7CisgICAgICAgICAgICBpZiAoUmVtb3RlSW5z cGVjdG9yOjpzdGFydEVuYWJsZWQpCisgICAgICAgICAgICAgICAgc2hhcmVkLmdldCgpLnN0YXJ0 KCk7CisgICAgICAgIH0KICAgICB9KTsKIAogICAgIHJldHVybiBzaGFyZWQ7Cg==