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>
Created attachment 237539 [details] [PATCH] Proposed Fix Still don't know how I feel about this but here is the patch.
Attachment 237539 [details] 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.
Comment on attachment 237539 [details] [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.
(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?
> > 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.
(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).
> 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.
Created attachment 237587 [details] [PATCH] For Landing
Comment on attachment 237587 [details] [PATCH] For Landing Clearing flags on attachment: 237587 Committed r173238: <http://trac.webkit.org/changeset/173238>
All reviewed patches have been landed. Closing bug.