Remove x509_certificate_model_openssl.cc

Remove x509_certificate_model_openssl.cc.

This file is the last net::IPAddressNumber consumer but
since it is part of an unsupported build configuration
and no longer needed, the file is removed rather than migrated.

BUG=496258
Committed: https://crrev.com/2fe47c26c73bab16d98dd1604cfc3a9702892b97
Cr-Commit-Position: refs/heads/master@{#390852}

[martijnc, 2016-04-24 12:52:49]

The CQ bit was checked by martijn@martijnc.be to run a CQ dry run

[commit-bot: I haz the power, 2016-04-24 12:53:01]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1893203005/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1893203005/20001

[commit-bot: I haz the power, 2016-04-24 13:24:45]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-04-24 13:24:46]

Dry run: This issue passed the CQ dry run.

[martijnc, 2016-04-25 17:22:10]

Patchset #2 (id:20001) has been deleted

[martijnc, 2016-04-25 18:01:04]

martijn@martijnc.be changed reviewers:
+ eroman@chromium.org

[martijnc, 2016-04-25 18:14:17]

martijn@martijnc.be changed reviewers:
+ mattm@chromium.org
- eroman@chromium.org

[martijnc, 2016-04-25 20:59:36]

Are there any build configurations and/or trybots that build this file? I tried
several but haven't had any luck.

[eroman, 2016-04-25 23:19:57]

eroman@chromium.org changed reviewers:
+ eroman@chromium.org

[eroman, 2016-04-25 23:19:57]

drive-by LGTM

[mattm, 2016-04-25 23:51:12]

On 2016/04/25 20:59:36, martijnc wrote:
> Are there any build configurations and/or trybots that build this file? I
tried
> several but haven't had any luck.

This used to be used when doing a use_openssl build on linux, but since the
openssl/NSS chimera that isn't the case. You would have to edit
build/config/crypto.gni to test it.

[eroman, 2016-04-25 23:54:46]

Is this file still needed then?

[mattm, 2016-04-26 00:08:27]

On 2016/04/25 23:54:46, eroman wrote:
> Is this file still needed then?

I think we can delete it. The plan was to move away from use_nss_certs
eventually, but use our own DER parsing stuff instead of the openssl ones,
right? So we probably won't need this again, and can always pull it from the
history if necessary.

[martijnc, 2016-04-26 20:45:51]

Patchset #1 (id:1) has been deleted

[martijnc, 2016-04-26 21:42:08]

On 2016/04/26 at 00:08:27, mattm wrote:
> On 2016/04/25 23:54:46, eroman wrote:
> > Is this file still needed then?
> 
> I think we can delete it. The plan was to move away from use_nss_certs
eventually, but use our own DER parsing stuff instead of the openssl ones,
right? So we probably won't need this again, and can always pull it from the
history if necessary.

The changes in patchset 1 compile with use_openssl_certs=true set (and
use_nss_certs=false) in build/config/crypto.gni but the full build fails (on
Linux):

../../chrome/browser/profiles/profile_io_data.cc -o
obj/chrome/browser/browser/profile_io_data.o
../../chrome/browser/profiles/profile_io_data.cc:966:2: error: Unknown platform.

Patchset 2 removes the file. This likely breaks the configuration further. But
the Chimera docs state the goal is to remove use_openssl_certs eventually, and
since it already appears to be broken that might not be an issue.

How would you like me to proceed with this?

[mattm, 2016-04-29 04:27:09]

On 2016/04/26 21:42:08, martijnc wrote:
> On 2016/04/26 at 00:08:27, mattm wrote:
> > On 2016/04/25 23:54:46, eroman wrote:
> > > Is this file still needed then?
> > 
> > I think we can delete it. The plan was to move away from use_nss_certs
> eventually, but use our own DER parsing stuff instead of the openssl ones,
> right? So we probably won't need this again, and can always pull it from the
> history if necessary.
> 
> The changes in patchset 1 compile with use_openssl_certs=true set (and
> use_nss_certs=false) in build/config/crypto.gni but the full build fails (on
> Linux):
> 
> ../../chrome/browser/profiles/profile_io_data.cc -o
> obj/chrome/browser/browser/profile_io_data.o
> ../../chrome/browser/profiles/profile_io_data.cc:966:2: error: Unknown
platform.
> 
> Patchset 2 removes the file. This likely breaks the configuration further. But
> the Chimera docs state the goal is to remove use_openssl_certs eventually, and
> since it already appears to be broken that might not be an issue.
> 
> How would you like me to proceed with this?

Yeah, removing is fine. There are also some test related stuff that could be
cleaned up too though.
Remove the use_openssl_certs conditions for x509_certificate_model_unittest.cc
in chrome/chrome_tests_unit.gypi and chrome/test/BUILD.gn
Remove the USE_OPENSSL_CERTS conditions in
chrome/common/net/x509_certificate_model_unittest.cc

[mattm, 2016-04-29 04:27:25]

