Towards making use of CryptoDigest in the implementation of Content Security Policy inline script and inline stylesheet hashes (bug #155007) we should move CryptoDigest from Source/WebCore/crypto to a place that conveys that it is not specific to the Web Crypto code. One idea is to move it to WebCore/platform/crypto.
<rdar://problem/24969787>
Created attachment 272830 [details] Patch
Attachment 272830 [details] did not pass style-queue: ERROR: Source/WebCore/platform/crypto/gnutls/CryptoDigestGnuTLS.cpp:29: Alphabetical sorting problem. [build/include_order] [4] ERROR: Source/WebCore/platform/crypto/CryptoDigest.h:40: enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums. [readability/enum_casing] [4] ERROR: Source/WebCore/platform/crypto/CryptoDigest.h:41: enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums. [readability/enum_casing] [4] ERROR: Source/WebCore/platform/crypto/CryptoDigest.h:42: enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums. [readability/enum_casing] [4] ERROR: Source/WebCore/platform/crypto/CryptoDigest.h:43: enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums. [readability/enum_casing] [4] ERROR: Source/WebCore/platform/crypto/CryptoDigest.h:44: enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums. [readability/enum_casing] [4] Total errors found: 6 in 15 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 272830 [details] Patch r=me. Please confirm tests pass before landing.
Comment on attachment 272830 [details] Patch Clearing flags on attachment: 272830 Committed r197575: <http://trac.webkit.org/changeset/197575>
All reviewed patches have been landed. Closing bug.
It broke the GTK build unless you pass -DENABLE_SUBTLE_CRYPTO=ON to CMake to enable that experimental option, but you couldn't have noticed because this is what the bots do. Follow-up in bug #155073. At first I considered rolling this out, because it adds a "new" dependency on GnuTLS. (We normally make new dependencies optional by allowing users to disable the corresponding new feature.) But, looking over CryptoDigestGnuTLS.cpp, I believe all the GnuTLS functionality it requires is fairly old, so it's unlikely to cause problems for our distributors in practice beyond one failed build to discover the new dependency. So I think a required dependency on GnuTLS is fine, we just need to remember to mention this in our NEWS. I'm testing a build fix now.
Committed r197623: <http://trac.webkit.org/changeset/197623>
(In reply to comment #7) > At first I considered rolling this out, because it adds a "new" dependency > on GnuTLS. (We normally make new dependencies optional by allowing users to > disable the corresponding new feature.) But, looking over > CryptoDigestGnuTLS.cpp, I believe all the GnuTLS functionality it requires > is fairly old, so it's unlikely to cause problems for our distributors in > practice beyond one failed build to discover the new dependency. So I think > a required dependency on GnuTLS is fine, we just need to remember to mention > this in our NEWS. Turns out this introduced a legal problem, see bug #162913.
(In reply to comment #9) > (In reply to comment #7) > > At first I considered rolling this out, because it adds a "new" dependency > > on GnuTLS. (We normally make new dependencies optional by allowing users to > > disable the corresponding new feature.) But, looking over > > CryptoDigestGnuTLS.cpp, I believe all the GnuTLS functionality it requires > > is fairly old, so it's unlikely to cause problems for our distributors in > > practice beyond one failed build to discover the new dependency. So I think > > a required dependency on GnuTLS is fine, we just need to remember to mention > > this in our NEWS. > > Turns out this introduced a legal problem, see bug #162913. :(
(In reply to comment #10) > :( Turns out I misunderstood the licenses. It's OK. We'll still try to remove the dependency.