Built crypto and crypto_unittests for iOS

Built crypto and crypto_unittests for iOS

Adds iOS support to crypto.gyp. Both targets now build, but the tests may not
run correctly, and not all the crypto code is correct yet for iOS.

BUG=NONE

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=150775

[msarda, 2012-08-06 15:54:53]

This is my first upstreaming CL for iOS code.
PTAL.

[stuartmorgan, 2012-08-06 16:07:31]

https://chromiumcodereview.appspot.com/10830183/diff/1/build/all.gyp
File build/all.gyp (right):

https://chromiumcodereview.appspot.com/10830183/diff/1/build/all.gyp#newcode14
build/all.gyp:14: '../crypto/crypto.gyp:*',
There should be a similar change you can also make in net.gyp

https://chromiumcodereview.appspot.com/10830183/diff/1/crypto/crypto.gyp
File crypto/crypto.gyp (right):

https://chromiumcodereview.appspot.com/10830183/diff/1/crypto/crypto.gyp#newc...
crypto/crypto.gyp:164: # TODO(wtc): sort out nss/openssl stuff
This isn't a useful comment; discuss with wtc to find out what the situation
here is and change the comment accordingly.

https://chromiumcodereview.appspot.com/10830183/diff/1/crypto/crypto.gyp#newc...
crypto/crypto.gyp:301: # TODO(wtc): sort out nss/openssl stuff
Same.

https://chromiumcodereview.appspot.com/10830183/diff/1/crypto/rsa_private_key...
File crypto/rsa_private_key_ios.cc (right):

https://chromiumcodereview.appspot.com/10830183/diff/1/crypto/rsa_private_key...
crypto/rsa_private_key_ios.cc:13: // ideas.
Remove this from the CL, and start a discussion about this file.

https://chromiumcodereview.appspot.com/10830183/diff/1/crypto/run_all_unittes...
File crypto/run_all_unittests.cc (right):

https://chromiumcodereview.appspot.com/10830183/diff/1/crypto/run_all_unittes...
crypto/run_all_unittests.cc:16: MainHook hook(main, argc, argv);
This should be the very first call.

[msarda, 2012-08-07 08:39:44]

PTAL.

https://chromiumcodereview.appspot.com/10830183/diff/1/build/all.gyp
File build/all.gyp (right):

https://chromiumcodereview.appspot.com/10830183/diff/1/build/all.gyp#newcode14
build/all.gyp:14: '../crypto/crypto.gyp:*',
On 2012/08/06 16:07:31, stuartmorgan wrote:
> There should be a similar change you can also make in net.gyp

Done.

https://chromiumcodereview.appspot.com/10830183/diff/1/crypto/crypto.gyp
File crypto/crypto.gyp (right):

https://chromiumcodereview.appspot.com/10830183/diff/1/crypto/crypto.gyp#newc...
crypto/crypto.gyp:164: # TODO(wtc): sort out nss/openssl stuff
On 2012/08/06 16:07:31, stuartmorgan wrote:
> This isn't a useful comment; discuss with wtc to find out what the situation
> here is and change the comment accordingly.

I removed this section - this will be addressed by the bug b/6878217

https://chromiumcodereview.appspot.com/10830183/diff/1/crypto/crypto.gyp#newc...
crypto/crypto.gyp:301: # TODO(wtc): sort out nss/openssl stuff
On 2012/08/06 16:07:31, stuartmorgan wrote:
> Same.

I removed this section - this will be addressed by the bug b/6878217

https://chromiumcodereview.appspot.com/10830183/diff/1/crypto/rsa_private_key...
File crypto/rsa_private_key_ios.cc (right):

https://chromiumcodereview.appspot.com/10830183/diff/1/crypto/rsa_private_key...
crypto/rsa_private_key_ios.cc:13: // ideas.
On 2012/08/06 16:07:31, stuartmorgan wrote:
> Remove this from the CL, and start a discussion about this file.

Done.

https://chromiumcodereview.appspot.com/10830183/diff/1/crypto/run_all_unittes...
File crypto/run_all_unittests.cc (right):

https://chromiumcodereview.appspot.com/10830183/diff/1/crypto/run_all_unittes...
crypto/run_all_unittests.cc:16: MainHook hook(main, argc, argv);
On 2012/08/06 16:07:31, stuartmorgan wrote:
> This should be the very first call.

The code we have in the iOS tree does this call at the end of the function. I
double checked and it looks like this MainHook calls exit - all the code after 
MainHook is never run.

[stuartmorgan, 2012-08-07 09:21:28]

+wtc; see the question below (and also for eventual OWNERS review)

