Bug 1439326 - Add U2FTokenTransport::Drop() to better handle U2FHIDTokenManager destruction r=jcj

dom/webauthn/U2FHIDTokenManager.cpp

@@ -5,6 +5,7 @@
  * You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #include "mozilla/dom/U2FHIDTokenManager.h"
+#include "mozilla/ipc/BackgroundParent.h"
 #include "mozilla/StaticMutex.h"
 
 namespace mozilla {
@@ -23,7 +24,7 @@ u2f_register_callback(uint64_t aTransactionId, rust_u2f_result* aResult)
   }
 
   UniquePtr<U2FResult> rv = MakeUnique<U2FResult>(aTransactionId, aResult);
-  nsCOMPtr<nsIRunnable> r(NewNonOwningRunnableMethod<UniquePtr<U2FResult>&&>(
+  nsCOMPtr<nsIRunnable> r(NewRunnableMethod<UniquePtr<U2FResult>&&>(
       "U2FHIDTokenManager::HandleRegisterResult", gInstance,
       &U2FHIDTokenManager::HandleRegisterResult, Move(rv)));
 
@@ -40,7 +41,7 @@ u2f_sign_callback(uint64_t aTransactionId, rust_u2f_result* aResult)
   }
 
   UniquePtr<U2FResult> rv = MakeUnique<U2FResult>(aTransactionId, aResult);
-  nsCOMPtr<nsIRunnable> r(NewNonOwningRunnableMethod<UniquePtr<U2FResult>&&>(
+  nsCOMPtr<nsIRunnable> r(NewRunnableMethod<UniquePtr<U2FResult>&&>(
       "U2FHIDTokenManager::HandleSignResult", gInstance,
       &U2FHIDTokenManager::HandleSignResult, Move(rv)));
 
@@ -51,8 +52,8 @@ u2f_sign_callback(uint64_t aTransactionId, rust_u2f_result* aResult)
 U2FHIDTokenManager::U2FHIDTokenManager() : mTransactionId(0)
 {
   StaticMutexAutoLock lock(gInstanceMutex);
+  mozilla::ipc::AssertIsOnBackgroundThread();
   MOZ_ASSERT(XRE_IsParentProcess());
-  MOZ_ASSERT(!NS_IsMainThread());
   MOZ_ASSERT(!gInstance);
 
   mU2FManager = rust_u2f_mgr_new();
@@ -61,11 +62,12 @@ U2FHIDTokenManager::U2FHIDTokenManager() : mTransactionId(0)
   gInstance = this;
 }
 
-U2FHIDTokenManager::~U2FHIDTokenManager()
+void
+U2FHIDTokenManager::Drop()
 {
   {
     StaticMutexAutoLock lock(gInstanceMutex);
-    MOZ_ASSERT(NS_GetCurrentThread() == gPBackgroundThread);
+    mozilla::ipc::AssertIsOnBackgroundThread();
 
     mRegisterPromise.RejectIfExists(NS_ERROR_DOM_UNKNOWN_ERR, __func__);
     mSignPromise.RejectIfExists(NS_ERROR_DOM_UNKNOWN_ERR, __func__);
@@ -78,6 +80,9 @@ U2FHIDTokenManager::~U2FHIDTokenManager()
   // u2f_{register,sign}_callback to lock and return.
   rust_u2f_mgr_free(mU2FManager);
   mU2FManager = nullptr;
+
+  // Reset transaction ID so that queued runnables exit early.
+  mTransactionId = 0;
 }
 
 // A U2F Register operation causes a new key pair to be generated by the token.
@@ -106,7 +111,7 @@ U2FHIDTokenManager::Register(const nsTArray<WebAuthnScopedCredential>& aCredenti
                              const nsTArray<uint8_t>& aChallenge,
                              uint32_t aTimeoutMS)
 {
-  MOZ_ASSERT(NS_GetCurrentThread() == gPBackgroundThread);
+  mozilla::ipc::AssertIsOnBackgroundThread();
 
   uint64_t registerFlags = 0;
 
@@ -162,7 +167,7 @@ U2FHIDTokenManager::Sign(const nsTArray<WebAuthnScopedCredential>& aCredentials,
                          bool aRequireUserVerification,
                          uint32_t aTimeoutMS)
 {
-  MOZ_ASSERT(NS_GetCurrentThread() == gPBackgroundThread);
+  mozilla::ipc::AssertIsOnBackgroundThread();
 
   uint64_t signFlags = 0;
 
@@ -191,7 +196,7 @@ U2FHIDTokenManager::Sign(const nsTArray<WebAuthnScopedCredential>& aCredentials,
 void
 U2FHIDTokenManager::Cancel()
 {
-  MOZ_ASSERT(NS_GetCurrentThread() == gPBackgroundThread);
+  mozilla::ipc::AssertIsOnBackgroundThread();
 
   ClearPromises();
   mTransactionId = rust_u2f_mgr_cancel(mU2FManager);
@@ -200,7 +205,7 @@ U2FHIDTokenManager::Cancel()
 void
 U2FHIDTokenManager::HandleRegisterResult(UniquePtr<U2FResult>&& aResult)
 {
-  MOZ_ASSERT(NS_GetCurrentThread() == gPBackgroundThread);
+  mozilla::ipc::AssertIsOnBackgroundThread();
 
   if (aResult->GetTransactionId() != mTransactionId) {
     return;
@@ -221,7 +226,7 @@ U2FHIDTokenManager::HandleRegisterResult(UniquePtr<U2FResult>&& aResult)
 void
 U2FHIDTokenManager::HandleSignResult(UniquePtr<U2FResult>&& aResult)
 {
-  MOZ_ASSERT(NS_GetCurrentThread() == gPBackgroundThread);
+  mozilla::ipc::AssertIsOnBackgroundThread();
 
   if (aResult->GetTransactionId() != mTransactionId) {
     return;

dom/webauthn/U2FHIDTokenManager.h

@@ -93,14 +93,14 @@ class U2FHIDTokenManager final : public U2FTokenTransport
 public:
   explicit U2FHIDTokenManager();
 
-  virtual RefPtr<U2FRegisterPromise>
+  RefPtr<U2FRegisterPromise>
   Register(const nsTArray<WebAuthnScopedCredential>& aCredentials,
            const WebAuthnAuthenticatorSelection &aAuthenticatorSelection,
            const nsTArray<uint8_t>& aApplication,
            const nsTArray<uint8_t>& aChallenge,
            uint32_t aTimeoutMS) override;
 
-  virtual RefPtr<U2FSignPromise>
+  RefPtr<U2FSignPromise>
   Sign(const nsTArray<WebAuthnScopedCredential>& aCredentials,
        const nsTArray<uint8_t>& aApplication,
        const nsTArray<uint8_t>& aChallenge,
@@ -108,12 +108,13 @@ public:
        uint32_t aTimeoutMS) override;
 
   void Cancel() override;
+  void Drop() override;
 
   void HandleRegisterResult(UniquePtr<U2FResult>&& aResult);
   void HandleSignResult(UniquePtr<U2FResult>&& aResult);
 
 private:
-  ~U2FHIDTokenManager();
+  ~U2FHIDTokenManager() { }
 
   void ClearPromises() {
     mRegisterPromise.RejectIfExists(NS_ERROR_DOM_UNKNOWN_ERR, __func__);

dom/webauthn/U2FSoftTokenManager.h

@@ -23,21 +23,21 @@ class U2FSoftTokenManager final : public U2FTokenTransport
 public:
   explicit U2FSoftTokenManager(uint32_t aCounter);
 
-  virtual RefPtr<U2FRegisterPromise>
+  RefPtr<U2FRegisterPromise>
   Register(const nsTArray<WebAuthnScopedCredential>& aCredentials,
            const WebAuthnAuthenticatorSelection &aAuthenticatorSelection,
            const nsTArray<uint8_t>& aApplication,
            const nsTArray<uint8_t>& aChallenge,
            uint32_t aTimeoutMS) override;
 
-  virtual RefPtr<U2FSignPromise>
+  RefPtr<U2FSignPromise>
   Sign(const nsTArray<WebAuthnScopedCredential>& aCredentials,
        const nsTArray<uint8_t>& aApplication,
        const nsTArray<uint8_t>& aChallenge,
        bool aRequireUserVerification,
        uint32_t aTimeoutMS) override;
 
-  virtual void Cancel() override;
+  void Cancel() override;
 
 private:
   ~U2FSoftTokenManager() {}

dom/webauthn/U2FTokenManager.cpp

@@ -180,7 +180,10 @@ U2FTokenManager::ClearTransaction()
 {
   mTransactionParent = nullptr;
   // Drop managers at the end of all transactions
-  mTokenManagerImpl = nullptr;
+  if (mTokenManagerImpl) {
+    mTokenManagerImpl->Drop();
+    mTokenManagerImpl = nullptr;
+  }
   // Forget promises, if necessary.
   mRegisterPromise.DisconnectIfExists();
   mSignPromise.DisconnectIfExists();

dom/webauthn/U2FTokenTransport.h

@@ -43,6 +43,8 @@ public:
 
   virtual void Cancel() = 0;
 
+  virtual void Drop() { }
+
 protected:
   virtual ~U2FTokenTransport() = default;
 };