Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
151673005:
Add digestSynchronous WebCrypto method. (Closed)
|
Created:
6 years, 10 months ago by jww Modified:
6 years, 10 months ago CC:
blink-reviews, jamesr, dglazkov+blink, abarth-chromium Base URL:
https://chromium.googlesource.com/chromium/blink.git@master Visibility:
Public. |
DescriptionAdd digestSynchronous WebCrypto method.
BUG=327826
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=167383
Patch Set 1 #
Total comments: 4
Patch Set 2 : Simplified digestSynchronous API. #
Total comments: 2
Patch Set 3 : Updated the function's arguments. #Messages
Total messages: 15 (0 generated)
This adds a digestSynchronous method to the webcrypto API.
https://codereview.chromium.org/151673005/diff/1/public/platform/WebCrypto.h File public/platform/WebCrypto.h (right): https://codereview.chromium.org/151673005/diff/1/public/platform/WebCrypto.h#... public/platform/WebCrypto.h:181: // This is the one exception to the "Completeing the request" guarantees typo completing https://codereview.chromium.org/151673005/diff/1/public/platform/WebCrypto.h#... public/platform/WebCrypto.h:188: virtual void digestSynchronous(const WebCryptoAlgorithm&, const unsigned char* data, unsigned dataSize, WebCryptoResult result) { result.completeWithError(); } Is it going to be convenient enough having "WebCryptoResult" here? This mean clients will have to provide their own implementation to extract the result. What about having the return value be a boolean for success, and the input be a vector
https://codereview.chromium.org/151673005/diff/1/public/platform/WebCrypto.h File public/platform/WebCrypto.h (right): https://codereview.chromium.org/151673005/diff/1/public/platform/WebCrypto.h#... public/platform/WebCrypto.h:181: // This is the one exception to the "Completeing the request" guarantees On 2014/02/08 00:03:40, eroman wrote: > typo completing Done. https://codereview.chromium.org/151673005/diff/1/public/platform/WebCrypto.h#... public/platform/WebCrypto.h:188: virtual void digestSynchronous(const WebCryptoAlgorithm&, const unsigned char* data, unsigned dataSize, WebCryptoResult result) { result.completeWithError(); } On 2014/02/08 00:03:40, eroman wrote: > Is it going to be convenient enough having "WebCryptoResult" here? This mean > clients will have to provide their own implementation to extract the result. > > What about having the return value be a boolean for success, and the input be a > vector That works for me. I was trying to keep the API similar to the other methods, but I suppose there's no reason for that.
https://codereview.chromium.org/151673005/diff/70001/public/platform/WebCrypto.h File public/platform/WebCrypto.h (right): https://codereview.chromium.org/151673005/diff/70001/public/platform/WebCrypt... public/platform/WebCrypto.h:186: virtual bool digestSynchronous(const WebCryptoAlgorithm&, const unsigned char* data, unsigned dataSize, WebArrayBuffer& result) { return false; } * Pass a WebCryptoAlgorithmId instead (since none of the supported digests have any parameters, and constructing WebCryptoAlgorithm is clumsy * I think WebVector would be a more appropriate output type.
https://codereview.chromium.org/151673005/diff/70001/public/platform/WebCrypto.h File public/platform/WebCrypto.h (right): https://codereview.chromium.org/151673005/diff/70001/public/platform/WebCrypt... public/platform/WebCrypto.h:186: virtual bool digestSynchronous(const WebCryptoAlgorithm&, const unsigned char* data, unsigned dataSize, WebArrayBuffer& result) { return false; } On 2014/02/08 01:40:22, eroman wrote: > * Pass a WebCryptoAlgorithmId instead (since none of the supported digests have > any parameters, and constructing WebCryptoAlgorithm is clumsy Done. > > * I think WebVector would be a more appropriate output type. I'm happy to do this if you think it's best, but given that it's a WebArrayBuffer everywhere else, it seems a bit inconsistent to start using WebVectors here (and it's a simple conversion to WebVector for the consumer if they choose to use it). Let me know either way.
On 2014/02/08 02:31:18, jww wrote: > https://codereview.chromium.org/151673005/diff/70001/public/platform/WebCrypto.h > File public/platform/WebCrypto.h (right): > > https://codereview.chromium.org/151673005/diff/70001/public/platform/WebCrypt... > public/platform/WebCrypto.h:186: virtual bool digestSynchronous(const > WebCryptoAlgorithm&, const unsigned char* data, unsigned dataSize, > WebArrayBuffer& result) { return false; } > On 2014/02/08 01:40:22, eroman wrote: > > * Pass a WebCryptoAlgorithmId instead (since none of the supported digests > have > > any parameters, and constructing WebCryptoAlgorithm is clumsy > Done. > > > > * I think WebVector would be a more appropriate output type. > I'm happy to do this if you think it's best, but given that it's a > WebArrayBuffer everywhere else, it seems a bit inconsistent to start using > WebVectors here (and it's a simple conversion to WebVector for the consumer if > they choose to use it). Let me know either way. Hey Eric, any thoughts on this latest CL?
lgtm On Thu, Feb 13, 2014 at 1:43 PM, <jww@chromium.org> wrote: > On 2014/02/08 02:31:18, jww wrote: > > https://codereview.chromium.org/151673005/diff/70001/ > public/platform/WebCrypto.h > >> File public/platform/WebCrypto.h (right): >> > > > https://codereview.chromium.org/151673005/diff/70001/ > public/platform/WebCrypto.h#newcode186 > >> public/platform/WebCrypto.h:186: virtual bool digestSynchronous(const >> WebCryptoAlgorithm&, const unsigned char* data, unsigned dataSize, >> WebArrayBuffer& result) { return false; } >> On 2014/02/08 01:40:22, eroman wrote: >> > * Pass a WebCryptoAlgorithmId instead (since none of the supported >> digests >> have >> > any parameters, and constructing WebCryptoAlgorithm is clumsy >> Done. >> > >> > * I think WebVector would be a more appropriate output type. >> I'm happy to do this if you think it's best, but given that it's a >> WebArrayBuffer everywhere else, it seems a bit inconsistent to start using >> WebVectors here (and it's a simple conversion to WebVector for the >> consumer if >> they choose to use it). Let me know either way. >> > > Hey Eric, any thoughts on this latest CL? > > https://codereview.chromium.org/151673005/ > To unsubscribe from this group and stop receiving emails from it, send an email to blink-reviews+unsubscribe@chromium.org.
lgtm
The CQ bit was checked by jww@chromium.org
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/jww@chromium.org/151673005/130002
The CQ bit was unchecked by commit-bot@chromium.org
Retried try job too often on linux_blink for step(s) webkit_tests http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_blin...
The CQ bit was checked by jww@chromium.org
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/jww@chromium.org/151673005/130002
Message was sent while issue was closed.
Change committed as 167383 |
