Bug 1540658 - Web Authentication - U2FTokenManager must obey the IPC state machine r=keeler
authorJ.C. Jones <jjones@mozilla.com>
Tue, 02 Apr 2019 18:26:38 +0000
changeset 467643 10f14d91c8249c211f301b35cfff85907f8580dc
parent 467642 0f5833c5f5f7dd895a8b57d0be645825fa75aba1
child 467644 88a9c30783a6ba2b184aef3f0a5304c4387775f1
push id35806
push userrgurzau@mozilla.com
push dateWed, 03 Apr 2019 04:07:39 +0000
treeherdermozilla-central@45808ab18609 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerskeeler
bugs1540658, 1448408
milestone68.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1540658 - Web Authentication - U2FTokenManager must obey the IPC state machine r=keeler In Bug 1448408 ("Don't listen to visibility events"), I changed `U2FTokenManager:: ClearTransaction` to send aborts, to handle the new visibility states. However, `WebAuthnTransactionParent::ActorDestroy` is called at the conclusion of IPC shutdown, which calls `MaybeClearTransaction` in `U2FTokenManager`, which calls ClearTransaction, which then tries to send an Abort, which is a state machine failure since we just shut the IPC down. This patch creates a new `AbortOngoingTransaction` method which is used to send the aborts instead of shoehorning that into `ClearTransaction`, reverting `ClearTransaction` back to the prior form, and instead changes `Register` and `Sign` to call the new method. Differential Revision: https://phabricator.services.mozilla.com/D25687
dom/webauthn/U2FTokenManager.cpp
dom/webauthn/U2FTokenManager.h
--- a/dom/webauthn/U2FTokenManager.cpp
+++ b/dom/webauthn/U2FTokenManager.cpp
@@ -161,32 +161,38 @@ U2FTokenManager* U2FTokenManager::Get() 
 }
 
 void U2FTokenManager::AbortTransaction(const uint64_t& aTransactionId,
                                        const nsresult& aError) {
   Unused << mTransactionParent->SendAbort(aTransactionId, aError);
   ClearTransaction();
 }
 
+void U2FTokenManager::AbortOngoingTransaction() {
+  if (mLastTransactionId > 0 && mTransactionParent) {
+    // Send an abort to any other ongoing transaction
+    Unused << mTransactionParent->SendAbort(mLastTransactionId,
+                                            NS_ERROR_DOM_ABORT_ERR);
+  }
+  ClearTransaction();
+}
+
 void U2FTokenManager::MaybeClearTransaction(
     PWebAuthnTransactionParent* aParent) {
   // Only clear if we've been requested to do so by our current transaction
   // parent.
   if (mTransactionParent == aParent) {
     ClearTransaction();
   }
 }
 
 void U2FTokenManager::ClearTransaction() {
-  if (mLastTransactionId > 0 && mTransactionParent) {
+  if (mLastTransactionId) {
     // Remove any prompts we might be showing for the current transaction.
     SendPromptNotification(kCancelPromptNotifcation, mLastTransactionId);
-    // Send an abort to any other ongoing transaction
-    Unused << mTransactionParent->SendAbort(mLastTransactionId,
-                                            NS_ERROR_DOM_ABORT_ERR);
   }
 
   mTransactionParent = nullptr;
 
   // Drop managers at the end of all transactions
   if (mTokenManagerImpl) {
     mTokenManagerImpl->Drop();
     mTokenManagerImpl = nullptr;
@@ -266,17 +272,17 @@ RefPtr<U2FTokenTransport> U2FTokenManage
 }
 
 void U2FTokenManager::Register(
     PWebAuthnTransactionParent* aTransactionParent,
     const uint64_t& aTransactionId,
     const WebAuthnMakeCredentialInfo& aTransactionInfo) {
   MOZ_LOG(gU2FTokenManagerLog, LogLevel::Debug, ("U2FAuthRegister"));
 
-  ClearTransaction();
+  AbortOngoingTransaction();
   mTransactionParent = aTransactionParent;
   mTokenManagerImpl = GetTokenManagerImpl();
 
   if (!mTokenManagerImpl) {
     AbortTransaction(aTransactionId, NS_ERROR_DOM_NOT_ALLOWED_ERR);
     return;
   }
 
@@ -363,17 +369,17 @@ void U2FTokenManager::MaybeAbortRegister
   AbortTransaction(aTransactionId, aError);
 }
 
 void U2FTokenManager::Sign(PWebAuthnTransactionParent* aTransactionParent,
                            const uint64_t& aTransactionId,
                            const WebAuthnGetAssertionInfo& aTransactionInfo) {
   MOZ_LOG(gU2FTokenManagerLog, LogLevel::Debug, ("U2FAuthSign"));
 
-  ClearTransaction();
+  AbortOngoingTransaction();
   mTransactionParent = aTransactionParent;
   mTokenManagerImpl = GetTokenManagerImpl();
 
   if (!mTokenManagerImpl) {
     AbortTransaction(aTransactionId, NS_ERROR_DOM_NOT_ALLOWED_ERR);
     return;
   }
 
--- a/dom/webauthn/U2FTokenManager.h
+++ b/dom/webauthn/U2FTokenManager.h
@@ -44,16 +44,17 @@ class U2FTokenManager final : public nsI
   void MaybeClearTransaction(PWebAuthnTransactionParent* aParent);
   static void Initialize();
 
  private:
   U2FTokenManager();
   ~U2FTokenManager() {}
   RefPtr<U2FTokenTransport> GetTokenManagerImpl();
   void AbortTransaction(const uint64_t& aTransactionId, const nsresult& aError);
+  void AbortOngoingTransaction();
   void ClearTransaction();
   // Step two of "Register", kicking off the actual transaction.
   void DoRegister(const WebAuthnMakeCredentialInfo& aInfo,
                   bool aForceNoneAttestation);
   void MaybeConfirmRegister(const uint64_t& aTransactionId,
                             const WebAuthnMakeCredentialResult& aResult);
   void MaybeAbortRegister(const uint64_t& aTransactionId,
                           const nsresult& aError);