Closed Bug 1019814 Opened 7 years ago Closed 7 years ago

Remove CERTCertificate dependencies from TrustDomain::GetCertTrust

Categories

(Core :: Security: PSM, enhancement)

enhancement
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla32

People

(Reporter: briansmith, Assigned: briansmith)

References

Details

Attachments

(1 file)

See bug 1019771 for backgronud.

This patch just shifts the CERTCertificate dependency into the TrustDomain. The conversion from SECItem* to CERTCertificate should be a pretty quick hashtable lookup + reference count increment because NSS maintains a cache from SECItem* -> CERTCertificate. Thus, the change shouldn't have a (significant) performance impact.
Attachment #8433521 - Flags: review?(dkeeler)
Comment on attachment 8433521 [details] [diff] [review]
remove-CERTCertificate-dependency-from-TrustDomain-GetCertTrust.patch

Review of attachment 8433521 [details] [diff] [review]:
-----------------------------------------------------------------

LGTM. Did you want to include in the TrustDomain implementations a comment explaining the expectation that NSS will maintain a hash of the SECItem -> CERTCertificate conversion so it won't be a performance concern?
Attachment #8433521 - Flags: review?(dkeeler) → review+
(In reply to David Keeler (:keeler) [use needinfo?] from comment #1)
> LGTM. Did you want to include in the TrustDomain implementations a comment
> explaining the expectation that NSS will maintain a hash of the SECItem ->
> CERTCertificate conversion so it won't be a performance concern?

I added a note to that effect in NSSCertDBTrustDomain.cpp. I didn't bother doing so for AppTrustDomain because we don't care (as much) about tiny performance issues in that class.

Thanks for the quick review!

https://hg.mozilla.org/integration/mozilla-inbound/rev/44be87ea2e1b
Target Milestone: --- → mozilla32
https://hg.mozilla.org/mozilla-central/rev/44be87ea2e1b
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.