Get rid of WTF::SHA1.

Get rid of WTF::SHA1.

WTF::SHA1 is redundant now with WebCrypto support in place, and Blink probably
should not roll its own crypto. This removes WTF::SHA1 and replaces its
consumers with WebCrypto calls.

BUG=349514
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=170897

[eroman, 2014-03-07 19:52:33]

Thanks

The new way of calling the functions is quite a bit more verbose.

I think it would be better to create a wrapper that calls all the Platform goop.

For instance, how about adding a simpler utility function in modules/crypto
which wraps all of the blink::WebCrypto* stuff, and the call to Platform.
Similarly I would create a separate enum that mirrors the one in blink:: so
don't run out of breath typing :)

My only other concern then is regarding performance. The new interface is only
for a one-shot digest. Is the thing in DomPatch performance critical and will be
hurt by this?

We could also create a multi-part version if needed

Cheers

[jww, 2014-03-11 20:48:58]

On 2014/03/07 19:52:33, eroman wrote:
> Thanks
> 
> The new way of calling the functions is quite a bit more verbose.
> 
> I think it would be better to create a wrapper that calls all the Platform
goop.
> 
> For instance, how about adding a simpler utility function in modules/crypto
> which wraps all of the blink::WebCrypto* stuff, and the call to Platform.
> Similarly I would create a separate enum that mirrors the one in blink:: so
> don't run out of breath typing :)
Done.
> 
> My only other concern then is regarding performance. The new interface is only
> for a one-shot digest. Is the thing in DomPatch performance critical and will
be
> hurt by this?
My intuition is that we shouldn't be any worse off. The new crypto callsites in
DOMPatchSupport are identical to the old ones, so unless WebCrypto is
particularly slow, I don't think that should be an issue. That having been said,
I'll add pfeldman as a reviewer to get his opinion (and since we'll need
approval from an OWNER of inspector anyway).
> 
> We could also create a multi-part version if needed
> 
> Cheers

[jww, 2014-03-11 20:50:46]

pfeldman@, would you mind taking a look at this CL, in particular
Source/core/inspector/DOMPatchSupport.cpp? eroman@ pointed out that we should be
sure that we're not adding any extra performance issues here. I don't think we
are, but it would great if you could look at that. Thanks!

[eroman, 2014-03-11 20:55:29]

The difference is that now it has to build a complete string to pass in to the
digest function. Whereas before it just fed the data into the digest function.

Say you were traversing through 100MB of data -- now you keep appending to build
that entire string, whereas previously the SHA1 class would just keep a little
bit of state while computing the digest and consuming the data.

Anyhow, pfeldman can comment if that will be a problem for this callsite. I will
review the rest of the code shortly.

[jww, 2014-03-11 21:06:40]

On 2014/03/11 20:55:29, eroman wrote:
> The difference is that now it has to build a complete string to pass in to the
> digest function. Whereas before it just fed the data into the digest function.
> 
> Say you were traversing through 100MB of data -- now you keep appending to
build
> that entire string, whereas previously the SHA1 class would just keep a little
> bit of state while computing the digest and consuming the data.
Ah, got it. I definitely misunderstood your point before. Thanks.
> 
> Anyhow, pfeldman can comment if that will be a problem for this callsite. I
will
> review the rest of the code shortly.

[pfeldman, 2014-03-12 04:38:18]

On 2014/03/11 20:50:46, jww wrote:
> pfeldman@, would you mind taking a look at this CL, in particular
> Source/core/inspector/DOMPatchSupport.cpp? eroman@ pointed out that we should
be
> sure that we're not adding any extra performance issues here. I don't think we
> are, but it would great if you could look at that. Thanks!

DOMPatchSupport is not performance critical, you are not adding much in this
case. But the new API, being the only digest API for Blink core, suggests that
the source data needs to be concatenated. This is somewhat misleading and
suggests bad coding practice memory-wise. SHA1 you remove was way better with
that regard.

[jww, 2014-03-12 05:09:51]

