Bug 123216 – Add CryptoKey base class and bindings

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[Alexey Proskuryakov]

Add CryptoKey base class and bindings.

[Alexey Proskuryakov]

Created attachment 214994 [details]
proposed patch

[Alexey Proskuryakov]

Created attachment 214997 [details]
with release build fix

[Sam Weinig]

Comment on attachment 214997 [details]
with release build fix

View in context: https://bugs.webkit.org/attachment.cgi?id=214997&action=review

r=me

> Source/WebCore/bindings/js/JSCryptoAlgorithmBuilder.h:49
> +    JSC::ExecState* m_exec;

It might be slightly more efficient to store a VM& here.  I think everything except perhaps constructEmptyObject has an override that takes a VM& or VM*.

> Source/WebCore/bindings/js/JSCryptoAlgorithmBuilder.h:50
> +    JSC::Strong<JSC::JSObject> m_dictionary;

If this is only ever used on the stack, does this have to be a Strong?

[Alexey Proskuryakov]

> It might be slightly more efficient to store a VM& here.  I think everything except perhaps constructEmptyObject has an override that takes a VM& or VM*.

Yes, I started with VM, but had to switch to ExecState because I'll need to call constructEmptyObject again for nested dictionaries (it's not in the patch right now).

> If this is only ever used on the stack, does this have to be a Strong?

Good point, I copied it from elsewhere thoughtlessly.

[Alexey Proskuryakov]

Committed <http://trac.webkit.org/r157936>.

[Darin Adler]

Comment on attachment 214997 [details]
with release build fix

View in context: https://bugs.webkit.org/attachment.cgi?id=214997&action=review

> Source/WebCore/bindings/js/JSCryptoAlgorithmBuilder.h:45
> +    virtual void add(const char*, unsigned long) OVERRIDE;

Why “unsigned long” instead of unsigned?

[Alexey Proskuryakov]

> Why “unsigned long” instead of unsigned?

This is for adding dictionary items that are marked "unsigned long" in IDL, what should this map to in C++ code?

In practice, key length could as well be 16 bit:

dictionary HmacKeyParams : Algorithm {
  // The inner hash function to use.
  AlgorithmIdentifier hash;

  // The length (in bytes) of the key to generate. If unspecified, the
  // recommended length will be used, which is the size of the associated hash function's block
  // size.
  unsigned long length?;
};

[Darin Adler]

(In reply to comment #7)
> > Why “unsigned long” instead of unsigned?
> 
> This is for adding dictionary items that are marked "unsigned long" in IDL, what should this map to in C++ code?

unsigned.

I know, it’s not obvious.