Bug 160067 – Rename SubtleCrypto to WebKitSubtleCrypto

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

[Jiewen Tan]

Rename SubtleCrypto to WebKitSubtleCrypto such that we are backward compatible and meanwhile allow the new implementation to use the name SubtleCrypto.

[Radar WebKit Bug Importer]

<rdar://problem/27483617>

[Jiewen Tan]

Created attachment 284292 [details]
Patch

[WebKit Commit Bot]

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


ERROR: Source/WebCore/bindings/js/JSWebKitSubtleCryptoCustom.cpp:55:  enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums.  [readability/enum_casing] [4]
ERROR: Source/WebCore/bindings/js/JSWebKitSubtleCryptoCustom.cpp:58:  enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums.  [readability/enum_casing] [4]
ERROR: Source/WebCore/bindings/js/JSWebKitSubtleCryptoCustom.cpp:61:  enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums.  [readability/enum_casing] [4]
WARNING: Not running on native Windows.
Total errors found: 3 in 141 files


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

[Jiewen Tan]

Created attachment 284298 [details]
Patch

[WebKit Commit Bot]

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


ERROR: Source/WebCore/bindings/js/JSWebKitSubtleCryptoCustom.cpp:55:  enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums.  [readability/enum_casing] [4]
ERROR: Source/WebCore/bindings/js/JSWebKitSubtleCryptoCustom.cpp:58:  enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums.  [readability/enum_casing] [4]
ERROR: Source/WebCore/bindings/js/JSWebKitSubtleCryptoCustom.cpp:61:  enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums.  [readability/enum_casing] [4]
WARNING: Not running on native Windows.
Total errors found: 3 in 142 files


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

[Brent Fulgham]

Comment on attachment 284298 [details]
Patch

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

This looks great, but I disagree with moving all of the crypto things into the new "webkitCrypto" folder. Can you please only move the things actually using the "Crypto.webkitSubtle" JS API?

R- to correct that problem, but otherwise this looks great.

> Source/WebCore/ChangeLog:67
> +               crypto/webkitSubtle/wrapKey-check-usage.html

I don't think you need to move all of these to a new directory. Many of these tests invoke "Crypto.subtle" or AES/RSA stuff directly. Those should stay in Crypo since they are unchanged.

Only the things invoking "Crypto.webkitSubtle" should be in a crypto/webkitSubtle directory.

> Source/WebCore/ChangeLog:71
> +        current JSBindings use, and therefore should not introduce any change of behavoir. 

Behavoir -> behavior.

> Source/WebCore/ChangeLog:73
> +        * CMakeLists.txt:

Add comment: "Revise project files for for new file names"

> Source/WebCore/ChangeLog:95
> +        (WebCore::JSWebKitSubtleCrypto::unwrapKey):

Delete lines 81-95. We don't need to see all of the renamed things here.

> Source/WebCore/crypto/WebKitSubtleCrypto.h:27
> +#define WebKitSubtleCrypto_h

#pragma once

> Source/WebCore/page/Crypto.h:67
> +    RefPtr<WebKitSubtleCrypto> m_webkitSubtle;

You don't really need this rename, but I assume you will be adding a "RefPtr<SubtleCrypto> m_subtle;" in a future patch.

[Jiewen Tan]

Comment on attachment 284298 [details]
Patch

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

Thanks Brent for reviewing my patch.

>> Source/WebCore/ChangeLog:67
>> +               crypto/webkitSubtle/wrapKey-check-usage.html
> 
> I don't think you need to move all of these to a new directory. Many of these tests invoke "Crypto.subtle" or AES/RSA stuff directly. Those should stay in Crypo since they are unchanged.
> 
> Only the things invoking "Crypto.webkitSubtle" should be in a crypto/webkitSubtle directory.

Hmmm, I found out that codes in resources/common.js:105:
      if (!crypto.subtle)
           crypto.subtle = crypto.webkitSubtle;
enforce all the tests running against crypto.webkitSubtle. That's why even though we haven't implement the interface crypto.subtle, tests can still use that interface.

Despite this little finding, I agree with you that we should keep those tests in the original folder.

>> Source/WebCore/ChangeLog:71
>> +        current JSBindings use, and therefore should not introduce any change of behavoir. 
> 
> Behavoir -> behavior.

Fixed.

>> Source/WebCore/ChangeLog:73
>> +        * CMakeLists.txt:
> 
> Add comment: "Revise project files for for new file names"

Added.

>> Source/WebCore/ChangeLog:95
>> +        (WebCore::JSWebKitSubtleCrypto::unwrapKey):
> 
> Delete lines 81-95. We don't need to see all of the renamed things here.

Deleted.

>> Source/WebCore/crypto/WebKitSubtleCrypto.h:27
>> +#define WebKitSubtleCrypto_h
> 
> #pragma once

Added.

>> Source/WebCore/page/Crypto.h:67
>> +    RefPtr<WebKitSubtleCrypto> m_webkitSubtle;
> 
> You don't really need this rename, but I assume you will be adding a "RefPtr<SubtleCrypto> m_subtle;" in a future patch.

Yes, I will add it in the future.

[Jiewen Tan]

Created attachment 284353 [details]
Patch

[WebKit Commit Bot]

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


WARNING: Not running on native Windows.
ERROR: Source/WebCore/bindings/js/JSWebKitSubtleCryptoCustom.cpp:55:  enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums.  [readability/enum_casing] [4]
ERROR: Source/WebCore/bindings/js/JSWebKitSubtleCryptoCustom.cpp:58:  enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums.  [readability/enum_casing] [4]
ERROR: Source/WebCore/bindings/js/JSWebKitSubtleCryptoCustom.cpp:61:  enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums.  [readability/enum_casing] [4]
Total errors found: 3 in 26 files


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

[Brent Fulgham]

Comment on attachment 284353 [details]
Patch

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

r=me.

> LayoutTests/crypto/webkitSubtle/gc-2-expected.txt:1
> +Test that window.crypto.subtle wrapper preserves custom properties.

Please change this to say "Test that window.crypto.webkitSubtle wrapper preserves custom properties"

> LayoutTests/crypto/webkitSubtle/gc-3-expected.txt:1
> +Test that window.crypto wrapper protects all dependencies, so it can always be used to create crypto.subtle.

Please change this to say "Test that window.crypto.webkitSubtle wrapper protects all ..."

> LayoutTests/crypto/webkitSubtle/gc-expected.txt:1
> +Test that window.crypto.subtle wrapper preserves custom properties.

Please change this to say "Test that window.crypto.webkitSubtle wrapper preserves custom properties"

[Jiewen Tan]

Created attachment 284500 [details]
Patch for landing

[WebKit Commit Bot]

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


WARNING: Not running on native Windows.
ERROR: Source/WebCore/bindings/js/JSWebKitSubtleCryptoCustom.cpp:55:  enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums.  [readability/enum_casing] [4]
ERROR: Source/WebCore/bindings/js/JSWebKitSubtleCryptoCustom.cpp:58:  enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums.  [readability/enum_casing] [4]
ERROR: Source/WebCore/bindings/js/JSWebKitSubtleCryptoCustom.cpp:61:  enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums.  [readability/enum_casing] [4]
Total errors found: 3 in 26 files


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

[WebKit Commit Bot]

Comment on attachment 284500 [details]
Patch for landing

Clearing flags on attachment: 284500

Committed r203696: <http://trac.webkit.org/changeset/203696>