On macOS, stop logging mach-lookup sandbox violations of com.apple.CoreDisplay.Notification for performance reasons.
Created attachment 407342 [details] Patch
<rdar://problem/67831223>
Created attachment 407353 [details] Patch
Comment on attachment 407353 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=407353&action=review > Source/WebKit/ChangeLog:8 > + On macOS, stop logging mach-lookup sandbox violations of com.apple.CoreDisplay.Notification for performance reasons. This makes it sound like it’s something already denied, and this patch simply changes it to no-log. > Source/WebKit/WebProcess/com.apple.WebProcess.sb.in:771 > +#if __MAC_OS_X_VERSION_MIN_REQUIRED >= 110000 > +(deny mach-lookup (with no-log) > + (global-name "com.apple.CoreDisplay.Notification)) > +#endif But this is a new deny rule, not changing an existing one to no-log mode. What am I missing?
(In reply to Darin Adler from comment #4) > Comment on attachment 407353 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=407353&action=review > > > Source/WebKit/ChangeLog:8 > > + On macOS, stop logging mach-lookup sandbox violations of com.apple.CoreDisplay.Notification for performance reasons. > > This makes it sound like it’s something already denied, and this patch > simply changes it to no-log. > Yes, that is correct. > > Source/WebKit/WebProcess/com.apple.WebProcess.sb.in:771 > > +#if __MAC_OS_X_VERSION_MIN_REQUIRED >= 110000 > > +(deny mach-lookup (with no-log) > > + (global-name "com.apple.CoreDisplay.Notification)) > > +#endif > > But this is a new deny rule, not changing an existing one to no-log mode. > > What am I missing? Yes, you are right. There is no previous rule for this service, so the default behavior is in effect, where the service is being denied, but with logging enabled. This change is only changing the logging behavior. Thanks for reviewing!
Comment on attachment 407353 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=407353&action=review >>> Source/WebKit/WebProcess/com.apple.WebProcess.sb.in:771 >>> +#endif >> >> But this is a new deny rule, not changing an existing one to no-log mode. >> >> What am I missing? > > Yes, you are right. There is no previous rule for this service, so the default behavior is in effect, where the service is being denied, but with logging enabled. > > This change is only changing the logging behavior. > > Thanks for reviewing! Oh, didn’t realize that "deny" was default for everything. I guess that should have been obvious.
(In reply to Darin Adler from comment #6) > Comment on attachment 407353 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=407353&action=review > > >>> Source/WebKit/WebProcess/com.apple.WebProcess.sb.in:771 > >>> +#endif > >> > >> But this is a new deny rule, not changing an existing one to no-log mode. > >> > >> What am I missing? > > > > Yes, you are right. There is no previous rule for this service, so the default behavior is in effect, where the service is being denied, but with logging enabled. > > > > This change is only changing the logging behavior. > > > > Thanks for reviewing! > > Oh, didn’t realize that "deny" was default for everything. I guess that > should have been obvious. Actually, in some cases, "allow" is the default, but not in this case :) Thanks for reviewing!
Committed r266216: <https://trac.webkit.org/changeset/266216> All reviewed patches have been landed. Closing bug and clearing flags on attachment 407353 [details].
Comment on attachment 407353 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=407353&action=review > Source/WebKit/WebProcess/com.apple.WebProcess.sb.in:770 > + (global-name "com.apple.CoreDisplay.Notification)) Missing double quote here :( https://trac.webkit.org/changeset/266238/webkit