Bug 160067

<rdar://problem/27483617>

Created attachment 284292 [details] Patch

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.

Created attachment 284298 [details] Patch

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.

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.

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.

Created attachment 284353 [details] Patch

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.

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"

Created attachment 284500 [details] Patch for landing

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.

Comment on attachment 284500 [details] Patch for landing Clearing flags on attachment: 284500 Committed r203696: <http://trac.webkit.org/changeset/203696>