LayoutTests crypto/subtle/ecdsa-generate-key-sign-verify-p384.html and crypto/subtle/ecdsa-generate-key-sign-verify-p256.html are flaky failures https://webkit-test-results.webkit.org/dashboards/flakiness_dashboard.html#tests=crypto%2Fsubtle%2Fecdsa- --- /Volumes/Data/slave/sierra-debug-tests-wk1/build/layout-test-results/crypto/subtle/ecdsa-generate-key-sign-verify-p384-expected.txt +++ /Volumes/Data/slave/sierra-debug-tests-wk1/build/layout-test-results/crypto/subtle/ecdsa-generate-key-sign-verify-p384-actual.txt @@ -7,7 +7,7 @@ Signing PASS signature.byteLength is 96 Verifying -PASS verified is true +FAIL verified should be true. Was false. PASS successfullyParsed is true TEST COMPLETE --- /Volumes/Data/slave/sierra-debug-tests-wk1/build/layout-test-results/crypto/subtle/ecdsa-generate-key-sign-verify-p256-expected.txt +++ /Volumes/Data/slave/sierra-debug-tests-wk1/build/layout-test-results/crypto/subtle/ecdsa-generate-key-sign-verify-p256-actual.txt @@ -7,7 +7,7 @@ Signing PASS signature.byteLength is 64 Verifying -PASS verified is true +FAIL verified should be true. Was false. PASS successfullyParsed is true TEST COMPLETE
<rdar://problem/31734958>
Marked tests as flaky in https://trac.webkit.org/changeset/215563/webkit
*** Bug 170921 has been marked as a duplicate of this bug. ***
Created attachment 308009 [details] Patch
Created attachment 308011 [details] Patch
Comment on attachment 308011 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=308011&action=review Looks fine, but I think you might want to clean up the code a little. r=me with cleanups tp signECDSA. > Source/WebCore/crypto/mac/CryptoAlgorithmECDSAMac.cpp:92 > + size_t pos = newSignature.size(); Isn't pos always zero here, since you only reserved capacity above, and did not actually fill the vector with anything? > Source/WebCore/crypto/mac/CryptoAlgorithmECDSAMac.cpp:97 > + // Otherwise skipping the leading 0s of r. // Otherwise skip the leading 0s of r. > Source/WebCore/crypto/mac/CryptoAlgorithmECDSAMac.cpp:98 > + if (signature[offset] > keyLengthInBytes) Should this be "else if"? > Source/WebCore/crypto/mac/CryptoAlgorithmECDSAMac.cpp:113 > + // Otherwise skipping the leading 0s of s. "// Otherwise skip the leading 0s of s."
Comment on attachment 308011 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=308011&action=review Thanks Brent for r+ my patch. >> Source/WebCore/crypto/mac/CryptoAlgorithmECDSAMac.cpp:92 >> + size_t pos = newSignature.size(); > > Isn't pos always zero here, since you only reserved capacity above, and did not actually fill the vector with anything? True. I just want to make it more general. I can remove pos under this check completely. >> Source/WebCore/crypto/mac/CryptoAlgorithmECDSAMac.cpp:97 >> + // Otherwise skipping the leading 0s of r. > > // Otherwise skip the leading 0s of r. Fixed. >> Source/WebCore/crypto/mac/CryptoAlgorithmECDSAMac.cpp:98 >> + if (signature[offset] > keyLengthInBytes) > > Should this be "else if"? I remember there is style check on if ... else ... We can only do 'if ... else if ... else ...' not 'if ... else if ...'. Maybe I am wrong. Let me double check. >> Source/WebCore/crypto/mac/CryptoAlgorithmECDSAMac.cpp:113 >> + // Otherwise skipping the leading 0s of s. > > "// Otherwise skip the leading 0s of s." Fixed.
Created attachment 308043 [details] Patch for landing
Comment on attachment 308043 [details] Patch for landing Clearing flags on attachment: 308043 Committed r215721: <http://trac.webkit.org/changeset/215721>