Bug 1421616 - Have one WebAuthnManager instance per CredentialsContainer r=jcj

Summary:
We currently have a single WebAuthnManager instance per process that's shared
between all CredentialContainers. That way the nsPIDOMWindowInner parent has
to be tracked by the transaction, as multiple containers could kick off
requests simultaneously.

This patch lets us we have one WebAuthnManager instance per each
CredentialsContainer and thus each nsPIDOMWindowInner. This matches the current
U2F implementation where there is one instance per parent window too.

This somewhat simplifies the communication diagram (at least in my head), as
each U2F/WebAuthnManager instance also has their own TransactionChild/Parent
pair for IPC protocol communication. The manager and child/parent pair are
destroyed when the window is.

Reviewers: jcj

Reviewed By: jcj

Bug #: 1421616

Differential Revision: https://phabricator.services.mozilla.com/D305

Author: Tim Taubert

dom/credentialmanagement/CredentialsContainer.cpp

@@ -11,7 +11,7 @@
 namespace mozilla {
 namespace dom {
 
-NS_IMPL_CYCLE_COLLECTION_WRAPPERCACHE(CredentialsContainer, mParent)
+NS_IMPL_CYCLE_COLLECTION_WRAPPERCACHE(CredentialsContainer, mParent, mManager)
 NS_IMPL_CYCLE_COLLECTING_ADDREF(CredentialsContainer)
 NS_IMPL_CYCLE_COLLECTING_RELEASE(CredentialsContainer)
 NS_INTERFACE_MAP_BEGIN_CYCLE_COLLECTION(CredentialsContainer)
@@ -28,6 +28,16 @@ CredentialsContainer::CredentialsContainer(nsPIDOMWindowInner* aParent) :
 CredentialsContainer::~CredentialsContainer()
 {}
 
+void
+CredentialsContainer::EnsureWebAuthnManager()
+{
+  MOZ_ASSERT(NS_IsMainThread());
+
+  if (!mManager) {
+    mManager = new WebAuthnManager(mParent);
+  }
+}
+
 JSObject*
 CredentialsContainer::WrapObject(JSContext* aCx, JS::Handle<JSObject*> aGivenProto)
 {
@@ -37,22 +47,22 @@ CredentialsContainer::WrapObject(JSContext* aCx, JS::Handle<JSObject*> aGivenPro
 already_AddRefed<Promise>
 CredentialsContainer::Get(const CredentialRequestOptions& aOptions)
 {
-  RefPtr<WebAuthnManager> mgr = WebAuthnManager::GetOrCreate();
-  return mgr->GetAssertion(mParent, aOptions.mPublicKey, aOptions.mSignal);
+  EnsureWebAuthnManager();
+  return mManager->GetAssertion(aOptions.mPublicKey, aOptions.mSignal);
 }
 
 already_AddRefed<Promise>
 CredentialsContainer::Create(const CredentialCreationOptions& aOptions)
 {
-  RefPtr<WebAuthnManager> mgr = WebAuthnManager::GetOrCreate();
-  return mgr->MakeCredential(mParent, aOptions.mPublicKey, aOptions.mSignal);
+  EnsureWebAuthnManager();
+  return mManager->MakeCredential(aOptions.mPublicKey, aOptions.mSignal);
 }
 
 already_AddRefed<Promise>
 CredentialsContainer::Store(const Credential& aCredential)
 {
-  RefPtr<WebAuthnManager> mgr = WebAuthnManager::GetOrCreate();
-  return mgr->Store(mParent, aCredential);
+  EnsureWebAuthnManager();
+  return mManager->Store(aCredential);
 }
 
 } // namespace dom

dom/credentialmanagement/CredentialsContainer.h

@@ -12,6 +12,8 @@
 namespace mozilla {
 namespace dom {
 
+class WebAuthnManager;
+
 class CredentialsContainer final : public nsISupports
                                  , public nsWrapperCache
 {
@@ -32,15 +34,20 @@ class CredentialsContainer final : public nsISupports
 
   already_AddRefed<Promise>
   Get(const CredentialRequestOptions& aOptions);
+
   already_AddRefed<Promise>
   Create(const CredentialCreationOptions& aOptions);
+
   already_AddRefed<Promise>
   Store(const Credential& aCredential);
 
 private:
   ~CredentialsContainer();
 
+  void EnsureWebAuthnManager();
+
   nsCOMPtr<nsPIDOMWindowInner> mParent;
+  RefPtr<WebAuthnManager> mManager;
 };
 
 } // namespace dom

dom/u2f/U2F.cpp

@@ -9,13 +9,13 @@
 #include "mozilla/dom/WebCryptoCommon.h"
 #include "mozilla/ipc/PBackgroundChild.h"
 #include "mozilla/ipc/BackgroundChild.h"
+#include "mozilla/dom/WebAuthnTransactionChild.h"
 #include "nsContentUtils.h"
 #include "nsICryptoHash.h"
 #include "nsIEffectiveTLDService.h"
 #include "nsNetCID.h"
 #include "nsNetUtil.h"
 #include "nsURLParsers.h"
-#include "U2FTransactionChild.h"
 #include "U2FUtil.h"
 #include "hasht.h"
 
@@ -293,9 +293,9 @@ U2F::~U2F()
   }
 
   if (mChild) {
-    RefPtr<U2FTransactionChild> c;
+    RefPtr<WebAuthnTransactionChild> c;
     mChild.swap(c);
-    c->Send__delete__(c);
+    c->Disconnect();
   }
 
   mRegisterCallback.reset();
@@ -437,8 +437,8 @@ U2F::Register(const nsAString& aAppId,
 }
 
 void
-U2F::FinishRegister(const uint64_t& aTransactionId,
-                    nsTArray<uint8_t>& aRegBuffer)
+U2F::FinishMakeCredential(const uint64_t& aTransactionId,
+                          nsTArray<uint8_t>& aRegBuffer)
 {
   MOZ_ASSERT(NS_IsMainThread());
 
@@ -566,9 +566,9 @@ U2F::Sign(const nsAString& aAppId,
 }
 
 void
-U2F::FinishSign(const uint64_t& aTransactionId,
-                nsTArray<uint8_t>& aCredentialId,
-                nsTArray<uint8_t>& aSigBuffer)
+U2F::FinishGetAssertion(const uint64_t& aTransactionId,
+                        nsTArray<uint8_t>& aCredentialId,
+                        nsTArray<uint8_t>& aSigBuffer)
 {
   MOZ_ASSERT(NS_IsMainThread());
 
@@ -749,7 +749,7 @@ U2F::MaybeCreateBackgroundActor()
     return false;
   }
 
-  RefPtr<U2FTransactionChild> mgr(new U2FTransactionChild(this));
+  RefPtr<WebAuthnTransactionChild> mgr(new WebAuthnTransactionChild(this));
   PWebAuthnTransactionChild* constructedMgr =
     actorChild->SendPWebAuthnTransactionConstructor(mgr);
 

dom/u2f/U2F.h

@@ -12,6 +12,7 @@
 #include "mozilla/dom/BindingDeclarations.h"
 #include "mozilla/dom/Nullable.h"
 #include "mozilla/dom/U2FBinding.h"
+#include "mozilla/dom/WebAuthnManagerBase.h"
 #include "mozilla/ErrorResult.h"
 #include "mozilla/MozPromise.h"
 #include "nsProxyRelease.h"
@@ -24,7 +25,7 @@ class nsISerialEventTarget;
 namespace mozilla {
 namespace dom {
 
-class U2FTransactionChild;
+class WebAuthnTransactionChild;
 class U2FRegisterCallback;
 class U2FSignCallback;
 
@@ -59,6 +60,7 @@ class U2FTransaction
 };
 
 class U2F final : public nsIDOMEventListener
+                , public WebAuthnManagerBase
                 , public nsWrapperCache
 {
 public:
@@ -97,18 +99,22 @@ class U2F final : public nsIDOMEventListener
        const Optional<Nullable<int32_t>>& opt_aTimeoutSeconds,
        ErrorResult& aRv);
 
+  // WebAuthnManagerBase
+
   void
-  FinishRegister(const uint64_t& aTransactionId, nsTArray<uint8_t>& aRegBuffer);
+  FinishMakeCredential(const uint64_t& aTransactionId,
+                       nsTArray<uint8_t>& aRegBuffer) override;
 
   void
-  FinishSign(const uint64_t& aTransactionId,
-             nsTArray<uint8_t>& aCredentialId,
-             nsTArray<uint8_t>& aSigBuffer);
+  FinishGetAssertion(const uint64_t& aTransactionId,
+                     nsTArray<uint8_t>& aCredentialId,
+                     nsTArray<uint8_t>& aSigBuffer) override;
 
   void
-  RequestAborted(const uint64_t& aTransactionId, const nsresult& aError);
+  RequestAborted(const uint64_t& aTransactionId,
+                 const nsresult& aError) override;
 
-  void ActorDestroyed();
+  void ActorDestroyed() override;
 
 private:
   ~U2F();
@@ -135,7 +141,7 @@ class U2F final : public nsIDOMEventListener
   Maybe<nsMainThreadPtrHandle<U2FSignCallback>> mSignCallback;
 
   // IPC Channel to the parent process.
-  RefPtr<U2FTransactionChild> mChild;
+  RefPtr<WebAuthnTransactionChild> mChild;
 
   // The current transaction, if any.
   Maybe<U2FTransaction> mTransaction;