Created attachment 286387 [details] [review]
Patch to populate_ui()

Proposed patch runs only once through list of certificates.

I intended to change function load_certs to load certs for all three pages, but this function is used in import_cert() to refresh only one page.

So I implemented a similar function which loads certs on all three pages.

Thanks for looking into this issue.

This patch is an improvement, but it really wants to be done asynchronously rather than in the main thread. It's great that you reduce it to a single call to PK11_ListCerts() but even *one* call to that function, blocking in the main thread, is still too much.

As well as making it asynchronous, we should also ditch PK11_ListCerts(). If you look at that function, you can see it's just iterating over all the certificates calling its pk11ListCertCallback() function:

http://dxr.mozilla.org/mozilla-central/source/security/nss/lib/pk11wrap/pk11cert.c#2508

I think the inefficiency is actually in the NSSTrustDomain_TraverseCertificates() function that it calls. See the 'collector' callback at

http://dxr.mozilla.org/mozilla-central/source/security/nss/lib/pki/trustdomain.c#985

which is adding the newly-found objects to a *list*, eliminating duplicates. That's where the time is spent. It should be relatively simple to implement our own version of NSSTrustDomain_TraverseCertificates() which just iterates over the available objects in the available tokens, calling a more efficient function to process each one. We definitely want to avoid the O(n²) behaviour of the default NSS function.

Comment on attachment 286387 [details] [review]
Patch to populate_ui()

I'm setting this to reviewed, even it being done by David. I do not mind much currently, but if you (David) prefers, then I can commit at least this version and wait for the improvement or just wait for the improvement.

I suppose I have no objection to committing the patch as-is. It's better than nothing.

It would be *very* good to fix the problem properly though, as described in comment 2.

Then we could actually merge and enable the evolution-pkcs11 token by default, which would be excellent.

I finally got round to filing https://bugzilla.mozilla.org/1253211 for this, although that's still only part of the issue — we still shouldn't be calling it from the main thread. We should populate the dialog asynchronously.

I extended Yuuma's patch to work asynchronously.

Created commit 76ad2ab in evo master (3.21.1+)
Created commit c5af05b in evo gnome-3-20 (3.20.1+)