The description of this CL is still confusing; you say it's minimimal, but don't
exclude any files. (Also, there's no need to say you enabled the dependencies in
other files; that's an expected part of getting it building). How about:

-----
Built crypto and crypto_unittests for iOS

Adds iOS support to crypto.gyp. Both targets now build, but the tests may not
run correctly, and not all the crypto code is correct yet for iOS.
-----

https://chromiumcodereview.appspot.com/10830183/diff/1/crypto/run_all_unittes...
File crypto/run_all_unittests.cc (right):

https://chromiumcodereview.appspot.com/10830183/diff/1/crypto/run_all_unittes...
crypto/run_all_unittests.cc:16: MainHook hook(main, argc, argv);
On 2012/08/07 08:39:44, msarda wrote:
> On 2012/08/06 16:07:31, stuartmorgan wrote:
> > This should be the very first call.
> 
> The code we have in the iOS tree does this call at the end of the function.

And it's wrong, which is why I want you to change it.

> I double checked and it looks like this MainHook calls exit

After calling back into main. Anything before MainHook will be run twice, which
is incorrect. Probably harmless here given the name of the method, but MainHook
must *always* be the first line.

https://chromiumcodereview.appspot.com/10830183/diff/8001/crypto/crypto.gyp
File crypto/crypto.gyp (right):

https://chromiumcodereview.appspot.com/10830183/diff/8001/crypto/crypto.gyp#n...
crypto/crypto.gyp:290: 'rsa_private_key_unittest.cc',
Now it's not clear why these are excluded.

+wtc; can you explain what kind of TODO would make sense here, if any? Should we
file a bug for future work?

[msarda, 2012-08-07 09:42:17]

I fixed the CL's description (thanks for the suggestion).
We'll wait for wtc's review and TODO suggestion before moving forward with this
CL.

https://chromiumcodereview.appspot.com/10830183/diff/1/crypto/run_all_unittes...
File crypto/run_all_unittests.cc (right):

https://chromiumcodereview.appspot.com/10830183/diff/1/crypto/run_all_unittes...
crypto/run_all_unittests.cc:16: MainHook hook(main, argc, argv);
On 2012/08/07 09:21:28, stuartmorgan wrote:
> On 2012/08/07 08:39:44, msarda wrote:
> > On 2012/08/06 16:07:31, stuartmorgan wrote:
> > > This should be the very first call.
> > 
> > The code we have in the iOS tree does this call at the end of the function.
> 
> And it's wrong, which is why I want you to change it.
> 
> > I double checked and it looks like this MainHook calls exit
> 
> After calling back into main. Anything before MainHook will be run twice,
which
> is incorrect. Probably harmless here given the name of the method, but
MainHook
> must *always* be the first line.

Done.

[msarda, 2012-08-08 15:48:54]

@Wan-Teh: Please take a look at this CL. I need to land this before moving to
upstream the rest of the crypto as this is the build part (and being in the
Paris time-zone does not help).
Thanks.

[wtc, 2012-08-08 17:57:02]

Patch set 3 LGTM.

[wtc, 2012-08-08 18:09:58]

I just noticed there were two questions for me.  I answer
them below.

https://chromiumcodereview.appspot.com/10830183/diff/1/crypto/crypto.gyp
File crypto/crypto.gyp (right):

https://chromiumcodereview.appspot.com/10830183/diff/1/crypto/crypto.gyp#newc...
crypto/crypto.gyp:164: # TODO(wtc): sort out nss/openssl stuff

On 2012/08/06 16:07:31, stuartmorgan wrote:
> This isn't a useful comment; discuss with wtc to find out what the situation
> here is and change the comment accordingly.

I don't know what this comment means.  Perhaps someone else
added it? :-)

I believe the RSAPrivateKey class is implemented entirely
in rsa_private_key_ios.cc for iOS, which is why we need to
exclude rsa_private_key.cc.

In any case, we should remove (and not add back) this TODO
comment.

https://chromiumcodereview.appspot.com/10830183/diff/8001/crypto/crypto.gyp
File crypto/crypto.gyp (right):

https://chromiumcodereview.appspot.com/10830183/diff/8001/crypto/crypto.gyp#n...
crypto/crypto.gyp:290: 'rsa_private_key_unittest.cc',

On 2012/08/07 09:21:28, stuartmorgan wrote:
> Now it's not clear why these are excluded.
> 
> +wtc; can you explain what kind of TODO would make sense here, if any? Should
we
> file a bug for future work?

These tests are excluded because the classes they are testing
(RSAPrivateKey, SignatureCreator, and SignatureVerifier)
are not implemented for iOS.  I remember these classes
are used to sign and verify Chrome extensions.  Perhaps
Chrome on iOS does not support extensions?

In any case, we should add a comment that these tests
are excluded because they test classes that are not
implemented.

[commit-bot: I haz the power, 2012-08-09 08:33:09]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/msarda@chromium.org/10830183/5003

[commit-bot: I haz the power, 2012-08-09 10:24:55]

Change committed as 150775

[msarda, 2012-08-09 10:26:49]

Thanks for the review.