[GCrypt] Only report libgcrypt errors when logging is enabled
Created attachment 321535 [details] Patch
Comment on attachment 321535 [details] Patch I would expect errors to always be printed to stdout regardless of whether logging is enabled or not. What sort of error spam are you seeing that makes it preferable to silence all errors from libgcrypt? I don't think using LOG_DISABLED to guard a call to WTFLogAlways makes sense, since WTFLogAlways is what you use to log something even when logs are disabled. You could use ENABLE(DEVELOPER_MODE), but even that doesn't make sense since there's no fundamental reason why errors should be silenced in release mode but not developer mode. If you really need to silence these errors, I think the right way would be to define a new log channel in Logging.h for WebCrypto. The problem is that's complicated by the fact that we have this weird transitional split between WebCore/platform and PAL right now, where PAL can't use WebCore/platform. I think PAL is causing more problems than it solves so long as we have it left in this transitional state. I guess PAL needs its own Logging.h. But logging is really intended for debug: errors should print always unless you have a *really* good reason not to.
In general, we rely on stderr output to debug problems all the time, and it would be really unfortunate if error messages that we're used to seeing in development builds are not present in release builds.
Currently this prints out libgcrypt errors whenever WebCrypto API is misused by the Web application. I don't see how it's helpful to the end-user who has no interest in debugging to see all this error output.
I mean, there's a bunch of logging used in GStreamer code that is only usable when compiling with !LOG_DISABLED. This isn't any different.
I don't know about GStreamer. I think we should *never* print any errors to stderr when the WebCrypto API is misused by the web application. But we *should* always send console messages to the web inspector.
(In reply to Michael Catanzaro from comment #6) > I think we should *never* print any errors to stderr when the WebCrypto API > is misused by the web application. This patch fixes that, while still enabling such logging for debugging purposes when !LOG_DISABLED is true. > But we *should* always send console messages to the web inspector. This isn't mandated anywhere. Neither do I know of any other API that does that in similar use cases. Regardless, providing libgcrypt-level details of these errors makes no sense to me.
I still think this is not the best approach, but I'll defer to you. You can ask Calvaris to reset the r+.
Created attachment 321688 [details] Patch Adds a FIXME that instructs using a WTF log channel once available in PAL.
Comment on attachment 321688 [details] Patch Clearing flags on attachment: 321688 Committed r222497: <http://trac.webkit.org/changeset/222497>
All reviewed patches have been landed. Closing bug.
<rdar://problem/34692969>