Bug 163541 – [WebIDL] Support BufferSource

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Zan Dobersek]

[WebIDL] Support BufferSource

[Zan Dobersek]

Created attachment 291812 [details]
WIP

[WebKit Commit Bot]

Attachment 291812 [details] did not pass style-queue:


ERROR: Source/WebCore/crypto/algorithms/CryptoAlgorithmRSA_OAEP.cpp:72:  CryptoAlgorithmRSA_OAEP::encrypt is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/crypto/algorithms/CryptoAlgorithmRSA_OAEP.cpp:84:  CryptoAlgorithmRSA_OAEP::decrypt is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/crypto/gnutls/CryptoAlgorithmAES_CBCGnuTLS.cpp:38:  CryptoAlgorithmAES_CBC::platformEncrypt is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/crypto/gnutls/CryptoAlgorithmAES_CBCGnuTLS.cpp:50:  CryptoAlgorithmAES_CBC::platformDecrypt is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/crypto/mac/CryptoAlgorithmRSASSA_PKCS1_v1_5Mac.cpp:62:  CryptoAlgorithmRSASSA_PKCS1_v1_5::platformSign is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/crypto/mac/CryptoAlgorithmRSASSA_PKCS1_v1_5Mac.cpp:99:  CryptoAlgorithmRSASSA_PKCS1_v1_5::platformVerify is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/crypto/gnutls/CryptoAlgorithmRSA_OAEPGnuTLS.cpp:38:  CryptoAlgorithmRSA_OAEP::platformEncrypt is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/crypto/gnutls/CryptoAlgorithmRSA_OAEPGnuTLS.cpp:50:  CryptoAlgorithmRSA_OAEP::platformDecrypt is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/crypto/mac/CryptoAlgorithmRSA_OAEPMac.cpp:38:  CryptoAlgorithmRSA_OAEP::platformEncrypt is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/crypto/mac/CryptoAlgorithmRSA_OAEPMac.cpp:59:  CryptoAlgorithmRSA_OAEP::platformDecrypt is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/crypto/mac/CryptoAlgorithmAES_CBCMac.cpp:38:  transformAES_CBC is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/crypto/mac/CryptoAlgorithmAES_CBCMac.cpp:85:  CryptoAlgorithmAES_CBC::platformEncrypt is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/crypto/mac/CryptoAlgorithmAES_CBCMac.cpp:90:  CryptoAlgorithmAES_CBC::platformDecrypt is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/crypto/gnutls/CryptoAlgorithmAES_KWGnuTLS.cpp:37:  CryptoAlgorithmAES_KW::platformEncrypt is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/crypto/gnutls/CryptoAlgorithmAES_KWGnuTLS.cpp:48:  CryptoAlgorithmAES_KW::platformDecrypt is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/crypto/algorithms/CryptoAlgorithmAES_CBC.cpp:68:  CryptoAlgorithmAES_CBC::encrypt is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/crypto/algorithms/CryptoAlgorithmAES_CBC.cpp:80:  CryptoAlgorithmAES_CBC::decrypt is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/crypto/mac/CryptoAlgorithmRSAES_PKCS1_v1_5Mac.cpp:36:  CryptoAlgorithmRSAES_PKCS1_v1_5::platformEncrypt is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/crypto/mac/CryptoAlgorithmRSAES_PKCS1_v1_5Mac.cpp:50:  CryptoAlgorithmRSAES_PKCS1_v1_5::platformDecrypt is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/crypto/algorithms/CryptoAlgorithmAES_KW.cpp:67:  CryptoAlgorithmAES_KW::encryptForWrapKey is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/crypto/algorithms/CryptoAlgorithmAES_KW.cpp:77:  CryptoAlgorithmAES_KW::decryptForUnwrapKey is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/crypto/algorithms/CryptoAlgorithmRSASSA_PKCS1_v1_5.cpp:72:  CryptoAlgorithmRSASSA_PKCS1_v1_5::sign is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/crypto/algorithms/CryptoAlgorithmRSASSA_PKCS1_v1_5.cpp:84:  CryptoAlgorithmRSASSA_PKCS1_v1_5::verify is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/crypto/algorithms/CryptoAlgorithmRSAES_PKCS1_v1_5.cpp:68:  CryptoAlgorithmRSAES_PKCS1_v1_5::encrypt is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/crypto/algorithms/CryptoAlgorithmRSAES_PKCS1_v1_5.cpp:78:  CryptoAlgorithmRSAES_PKCS1_v1_5::decrypt is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/crypto/gnutls/CryptoAlgorithmRSASSA_PKCS1_v1_5GnuTLS.cpp:38:  CryptoAlgorithmRSASSA_PKCS1_v1_5::platformSign is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/crypto/gnutls/CryptoAlgorithmRSASSA_PKCS1_v1_5GnuTLS.cpp:50:  CryptoAlgorithmRSASSA_PKCS1_v1_5::platformVerify is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/ChangeLog:8:  You should remove the 'No new tests' and either add and list tests, or explain why no new tests were possible.  [changelog/nonewtests] [5]
ERROR: Source/WebCore/crypto/gnutls/CryptoAlgorithmRSAES_PKCS1_v1_5GnuTLS.cpp:37:  CryptoAlgorithmRSAES_PKCS1_v1_5::platformEncrypt is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/crypto/gnutls/CryptoAlgorithmRSAES_PKCS1_v1_5GnuTLS.cpp:48:  CryptoAlgorithmRSAES_PKCS1_v1_5::platformDecrypt is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/crypto/mac/CryptoAlgorithmAES_KWMac.cpp:37:  CryptoAlgorithmAES_KW::platformEncrypt is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/crypto/mac/CryptoAlgorithmAES_KWMac.cpp:57:  CryptoAlgorithmAES_KW::platformDecrypt is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/bindings/js/JSWebKitSubtleCryptoCustom.cpp:673:  Missing space before {  [whitespace/braces] [5]
ERROR: Source/WebCore/bindings/js/JSWebKitSubtleCryptoCustom.cpp:774:  Missing space before {  [whitespace/braces] [5]
Total errors found: 34 in 50 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[youenn fablet]

Comment on attachment 291812 [details]
WIP

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

> Source/WebCore/bindings/js/JSDOMConvert.h:657
> +        throwTypeError(&state, scope);

I would add an error message explaining the reason of the error.

> Source/WebCore/bindings/js/JSWebKitSubtleCryptoCustom.cpp:-169
> -    auto success = cryptoOperationDataFromJSValue(&state, state.uncheckedArgument(2), data);

Can we remove Source/WebCore/bindings/js/JSCryptoAlgorithmDictionary.cpp as well?

> Source/WebCore/crypto/WebKitSubtleCrypto.idl:-35
> -    [Custom] Promise digest(AlgorithmIdentifier algorithm, sequence<CryptoOperationData> data);

This is a legacy API and is not supposed to be changed.
But at the same time, all of these methods are custom so it is probably fine.

[youenn fablet]

> Can we remove Source/WebCore/bindings/js/JSCryptoAlgorithmDictionary.cpp as
> well?
Would bad copy-paste..
I was meaning "Can we remove cryptoOperationDataFromJSValue function"...

[Zan Dobersek]

(In reply to comment #4)
> > Can we remove Source/WebCore/bindings/js/JSCryptoAlgorithmDictionary.cpp as
> > well?
> Would bad copy-paste..
> I was meaning "Can we remove cryptoOperationDataFromJSValue function"...

The two uses of cryptoOperationDataFromJSValue() in JSCryptoAlgorithmDictionary.cpp can be replaced with BufferSource as well. That would enable removing the JSCryptoOperationData files.

[Zan Dobersek]

Created attachment 291824 [details]
WIP

Need to fix xcode builds.

[Jiewen Tan]

Hi Youenn and Zan,

Thank you for making the support for BufferSource. I would definitely like to use the BufferSource in my new WebCrypto API once it is ready. However, my opinion of the old interface remains the same: keep it intact. I am not quite sure why there is a must to change the old CryptoOperationData to BufferSource. Even though this change probably wouldn't break anything, I just don't see a must for replacing CryptoOperationData with BufferSource.

[youenn fablet]

(In reply to comment #7)
> Hi Youenn and Zan,
> 
> Thank you for making the support for BufferSource. I would definitely like
> to use the BufferSource in my new WebCrypto API once it is ready. However,
> my opinion of the old interface remains the same: keep it intact. I am not
> quite sure why there is a must to change the old CryptoOperationData to
> BufferSource. Even though this change probably wouldn't break anything, I
> just don't see a must for replacing CryptoOperationData with BufferSource.

From the legacy crypto API point of view, AFAICS, it is just renaming the cryptoOperationDataFromJSValue function. I think it is good to do that since the function is more generic than crypto and we do not want twice the same method in our code base.

The change to WebKit legacy crypto IDL is not actually needed, it does not change anything since all methods are custom I think. But it clarifies things since the custom binding is not supporting sequence<> parameters.

[Zan Dobersek]

Created attachment 291833 [details]
WIP

[WebKit Commit Bot]

Attachment 291833 [details] did not pass style-queue:


ERROR: Source/WebCore/crypto/algorithms/CryptoAlgorithmRSA_OAEP.cpp:72:  CryptoAlgorithmRSA_OAEP::encrypt is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/crypto/algorithms/CryptoAlgorithmRSA_OAEP.cpp:84:  CryptoAlgorithmRSA_OAEP::decrypt is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/crypto/gnutls/CryptoAlgorithmAES_CBCGnuTLS.cpp:38:  CryptoAlgorithmAES_CBC::platformEncrypt is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/crypto/gnutls/CryptoAlgorithmAES_CBCGnuTLS.cpp:50:  CryptoAlgorithmAES_CBC::platformDecrypt is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/crypto/mac/CryptoAlgorithmRSASSA_PKCS1_v1_5Mac.cpp:62:  CryptoAlgorithmRSASSA_PKCS1_v1_5::platformSign is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/crypto/mac/CryptoAlgorithmRSASSA_PKCS1_v1_5Mac.cpp:99:  CryptoAlgorithmRSASSA_PKCS1_v1_5::platformVerify is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/crypto/mac/CryptoAlgorithmRSA_OAEPMac.cpp:38:  CryptoAlgorithmRSA_OAEP::platformEncrypt is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/crypto/mac/CryptoAlgorithmRSA_OAEPMac.cpp:59:  CryptoAlgorithmRSA_OAEP::platformDecrypt is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/crypto/mac/CryptoAlgorithmAES_CBCMac.cpp:38:  transformAES_CBC is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/crypto/mac/CryptoAlgorithmAES_CBCMac.cpp:85:  CryptoAlgorithmAES_CBC::platformEncrypt is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/crypto/mac/CryptoAlgorithmAES_CBCMac.cpp:90:  CryptoAlgorithmAES_CBC::platformDecrypt is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/crypto/gnutls/CryptoAlgorithmAES_KWGnuTLS.cpp:37:  CryptoAlgorithmAES_KW::platformEncrypt is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/crypto/gnutls/CryptoAlgorithmAES_KWGnuTLS.cpp:48:  CryptoAlgorithmAES_KW::platformDecrypt is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/crypto/algorithms/CryptoAlgorithmAES_CBC.cpp:68:  CryptoAlgorithmAES_CBC::encrypt is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/crypto/algorithms/CryptoAlgorithmAES_CBC.cpp:80:  CryptoAlgorithmAES_CBC::decrypt is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/crypto/mac/CryptoAlgorithmRSAES_PKCS1_v1_5Mac.cpp:36:  CryptoAlgorithmRSAES_PKCS1_v1_5::platformEncrypt is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/crypto/mac/CryptoAlgorithmRSAES_PKCS1_v1_5Mac.cpp:50:  CryptoAlgorithmRSAES_PKCS1_v1_5::platformDecrypt is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/crypto/algorithms/CryptoAlgorithmAES_KW.cpp:67:  CryptoAlgorithmAES_KW::encryptForWrapKey is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/crypto/algorithms/CryptoAlgorithmAES_KW.cpp:77:  CryptoAlgorithmAES_KW::decryptForUnwrapKey is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/crypto/algorithms/CryptoAlgorithmRSASSA_PKCS1_v1_5.cpp:72:  CryptoAlgorithmRSASSA_PKCS1_v1_5::sign is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/crypto/algorithms/CryptoAlgorithmRSASSA_PKCS1_v1_5.cpp:84:  CryptoAlgorithmRSASSA_PKCS1_v1_5::verify is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/crypto/gnutls/CryptoAlgorithmRSA_OAEPGnuTLS.cpp:38:  CryptoAlgorithmRSA_OAEP::platformEncrypt is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/crypto/gnutls/CryptoAlgorithmRSA_OAEPGnuTLS.cpp:50:  CryptoAlgorithmRSA_OAEP::platformDecrypt is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/crypto/algorithms/CryptoAlgorithmRSAES_PKCS1_v1_5.cpp:68:  CryptoAlgorithmRSAES_PKCS1_v1_5::encrypt is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/crypto/algorithms/CryptoAlgorithmRSAES_PKCS1_v1_5.cpp:78:  CryptoAlgorithmRSAES_PKCS1_v1_5::decrypt is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/crypto/gnutls/CryptoAlgorithmRSASSA_PKCS1_v1_5GnuTLS.cpp:38:  CryptoAlgorithmRSASSA_PKCS1_v1_5::platformSign is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/crypto/gnutls/CryptoAlgorithmRSASSA_PKCS1_v1_5GnuTLS.cpp:50:  CryptoAlgorithmRSASSA_PKCS1_v1_5::platformVerify is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/ChangeLog:8:  You should remove the 'No new tests' and either add and list tests, or explain why no new tests were possible.  [changelog/nonewtests] [5]
ERROR: Source/WebCore/crypto/gnutls/CryptoAlgorithmRSAES_PKCS1_v1_5GnuTLS.cpp:37:  CryptoAlgorithmRSAES_PKCS1_v1_5::platformEncrypt is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/crypto/gnutls/CryptoAlgorithmRSAES_PKCS1_v1_5GnuTLS.cpp:48:  CryptoAlgorithmRSAES_PKCS1_v1_5::platformDecrypt is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/crypto/mac/CryptoAlgorithmAES_KWMac.cpp:37:  CryptoAlgorithmAES_KW::platformEncrypt is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/crypto/mac/CryptoAlgorithmAES_KWMac.cpp:57:  CryptoAlgorithmAES_KW::platformDecrypt is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/bindings/js/JSWebKitSubtleCryptoCustom.cpp:673:  Missing space before {  [whitespace/braces] [5]
ERROR: Source/WebCore/bindings/js/JSWebKitSubtleCryptoCustom.cpp:774:  Missing space before {  [whitespace/braces] [5]
Total errors found: 34 in 52 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Zan Dobersek]

(In reply to comment #7)
> Hi Youenn and Zan,
> 
> Thank you for making the support for BufferSource. I would definitely like
> to use the BufferSource in my new WebCrypto API once it is ready. However,
> my opinion of the old interface remains the same: keep it intact. I am not
> quite sure why there is a must to change the old CryptoOperationData to
> BufferSource. Even though this change probably wouldn't break anything, I
> just don't see a must for replacing CryptoOperationData with BufferSource.

FWIW I'm totally fine reverting any changes to the crypto interfaces. I started with those because that's where I spotted BufferSource first, but later also found it could be used on the SourceBuffer interface.

[Zan Dobersek]

Created attachment 291840 [details]
WIP

Loses the crypto changes, should build everywhere, needs a bindings test.

[Jiewen Tan]

(In reply to comment #11)
> (In reply to comment #7)
> > Hi Youenn and Zan,
> > 
> > Thank you for making the support for BufferSource. I would definitely like
> > to use the BufferSource in my new WebCrypto API once it is ready. However,
> > my opinion of the old interface remains the same: keep it intact. I am not
> > quite sure why there is a must to change the old CryptoOperationData to
> > BufferSource. Even though this change probably wouldn't break anything, I
> > just don't see a must for replacing CryptoOperationData with BufferSource.
> 
> FWIW I'm totally fine reverting any changes to the crypto interfaces. I
> started with those because that's where I spotted BufferSource first, but
> later also found it could be used on the SourceBuffer interface.

Thank you. Actually the old spec the old interface is built against doesn't use BufferSource. So I will really appreciate it if you can revert your changes against to the crypto interfaces.

[WebKit Commit Bot]

Attachment 291840 [details] did not pass style-queue:


ERROR: Source/WebCore/ChangeLog:8:  You should remove the 'No new tests' and either add and list tests, or explain why no new tests were possible.  [changelog/nonewtests] [5]
Total errors found: 1 in 11 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Jiewen Tan]

(In reply to comment #8)
> (In reply to comment #7)
> > Hi Youenn and Zan,
> > 
> > Thank you for making the support for BufferSource. I would definitely like
> > to use the BufferSource in my new WebCrypto API once it is ready. However,
> > my opinion of the old interface remains the same: keep it intact. I am not
> > quite sure why there is a must to change the old CryptoOperationData to
> > BufferSource. Even though this change probably wouldn't break anything, I
> > just don't see a must for replacing CryptoOperationData with BufferSource.
> 
> From the legacy crypto API point of view, AFAICS, it is just renaming the
> cryptoOperationDataFromJSValue function. I think it is good to do that since
> the function is more generic than crypto and we do not want twice the same
> method in our code base.
> 
> The change to WebKit legacy crypto IDL is not actually needed, it does not
> change anything since all methods are custom I think. But it clarifies
> things since the custom binding is not supporting sequence<> parameters.

I don't quite get why cryptoOperationDataFromJSValue is more generic than crypto. Besides the old WebCrypto interface, I don't see any other places that use it. In my new implementation, I don't have any plan to use this function at least at this point.

[Zan Dobersek]

Created attachment 291921 [details]
Patch

[youenn fablet]

Comment on attachment 291921 [details]
Patch

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

> Source/WebCore/bindings/js/JSDOMBinding.h:26
> +#include "BufferSource.h"

Can we do without this include here?

> Source/WebCore/bindings/js/JSDOMConvert.h:645
> +    using ReturnType = BufferSource;

Not sure this adds much?

> Source/WebCore/bindings/scripts/CodeGenerator.pm:575
> +sub IsBufferSourceType

Not sure this function is really needed.

> Source/WebCore/bindings/scripts/CodeGenerator.pm:878
> +    return 0 if $object->IsTypedArrayType($type) || $object->IsBufferSourceType($type);

If we remove IsBufferSourceType, we could write return 0 if $type eq "BufferSource" instead which sounds good enough.

> Source/WebCore/bindings/scripts/CodeGeneratorJS.pm:322
> +    return if $codeGenerator->IsTypedArrayType($interfaceName) || $codeGenerator->IsBufferSourceType($interfaceName);

Maybe do the three checks in the same way: the first is separated from IsTypedArrayType and the next twos are combined.

[youenn fablet]

> I don't quite get why cryptoOperationDataFromJSValue is more generic than
> crypto. Besides the old WebCrypto interface, I don't see any other places
> that use it. In my new implementation, I don't have any plan to use this
> function at least at this point.

There is no crypto-specific bits in cryptoOperationDataFromJSValue, it is just converting a BufferSource in a struct that can be used more easily by WebCore.

In general, the fact that the Crypto API is legacy and its functionality should be kept the same does not prevent us to do some refactoring to improve the code base (removing duplicate code).

I agree this could be fixed as a follow-up patch.

[Zan Dobersek]

Created attachment 291935 [details]
Patch for landing

[Zan Dobersek]

Comment on attachment 291935 [details]
Patch for landing

Clearing flags on attachment: 291935

Committed r207462: <http://trac.webkit.org/changeset/207462>

[Zan Dobersek]

All reviewed patches have been landed.  Closing bug.

[Jiewen Tan]

(In reply to comment #18)
> > I don't quite get why cryptoOperationDataFromJSValue is more generic than
> > crypto. Besides the old WebCrypto interface, I don't see any other places
> > that use it. In my new implementation, I don't have any plan to use this
> > function at least at this point.
> 
> There is no crypto-specific bits in cryptoOperationDataFromJSValue, it is
> just converting a BufferSource in a struct that can be used more easily by
> WebCore.
> 
> In general, the fact that the Crypto API is legacy and its functionality
> should be kept the same does not prevent us to do some refactoring to
> improve the code base (removing duplicate code).
> 
> I agree this could be fixed as a follow-up patch.

I see. We could remove this function or the file together when we deprecate the whole WebkitSubtleCrypto thing.