Move crypto files out of base, to a top level directory.

Move crypto files out of base, to a top level directory.

src/crypto is now an independent project that contains our
cryptographic primitives (except md5 and sha1).

This removes the base dependency from nss, openssl and sqlite.

BUG=76996
TEST=none
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=81611

[rvargas (doing something else), 2011-04-06 22:02:38]

This is not complete, but it's probably better if you start taking a look now :)

In particular, I have not changed all the callers from src\remoting and
src\chrome so I expect a few compilation errors with these files.

[wtc, 2011-04-07 05:35:53]

LGTM.  Thanks a lot for writing this CL.  I've wanted to do
this for a long time.  (I forgot if I filed a bug about this
in the issue tracker.)

Eventually we should move sha1 into the crypto directory.

http://codereview.chromium.org/6805019/diff/1/base/nss_util.h
File base/nss_util.h (left):

http://codereview.chromium.org/6805019/diff/1/base/nss_util.h#oldcode88
base/nss_util.h:88: Time PRTimeToBaseTime(int64 prtime);
Can you find out if any code in 'base' calls PRTimeToBaseTime?

http://codereview.chromium.org/6805019/diff/1/crypto/encryptor_unittest.cc
File crypto/encryptor_unittest.cc (left):

http://codereview.chromium.org/6805019/diff/1/crypto/encryptor_unittest.cc#ol...
crypto/encryptor_unittest.cc:5: #include "base/crypto/encryptor.h"
It's correct to include "crypto/encryptor.h" first
in the encryptor_unittest.cc file.  The Style Guide shows
an example of this.

http://codereview.chromium.org/6805019/diff/1/crypto/nss_util.h
File crypto/nss_util.h (right):

http://codereview.chromium.org/6805019/diff/1/crypto/nss_util.h#newcode39
crypto/nss_util.h:39: // ever be initialized once.  NSS will be properly shut
down on program exit.
I believe EnsureNSSInit() no longer shuts down NSS on
program exit now.  Could you verify that and update this
comment?

Please also check if NSPR is shut down on program exit and
update the comment on lines 34-35 above if necessary.

http://codereview.chromium.org/6805019/diff/1/crypto/nss_util_internal.h
File crypto/nss_util_internal.h (right):

http://codereview.chromium.org/6805019/diff/1/crypto/nss_util_internal.h#newc...
crypto/nss_util_internal.h:5: #ifndef CRYPTO_NSS_SLOT_UTIL_H_
The include guard macro should be CRYPTO_NSS_UTIL_INTERNAL_H_.
(This is a pre-existing problem.)

http://codereview.chromium.org/6805019/diff/1/crypto/rsa_private_key_nss_unit...
File crypto/rsa_private_key_nss_unittest.cc (left):

http://codereview.chromium.org/6805019/diff/1/crypto/rsa_private_key_nss_unit...
crypto/rsa_private_key_nss_unittest.cc:5: #include
"base/crypto/rsa_private_key.h"
It is correct to include "crypto/rsa_private_key.h" first.

http://codereview.chromium.org/6805019/diff/1/crypto/rsa_private_key_unittest.cc
File crypto/rsa_private_key_unittest.cc (left):

http://codereview.chromium.org/6805019/diff/1/crypto/rsa_private_key_unittest...
crypto/rsa_private_key_unittest.cc:5: #include "base/crypto/rsa_private_key.h"
It is correct to include "crypto/rsa_private_key.h" first.

http://codereview.chromium.org/6805019/diff/1/crypto/run_all_unittests.cc
File crypto/run_all_unittests.cc (right):

http://codereview.chromium.org/6805019/diff/1/crypto/run_all_unittests.cc#new...
crypto/run_all_unittests.cc:10: crypto::EnsureNSSInit();
Why is this necessary?  Is this copied from
base/test/test_suite.cc?

Please add a comment to explain.

http://codereview.chromium.org/6805019/diff/1/crypto/secure_hash_unittest.cc
File crypto/secure_hash_unittest.cc (left):

http://codereview.chromium.org/6805019/diff/1/crypto/secure_hash_unittest.cc#...
crypto/secure_hash_unittest.cc:5: #include "base/crypto/secure_hash.h"
It's correct to include "crypto/secure_hash.h" first.

http://codereview.chromium.org/6805019/diff/1/crypto/sha2_unittest.cc
File crypto/sha2_unittest.cc (left):

http://codereview.chromium.org/6805019/diff/1/crypto/sha2_unittest.cc#oldcode5
crypto/sha2_unittest.cc:5: #include "base/sha2.h"
It is correct to include "crypto/sha2.h" first.

http://codereview.chromium.org/6805019/diff/1/crypto/symmetric_key_unittest.cc
File crypto/symmetric_key_unittest.cc (left):

