Update keygen to use correct NSS slot on ChromeOS multiprofile.

crypto/nss_crypto_module_delegate.h

Index: crypto/nss_crypto_module_delegate.h
diff --git a/crypto/crypto_module_blocking_password_delegate.h b/crypto/nss_crypto_module_delegate.h
similarity index 44%
rename from crypto/crypto_module_blocking_password_delegate.h
rename to crypto/nss_crypto_module_delegate.h
index c4acf1a8c5518de3d2a264b308df936a64f62dff..09f3020974c60a25f4e4ded0f6b00a9b2af48bc3 100644
--- a/crypto/crypto_module_blocking_password_delegate.h
+++ b/crypto/nss_crypto_module_delegate.h
@@ -2,21 +2,30 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#ifndef CRYPTO_CRYPTO_MODULE_BLOCKING_PASSWORD_DELEGATE_H_
-#define CRYPTO_CRYPTO_MODULE_BLOCKING_PASSWORD_DELEGATE_H_
+#ifndef CRYPTO_NSS_CRYPTO_MODULE_DELEGATE_H_
+#define CRYPTO_NSS_CRYPTO_MODULE_DELEGATE_H_
 
 #include <string>
 
+#include "base/callback_forward.h"
+#include "crypto/scoped_nss_types.h"
+
 namespace crypto {
 
 // PK11_SetPasswordFunc is a global setting.  An implementation of
-// CryptoModuleBlockingPasswordDelegate should be passed as the user data
-// argument (|wincx|) to relevant NSS functions, which the global password
-// handler will call to do the actual work.
+// CryptoModuleBlockingPasswordDelegate should be passed using wincx() as the
+// user data argument (|wincx|) to relevant NSS functions, which the global
+// password handler will call to do the actual work.
 class CryptoModuleBlockingPasswordDelegate {
  public:
   virtual ~CryptoModuleBlockingPasswordDelegate() {}
 
+  // Return a value suitable for passing to the |wincx| argument of relevant NSS
+  // functions. This should be used instead of passing the object pointer
+  // directly to avoid accidentally casting a pointer to a subclass to void* and
+  // then casting back to a pointer of the base class
+  void* wincx() { return this; }
+
   // Requests a password to unlock |slot_name|. The interface is
   // synchronous because NSS cannot issue an asynchronous
   // request. |retry| is true if this is a request for the retry
@@ -26,8 +35,22 @@ class CryptoModuleBlockingPasswordDelegate {
   // user entered.
   virtual std::string RequestPassword(const std::string& slot_name, bool retry,
                                       bool* cancelled) = 0;
+
+};
+
+class NSSCryptoModuleDelegate : public CryptoModuleBlockingPasswordDelegate {

[Ryan Sleevi, 2013/11/22 00:03:30]

I don't understand why the separate class with the inheritance here. These are
both NSS-only classes. The comments don't indicate the distinction in expected
uses.

[mattm, 2013/11/22 00:30:31]

I wasn't entirely sure about this, but it's because
CryptoModuleBlockingPasswordDelegate could theoretically be used for other cases
where requesting a slot doesn't make sense. Like if we moved other NSS
operations to the worker thread and passed a
CryptoModuleBlockingPasswordDelegate to use as the wincx, it might be weird to
have to construct a delegate that also has the ability to get a PK11Slot. I'm
not too wedded to this separation, so I could combine them or just add some more
comments, what do you think?

+ public:
+  virtual ~NSSCryptoModuleDelegate() {}
+
+  // Initialize the delegate. |callback| is run when initialization is complete.
+  // |callback| may be run synchronously.  Caller must ensure the delegate
+  // remains alive until |callback| is run.
+  virtual void Initialize(const base::Closure& callback) = 0;
+
+  // Get the slot to store the generated key.
+  virtual ScopedPK11Slot RequestSlot() = 0;
 };
 
 }  // namespace crypto
 
-#endif  // CRYPTO_CRYPTO_MODULE_BLOCKING_PASSWORD_DELEGATE_H_
+#endif  // CRYPTO_NSS_CRYPTO_MODULE_DELEGATE_H_