Add content implementation of digestSynchcronous.

Add content implementation of digestSynchcronous.

BUG=327826
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=252783

[jww, 2014-02-07 23:24:17]

This is dependent on the Blink CL: https://codereview.chromium.org/151673005/

I wasn't entirely sure of the best way to implement this. In practice, the
signature of DigestInternal currently guarantees that it will provide
synchronous results, so it seemed really silly to just copy and paste the
digest() behavior into digestSynchronous(), in case changes were ever made to
digest(). Thus, I made digestSynchronous() simple call digest().

However this has the obvious disadvantage that if digest() ever becomes
asynchronous, digestSynchronous() will inadvertently become asynchronous as well
and will need to be reimplemented. Additionally, it means that there's really no
way to unit test digestSychronous since all of the unit tests are based on
DigestInternal(), not digest().

Long story short, comments and thoughts on the implementation are more than
welcome.

[jww, 2014-02-08 00:44:55]

This is updated with the simplified digestSychronous API in CL
https://codereview.chromium.org/151673005/. These changes actually answer a
bunch of my questions.

[jww, 2014-02-19 02:24:42]

On 2014/02/08 02:32:30, jww wrote:

Hi eroman. Now that the Blink CL is in, let me know how this looks. Thanks!

[eroman, 2014-02-21 23:11:20]

https://codereview.chromium.org/138343008/diff/170001/content/renderer/webcry...
File content/renderer/webcrypto/webcrypto_impl.cc (right):

https://codereview.chromium.org/138343008/diff/170001/content/renderer/webcry...
content/renderer/webcrypto/webcrypto_impl.cc:197: // This method synchronously
computes a digest for the given data, returning
Can you move this comment to the header file?

https://codereview.chromium.org/138343008/diff/170001/content/renderer/webcry...
content/renderer/webcrypto/webcrypto_impl.cc:199: // webcrypto::Digest()
implementation being synchronous as well, so if that ever
Please remove this comment, I think the function signatures already explain
that.

https://codereview.chromium.org/138343008/diff/170001/content/renderer/webcry...
content/renderer/webcrypto/webcrypto_impl.cc:202: const
blink::WebCryptoAlgorithmId algorithm_id,
in the header this is called "algorithm". Please use a consistent name
throughout.

https://codereview.chromium.org/138343008/diff/170001/content/renderer/webcry...
content/renderer/webcrypto/webcrypto_impl.cc:205: blink::WebArrayBuffer& result)
{
did you run "git cl format". If not please do, trying to keep these files
formatted by that tool.

https://codereview.chromium.org/138343008/diff/170001/content/renderer/webcry...
File content/renderer/webcrypto/webcrypto_impl.h (right):

https://codereview.chromium.org/138343008/diff/170001/content/renderer/webcry...
content/renderer/webcrypto/webcrypto_impl.h:67: blink::WebArrayBuffer& result);
Have you tested if this works? Try adding an OVERRIDE as a test to make sure the
signature is right. I seem to remember we settled on WebVector rather than
WebArrayBuffer.

Other than that, L G T M.

[jww, 2014-02-21 23:23:33]

https://codereview.chromium.org/138343008/diff/170001/content/renderer/webcry...
File content/renderer/webcrypto/webcrypto_impl.cc (right):

https://codereview.chromium.org/138343008/diff/170001/content/renderer/webcry...
content/renderer/webcrypto/webcrypto_impl.cc:197: // This method synchronously
computes a digest for the given data, returning
On 2014/02/21 23:11:20, eroman wrote:
> Can you move this comment to the header file?

Done.

https://codereview.chromium.org/138343008/diff/170001/content/renderer/webcry...
content/renderer/webcrypto/webcrypto_impl.cc:199: // webcrypto::Digest()
implementation being synchronous as well, so if that ever
On 2014/02/21 23:11:20, eroman wrote:
> Please remove this comment, I think the function signatures already explain
> that.

Done.

https://codereview.chromium.org/138343008/diff/170001/content/renderer/webcry...
content/renderer/webcrypto/webcrypto_impl.cc:202: const
blink::WebCryptoAlgorithmId algorithm_id,
On 2014/02/21 23:11:20, eroman wrote:
> in the header this is called "algorithm". Please use a consistent name
> throughout.

Done.

https://codereview.chromium.org/138343008/diff/170001/content/renderer/webcry...
content/renderer/webcrypto/webcrypto_impl.cc:205: blink::WebArrayBuffer& result)
{
On 2014/02/21 23:11:20, eroman wrote:
> did you run "git cl format". If not please do, trying to keep these files
> formatted by that tool.

Yup, I ran git-cl format over this. In fact, the wacky bit below was the result
of that. If you have a preferred formatting for this, I'm happy to format it as
such, as this is objectively pretty ugly :-)

https://codereview.chromium.org/138343008/diff/170001/content/renderer/webcry...
File content/renderer/webcrypto/webcrypto_impl.h (right):

https://codereview.chromium.org/138343008/diff/170001/content/renderer/webcry...
content/renderer/webcrypto/webcrypto_impl.h:67: blink::WebArrayBuffer& result);
On 2014/02/21 23:11:20, eroman wrote:
> Have you tested if this works? Try adding an OVERRIDE as a test to make sure
the
> signature is right. I seem to remember we settled on WebVector rather than
> WebArrayBuffer.
> 
> Other than that, L G T M.

In the end, you approved the version that was WebArrayBuffer and that's what was
committed. I've definitely confirmed locally that this works.

[eroman, 2014-02-21 23:39:05]

lgtm

[jww, 2014-02-22 00:39:07]

The CQ bit was checked by jww@chromium.org

[commit-bot: I haz the power, 2014-02-22 00:39:12]

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

[commit-bot: I haz the power, 2014-02-22 01:33:57]

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

[commit-bot: I haz the power, 2014-02-22 08:32:57]

Change committed as 252783