http://codereview.chromium.org/6805019/diff/1/crypto/symmetric_key_unittest.c...
crypto/symmetric_key_unittest.cc:5: #include "base/crypto/symmetric_key.h"
It is correct to include "crypto/symmetric_key.h" first.

http://codereview.chromium.org/6805019/diff/1/net/base/run_all_unittests.cc
File net/base/run_all_unittests.cc (right):

http://codereview.chromium.org/6805019/diff/1/net/base/run_all_unittests.cc#n...
net/base/run_all_unittests.cc:16: // We want to be sure to init NSPR on the main
thread.
This should be unnecessary now that we leak NSPR on shutdown.
But you can keep this.

http://codereview.chromium.org/6805019/diff/1/net/base/x509_certificate.h
File net/base/x509_certificate.h (right):

http://codereview.chromium.org/6805019/diff/1/net/base/x509_certificate.h#new...
net/base/x509_certificate.h:41: }  // namespace base
base => crypto

[wtc, 2011-04-08 19:09:17]

rvargas: can you add an OWNERS file to the crypto directory?
You can list you, me, agl, and rsleevi.  Thanks.

[rvargas (doing something else), 2011-04-08 19:19:54]

Sure... do you want the file to block looking at the parent directory?

On Fri, Apr 8, 2011 at 12:09 PM, <wtc@chromium.org> wrote:

> rvargas: can you add an OWNERS file to the crypto directory?
> You can list you, me, agl, and rsleevi.  Thanks.
>
>
> http://codereview.chromium.org/6805019/
>

[wtc, 2011-04-08 19:53:47]

On 2011/04/08 19:19:54, rvargas wrote:
> Sure... do you want the file to block looking at the parent directory?

I think so.  (I didn't know it's possible to specify that in an OWNERS
file.)

[rvargas (doing something else), 2011-04-08 21:04:07]

Thanks.

I addressed all the comments, added the owners file and a bunch of other files
outside of base and net.

I'm still having issues with the try bots and chrome frame, but I expect Eric to
review the rest. Thanks again.

http://codereview.chromium.org/6805019/diff/1/base/nss_util.h
File base/nss_util.h (left):

http://codereview.chromium.org/6805019/diff/1/base/nss_util.h#oldcode88
base/nss_util.h:88: Time PRTimeToBaseTime(int64 prtime);
On 2011/04/07 05:35:53, wtc wrote:
> Can you find out if any code in 'base' calls PRTimeToBaseTime?

Nobody.

http://codereview.chromium.org/6805019/diff/1/crypto/encryptor_unittest.cc
File crypto/encryptor_unittest.cc (left):

http://codereview.chromium.org/6805019/diff/1/crypto/encryptor_unittest.cc#ol...
crypto/encryptor_unittest.cc:5: #include "base/crypto/encryptor.h"
On 2011/04/07 05:35:53, wtc wrote:
> It's correct to include "crypto/encryptor.h" first
> in the encryptor_unittest.cc file.  The Style Guide shows
> an example of this.

I wish the style guide did't change under me.

http://codereview.chromium.org/6805019/diff/1/crypto/nss_util.h
File crypto/nss_util.h (right):

http://codereview.chromium.org/6805019/diff/1/crypto/nss_util.h#newcode39
crypto/nss_util.h:39: // ever be initialized once.  NSS will be properly shut
down on program exit.
On 2011/04/07 05:35:53, wtc wrote:
> I believe EnsureNSSInit() no longer shuts down NSS on
> program exit now.  Could you verify that and update this
> comment?
> 
> Please also check if NSPR is shut down on program exit and
> update the comment on lines 34-35 above if necessary.

Done.

http://codereview.chromium.org/6805019/diff/1/crypto/run_all_unittests.cc
File crypto/run_all_unittests.cc (right):

http://codereview.chromium.org/6805019/diff/1/crypto/run_all_unittests.cc#new...
crypto/run_all_unittests.cc:10: crypto::EnsureNSSInit();
On 2011/04/07 05:35:53, wtc wrote:
> Why is this necessary?  Is this copied from
> base/test/test_suite.cc?
> 
> Please add a comment to explain.

Yes this comes from base, but base is not testing crypto code anymore. Instead,
this crypto_unittests should make sure that the library is initialized. Note
that base_unittests used to perform this in another place

[wtc, 2011-04-08 21:14:18]

crypto/OWNERS looks good to me.

http://codereview.chromium.org/6805019/diff/1/base/nss_util.h
File base/nss_util.h (left):

http://codereview.chromium.org/6805019/diff/1/base/nss_util.h#oldcode88
base/nss_util.h:88: Time PRTimeToBaseTime(int64 prtime);
Thank you for checking.  The reason I asked is that
PRTimeToBaseTime doesn't seem related to NSS, so I was
wondering if we should create nspr_util.h and move PRTimeToBaseTime
there.

