Remove nss_util dependency from ClientCertStoreChromeOS.

Remove nss_util dependency from ClientCertStoreChromeOS.

This required for the new CertDatabaseService
  https://codereview.chromium.org/419013003/
which will (at least temporarily) depend on net/.
Without this change, that would lead to a cyclic dependency between net/ and cert_database/ .

This makes several tests in client_cert_store_chromeos_unittest, like cross-reading between user slots and the system slot, unnecessary.
These are already covered in the unit tests for NSSProfileFilterChromeOS.
This also removes this knowledge about users and the system slot from net/, which are Chrome/ChromeOS details that don't belong into net/.

BUG=413219
Committed: https://crrev.com/a23ed1aa7781d6606fe95bd72bf5afdfc27cf5bf
Cr-Commit-Position: refs/heads/master@{#302430}

[pneubeck (no reviews), 2014-10-26 18:02:28]

Patchset #1 (id:1) has been deleted

[pneubeck (no reviews), 2014-10-27 07:52:37]

Patchset #1 (id:2) has been deleted

[pneubeck (no reviews), 2014-10-27 08:34:44]

Patchset #1 (id:30001) has been deleted

[pneubeck (no reviews), 2014-10-27 09:08:24]

pneubeck@chromium.org changed reviewers:
+ mattm@chromium.org

[pneubeck (no reviews), 2014-10-27 09:08:24]

pneubeck@chromium.org changed required reviewers:
+ mattm@chromium.org

[pneubeck (no reviews), 2014-10-27 09:08:25]

Hi Matt,

I separated the ClientCertStoreChromeOS change from the larger CL.
I wasn't happy with my first proposal adding NSSProfileFilterChromeOSFactory.
That was too hacky for my taste.

I cleaned this up. Please review and tell me whether this makes now more sense
to you.

Thanks!

[mattm, 2014-10-28 03:00:24]

This approach seems fine to me.

https://codereview.chromium.org/663583006/diff/70001/chrome/browser/chromeos/...
File chrome/browser/chromeos/net/cert_profile_filter.cc (right):

https://codereview.chromium.org/663583006/diff/70001/chrome/browser/chromeos/...
chrome/browser/chromeos/net/cert_profile_filter.cc:58:
got_private_slot_callback.Run(private_slot.Pass());
It looks like this means the callback will be run even in the synchronous Init
case

https://codereview.chromium.org/663583006/diff/70001/chrome/browser/chromeos/...
File chrome/browser/chromeos/net/cert_profile_filter.h (right):

https://codereview.chromium.org/663583006/diff/70001/chrome/browser/chromeos/...
chrome/browser/chromeos/net/cert_profile_filter.h:20: class
ClientCertFilterChromeOS
update filename to match

https://codereview.chromium.org/663583006/diff/70001/chrome/browser/chromeos/...
chrome/browser/chromeos/net/cert_profile_filter.h:24: // and private of the user
with |username_hash| and with the system slot if
Did you mean s/private of/private slot of/ ?

https://codereview.chromium.org/663583006/diff/70001/chrome/browser/chromeos/...
chrome/browser/chromeos/net/cert_profile_filter.h:26: explicit
ClientCertFilterChromeOS(bool use_system_slot,
"explicit" unnecessary

[pneubeck (no reviews), 2014-10-28 09:41:40]

Patchset #3 (id:90001) has been deleted

[pneubeck (no reviews), 2014-10-28 09:44:37]

https://codereview.chromium.org/663583006/diff/70001/chrome/browser/chromeos/...
File chrome/browser/chromeos/net/cert_profile_filter.cc (right):

https://codereview.chromium.org/663583006/diff/70001/chrome/browser/chromeos/...
chrome/browser/chromeos/net/cert_profile_filter.cc:58:
got_private_slot_callback.Run(private_slot.Pass());
On 2014/10/28 03:00:24, mattm wrote:
> It looks like this means the callback will be run even in the synchronous Init
> case

Uugh. That's a principle flaw. Fixing it with this callback sequence would have
been even more ugly.

ptal at the latest version. That should be ok finally.

https://codereview.chromium.org/663583006/diff/70001/chrome/browser/chromeos/...
File chrome/browser/chromeos/net/cert_profile_filter.h (right):

https://codereview.chromium.org/663583006/diff/70001/chrome/browser/chromeos/...
chrome/browser/chromeos/net/cert_profile_filter.h:20: class
ClientCertFilterChromeOS
On 2014/10/28 03:00:24, mattm wrote:
> update filename to match

Done.

https://codereview.chromium.org/663583006/diff/70001/chrome/browser/chromeos/...
chrome/browser/chromeos/net/cert_profile_filter.h:24: // and private of the user
with |username_hash| and with the system slot if
On 2014/10/28 03:00:24, mattm wrote:
> Did you mean s/private of/private slot of/ ?

Done.

https://codereview.chromium.org/663583006/diff/70001/chrome/browser/chromeos/...
chrome/browser/chromeos/net/cert_profile_filter.h:26: explicit
ClientCertFilterChromeOS(bool use_system_slot,
On 2014/10/28 03:00:24, mattm wrote:
> "explicit" unnecessary

Done.

[pneubeck (no reviews), 2014-10-28 09:49:02]

pneubeck@chromium.org changed reviewers:
+ davemoore@chromium.org

[pneubeck (no reviews), 2014-10-28 09:49:02]

pneubeck@chromium.org changed required reviewers:
+ davemoore@chromium.org

[pneubeck (no reviews), 2014-10-28 09:49:02]

Dave please owner review the changes in
  chrome/browser

Matt, who previously changed these parts, is reviewing it as well.

[DaveMoore, 2014-10-29 15:41:31]

On 2014/10/28 09:49:02, pneubeck wrote:
> Dave please owner review the changes in
>   chrome/browser
> 
> Matt, who previously changed these parts, is reviewing it as well.

I don't feel qualified to review this, even for owners. Could you try someone
else?

[pneubeck (no reviews), 2014-10-29 15:53:06]

pneubeck@chromium.org changed reviewers:
+ rsleevi@chromium.org

[pneubeck (no reviews), 2014-10-29 15:53:07]

pneubeck@chromium.org changed required reviewers:
+ rsleevi@chromium.org

[pneubeck (no reviews), 2014-10-29 15:53:07]

Are you all OK with adding Matt and Ryan as owners of the files in question?
I.e.
  chrome/browser/chromeos/net/client_cert_filter_chromeos.*

@Dave, can you lgtm only the profile_io_data and OWNERS change?

[mattm, 2014-10-29 21:10:53]

lgtm

https://codereview.chromium.org/663583006/diff/150001/chrome/browser/chromeos...
File chrome/browser/chromeos/net/client_cert_filter_chromeos.h (right):

https://codereview.chromium.org/663583006/diff/150001/chrome/browser/chromeos...
chrome/browser/chromeos/net/client_cert_filter_chromeos.h:53:
crypto::ScopedPK11Slot private_slot_;
maybe add a comment that these are only used during initialization and will be
null after the filter is initialized

[Ryan Sleevi, 2014-10-29 23:48:08]

lgtm

https://codereview.chromium.org/663583006/diff/150001/net/ssl/client_cert_sto...
File net/ssl/client_cert_store_chromeos.h (right):

https://codereview.chromium.org/663583006/diff/150001/net/ssl/client_cert_sto...
net/ssl/client_cert_store_chromeos.h:22: // Initializes this filter. Returns
true if it finished initialization,
nit: newline

https://codereview.chromium.org/663583006/diff/150001/net/ssl/client_cert_sto...
net/ssl/client_cert_store_chromeos.h:35: // This ClientCertStore will createy a
filter using |profile_filter_factory|
nit: creaty -> create

This comment is no longer accurate though - it doesn't use _factory

[pneubeck (no reviews), 2014-10-30 08:24:40]

https://codereview.chromium.org/663583006/diff/150001/chrome/browser/chromeos...
File chrome/browser/chromeos/net/client_cert_filter_chromeos.h (right):

https://codereview.chromium.org/663583006/diff/150001/chrome/browser/chromeos...
chrome/browser/chromeos/net/client_cert_filter_chromeos.h:53:
crypto::ScopedPK11Slot private_slot_;
On 2014/10/29 21:10:53, mattm wrote:
> maybe add a comment that these are only used during initialization and will be
> null after the filter is initialized

Done.

https://codereview.chromium.org/663583006/diff/150001/net/ssl/client_cert_sto...
File net/ssl/client_cert_store_chromeos.h (right):

https://codereview.chromium.org/663583006/diff/150001/net/ssl/client_cert_sto...
net/ssl/client_cert_store_chromeos.h:22: // Initializes this filter. Returns
true if it finished initialization,
On 2014/10/29 23:48:08, Ryan Sleevi wrote:
> nit: newline

Done.

https://codereview.chromium.org/663583006/diff/150001/net/ssl/client_cert_sto...
net/ssl/client_cert_store_chromeos.h:35: // This ClientCertStore will createy a
filter using |profile_filter_factory|
On 2014/10/29 23:48:08, Ryan Sleevi wrote:
> nit: creaty -> create
> 
> This comment is no longer accurate though - it doesn't use _factory

Done.

[pneubeck (no reviews), 2014-10-31 16:16:00]

pneubeck@chromium.org changed reviewers:
+ thestig@chromium.org

[pneubeck (no reviews), 2014-10-31 16:16:01]

pneubeck@chromium.org changed required reviewers:
- davemoore@chromium.org

[pneubeck (no reviews), 2014-10-31 16:16:01]

@thestig
 please lgtm the OWNER change if acceptable.

[Lei Zhang, 2014-10-31 18:25:42]

On 2014/10/31 16:16:01, pneubeck wrote:
> @thestig
>  please lgtm the OWNER change if acceptable.

lgtm if you sort the OWNERS file.

[pneubeck (no reviews), 2014-11-03 10:17:54]

pneubeck@chromium.org changed reviewers:
- davemoore@chromium.org

[pneubeck (no reviews), 2014-11-03 10:17:54]

pneubeck@chromium.org changed required reviewers:
+ thestig@chromium.org

[pneubeck (no reviews), 2014-11-03 10:17:55]

Sorry, I missed to add the client_cert_filter* to the OWNERS in the last patch.

Added now. Inheriting the lgtm. I can revert if you disagree.

[pneubeck (no reviews), 2014-11-03 10:19:41]

The CQ bit was checked by pneubeck@chromium.org

[commit-bot: I haz the power, 2014-11-03 10:20:05]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/663583006/220001

[commit-bot: I haz the power, 2014-11-03 10:55:31]

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

[commit-bot: I haz the power, 2014-11-03 10:55:33]

Try jobs failed on following builders:
  linux_chromium_chromeos_rel on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[pneubeck (no reviews), 2014-11-03 14:57:41]

The CQ bit was checked by pneubeck@chromium.org

[commit-bot: I haz the power, 2014-11-03 14:58:21]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/663583006/240001

[commit-bot: I haz the power, 2014-11-03 15:40:52]

Committed patchset #10 (id:240001)

[commit-bot: I haz the power, 2014-11-03 15:41:23]

Patchset 10 (id:??) landed as
https://crrev.com/a23ed1aa7781d6606fe95bd72bf5afdfc27cf5bf
Cr-Commit-Position: refs/heads/master@{#302430}