net: remove CryptoModuleList typedef

chrome/browser/ui/crypto_module_password_dialog_nss.cc

 // Copyright (c) 2011 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/ui/crypto_module_password_dialog_nss.h"
 
 #include <pk11pub.h>
 #include <stddef.h>
 
 #include "base/bind.h"
 #include "base/logging.h"
 #include "content/public/browser/browser_thread.h"
 #include "net/base/crypto_module.h"
 #include "net/cert/x509_certificate.h"
 
 using content::BrowserThread;
 
 namespace {
 
-bool ShouldShowDialog(const net::CryptoModule* module) {
+bool ShouldShowDialog(PK11SlotInfo* slot) {
   // The wincx arg is unused since we don't call PK11_SetIsLoggedInFunc.
-  return (PK11_NeedLogin(module->os_module_handle()) &&
-          !PK11_IsLoggedIn(module->os_module_handle(), NULL /* wincx */));
+  return (PK11_NeedLogin(slot) && !PK11_IsLoggedIn(slot, NULL /* wincx */));
 }
 
 // Basically an asynchronous implementation of NSS's PK11_DoPassword.
 // Note: This currently handles only the simple case.  See the TODOs in
 // GotPassword for what is yet unimplemented.
 class SlotUnlocker {
  public:
-  SlotUnlocker(const net::CryptoModuleList& modules,
+  SlotUnlocker(const std::vector<crypto::ScopedPK11Slot>& modules,
                chrome::CryptoModulePasswordReason reason,
                const net::HostPortPair& server,
                gfx::NativeWindow parent,
                const base::Closure& callback);
 
   void Start();
 
  private:
   void GotPassword(const std::string& password);
   void Done();
 
   size_t current_;
-  net::CryptoModuleList modules_;
+  std::vector<crypto::ScopedPK11Slot> modules_;
   chrome::CryptoModulePasswordReason reason_;
   net::HostPortPair server_;
   gfx::NativeWindow parent_;
   base::Closure callback_;
   PRBool retry_;
 };
 
-SlotUnlocker::SlotUnlocker(const net::CryptoModuleList& modules,
+SlotUnlocker::SlotUnlocker(const std::vector<crypto::ScopedPK11Slot>& modules,
                            chrome::CryptoModulePasswordReason reason,
                            const net::HostPortPair& server,
                            gfx::NativeWindow parent,
                            const base::Closure& callback)
     : current_(0),
-      modules_(modules),
       reason_(reason),
       server_(server),
       parent_(parent),
       callback_(callback),
       retry_(PR_FALSE) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
+  for (const auto& slot : modules) {

[tfarina, 2017/02/24 14:30:49]

It was not possible to do modules_(std::move(modules)).

[mattm, 2017/02/24 21:03:39]

Hm, it should work. Did you change the parameter type to not be a const-ref?

[tfarina, 2017/02/25 02:04:42]

When it is non const-ref I get this:

../../build/linux/debian_wheezy_amd64-sysroot/usr/lib/gcc/x86_64-linux-gnu/4.6/../../../../include/c++/4.6/bits/stl_construct.h:76:38:
error: call to deleted constructor of 'std::unique_ptr<PK11SlotInfoStr,
crypto::NSSDestroyer<PK11SlotInfoStr, &PK11_FreeSlot> >'
    { ::new(static_cast<void*>(__p)) _T1(std::forward<_Args>(__args)...); }
                                     ^   ~~~~~~~~~~~~~~~~~~~~~~~~~~~
../../build/linux/debian_wheezy_amd64-sysroot/usr/lib/gcc/x86_64-linux-gnu/4.6/../../../../include/c++/4.6/bits/stl_uninitialized.h:77:8:
note: in instantiation of function template specialization
'std::_Construct<std::unique_ptr<PK11SlotInfoStr,
crypto::NSSDestroyer<PK11SlotInfoStr, &PK11_FreeSlot> >, const
std::unique_ptr<PK11SlotInfoStr, crypto::NSSDestroyer<PK11SlotInfoStr,
&PK11_FreeSlot> > &>' requested here
                std::_Construct(std::__addressof(*__cur), *__first);
                     ^
../../build/linux/debian_wheezy_amd64-sysroot/usr/lib/gcc/x86_64-linux-gnu/4.6/../../../../include/c++/4.6/bits/stl_uninitialized.h:119:2:
note: in instantiation of function template specialization
'std::__uninitialized_copy<false>::__uninit_copy<__gnu_cxx::__normal_iterator<const
std::unique_ptr<PK11SlotInfoStr, crypto::NSSDestroyer<PK11SlotInfoStr,
&PK11_FreeSlot> > *, std::__cxx1998::vector<std::unique_ptr<PK11SlotInfoStr,
crypto::NSSDestroyer<PK11SlotInfoStr, &PK11_FreeSlot> >,
std::allocator<std::unique_ptr<PK11SlotInfoStr,
crypto::NSSDestroyer<PK11SlotInfoStr, &PK11_FreeSlot> > > > >,
std::unique_ptr<PK11SlotInfoStr, crypto::NSSDestroyer<PK11SlotInfoStr,
&PK11_FreeSlot> > *>' requested here
        __uninit_copy(__first, __last, __result);
        ^
../../build/linux/debian_wheezy_amd64-sysroot/usr/lib/gcc/x86_64-linux-gnu/4.6/../../../../include/c++/4.6/bits/stl_uninitialized.h:259:19:
note: in instantiation of function template specialization
'std::uninitialized_copy<__gnu_cxx::__normal_iterator<const
std::unique_ptr<PK11SlotInfoStr, crypto::NSSDestroyer<PK11SlotInfoStr,
&PK11_FreeSlot> > *, std::__cxx1998::vector<std::unique_ptr<PK11SlotInfoStr,
crypto::NSSDestroyer<PK11SlotInfoStr, &PK11_FreeSlot> >,
std::allocator<std::unique_ptr<PK11SlotInfoStr,
crypto::NSSDestroyer<PK11SlotInfoStr, &PK11_FreeSlot> > > > >,
std::unique_ptr<PK11SlotInfoStr, crypto::NSSDestroyer<PK11SlotInfoStr,
&PK11_FreeSlot> > *>' requested here
    { return std::uninitialized_copy(__first, __last, __result); }
                  ^

When it is const-ref and I use std::move, I get this:

../../build/linux/debian_wheezy_amd64-sysroot/usr/lib/gcc/x86_64-linux-gnu/4.6/../../../../include/c++/4.6/bits/stl_construct.h:76:38:
error: call to deleted constructor of 'std::unique_ptr<PK11SlotInfoStr,
crypto::NSSDestroyer<PK11SlotInfoStr, &PK11_FreeSlot> >'
    { ::new(static_cast<void*>(__p)) _T1(std::forward<_Args>(__args)...); }
                                     ^   ~~~~~~~~~~~~~~~~~~~~~~~~~~~
../../build/linux/debian_wheezy_amd64-sysroot/usr/lib/gcc/x86_64-linux-gnu/4.6/../../../../include/c++/4.6/bits/stl_uninitialized.h:77:8:
note: in instantiation of function template specialization
'std::_Construct<std::unique_ptr<PK11SlotInfoStr,
crypto::NSSDestroyer<PK11SlotInfoStr, &PK11_FreeSlot> >, const
std::unique_ptr<PK11SlotInfoStr, crypto::NSSDestroyer<PK11SlotInfoStr,
&PK11_FreeSlot> > &>' requested here
                std::_Construct(std::__addressof(*__cur), *__first);
                     ^
../../build/linux/debian_wheezy_amd64-sysroot/usr/lib/gcc/x86_64-linux-gnu/4.6/../../../../include/c++/4.6/bits/stl_uninitialized.h:119:2:
note: in instantiation of function template specialization
'std::__uninitialized_copy<false>::__uninit_copy<__gnu_cxx::__normal_iterator<const
std::unique_ptr<PK11SlotInfoStr, crypto::NSSDestroyer<PK11SlotInfoStr,
&PK11_FreeSlot> > *, std::__cxx1998::vector<std::unique_ptr<PK11SlotInfoStr,
crypto::NSSDestroyer<PK11SlotInfoStr, &PK11_FreeSlot> >,
std::allocator<std::unique_ptr<PK11SlotInfoStr,
crypto::NSSDestroyer<PK11SlotInfoStr, &PK11_FreeSlot> > > > >,
std::unique_ptr<PK11SlotInfoStr, crypto::NSSDestroyer<PK11SlotInfoStr,
&PK11_FreeSlot> > *>' requested here
        __uninit_copy(__first, __last, __result);
        ^
../../build/linux/debian_wheezy_amd64-sysroot/usr/lib/gcc/x86_64-linux-gnu/4.6/../../../../include/c++/4.6/bits/stl_uninitialized.h:259:19:
note: in instantiation of function template specialization
'std::uninitialized_copy<__gnu_cxx::__normal_iterator<const
std::unique_ptr<PK11SlotInfoStr, crypto::NSSDestroyer<PK11SlotInfoStr,
&PK11_FreeSlot> > *, std::__cxx1998::vector<std::unique_ptr<PK11SlotInfoStr,
crypto::NSSDestroyer<PK11SlotInfoStr, &PK11_FreeSlot> >,
std::allocator<std::unique_ptr<PK11SlotInfoStr,
crypto::NSSDestroyer<PK11SlotInfoStr, &PK11_FreeSlot> > > > >,
std::unique_ptr<PK11SlotInfoStr, crypto::NSSDestroyer<PK11SlotInfoStr,
&PK11_FreeSlot> > *>' requested here
    { return std::uninitialized_copy(__first, __last, __result); }
                  ^
../../build/linux/debian_wheezy_amd64-sysroot/usr/lib/gcc/x86_64-linux-gnu/4.6/../../../../include/c++/4.6/bits/stl_vector.h:281:9:
note: in instantiation of function template specialization
'std::__uninitialized_copy_a<__gnu_cxx::__normal_iterator<const
std::unique_ptr<PK11SlotInfoStr, crypto::NSSDestroyer<PK11SlotInfoStr,
&PK11_FreeSlot> > *, std::__cxx1998::vector<std::unique_ptr<PK11SlotInfoStr,
crypto::NSSDestroyer<PK11SlotInfoStr, &PK11_FreeSlot> >,
std::allocator<std::unique_ptr<PK11SlotInfoStr,
crypto::NSSDestroyer<PK11SlotInfoStr, &PK11_FreeSlot> > > > >,
std::unique_ptr<PK11SlotInfoStr, crypto::NSSDestroyer<PK11SlotInfoStr,
&PK11_FreeSlot> > *, std::unique_ptr<PK11SlotInfoStr,
crypto::NSSDestroyer<PK11SlotInfoStr, &PK11_FreeSlot> > >' requested here
          std::__uninitialized_copy_a(__x.begin(), __x.end(),
               ^
../../build/linux/debian_wheezy_amd64-sysroot/usr/lib/gcc/x86_64-linux-gnu/4.6/../../../../include/c++/4.6/debug/vector:105:9:
note: in instantiation of member function
'std::__cxx1998::vector<std::unique_ptr<PK11SlotInfoStr,
crypto::NSSDestroyer<PK11SlotInfoStr, &PK11_FreeSlot> >,
std::allocator<std::unique_ptr<PK11SlotInfoStr,
crypto::NSSDestroyer<PK11SlotInfoStr, &PK11_FreeSlot> > > >::vector' requested
here
      : _Base(__x), _Safe_base(), _M_guaranteed_capacity(__x.size()) { }
        ^
../../chrome/browser/ui/crypto_module_password_dialog_nss.cc:57:7: note: in
instantiation of member function
'std::__debug::vector<std::unique_ptr<PK11SlotInfoStr,
crypto::NSSDestroyer<PK11SlotInfoStr, &PK11_FreeSlot> >,
std::allocator<std::unique_ptr<PK11SlotInfoStr,
crypto::NSSDestroyer<PK11SlotInfoStr, &PK11_FreeSlot> > > >::vector' requested
here
      modules_(std::move(modules)),
      ^
../../build/linux/debian_wheezy_amd64-sysroot/usr/lib/gcc/x86_64-linux-gnu/4.6/../../../../include/c++/4.6/bits/unique_ptr.h:256:7:
note: 'unique_ptr' has been explicitly marked deleted here
      unique_ptr(const unique_ptr&) = delete;
      ^

[mattm, 2017/02/25 02:45:01]

Yeah, those are some nice error messages :)

I think you just didn't propagate the moves through all the steps.  I took a
stab at it and was able to make it work:
https://codereview.chromium.org/2715063002/

+    modules_.push_back(crypto::ScopedPK11Slot(PK11_ReferenceSlot(slot.get())));
+  }
 }
 
 void SlotUnlocker::Start() {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
 
   for (; current_ < modules_.size(); ++current_) {
     if (ShouldShowDialog(modules_[current_].get())) {
       ShowCryptoModulePasswordDialog(
-          modules_[current_]->GetTokenName(),
-          retry_,
-          reason_,
-          server_.host(),
-          parent_,
+          PK11_GetTokenName(modules_[current_].get()), retry_, reason_,
+          server_.host(), parent_,
           base::Bind(&SlotUnlocker::GotPassword, base::Unretained(this)));
       return;
     }
   }
   Done();
 }
 
 void SlotUnlocker::GotPassword(const std::string& password) {
   // TODO(mattm): PK11_DoPassword has something about PK11_Global.verifyPass.
   // Do we need it?
   // http://mxr.mozilla.org/mozilla/source/security/nss/lib/pk11wrap/pk11auth.c#577
 
   if (password.empty()) {
     // User cancelled entering password.  Oh well.
     ++current_;
     Start();
     return;
   }
 
   // TODO(mattm): handle protectedAuthPath
-  SECStatus rv = PK11_CheckUserPassword(modules_[current_]->os_module_handle(),
-                                        password.c_str());
+  SECStatus rv =
+      PK11_CheckUserPassword(modules_[current_].get(), password.c_str());
   if (rv == SECWouldBlock) {
     // Incorrect password.  Try again.
     retry_ = PR_TRUE;
     Start();
     return;
   }
 
   // TODO(mattm): PK11_DoPassword calls nssTrustDomain_UpdateCachedTokenCerts on
   // non-friendly slots.  How important is that?
 
   // Correct password (SECSuccess) or too many attempts/other failure
   // (SECFailure).  Either way we're done with this slot.
   ++current_;
   Start();
 }
 
 void SlotUnlocker::Done() {
   DCHECK_EQ(current_, modules_.size());
   callback_.Run();
   delete this;
 }
 
 }  // namespace
 
 namespace chrome {
 
-void UnlockSlotsIfNecessary(const net::CryptoModuleList& modules,
+void UnlockSlotsIfNecessary(const std::vector<crypto::ScopedPK11Slot>& modules,
                             chrome::CryptoModulePasswordReason reason,
                             const net::HostPortPair& server,
                             gfx::NativeWindow parent,
                             const base::Closure& callback) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
   for (size_t i = 0; i < modules.size(); ++i) {
     if (ShouldShowDialog(modules[i].get())) {
       (new SlotUnlocker(modules, reason, server, parent, callback))->Start();
       return;
     }
   }
   callback.Run();
 }
 
 void UnlockCertSlotIfNecessary(net::X509Certificate* cert,
                                chrome::CryptoModulePasswordReason reason,
                                const net::HostPortPair& server,
                                gfx::NativeWindow parent,
                                const base::Closure& callback) {
-  net::CryptoModuleList modules;
-  modules.push_back(net::CryptoModule::CreateFromHandle(
-      cert->os_cert_handle()->slot));
+  std::vector<crypto::ScopedPK11Slot> modules;
+  modules.push_back(
+      crypto::ScopedPK11Slot(PK11_ReferenceSlot(cert->os_cert_handle()->slot)));
   UnlockSlotsIfNecessary(modules, reason, server, parent, callback);
 }
 
 }  // namespace chrome