177360 2017-09-22 05:54:39 -0700 [GCrypt] Only report libgcrypt errors when logging is enabled 2017-09-27 12:17:39 -0700 1 1 1 Unclassified WebKit New Bugs WebKit Nightly Build Unspecified Unspecified RESOLVED FIXED InRadar P2 Normal --- 1 zan zan buildbot calvaris jiewen_tan mcatanzaro webkit-bug-importer oldest_to_newest 1351618 0 zan 2017-09-22 05:54:39 -0700 [GCrypt] Only report libgcrypt errors when logging is enabled 1351619 1 321535 zan 2017-09-22 05:59:03 -0700 Created attachment 321535 Patch 1351625 2 321535 mcatanzaro 2017-09-22 06:29:11 -0700 Comment on attachment 321535 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. 1351626 3 mcatanzaro 2017-09-22 06:30:08 -0700 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. 1352054 4 zan 2017-09-24 03:30:32 -0700 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. 1352057 5 zan 2017-09-24 03:34:15 -0700 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. 1352075 6 mcatanzaro 2017-09-24 08:24:12 -0700 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. 1352165 7 zan 2017-09-25 04:45:21 -0700 (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. 1352186 8 mcatanzaro 2017-09-25 07:21:41 -0700 I still think this is not the best approach, but I'll defer to you. You can ask Calvaris to reset the r+. 1352200 9 321688 zan 2017-09-25 08:29:08 -0700 Created attachment 321688 Patch Adds a FIXME that instructs using a WTF log channel once available in PAL. 1352615 10 321688 zan 2017-09-26 01:38:19 -0700 Comment on attachment 321688 Patch Clearing flags on attachment: 321688 Committed r222497: <http://trac.webkit.org/changeset/222497> 1352616 11 zan 2017-09-26 01:38:23 -0700 All reviewed patches have been landed. Closing bug. 1353266 12 webkit-bug-importer 2017-09-27 12:17:39 -0700 <rdar://problem/34692969> 321535 2017-09-22 05:59:03 -0700 2017-09-25 08:29:05 -0700 Patch bug-177360-20170922145902.patch text/plain 1590 zan U3VidmVyc2lvbiBSZXZpc2lvbjogMjIyMzgwCmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9Q QUwvQ2hhbmdlTG9nIGIvU291cmNlL1dlYkNvcmUvUEFML0NoYW5nZUxvZwppbmRleCA2OTg4MGMx OGFlMTFmY2U4N2Q5ODMxNGQ0MzQ2MGYzMTFkNDU4NGM4Li5jMjMxZTRiOTM3ZWIxY2Q5YmNlYTY3 ZGRmODgzYzhmNGUzOWUwODZlIDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViQ29yZS9QQUwvQ2hhbmdl TG9nCisrKyBiL1NvdXJjZS9XZWJDb3JlL1BBTC9DaGFuZ2VMb2cKQEAgLTEsMyArMSwxNSBAQAor MjAxNy0wOS0yMiAgWmFuIERvYmVyc2VrICA8emRvYmVyc2VrQGlnYWxpYS5jb20+CisKKyAgICAg ICAgW0dDcnlwdF0gT25seSByZXBvcnQgbGliZ2NyeXB0IGVycm9ycyB3aGVuIGxvZ2dpbmcgaXMg ZW5hYmxlZAorICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9 MTc3MzYwCisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAg KiBwYWwvY3J5cHRvL2djcnlwdC9VdGlsaXRpZXMuaDoKKyAgICAgICAgKFBBTDo6R0NyeXB0Ojps b2dFcnJvcik6IE9ubHkgcHJpbnQgb3V0IHRoZSBsaWJnY3J5cHQgZXJyb3Igb24gc3RkZXJyCisg ICAgICAgIHdoZW4gbG9nZ2luZyBpcyBlbmFibGVkIGF0IGJ1aWxkLXRpbWUsIGF2b2lkaW5nIHVu bmVjZXNzYXJ5IG91dHB1dCBpbgorICAgICAgICBub24tZGVidWdnaW5nIHVzZSBjYXNlcy4KKwog MjAxNy0wOS0yMSAgWW9zaGlha2kgSml0c3VrYXdhICA8WW9zaGlha2kuSml0c3VrYXdhQHNvbnku Y29tPgogCiAgICAgICAgIFtQQUxdIE1vdmUgTWVkaWFUaW1lQVZGb3VuZGF0aW9uIGludG8gUEFM CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9QQUwvcGFsL2NyeXB0by9nY3J5cHQvVXRpbGl0 aWVzLmggYi9Tb3VyY2UvV2ViQ29yZS9QQUwvcGFsL2NyeXB0by9nY3J5cHQvVXRpbGl0aWVzLmgK aW5kZXggYTc4ODQ4ZTM3NzZjNzUzZTA3ZjNkOGFiZjE3NWI2NDk5NjhmYzQ4NS4uNTM3MjlhOTg4 ZjJkOWZiYWIyYTRmZjgwNDdhYzllZmM4ODFjZDIxMSAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNv cmUvUEFML3BhbC9jcnlwdG8vZ2NyeXB0L1V0aWxpdGllcy5oCisrKyBiL1NvdXJjZS9XZWJDb3Jl L1BBTC9wYWwvY3J5cHRvL2djcnlwdC9VdGlsaXRpZXMuaApAQCAtNDMsOCArNDMsMTIgQEAgdXNp bmcgQ2lwaGVyT3BlcmF0aW9uID0gZ2NyeV9lcnJvcl90KGdjcnlfY2lwaGVyX2hkX3QsIHZvaWQq LCBzaXplX3QsIGNvbnN0IHZvaWQKIAogc3RhdGljIGlubGluZSB2b2lkIGxvZ0Vycm9yKGdjcnlf ZXJyb3JfdCBlcnJvcikKIHsKKyNpZiAhTE9HX0RJU0FCTEVECiAgICAgV1RGTG9nQWx3YXlzKCJs aWJnY3J5cHQgZXJyb3I6IHNvdXJjZSAnJXMnLCBkZXNjcmlwdGlvbiAnJXMnIiwKICAgICAgICAg Z2NyeV9zdHJzb3VyY2UoZXJyb3IpLCBnY3J5X3N0cmVycm9yKGVycm9yKSk7CisjZWxzZQorICAg IFVOVVNFRF9QQVJBTShlcnJvcik7CisjZW5kaWYKIH0KIAogc3RhdGljIGlubGluZSBzdGQ6Om9w dGlvbmFsPGludD4gYWVzQWxnb3JpdGhtRm9yS2V5U2l6ZShzaXplX3Qga2V5U2l6ZSkK 321688 2017-09-25 08:29:08 -0700 2017-09-26 01:38:19 -0700 Patch bug-177360-20170925172907.patch text/plain 1661 zan U3VidmVyc2lvbiBSZXZpc2lvbjogMjIyNDQ0CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9Q QUwvQ2hhbmdlTG9nIGIvU291cmNlL1dlYkNvcmUvUEFML0NoYW5nZUxvZwppbmRleCBhMjVjZWU4 YTkwMTlkMDU1ZDZiNGE3MjQzMzRmOTk4ODJlMThkOTYxLi5jZjIxNjYwOTljMDY2YjMxY2Y4YzRj NDhjZTc2ZWNlYzJmNWQ1ZjlmIDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViQ29yZS9QQUwvQ2hhbmdl TG9nCisrKyBiL1NvdXJjZS9XZWJDb3JlL1BBTC9DaGFuZ2VMb2cKQEAgLTEsMyArMSwxNSBAQAor MjAxNy0wOS0yNSAgWmFuIERvYmVyc2VrICA8emRvYmVyc2VrQGlnYWxpYS5jb20+CisKKyAgICAg ICAgW0dDcnlwdF0gT25seSByZXBvcnQgbGliZ2NyeXB0IGVycm9ycyB3aGVuIGxvZ2dpbmcgaXMg ZW5hYmxlZAorICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9 MTc3MzYwCisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAg KiBwYWwvY3J5cHRvL2djcnlwdC9VdGlsaXRpZXMuaDoKKyAgICAgICAgKFBBTDo6R0NyeXB0Ojps b2dFcnJvcik6IE9ubHkgcHJpbnQgb3V0IHRoZSBsaWJnY3J5cHQgZXJyb3Igb24gc3RkZXJyCisg ICAgICAgIHdoZW4gbG9nZ2luZyBpcyBlbmFibGVkIGF0IGJ1aWxkLXRpbWUsIGF2b2lkaW5nIHVu bmVjZXNzYXJ5IG91dHB1dCBpbgorICAgICAgICBub24tZGVidWdnaW5nIHVzZSBjYXNlcy4KKwog MjAxNy0wOS0yMiAgWmFsYW4gQnVqdGFzICA8emFsYW5AYXBwbGUuY29tPgogCiAgICAgICAgIFdl YWtQdHJGYWN0b3J5IHNob3VsZCBwb3B1bGF0ZSBtX3JlZiBsYXppbHkuCmRpZmYgLS1naXQgYS9T b3VyY2UvV2ViQ29yZS9QQUwvcGFsL2NyeXB0by9nY3J5cHQvVXRpbGl0aWVzLmggYi9Tb3VyY2Uv V2ViQ29yZS9QQUwvcGFsL2NyeXB0by9nY3J5cHQvVXRpbGl0aWVzLmgKaW5kZXggYTc4ODQ4ZTM3 NzZjNzUzZTA3ZjNkOGFiZjE3NWI2NDk5NjhmYzQ4NS4uODJmZTE3YzAzYjQxMTczZWMxM2QzMGIy ZmMzMWZhYzFkMjhhNThkZiAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvUEFML3BhbC9jcnlw dG8vZ2NyeXB0L1V0aWxpdGllcy5oCisrKyBiL1NvdXJjZS9XZWJDb3JlL1BBTC9wYWwvY3J5cHRv L2djcnlwdC9VdGlsaXRpZXMuaApAQCAtNDMsOCArNDMsMTMgQEAgdXNpbmcgQ2lwaGVyT3BlcmF0 aW9uID0gZ2NyeV9lcnJvcl90KGdjcnlfY2lwaGVyX2hkX3QsIHZvaWQqLCBzaXplX3QsIGNvbnN0 IHZvaWQKIAogc3RhdGljIGlubGluZSB2b2lkIGxvZ0Vycm9yKGdjcnlfZXJyb3JfdCBlcnJvcikK IHsKKyAgICAvLyBGSVhNRTogVXNlIGEgV2ViQ3J5cHRvIFdURiBsb2cgY2hhbm5lbCBoZXJlIG9u Y2UgdGhvc2UgYXJlIG1vdmVkIGRvd24gdG8gUEFMLgorI2lmICFMT0dfRElTQUJMRUQKICAgICBX VEZMb2dBbHdheXMoImxpYmdjcnlwdCBlcnJvcjogc291cmNlICclcycsIGRlc2NyaXB0aW9uICcl cyciLAogICAgICAgICBnY3J5X3N0cnNvdXJjZShlcnJvciksIGdjcnlfc3RyZXJyb3IoZXJyb3Ip KTsKKyNlbHNlCisgICAgVU5VU0VEX1BBUkFNKGVycm9yKTsKKyNlbmRpZgogfQogCiBzdGF0aWMg aW5saW5lIHN0ZDo6b3B0aW9uYWw8aW50PiBhZXNBbGdvcml0aG1Gb3JLZXlTaXplKHNpemVfdCBr ZXlTaXplKQo=