I just searched for the users of PRTimeToBaseTime:
net/base/x509_certificate_nss.cc
common/net/x509_certificate_model_nss.cc

So it is only used in the context of NSS.  That's good.

[eroman, 2011-04-08 21:37:15]

sure, i can take this over in wtc's absence.
let me know when it is ready for me to look at.

[bradn, 2011-04-12 21:28:50]

http://codereview.chromium.org/6805019/diff/12032/crypto/crypto.gyp
File crypto/crypto.gyp (right):

http://codereview.chromium.org/6805019/diff/12032/crypto/crypto.gyp#newcode210
crypto/crypto.gyp:210: 8],
This might be bad.

[rvargas (doing something else), 2011-04-12 22:03:04]

http://codereview.chromium.org/6805019/diff/12032/crypto/crypto.gyp
File crypto/crypto.gyp (right):

http://codereview.chromium.org/6805019/diff/12032/crypto/crypto.gyp#newcode210
crypto/crypto.gyp:210: 8],
On 2011/04/12 21:28:50, bradn wrote:
> This might be bad.

Thanks a lot. That was it.

[rvargas (doing something else), 2011-04-13 01:51:58]

Eric:

Patch set 4 should pass all tests (just rebased from every test passing).
Unfortunately, I had to undo the move to be able to use the try servers (I'll
fix that before commit).

The last version that wtc looked at was 3, but only to base, crypto and net.
Anything outside those directories has not been reviewed.

Also, I added DEPS files everywhere, and there are minor changes to files
reviewed by wtc, mostly inside base (but the whole change is basically just
mechanic anyway). Sorry about that.

[eroman, 2011-04-13 03:25:22]

LGTM.

Two general suggestions:

 (1) You may want to grep for 'base/crypto' to verify everything got converted

 (2) Be aware that when you land this, the valgrind tests might start failing! I
believe we have some valgrind suppressions that include code in crypto, which
will no longer be matched once the mangled name changes.

http://codereview.chromium.org/6805019/diff/12032/crypto/run_all_unittests.cc
File crypto/run_all_unittests.cc (right):

http://codereview.chromium.org/6805019/diff/12032/crypto/run_all_unittests.cc...
crypto/run_all_unittests.cc:12: TODO(rvargas): remove this.
What is your plan to remove this? Is this a comment during development, or
something you plan to look into post landing? (If so, I suggest attaching the
bug number to it).

http://codereview.chromium.org/6805019/diff/12032/crypto/scoped_nss_types.h
File crypto/scoped_nss_types.h (right):

http://codereview.chromium.org/6805019/diff/12032/crypto/scoped_nss_types.h#n...
crypto/scoped_nss_types.h:5: #ifndef BASE_SCOPED_NSS_TYPES_H_
Update this header guard to use CRYPTO_

http://codereview.chromium.org/6805019/diff/12032/crypto/symmetric_key_nss.cc
File crypto/symmetric_key_nss.cc (right):

http://codereview.chromium.org/6805019/diff/12032/crypto/symmetric_key_nss.cc...
crypto/symmetric_key_nss.cc:5: #include "base/crypto/symmetric_key.h"
This needs to be updated yes?

http://codereview.chromium.org/6805019/diff/12032/crypto/symmetric_key_nss.cc...
crypto/symmetric_key_nss.cc:11: #include "base/logging.h"
nit: can you order these? (pre-existing problem).

http://codereview.chromium.org/6805019/diff/12032/crypto/symmetric_key_openss...
File crypto/symmetric_key_openssl.cc (right):

http://codereview.chromium.org/6805019/diff/12032/crypto/symmetric_key_openss...
crypto/symmetric_key_openssl.cc:5: #include "base/crypto/symmetric_key.h"
Update.

http://codereview.chromium.org/6805019/diff/12032/net/base/cert_database_nss_...
File net/base/cert_database_nss_unittest.cc (right):

http://codereview.chromium.org/6805019/diff/12032/net/base/cert_database_nss_...
net/base/cert_database_nss_unittest.cc:23: #include "net/base/crypto_module.h"
Has this file not moved to crypto?

http://codereview.chromium.org/6805019/diff/19001/crypto/capi_util.cc
File crypto/capi_util.cc (right):

