Created attachment 313933 [details] WIP patch

Attachment 313933 [details] did not pass style-queue: ERROR: Source/WebCore/PAL/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/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: 2 in 6 files If any of these errors are false positives, please file a bug against check-webkit-style.

Created attachment 314031 [details] Patch

(In reply to Zan Dobersek from comment #3) > Created attachment 314031 [details] > Patch This is ready for review, but the patch still relies on libtasn1 helpers that are being added in bug #173646.

Created attachment 314464 [details] Patch Now buildable.

Comment on attachment 314464 [details] Patch In general, I don't believe we need to encrypt CryptoKey objects when they are stored into the indexedDB. However, this functionality has been implemented before I take over the WebCrypto API, and it is hard to remove for backward compatibility. I recommend GTK+ not to follow this approach at least you have legitimate reasons for it.

(In reply to Jiewen Tan from comment #6) > Comment on attachment 314464 [details] > Patch > > In general, I don't believe we need to encrypt CryptoKey objects when they > are stored into the indexedDB. However, this functionality has been > implemented before I take over the WebCrypto API, and it is hard to remove > for backward compatibility. I recommend GTK+ not to follow this approach at > least you have legitimate reasons for it. If I'm not mistaken, serialization support is still required for passing keys into worker contexts. The main intent was to gain support for that. Do you plan to change how that is supported?

(In reply to Zan Dobersek from comment #7) > (In reply to Jiewen Tan from comment #6) > > Comment on attachment 314464 [details] > > Patch > > > > In general, I don't believe we need to encrypt CryptoKey objects when they > > are stored into the indexedDB. However, this functionality has been > > implemented before I take over the WebCrypto API, and it is hard to remove > > for backward compatibility. I recommend GTK+ not to follow this approach at > > least you have legitimate reasons for it. > > If I'm not mistaken, serialization support is still required for passing > keys into worker contexts. The main intent was to gain support for that. > > Do you plan to change how that is supported? Yes, that's true. But you don't have to encrypt/decrypt the binaries.

Understood, I will simplify the implementation.

Created attachment 315933 [details] WIP patch

Created attachment 315940 [details] Patch

Comment on attachment 315940 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=315940&action=review Thanks for taking my last comment. r- for the following reasons. Please address it. > Source/WebCore/crypto/gcrypt/SerializedCryptoKeyWrapGCrypt.cpp:44 > bool wrapSerializedCryptoKey(const Vector<uint8_t>& masterKey, const Vector<uint8_t>& key, Vector<uint8_t>& result) I am suggesting you just return the key. Maybe a copy is needed. That's it. The key here is already serialized. You don't have to do it again.

Created attachment 316444 [details] Patch

Comment on attachment 316444 [details] Patch Looks good to me. r=me.

Comment on attachment 316444 [details] Patch Clearing flags on attachment: 316444 Committed r219976: <http://trac.webkit.org/changeset/219976>

All reviewed patches have been landed. Closing bug.