On Mar 11, 2014 9:38 PM, <pfeldman@chromium.org> wrote:
>
> On 2014/03/11 20:50:46, jww wrote:
>>
>> pfeldman@, would you mind taking a look at this CL, in particular
>> Source/core/inspector/DOMPatchSupport.cpp? eroman@ pointed out that we
should
>
> be
>>
>> sure that we're not adding any extra performance issues here. I don't
think we
>> are, but it would great if you could look at that. Thanks!
>
>
> DOMPatchSupport is not performance critical, you are not adding much in
this
> case. But the new API, being the only digest API for Blink core, suggests
that
> the source data needs to be concatenated. This is somewhat misleading and
> suggests bad coding practice memory-wise. SHA1 you remove was way better
with
> that regard.
Agreed. I think we should provide a version of the digest API that can
consume the data piecemeal, as eroman suggested. But do you think that's
necessary for DOMPatchSupport in first swing at this? Or is that something
we can add in a separate CL? Thanks!
>
> https://codereview.chromium.org/189373010/

To unsubscribe from this group and stop receiving emails from it, send an email
to blink-reviews+unsubscribe@chromium.org.

[pfeldman, 2014-03-12 05:18:18]

> Agreed. I think we should provide a version of the digest API that can
> consume the data piecemeal, as eroman suggested. But do you think that's
> necessary for DOMPatchSupport in first swing at this? Or is that something
> we can add in a separate CL? Thanks!

DOMPatchSupport is not performance critical, it is fine to follow up in a
separate CL.

But I wonder if you should do it right away regardless the DOMPatchSupport. It
seems there is a consensus on the way API should look like and this change is
introducing a different one.

[eseidel, 2014-03-12 06:39:29]

This is OK, but I might have just swapped out the guts of SHA1.h with CryptoUtil
and left the callers alone. :)

[eseidel, 2014-03-12 06:41:04]

I realize now that CryptoUtil is your new wrapper.  I guess that's OK, and I
guess we're just bikeshedding, but it feels like we could have just replaced the
guts of SHA1.cpp and not had to change very much code here?

[eseidel, 2014-03-12 06:50:28]

I see now that you're replacing SHA1 with a more generic crypto helper.

nit:  Util isn't a very descriptive suffix.  I probably would have just called
your namespace "Crypto".

abarth has reviewed patches in this area more recently than I.

I'm not a big fan of all the reinterpret casts, adn I don't think you want to be
using String types when you really mean raw bytes.  We could always expose more
interfaces on the result types to avoid the reinterpet casts.  This interface
should feel very blink-like and speak in terms of blink objects.  The blink::
interfaces are APIs into Chromium and tend to speak in terms of chromium objects
so we have to convert when we wrap them for consumption by the rest of blink.

https://codereview.chromium.org/189373010/diff/40001/Source/core/inspector/DO...
File Source/core/inspector/DOMPatchSupport.cpp (right):

