nsIPKCS11 consists of addModule and deleteModule, which, in my opinion, belong in nsIPKCS11ModuleDB. At the same time, the implementation of nsIPKCS11ModuleDB, nsPKCS11ModuleDB, is currently in nsPKCS11Slot.cpp, which is unnecessary and confusing. This will merge nsIPKCS11 into nsIPKCS11ModuleDB and centralize the implementation.

Comment on attachment 8898454 [details] bug 1391404 - fold nsIPKCS11 into nsIPKCS11ModuleDB https://reviewboard.mozilla.org/r/169824/#review175350 Cool, I like how this also has the side benefit of moving more stuff under namespaces. Anyways, LGTM with comments addressed. ::: security/manager/ssl/nsNSSModule.cpp (Diff revision 1) > * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ > > #include "CertBlocklist.h" > #include "ContentSignatureVerifier.h" > #include "NSSErrorsService.h" > -#include "PKCS11.h" The definition of `NS_PKCS11MODULEDB_CID` has moved to PKCS11ModuleDB.h now, so we should probably `#include "PKCS11ModuleDB.h"` instead. ::: security/manager/ssl/tests/mochitest/browser/browser_loadPKCS11Module_ui.js:56 (Diff revision 1) > + > + listModules() { > + throw new Error("not expecting listModules() to be called"); > + }, > + > + getCanToggleFIPS() { I played around with this (by adding a call to `canToggleFIPS` in the implementation to try and get the test to fail), and it turns out that attributes in IDLs should also be attributes in JS implementations. So, this should be `get can...() {` instead. ::: security/manager/ssl/tests/mochitest/browser/browser_loadPKCS11Module_ui.js:64 (Diff revision 1) > + > + toggleFIPSMode() { > + throw new Error("not expecting toggleFIPSMode() to be called"); > + }, > + > + getIsFIPSEnabled() { Same thing as `getCanToggleFIPS`.

Comment on attachment 8898454 [details] bug 1391404 - fold nsIPKCS11 into nsIPKCS11ModuleDB https://reviewboard.mozilla.org/r/169824/#review175350 Thanks! > The definition of `NS_PKCS11MODULEDB_CID` has moved to PKCS11ModuleDB.h now, so we should probably `#include "PKCS11ModuleDB.h"` instead. Sounds good. > I played around with this (by adding a call to `canToggleFIPS` in the implementation to try and get the test to fail), and it turns out that attributes in IDLs should also be attributes in JS implementations. > So, this should be `get can...() {` instead. Ah - good catch.

Pushed by dkeeler@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/c76c0f1fadfe fold nsIPKCS11 into nsIPKCS11ModuleDB r=Cykesiopka