Remove GetPkcs11Id from x509_certificate_model

Remove GetPkcs11Id from x509_certificate_model

We have the same function in chromeos::CertLoader.
CertLibrary, which is the only user of x509_certificate_model::GetPkcs11Id,
already depends on the CertLoader, so there is no real reson not to use
CertLoader::GetPkcs11IdForCert.

Also, update CertLibrary::GetCertIndexByPEM and
CertLibrary::GetCertIndexByPkcs11 to work only for a single certificate
type (server ca and client certificates respectively), instead of taking
the certificate type as param.

BUG=236978
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=262257

[stevenjb, 2014-03-27 19:43:27]

lgtm

[Ryan Sleevi, 2014-03-27 19:51:44]

I'm curious why the User/Server change. That did not really seem obvious from
the CL description or the bug. Does something inherently not work?

[tbarzic, 2014-03-27 20:15:43]

On 2014/03/27 19:51:44, Ryan Sleevi wrote:
> I'm curious why the User/Server change. That did not really seem obvious from
> the CL description or the bug. Does something inherently not work?

Not really, it just mirrors how certs are identified by shill: server CA
certificated by PEM, and user certs by pkcs11 id.

[tbarzic, 2014-03-27 20:20:38]

On 2014/03/27 20:15:43, tbarzic wrote:
> On 2014/03/27 19:51:44, Ryan Sleevi wrote:
> > I'm curious why the User/Server change. That did not really seem obvious
from
> > the CL description or the bug. Does something inherently not work?
> 
> Not really, it just mirrors how certs are identified by shill: server CA
> certificated by PEM, and user certs by pkcs11 id.

"Not really" as "It's not that it inherently doesn't work"; it's just that
there's no need to identify user certs by PEM, or server certs by id.

[stevenjb, 2014-03-27 20:21:17]

On 2014/03/27 20:15:43, tbarzic wrote:
> On 2014/03/27 19:51:44, Ryan Sleevi wrote:
> > I'm curious why the User/Server change. That did not really seem obvious
from
> > the CL description or the bug. Does something inherently not work?
> 
> Not really, it just mirrors how certs are identified by shill: server CA
> certificated by PEM, and user certs by pkcs11 id.

I'm in favor of the name cleanup. "CertLoader" is rather poorly named, it should
really be something like "NetworkCertLoader". There's no reason to expose the
type implementation detail to the implementations.

(When we migrate these dialogs to WebUI, CertLoader may become redundant, or may
be needed to provide certs to the WebUI, not sure which yet, but at that point
it should either go away or get renamed).

[Ryan Sleevi, 2014-03-27 20:25:20]

https://codereview.chromium.org/182313004/diff/40001/chrome/common/net/x509_c...
File chrome/common/net/x509_certificate_model.h (left):

https://codereview.chromium.org/182313004/diff/40001/chrome/common/net/x509_c...
chrome/common/net/x509_certificate_model.h:82: std::string
GetPkcs11Id(net::X509Certificate::OSCertHandle cert_handle);
Why haven't you updated x509_certificate_model_nss.cc ?

[tbarzic, 2014-03-27 20:30:39]

https://codereview.chromium.org/182313004/diff/40001/chrome/common/net/x509_c...
File chrome/common/net/x509_certificate_model.h (left):

https://codereview.chromium.org/182313004/diff/40001/chrome/common/net/x509_c...
chrome/common/net/x509_certificate_model.h:82: std::string
GetPkcs11Id(net::X509Certificate::OSCertHandle cert_handle);
On 2014/03/27 20:25:21, Ryan Sleevi wrote:
> Why haven't you updated x509_certificate_model_nss.cc ?

d'oh. I thought I did... seems that I lost the diff somewhere along the line.
Done.

[Ryan Sleevi, 2014-03-27 20:31:27]

lgtm

[tbarzic, 2014-03-27 20:32:54]

The CQ bit was checked by tbarzic@chromium.org

[commit-bot: I haz the power, 2014-03-27 20:34:17]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/tbarzic@chromium.org/182313004/60001

[tbarzic, 2014-03-27 21:06:36]

oh, looks like there's no OWNERS file in chrome/common/net/
+sky for owner approval

[commit-bot: I haz the power, 2014-03-27 21:16:59]

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

[commit-bot: I haz the power, 2014-03-27 21:16:59]

Retried try job too often on chromium_presubmit for step(s) presubmit
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=chromium_p...

[sky, 2014-03-27 21:45:41]

LGTM

[tbarzic, 2014-03-27 22:54:22]

The CQ bit was checked by tbarzic@chromium.org

[commit-bot: I haz the power, 2014-03-27 22:57:13]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/tbarzic@chromium.org/182313004/60001

[commit-bot: I haz the power, 2014-03-28 02:49:25]

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

[commit-bot: I haz the power, 2014-03-28 02:49:26]

Retried try job too often on ios_dbg_simulator for step(s) crypto_unittests,
url_unittests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=ios_dbg_si...

[tbarzic, 2014-03-28 06:44:52]

The CQ bit was checked by tbarzic@chromium.org

[commit-bot: I haz the power, 2014-03-28 06:45:22]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/tbarzic@chromium.org/182313004/60001

[commit-bot: I haz the power, 2014-03-28 11:02:01]

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

[commit-bot: I haz the power, 2014-03-28 11:02:02]

Retried try job too often on ios_dbg_simulator for step(s) base_unittests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=ios_dbg_si...

[tbarzic, 2014-04-02 05:07:13]

The CQ bit was checked by tbarzic@chromium.org

[commit-bot: I haz the power, 2014-04-02 05:07:18]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/tbarzic@chromium.org/182313004/60001

[commit-bot: I haz the power, 2014-04-02 06:30:28]

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

[commit-bot: I haz the power, 2014-04-02 06:30:29]

Retried try job too often on win_rel for step(s) content_unittests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_rel&nu...

[tbarzic, 2014-04-07 20:36:07]

The CQ bit was checked by tbarzic@chromium.org

[commit-bot: I haz the power, 2014-04-07 20:42:12]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/tbarzic@chromium.org/182313004/60001

[commit-bot: I haz the power, 2014-04-07 23:54:10]

Change committed as 262257