http://codereview.chromium.org/6805019/diff/19001/crypto/capi_util.cc#newcode45
crypto/capi_util.cc:45: {
nit: can this brace move to previous line?

http://codereview.chromium.org/6805019/diff/19001/crypto/crypto.gyp
File crypto/crypto.gyp (right):

http://codereview.chromium.org/6805019/diff/19001/crypto/crypto.gyp#newcode32
crypto/crypto.gyp:32: 'base/crypto/scoped_nss_types.h',
this seems like an old file path, is base/crypto still going to exist?

http://codereview.chromium.org/6805019/diff/19001/crypto/crypto.gyp#newcode87
crypto/crypto.gyp:87: 'nss_util.cc',
nit: can you pipe this list through |sort| ?

http://codereview.chromium.org/6805019/diff/19001/crypto/crypto.gyp#newcode91
crypto/crypto.gyp:91: 'signature_creator_nss.cc',
nit: de-indent by 1 space.

[rvargas (doing something else), 2011-04-13 19:55:55]

Thanks for the review.

Brett: I'm touching some DEPS files and stuff inside src/content. Could you take
a look at that?

http://codereview.chromium.org/6805019/diff/12032/crypto/run_all_unittests.cc
File crypto/run_all_unittests.cc (right):

http://codereview.chromium.org/6805019/diff/12032/crypto/run_all_unittests.cc...
crypto/run_all_unittests.cc:12: TODO(rvargas): remove this.
On 2011/04/13 03:25:22, eroman wrote:
> What is your plan to remove this? Is this a comment during development, or
> something you plan to look into post landing? (If so, I suggest attaching the
> bug number to it).

Done.

http://codereview.chromium.org/6805019/diff/12032/crypto/scoped_nss_types.h
File crypto/scoped_nss_types.h (right):

http://codereview.chromium.org/6805019/diff/12032/crypto/scoped_nss_types.h#n...
crypto/scoped_nss_types.h:5: #ifndef BASE_SCOPED_NSS_TYPES_H_
On 2011/04/13 03:25:22, eroman wrote:
> Update this header guard to use CRYPTO_

Done.

http://codereview.chromium.org/6805019/diff/12032/crypto/symmetric_key_nss.cc
File crypto/symmetric_key_nss.cc (right):

http://codereview.chromium.org/6805019/diff/12032/crypto/symmetric_key_nss.cc...
crypto/symmetric_key_nss.cc:5: #include "base/crypto/symmetric_key.h"
On 2011/04/13 03:25:22, eroman wrote:
> This needs to be updated yes?

Weird. It's already fixed in my local version. In fact, it is fixed in this
review (patch set 4)

http://codereview.chromium.org/6805019/diff/12032/crypto/symmetric_key_nss.cc...
crypto/symmetric_key_nss.cc:11: #include "base/logging.h"
On 2011/04/13 03:25:22, eroman wrote:
> nit: can you order these? (pre-existing problem).

Same here... already fixed.

http://codereview.chromium.org/6805019/diff/12032/crypto/symmetric_key_openss...
File crypto/symmetric_key_openssl.cc (right):

http://codereview.chromium.org/6805019/diff/12032/crypto/symmetric_key_openss...
crypto/symmetric_key_openssl.cc:5: #include "base/crypto/symmetric_key.h"
On 2011/04/13 03:25:22, eroman wrote:
> Update.

Done.

http://codereview.chromium.org/6805019/diff/12032/net/base/cert_database_nss_...
File net/base/cert_database_nss_unittest.cc (right):

http://codereview.chromium.org/6805019/diff/12032/net/base/cert_database_nss_...
net/base/cert_database_nss_unittest.cc:23: #include "net/base/crypto_module.h"
On 2011/04/13 03:25:22, eroman wrote:
> Has this file not moved to crypto?

Nope. Only stuff from base, not from net/base.

http://codereview.chromium.org/6805019/diff/19001/crypto/capi_util.cc
File crypto/capi_util.cc (right):

http://codereview.chromium.org/6805019/diff/19001/crypto/capi_util.cc#newcode45
crypto/capi_util.cc:45: {
On 2011/04/13 03:25:22, eroman wrote:
> nit: can this brace move to previous line?

Done.

http://codereview.chromium.org/6805019/diff/19001/crypto/crypto.gyp
File crypto/crypto.gyp (right):

http://codereview.chromium.org/6805019/diff/19001/crypto/crypto.gyp#newcode32
crypto/crypto.gyp:32: 'base/crypto/scoped_nss_types.h',
On 2011/04/13 03:25:22, eroman wrote:
> this seems like an old file path, is base/crypto still going to exist?

This seems to be dead code... scoped_nss_types.h is already in the list of
sources.

http://codereview.chromium.org/6805019/diff/19001/crypto/crypto.gyp#newcode91
crypto/crypto.gyp:91: 'signature_creator_nss.cc',
On 2011/04/13 03:25:22, eroman wrote:
> nit: de-indent by 1 space.

Done.

[brettw, 2011-04-13 20:22:44]

content LGTM