https://codereview.chromium.org/189373010/diff/40001/Source/core/inspector/DO...
Source/core/inspector/DOMPatchSupport.cpp:407:
digestable.append(node->nodeName());
You could have just used Vector<uint8_t> here, StringBuilder tries to be smart
about appends to make them cheap, but it also provides you UTF16 access to your
data (which you don't want).  I think it's a bit of a lie to use it here.

https://codereview.chromium.org/189373010/diff/40001/Source/core/inspector/DO...
Source/core/inspector/DOMPatchSupport.cpp:430: digest->m_attrsSHA1 =
base64Encode(reinterpret_cast<const char*>(digestResult.data()), 10);
I take it we only want the first 10 bytes of the digest?  Odd.

https://codereview.chromium.org/189373010/diff/40001/Source/modules/websocket...
File Source/modules/websockets/WebSocketHandshake.cpp (right):

https://codereview.chromium.org/189373010/diff/40001/Source/modules/websocket...
Source/modules/websockets/WebSocketHandshake.cpp:131: return
base64Encode(reinterpret_cast<const char*>(digest.data()),
CryptoUtil::sha1HashSize);
Do we really need a reinterpret cast here?

https://codereview.chromium.org/189373010/diff/40001/Source/platform/CryptoUt...
File Source/platform/CryptoUtilities.cpp (right):

https://codereview.chromium.org/189373010/diff/40001/Source/platform/CryptoUt...
Source/platform/CryptoUtilities.cpp:70:
digestResult.append(reinterpret_cast<uint8_t*>(result.data()),
result.byteLength());
Do we need this reinterpret_cast?

[eseidel, 2014-03-12 06:51:22]

https://codereview.chromium.org/189373010/diff/40001/Source/platform/CryptoUt...
File Source/platform/CryptoUtilities.h (right):

https://codereview.chromium.org/189373010/diff/40001/Source/platform/CryptoUt...
Source/platform/CryptoUtilities.h:42: typedef Vector<uint8_t, kMaxDigestSize>
DigestValue;
I see, you're just typdefing to a vector directly, and I suspect that our base64
operations don't take uint_8_t and thus you ahve to reinterpet cast?

[abarth-chromium, 2014-03-12 18:59:01]

https://codereview.chromium.org/189373010/diff/40001/Source/core/frame/csp/CS...
File Source/core/frame/csp/CSPSourceList.h (right):

https://codereview.chromium.org/189373010/diff/40001/Source/core/frame/csp/CS...
Source/core/frame/csp/CSPSourceList.h:9: #include "platform/CryptoUtilities.h"
Yeah, I agree with Eric.  Drop the "Utilities".  It's cleaner.  :)

https://codereview.chromium.org/189373010/diff/40001/Source/core/frame/csp/CS...
Source/core/frame/csp/CSPSourceList.h:42: bool parseHash(const UChar* begin,
const UChar* end, CryptoUtil::DigestValue& hash,
ContentSecurityPolicyHashAlgorithm&);
Why not just DigestValue ?

https://codereview.chromium.org/189373010/diff/40001/Source/core/inspector/DO...
File Source/core/inspector/DOMPatchSupport.cpp (right):

https://codereview.chromium.org/189373010/diff/40001/Source/core/inspector/DO...
Source/core/inspector/DOMPatchSupport.cpp:407:
digestable.append(node->nodeName());
On 2014/03/12 06:50:29, eseidel wrote:
> You could have just used Vector<uint8_t> here, StringBuilder tries to be smart
> about appends to make them cheap, but it also provides you UTF16 access to
your
> data (which you don't want).  I think it's a bit of a lie to use it here.

Yeah, you've got an UTF-8 conversion at the end, which seems wrong.  I think you
just want to operate with a Vector of bytes here.

https://codereview.chromium.org/189373010/diff/40001/Source/platform/CryptoUt...
File Source/platform/CryptoUtilities.h (right):

https://codereview.chromium.org/189373010/diff/40001/Source/platform/CryptoUt...
Source/platform/CryptoUtilities.h:39: namespace CryptoUtil {
I'd just drop this namespace entirely.  WebCore is one big happy namespace.  :)

[jww, 2014-03-14 23:02:22]

On 2014/03/12 18:59:01, abarth wrote:
>
https://codereview.chromium.org/189373010/diff/40001/Source/core/frame/csp/CS...
> File Source/core/frame/csp/CSPSourceList.h (right):
> 
>
https://codereview.chromium.org/189373010/diff/40001/Source/core/frame/csp/CS...
> Source/core/frame/csp/CSPSourceList.h:9: #include "platform/CryptoUtilities.h"
> Yeah, I agree with Eric.  Drop the "Utilities".  It's cleaner.  :)
> 
>
https://codereview.chromium.org/189373010/diff/40001/Source/core/frame/csp/CS...
> Source/core/frame/csp/CSPSourceList.h:42: bool parseHash(const UChar* begin,
> const UChar* end, CryptoUtil::DigestValue& hash,
> ContentSecurityPolicyHashAlgorithm&);
> Why not just DigestValue ?
> 
>
https://codereview.chromium.org/189373010/diff/40001/Source/core/inspector/DO...
> File Source/core/inspector/DOMPatchSupport.cpp (right):
> 
>
https://codereview.chromium.org/189373010/diff/40001/Source/core/inspector/DO...
> Source/core/inspector/DOMPatchSupport.cpp:407:
> digestable.append(node->nodeName());
> On 2014/03/12 06:50:29, eseidel wrote:
> > You could have just used Vector<uint8_t> here, StringBuilder tries to be
smart
> > about appends to make them cheap, but it also provides you UTF16 access to
> your
> > data (which you don't want).  I think it's a bit of a lie to use it here.
> 
> Yeah, you've got an UTF-8 conversion at the end, which seems wrong.  I think
you
> just want to operate with a Vector of bytes here.
> 
>
https://codereview.chromium.org/189373010/diff/40001/Source/platform/CryptoUt...
> File Source/platform/CryptoUtilities.h (right):
> 
>
https://codereview.chromium.org/189373010/diff/40001/Source/platform/CryptoUt...
> Source/platform/CryptoUtilities.h:39: namespace CryptoUtil {
> I'd just drop this namespace entirely.  WebCore is one big happy namespace. 
:)

Just a heads up that I'll work on all the comments on this soon, but per
pfeldman's recommandation, I'd love to get a digest-by-chunk API into WebCrypto
first. The CLs for that are: https://codereview.chromium.org/195983024/ and
https://codereview.chromium.org/200763005/.

[jww, 2014-04-01 23:29:09]

Sorry for the long delay on this one, but I was waiting on
https://codereview.chromium.org/195983024/ and
https://codereview.chromium.org/200763005/ to land so we could use the new
"digestor" webcrypto API.

pfeldman, in particular, DOMPatchSupport now uses the new "digestor" API, so
it's now digesting chunk by chunk, rather thank a one off of a large vector, as
per your request. Let me know how it looks.

https://codereview.chromium.org/189373010/diff/40001/Source/core/frame/csp/CS...
File Source/core/frame/csp/CSPSourceList.h (right):

https://codereview.chromium.org/189373010/diff/40001/Source/core/frame/csp/CS...
Source/core/frame/csp/CSPSourceList.h:9: #include "platform/CryptoUtilities.h"
On 2014/03/12 18:59:02, abarth wrote:
> Yeah, I agree with Eric.  Drop the "Utilities".  It's cleaner.  :)

Done.

https://codereview.chromium.org/189373010/diff/40001/Source/core/frame/csp/CS...
Source/core/frame/csp/CSPSourceList.h:42: bool parseHash(const UChar* begin,
const UChar* end, CryptoUtil::DigestValue& hash,
ContentSecurityPolicyHashAlgorithm&);
On 2014/03/12 18:59:02, abarth wrote:
> Why not just DigestValue ?

I've now gotten rid of the CryptoUtil namespace, which I believe is what you're
asking for.

https://codereview.chromium.org/189373010/diff/40001/Source/core/inspector/DO...
File Source/core/inspector/DOMPatchSupport.cpp (right):

https://codereview.chromium.org/189373010/diff/40001/Source/core/inspector/DO...
Source/core/inspector/DOMPatchSupport.cpp:407:
digestable.append(node->nodeName());
On 2014/03/12 18:59:02, abarth wrote:
> On 2014/03/12 06:50:29, eseidel wrote:
> > You could have just used Vector<uint8_t> here, StringBuilder tries to be
smart
> > about appends to make them cheap, but it also provides you UTF16 access to
> your
> > data (which you don't want).  I think it's a bit of a lie to use it here.
> 
> Yeah, you've got an UTF-8 conversion at the end, which seems wrong.  I think
you
> just want to operate with a Vector of bytes here.

I think this is irrelevant now because I've modified this to use the new
"digestor" API we've added to WebCrytpo, so we can digest chunks one at a time.

https://codereview.chromium.org/189373010/diff/40001/Source/core/inspector/DO...
Source/core/inspector/DOMPatchSupport.cpp:430: digest->m_attrsSHA1 =
base64Encode(reinterpret_cast<const char*>(digestResult.data()), 10);
On 2014/03/12 06:50:29, eseidel wrote:
> I take it we only want the first 10 bytes of the digest?  Odd.

I don't ask questions; I just make compatible with the old version. But, yes, I
agree, odd :-)

https://codereview.chromium.org/189373010/diff/40001/Source/modules/websocket...
File Source/modules/websockets/WebSocketHandshake.cpp (right):

https://codereview.chromium.org/189373010/diff/40001/Source/modules/websocket...
Source/modules/websockets/WebSocketHandshake.cpp:131: return
base64Encode(reinterpret_cast<const char*>(digest.data()),
CryptoUtil::sha1HashSize);
On 2014/03/12 06:50:29, eseidel wrote:
> Do we really need a reinterpret cast here?

Yes. As per your other comment, it's a vector of uint8_ts, so we have to
reinterpret cast that to chars.

https://codereview.chromium.org/189373010/diff/40001/Source/platform/CryptoUt...
File Source/platform/CryptoUtilities.cpp (right):

https://codereview.chromium.org/189373010/diff/40001/Source/platform/CryptoUt...
Source/platform/CryptoUtilities.cpp:70:
digestResult.append(reinterpret_cast<uint8_t*>(result.data()),
result.byteLength());
On 2014/03/12 06:50:29, eseidel wrote:
> Do we need this reinterpret_cast?

It actually can be a static_cast, which I assume is preferably. I've changed it
to a static_cast but let me know if I'm mistaken about which is better to use.

https://codereview.chromium.org/189373010/diff/40001/Source/platform/CryptoUt...
File Source/platform/CryptoUtilities.h (right):

https://codereview.chromium.org/189373010/diff/40001/Source/platform/CryptoUt...
Source/platform/CryptoUtilities.h:39: namespace CryptoUtil {
On 2014/03/12 18:59:02, abarth wrote:
> I'd just drop this namespace entirely.  WebCore is one big happy namespace. 
:)

Done.

https://codereview.chromium.org/189373010/diff/40001/Source/platform/CryptoUt...
Source/platform/CryptoUtilities.h:42: typedef Vector<uint8_t, kMaxDigestSize>
DigestValue;
On 2014/03/12 06:51:23, eseidel wrote:
> I see, you're just typdefing to a vector directly, and I suspect that our
base64
> operations don't take uint_8_t and thus you ahve to reinterpet cast?

Correct.

[abarth-chromium, 2014-04-01 23:46:48]

lgtm

https://codereview.chromium.org/189373010/diff/60001/Source/core/frame/csp/CS...
File Source/core/frame/csp/CSPSourceList.cpp (right):

https://codereview.chromium.org/189373010/diff/60001/Source/core/frame/csp/CS...
Source/core/frame/csp/CSPSourceList.cpp:10: #include "platform/Crypto.h"
You already addes this to CSPSourceList.h.  No need to include it here again.

https://codereview.chromium.org/189373010/diff/60001/Source/core/inspector/DO...
File Source/core/inspector/DOMPatchSupport.cpp (right):

https://codereview.chromium.org/189373010/diff/60001/Source/core/inspector/DO...
Source/core/inspector/DOMPatchSupport.cpp:415: OwnPtr<blink::WebCryptoDigestor>
digestor = adoptPtr(createDigestor(HashAlgorithmSha1));
Why doesn't createDigestor return a PassOwnPtr?

https://codereview.chromium.org/189373010/diff/60001/Source/platform/Crypto.cpp
File Source/platform/Crypto.cpp (right):

https://codereview.chromium.org/189373010/diff/60001/Source/platform/Crypto.c...
Source/platform/Crypto.cpp:27: */
Please use new-style copyright blocks:

http://dev.chromium.org/blink/coding-style#TOC-License

https://codereview.chromium.org/189373010/diff/60001/Source/platform/Crypto.c...
Source/platform/Crypto.cpp:41: static blink::WebCryptoAlgorithmId
convertHashAlgorithm(HashAlgorithm algorithm)
convertHashAlgorithm -> toWebCryptoAlgorithmId

https://codereview.chromium.org/189373010/diff/60001/Source/platform/Crypto.c...
Source/platform/Crypto.cpp:78: return
crypto->createDigestor(convertHashAlgorithm(algorithm));
I'd just make this function all be one line.  This ASSERT doesn't buy us much
because the code will crash on the next line anyway.

https://codereview.chromium.org/189373010/diff/60001/Source/platform/Crypto.c...
Source/platform/Crypto.cpp:84: unsigned resultSize;
Please initialize scalars.

https://codereview.chromium.org/189373010/diff/60001/Source/platform/Crypto.h
File Source/platform/Crypto.h (right):

https://codereview.chromium.org/189373010/diff/60001/Source/platform/Crypto.h...
Source/platform/Crypto.h:53: PLATFORM_EXPORT blink::WebCryptoDigestor*
createDigestor(HashAlgorithm);
PassOwnPtr<blink::WebCryptoDigestor>

[jww, 2014-04-02 01:11:50]

Fixes from abarth are in.

pfeldman, I'll still wait for your lgtm, though.

https://codereview.chromium.org/189373010/diff/60001/Source/core/frame/csp/CS...
File Source/core/frame/csp/CSPSourceList.cpp (right):

https://codereview.chromium.org/189373010/diff/60001/Source/core/frame/csp/CS...
Source/core/frame/csp/CSPSourceList.cpp:10: #include "platform/Crypto.h"
On 2014/04/01 23:46:48, abarth wrote:
> You already addes this to CSPSourceList.h.  No need to include it here again.

Done.

https://codereview.chromium.org/189373010/diff/60001/Source/core/inspector/DO...
File Source/core/inspector/DOMPatchSupport.cpp (right):

https://codereview.chromium.org/189373010/diff/60001/Source/core/inspector/DO...
Source/core/inspector/DOMPatchSupport.cpp:415: OwnPtr<blink::WebCryptoDigestor>
digestor = adoptPtr(createDigestor(HashAlgorithmSha1));
On 2014/04/01 23:46:48, abarth wrote:
> Why doesn't createDigestor return a PassOwnPtr?

Fixed.

https://codereview.chromium.org/189373010/diff/60001/Source/platform/Crypto.cpp
File Source/platform/Crypto.cpp (right):

https://codereview.chromium.org/189373010/diff/60001/Source/platform/Crypto.c...
Source/platform/Crypto.cpp:27: */
On 2014/04/01 23:46:48, abarth wrote:
> Please use new-style copyright blocks:
> 
> http://dev.chromium.org/blink/coding-style#TOC-License

Done.

https://codereview.chromium.org/189373010/diff/60001/Source/platform/Crypto.c...
Source/platform/Crypto.cpp:41: static blink::WebCryptoAlgorithmId
convertHashAlgorithm(HashAlgorithm algorithm)
On 2014/04/01 23:46:48, abarth wrote:
> convertHashAlgorithm -> toWebCryptoAlgorithmId

Done.

https://codereview.chromium.org/189373010/diff/60001/Source/platform/Crypto.c...
Source/platform/Crypto.cpp:78: return
crypto->createDigestor(convertHashAlgorithm(algorithm));
On 2014/04/01 23:46:48, abarth wrote:
> I'd just make this function all be one line.  This ASSERT doesn't buy us much
> because the code will crash on the next line anyway.

Done.

https://codereview.chromium.org/189373010/diff/60001/Source/platform/Crypto.c...
Source/platform/Crypto.cpp:84: unsigned resultSize;
On 2014/04/01 23:46:48, abarth wrote:
> Please initialize scalars.

Done.

https://codereview.chromium.org/189373010/diff/60001/Source/platform/Crypto.h
File Source/platform/Crypto.h (right):

https://codereview.chromium.org/189373010/diff/60001/Source/platform/Crypto.h...
Source/platform/Crypto.h:53: PLATFORM_EXPORT blink::WebCryptoDigestor*
createDigestor(HashAlgorithm);
On 2014/04/01 23:46:48, abarth wrote:
> PassOwnPtr<blink::WebCryptoDigestor>

Done.

[jww, 2014-04-04 16:36:15]

On 2014/04/02 01:11:50, jww wrote:
> Fixes from abarth are in.
> 
> pfeldman, I'll still wait for your lgtm, though.
> 
>
https://codereview.chromium.org/189373010/diff/60001/Source/core/frame/csp/CS...
> File Source/core/frame/csp/CSPSourceList.cpp (right):
> 
>
https://codereview.chromium.org/189373010/diff/60001/Source/core/frame/csp/CS...
> Source/core/frame/csp/CSPSourceList.cpp:10: #include "platform/Crypto.h"
> On 2014/04/01 23:46:48, abarth wrote:
> > You already addes this to CSPSourceList.h.  No need to include it here
again.
> 
> Done.
> 
>
https://codereview.chromium.org/189373010/diff/60001/Source/core/inspector/DO...
> File Source/core/inspector/DOMPatchSupport.cpp (right):
> 
>
https://codereview.chromium.org/189373010/diff/60001/Source/core/inspector/DO...
> Source/core/inspector/DOMPatchSupport.cpp:415:
OwnPtr<blink::WebCryptoDigestor>
> digestor = adoptPtr(createDigestor(HashAlgorithmSha1));
> On 2014/04/01 23:46:48, abarth wrote:
> > Why doesn't createDigestor return a PassOwnPtr?
> 
> Fixed.
> 
>
https://codereview.chromium.org/189373010/diff/60001/Source/platform/Crypto.cpp
> File Source/platform/Crypto.cpp (right):
> 
>
https://codereview.chromium.org/189373010/diff/60001/Source/platform/Crypto.c...
> Source/platform/Crypto.cpp:27: */
> On 2014/04/01 23:46:48, abarth wrote:
> > Please use new-style copyright blocks:
> > 
> > http://dev.chromium.org/blink/coding-style#TOC-License
> 
> Done.
> 
>
https://codereview.chromium.org/189373010/diff/60001/Source/platform/Crypto.c...
> Source/platform/Crypto.cpp:41: static blink::WebCryptoAlgorithmId
> convertHashAlgorithm(HashAlgorithm algorithm)
> On 2014/04/01 23:46:48, abarth wrote:
> > convertHashAlgorithm -> toWebCryptoAlgorithmId
> 
> Done.
> 
>
https://codereview.chromium.org/189373010/diff/60001/Source/platform/Crypto.c...
> Source/platform/Crypto.cpp:78: return
> crypto->createDigestor(convertHashAlgorithm(algorithm));
> On 2014/04/01 23:46:48, abarth wrote:
> > I'd just make this function all be one line.  This ASSERT doesn't buy us
much
> > because the code will crash on the next line anyway.
> 
> Done.
> 
>
https://codereview.chromium.org/189373010/diff/60001/Source/platform/Crypto.c...
> Source/platform/Crypto.cpp:84: unsigned resultSize;
> On 2014/04/01 23:46:48, abarth wrote:
> > Please initialize scalars.
> 
> Done.
> 
> https://codereview.chromium.org/189373010/diff/60001/Source/platform/Crypto.h
> File Source/platform/Crypto.h (right):
> 
>
https://codereview.chromium.org/189373010/diff/60001/Source/platform/Crypto.h...
> Source/platform/Crypto.h:53: PLATFORM_EXPORT blink::WebCryptoDigestor*
> createDigestor(HashAlgorithm);
> On 2014/04/01 23:46:48, abarth wrote:
> > PassOwnPtr<blink::WebCryptoDigestor>
> 
> Done.

pfeldman gave approval offline. Since this has all the lgtms, I'm flipping the
commit bit.

[jww, 2014-04-04 16:36:21]

The CQ bit was checked by jww@chromium.org

[commit-bot: I haz the power, 2014-04-04 16:36:24]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jww@chromium.org/189373010/80001

[commit-bot: I haz the power, 2014-04-04 16:53:18]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-04-04 16:53:19]

Try jobs failed on following builders:
  tryserver.blink on win_blink_rel

[jww, 2014-04-04 19:22:21]

The CQ bit was checked by jww@chromium.org

[commit-bot: I haz the power, 2014-04-04 19:22:27]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jww@chromium.org/189373010/100001

[commit-bot: I haz the power, 2014-04-04 20:18:51]

Change committed as 170897