On 2016/04/29 04:27:09, mattm wrote:
> On 2016/04/26 21:42:08, martijnc wrote:
> > On 2016/04/26 at 00:08:27, mattm wrote:
> > > On 2016/04/25 23:54:46, eroman wrote:
> > > > Is this file still needed then?
> > > 
> > > I think we can delete it. The plan was to move away from use_nss_certs
> > eventually, but use our own DER parsing stuff instead of the openssl ones,
> > right? So we probably won't need this again, and can always pull it from the
> > history if necessary.
> > 
> > The changes in patchset 1 compile with use_openssl_certs=true set (and
> > use_nss_certs=false) in build/config/crypto.gni but the full build fails (on
> > Linux):
> > 
> > ../../chrome/browser/profiles/profile_io_data.cc -o
> > obj/chrome/browser/browser/profile_io_data.o
> > ../../chrome/browser/profiles/profile_io_data.cc:966:2: error: Unknown
> platform.
> > 
> > Patchset 2 removes the file. This likely breaks the configuration further.
But
> > the Chimera docs state the goal is to remove use_openssl_certs eventually,
and
> > since it already appears to be broken that might not be an issue.
> > 
> > How would you like me to proceed with this?
> 
> Yeah, removing is fine. There are also some test related stuff that could be
> cleaned up too though.
> Remove the use_openssl_certs conditions for x509_certificate_model_unittest.cc
> in chrome/chrome_tests_unit.gypi and chrome/test/BUILD.gn
> Remove the USE_OPENSSL_CERTS conditions in
> chrome/common/net/x509_certificate_model_unittest.cc

Oh, and update CL description. Thanks!

[martijnc, 2016-04-29 15:40:30]

Description was changed from

==========
Migrate x509_certificate_model_openssl.cc to net::IPAddress.

BUG=496258
==========

to

==========
Remove x509_certificate_model_openssl.cc.

This file is the last net::IPAddressNumber consumer but since it is part of an
unsupported build configuration and no longer needed, the file is removed rather
than migrated.

BUG=496258
==========

[martijnc, 2016-04-29 15:40:30]

martijn@martijnc.be changed reviewers:
+ thakis@chromium.org

[martijnc, 2016-04-29 17:31:17]

Thank you for the feedback! Patchset 3 also removes the test bits.

+thakis for the .gn and .gpyi changes in chrome/*

[Nico, 2016-04-29 18:40:04]

rs-lgtm based on the discussion above

[Nico, 2016-04-29 18:40:29]

Description was changed from

==========
Remove x509_certificate_model_openssl.cc.

This file is the last net::IPAddressNumber consumer but since it is part of an
unsupported build configuration and no longer needed, the file is removed rather
than migrated.

BUG=496258
==========

to

==========
Remove x509_certificate_model_openssl.cc.

This file is the last net::IPAddressNumber consumer but
since it is part of an unsupported build configuration
and no longer needed, the file is removed rather than migrated.

BUG=496258
==========

[mattm, 2016-04-29 18:47:38]

lgtm

[martijnc, 2016-04-30 11:39:57]

The CQ bit was checked by martijn@martijnc.be

[martijnc, 2016-04-30 11:39:57]

The patchset sent to the CQ was uploaded after l-g-t-m from eroman@chromium.org
Link to the patchset: https://codereview.chromium.org/1893203005/#ps80001
(title: "remove tests")

[commit-bot: I haz the power, 2016-04-30 11:40:07]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1893203005/80001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1893203005/80001

[commit-bot: I haz the power, 2016-04-30 12:17:56]

Description was changed from

==========
Remove x509_certificate_model_openssl.cc.

This file is the last net::IPAddressNumber consumer but
since it is part of an unsupported build configuration
and no longer needed, the file is removed rather than migrated.

BUG=496258
==========

to

==========
Remove x509_certificate_model_openssl.cc.

This file is the last net::IPAddressNumber consumer but
since it is part of an unsupported build configuration
and no longer needed, the file is removed rather than migrated.

BUG=496258
==========

[commit-bot: I haz the power, 2016-04-30 12:17:57]

Committed patchset #3 (id:80001)

[commit-bot: I haz the power, 2016-04-30 17:31:18]

Patchset 3 (id:??) landed as
https://crrev.com/2fe47c26c73bab16d98dd1604cfc3a9702892b97
Cr-Commit-Position: refs/heads/master@{#390852}

[commit-bot: I haz the power, 2016-04-30 18:04:14]

Description was changed from

==========
Remove x509_certificate_model_openssl.cc.

This file is the last net::IPAddressNumber consumer but
since it is part of an unsupported build configuration
and no longer needed, the file is removed rather than migrated.

BUG=496258
==========

to

==========
Remove x509_certificate_model_openssl.cc.

This file is the last net::IPAddressNumber consumer but
since it is part of an unsupported build configuration
and no longer needed, the file is removed rather than migrated.

BUG=496258

Committed: https://crrev.com/2fe47c26c73bab16d98dd1604cfc3a9702892b97
Cr-Commit-Position: refs/heads/master@{#390852}
==========