Bug 1517611 - Cycle collect WebAuthnManager and U2F more. r=smaug
authorAndrew McCreight <continuation@gmail.com>
Fri, 18 Jan 2019 23:21:46 +0000
changeset 514477 4145884732caac4d1c9f4dcce9326bd0a0b1e79b
parent 514476 eab01782e1aab26c84e09a6c2752d0ea998f6897
child 514478 f51c5057cc00e733f72ad2b97075d9621ea31552
push id1953
push userffxbld-merge
push dateMon, 11 Mar 2019 12:10:20 +0000
treeherdermozilla-release@9c35dcbaa899 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssmaug
bugs1517611
milestone66.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 1517611 - Cycle collect WebAuthnManager and U2F more. r=smaug Differential Revision: https://phabricator.services.mozilla.com/D17026
dom/base/nsWrapperCache.h
dom/u2f/U2F.cpp
dom/u2f/U2F.h
dom/webauthn/WebAuthnManager.cpp
dom/webauthn/WebAuthnManager.h
dom/webauthn/WebAuthnManagerBase.cpp
dom/webauthn/WebAuthnManagerBase.h
mfbt/Maybe.h
testing/web-platform/meta/webauthn/__dir__.ini
xpcom/threads/nsProxyRelease.h
--- a/dom/base/nsWrapperCache.h
+++ b/dom/base/nsWrapperCache.h
@@ -436,9 +436,22 @@ NS_DEFINE_STATIC_IID_ACCESSOR(nsWrapperC
     NS_IMPL_CYCLE_COLLECTION_UNLINK(__VA_ARGS__)           \
     NS_IMPL_CYCLE_COLLECTION_UNLINK_PRESERVED_WRAPPER      \
   NS_IMPL_CYCLE_COLLECTION_UNLINK_END                      \
   NS_IMPL_CYCLE_COLLECTION_TRAVERSE_BEGIN(_class)          \
     NS_IMPL_CYCLE_COLLECTION_TRAVERSE(__VA_ARGS__)         \
   NS_IMPL_CYCLE_COLLECTION_TRAVERSE_END                    \
   NS_IMPL_CYCLE_COLLECTION_TRACE_WRAPPERCACHE(_class)
 
+// This is used for wrapper cached classes that inherit from cycle
+// collected non-wrapper cached classes.
+#define NS_IMPL_CYCLE_COLLECTION_WRAPPERCACHE_INHERITED(_class, _base, ...) \
+  NS_IMPL_CYCLE_COLLECTION_CLASS(_class)                                    \
+  NS_IMPL_CYCLE_COLLECTION_UNLINK_BEGIN_INHERITED(_class, _base)            \
+    NS_IMPL_CYCLE_COLLECTION_UNLINK(__VA_ARGS__)                            \
+    NS_IMPL_CYCLE_COLLECTION_UNLINK_PRESERVED_WRAPPER                       \
+  NS_IMPL_CYCLE_COLLECTION_UNLINK_END                                       \
+  NS_IMPL_CYCLE_COLLECTION_TRAVERSE_BEGIN_INHERITED(_class, _base)          \
+    NS_IMPL_CYCLE_COLLECTION_TRAVERSE(__VA_ARGS__)                          \
+  NS_IMPL_CYCLE_COLLECTION_TRAVERSE_END                                     \
+  NS_IMPL_CYCLE_COLLECTION_TRACE_WRAPPERCACHE(_class)
+
 #endif /* nsWrapperCache_h___ */
--- a/dom/u2f/U2F.cpp
+++ b/dom/u2f/U2F.cpp
@@ -37,24 +37,23 @@ NS_NAMED_LITERAL_STRING(kGetAssertion, "
 NS_NAMED_LITERAL_STRING(kGoogleAccountsAppId1,
                         "https://www.gstatic.com/securitykey/origins.json");
 NS_NAMED_LITERAL_STRING(
     kGoogleAccountsAppId2,
     "https://www.gstatic.com/securitykey/a/google.com/origins.json");
 
 NS_INTERFACE_MAP_BEGIN_CYCLE_COLLECTION(U2F)
   NS_WRAPPERCACHE_INTERFACE_MAP_ENTRY
-  NS_INTERFACE_MAP_ENTRY(nsISupports)
-  NS_INTERFACE_MAP_ENTRY(nsIDOMEventListener)
-NS_INTERFACE_MAP_END
+NS_INTERFACE_MAP_END_INHERITING(WebAuthnManagerBase)
 
-NS_IMPL_CYCLE_COLLECTING_ADDREF(U2F)
-NS_IMPL_CYCLE_COLLECTING_RELEASE(U2F)
+NS_IMPL_ADDREF_INHERITED(U2F, WebAuthnManagerBase)
+NS_IMPL_RELEASE_INHERITED(U2F, WebAuthnManagerBase)
 
-NS_IMPL_CYCLE_COLLECTION_WRAPPERCACHE(U2F, mParent)
+NS_IMPL_CYCLE_COLLECTION_WRAPPERCACHE_INHERITED(U2F, WebAuthnManagerBase,
+                                                mTransaction)
 
 /***********************************************************************
  * Utility Functions
  **********************************************************************/
 
 static ErrorCode ConvertNSResultToErrorCode(const nsresult& aError) {
   if (aError == NS_ERROR_DOM_TIMEOUT_ERR) {
     return ErrorCode::TIMEOUT;
--- a/dom/u2f/U2F.h
+++ b/dom/u2f/U2F.h
@@ -9,16 +9,17 @@
 
 #include "js/TypeDecls.h"
 #include "mozilla/Attributes.h"
 #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/Maybe.h"
 #include "mozilla/MozPromise.h"
 #include "nsProxyRelease.h"
 #include "nsWrapperCache.h"
 #include "U2FAuthenticator.h"
 
 namespace mozilla {
 namespace dom {
 
@@ -72,18 +73,19 @@ class U2FTransaction {
   static uint64_t NextId() {
     static uint64_t id = 0;
     return ++id;
   }
 };
 
 class U2F final : public WebAuthnManagerBase, public nsWrapperCache {
  public:
-  NS_DECL_CYCLE_COLLECTING_ISUPPORTS
-  NS_DECL_CYCLE_COLLECTION_SCRIPT_HOLDER_CLASS(U2F)
+  NS_DECL_ISUPPORTS_INHERITED
+  NS_DECL_CYCLE_COLLECTION_SCRIPT_HOLDER_CLASS_INHERITED(U2F,
+                                                         WebAuthnManagerBase)
 
   explicit U2F(nsPIDOMWindowInner* aParent) : WebAuthnManagerBase(aParent) {}
 
   nsPIDOMWindowInner* GetParentObject() const { return mParent; }
 
   void Init(ErrorResult& aRv);
 
   virtual JSObject* WrapObject(JSContext* aCx,
@@ -131,12 +133,32 @@ class U2F final : public WebAuthnManager
   void RejectTransaction(const nsresult& aError);
 
   nsString mOrigin;
 
   // The current transaction, if any.
   Maybe<U2FTransaction> mTransaction;
 };
 
+inline void ImplCycleCollectionTraverse(
+    nsCycleCollectionTraversalCallback& aCallback, U2FTransaction& aTransaction,
+    const char* aName, uint32_t aFlags = 0) {
+  if (aTransaction.HasRegisterCallback()) {
+    CycleCollectionNoteChild(
+        aCallback, aTransaction.GetRegisterCallback().get(), aName, aFlags);
+  } else {
+    CycleCollectionNoteChild(aCallback, aTransaction.GetSignCallback().get(),
+                             aName, aFlags);
+  }
+}
+
+inline void ImplCycleCollectionUnlink(U2FTransaction& aTransaction) {
+  if (aTransaction.HasRegisterCallback()) {
+    aTransaction.GetRegisterCallback() = nullptr;
+  } else {
+    aTransaction.GetSignCallback() = nullptr;
+  }
+}
+
 }  // namespace dom
 }  // namespace mozilla
 
 #endif  // mozilla_dom_U2F_h
--- a/dom/webauthn/WebAuthnManager.cpp
+++ b/dom/webauthn/WebAuthnManager.cpp
@@ -26,17 +26,21 @@ namespace dom {
 /***********************************************************************
  * Statics
  **********************************************************************/
 
 namespace {
 static mozilla::LazyLogModule gWebAuthnManagerLog("webauthnmanager");
 }
 
-NS_IMPL_ISUPPORTS(WebAuthnManager, nsIDOMEventListener);
+NS_IMPL_ISUPPORTS_CYCLE_COLLECTION_INHERITED_0(WebAuthnManager,
+                                               WebAuthnManagerBase)
+
+NS_IMPL_CYCLE_COLLECTION_INHERITED(WebAuthnManager, WebAuthnManagerBase,
+                                   mFollowingSignal, mTransaction)
 
 /***********************************************************************
  * Utility Functions
  **********************************************************************/
 
 static nsresult AssembleClientData(
     const nsAString& aOrigin, const CryptoBuffer& aChallenge,
     const nsAString& aType,
--- a/dom/webauthn/WebAuthnManager.h
+++ b/dom/webauthn/WebAuthnManager.h
@@ -2,16 +2,17 @@
 /* vim: set ts=8 sts=2 et sw=2 tw=80: */
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #ifndef mozilla_dom_WebAuthnManager_h
 #define mozilla_dom_WebAuthnManager_h
 
+#include "mozilla/Maybe.h"
 #include "mozilla/MozPromise.h"
 #include "mozilla/dom/PWebAuthnTransaction.h"
 #include "mozilla/dom/WebAuthnManagerBase.h"
 
 /*
  * Content process manager for the WebAuthn protocol. Created on calls to the
  * WebAuthentication DOM object, this manager handles establishing IPC channels
  * for WebAuthn transactions, as well as keeping track of JS Promise objects
@@ -64,17 +65,18 @@ class WebAuthnTransaction {
   static uint64_t NextId() {
     static uint64_t id = 0;
     return ++id;
   }
 };
 
 class WebAuthnManager final : public WebAuthnManagerBase, public AbortFollower {
  public:
-  NS_DECL_ISUPPORTS
+  NS_DECL_ISUPPORTS_INHERITED
+  NS_DECL_CYCLE_COLLECTION_CLASS_INHERITED(WebAuthnManager, WebAuthnManagerBase)
 
   explicit WebAuthnManager(nsPIDOMWindowInner* aParent)
       : WebAuthnManagerBase(aParent) {}
 
   already_AddRefed<Promise> MakeCredential(
       const PublicKeyCredentialCreationOptions& aOptions,
       const Optional<OwningNonNull<AbortSignal>>& aSignal);
 
@@ -112,12 +114,22 @@ class WebAuthnManager final : public Web
   void ClearTransaction();
   // Rejects the current transaction and calls ClearTransaction().
   void RejectTransaction(const nsresult& aError);
 
   // The current transaction, if any.
   Maybe<WebAuthnTransaction> mTransaction;
 };
 
+inline void ImplCycleCollectionTraverse(
+    nsCycleCollectionTraversalCallback& aCallback,
+    WebAuthnTransaction& aTransaction, const char* aName, uint32_t aFlags = 0) {
+  ImplCycleCollectionTraverse(aCallback, aTransaction.mPromise, aName, aFlags);
+}
+
+inline void ImplCycleCollectionUnlink(WebAuthnTransaction& aTransaction) {
+  ImplCycleCollectionUnlink(aTransaction.mPromise);
+}
+
 }  // namespace dom
 }  // namespace mozilla
 
 #endif  // mozilla_dom_WebAuthnManager_h
--- a/dom/webauthn/WebAuthnManagerBase.cpp
+++ b/dom/webauthn/WebAuthnManagerBase.cpp
@@ -19,16 +19,26 @@ NS_NAMED_LITERAL_STRING(kVisibilityChang
 WebAuthnManagerBase::WebAuthnManagerBase(nsPIDOMWindowInner* aParent)
     : mParent(aParent) {
   MOZ_ASSERT(NS_IsMainThread());
   MOZ_ASSERT(aParent);
 }
 
 WebAuthnManagerBase::~WebAuthnManagerBase() { MOZ_ASSERT(NS_IsMainThread()); }
 
+NS_INTERFACE_MAP_BEGIN_CYCLE_COLLECTION(WebAuthnManagerBase)
+  NS_INTERFACE_MAP_ENTRY(nsISupports)
+  NS_INTERFACE_MAP_ENTRY(nsIDOMEventListener)
+NS_INTERFACE_MAP_END
+
+NS_IMPL_CYCLE_COLLECTION(WebAuthnManagerBase, mParent)
+
+NS_IMPL_CYCLE_COLLECTING_ADDREF(WebAuthnManagerBase)
+NS_IMPL_CYCLE_COLLECTING_RELEASE(WebAuthnManagerBase)
+
 /***********************************************************************
  * IPC Protocol Implementation
  **********************************************************************/
 
 bool WebAuthnManagerBase::MaybeCreateBackgroundActor() {
   MOZ_ASSERT(NS_IsMainThread());
 
   if (mChild) {
--- a/dom/webauthn/WebAuthnManagerBase.h
+++ b/dom/webauthn/WebAuthnManagerBase.h
@@ -20,33 +20,36 @@ namespace dom {
 class WebAuthnTransactionChild;
 class WebAuthnMakeCredentialResult;
 class WebAuthnGetAssertionResult;
 
 class WebAuthnManagerBase : public nsIDOMEventListener {
  public:
   NS_DECL_NSIDOMEVENTLISTENER
 
+  NS_DECL_CYCLE_COLLECTING_ISUPPORTS
+  NS_DECL_CYCLE_COLLECTION_CLASS(WebAuthnManagerBase)
+
   explicit WebAuthnManagerBase(nsPIDOMWindowInner* aParent);
 
   virtual void FinishMakeCredential(
       const uint64_t& aTransactionId,
       const WebAuthnMakeCredentialResult& aResult) = 0;
 
   virtual void FinishGetAssertion(
       const uint64_t& aTransactionId,
       const WebAuthnGetAssertionResult& aResult) = 0;
 
   virtual void RequestAborted(const uint64_t& aTransactionId,
                               const nsresult& aError) = 0;
 
   void ActorDestroyed();
 
  protected:
-  ~WebAuthnManagerBase();
+  virtual ~WebAuthnManagerBase();
 
   // Needed by HandleEvent() to cancel transactions.
   virtual void CancelTransaction(const nsresult& aError) = 0;
 
   // Visibility event handling.
   void ListenForVisibilityEvents();
   void StopListeningForVisibilityEvents();
 
--- a/mfbt/Maybe.h
+++ b/mfbt/Maybe.h
@@ -17,16 +17,23 @@
 #include "mozilla/OperatorNewExtensions.h"
 #include "mozilla/Poison.h"
 #include "mozilla/TypeTraits.h"
 
 #include <new>  // for placement new
 #include <ostream>
 #include <type_traits>
 
+class nsCycleCollectionTraversalCallback;
+
+template <typename T>
+inline void CycleCollectionNoteChild(
+    nsCycleCollectionTraversalCallback& aCallback, T* aChild, const char* aName,
+    uint32_t aFlags);
+
 namespace mozilla {
 
 struct Nothing {};
 
 namespace detail {
 
 // You would think that poisoning Maybe instances could just be a call
 // to mozWritePoison.  Unfortunately, using a simple call to
@@ -614,11 +621,27 @@ bool operator<=(const Maybe<T>& aLHS, co
   return aLHS < aRHS || aLHS == aRHS;
 }
 
 template <typename T>
 bool operator>=(const Maybe<T>& aLHS, const Maybe<T>& aRHS) {
   return !(aLHS < aRHS);
 }
 
+template <typename T>
+inline void ImplCycleCollectionTraverse(
+    nsCycleCollectionTraversalCallback& aCallback, mozilla::Maybe<T>& aField,
+    const char* aName, uint32_t aFlags = 0) {
+  if (aField) {
+    ImplCycleCollectionTraverse(aCallback, aField.ref(), aName, aFlags);
+  }
+}
+
+template <typename T>
+inline void ImplCycleCollectionUnlink(mozilla::Maybe<T>& aField) {
+  if (aField) {
+    ImplCycleCollectionUnlink(aField.ref());
+  }
+}
+
 }  // namespace mozilla
 
 #endif /* mozilla_Maybe_h */
--- a/testing/web-platform/meta/webauthn/__dir__.ini
+++ b/testing/web-platform/meta/webauthn/__dir__.ini
@@ -1,2 +1,1 @@
 prefs: [security.webauth.webauthn:true]
-lsan-allowed: [Alloc, alloc_system::platform::_$LT$impl$u20$core..alloc..GlobalAlloc$u20$for$u20$alloc_system..System$GT$::alloc::, alloc_system::platform::_$LT$impl$u20$core..alloc..GlobalAlloc$u20$for$u20$alloc_system..System$GT$::realloc::, mozilla::dom::DOMException::Create, mozilla::dom::Navigator::Credentials, mozilla::dom::Performance::CreateForMainThread]
--- a/xpcom/threads/nsProxyRelease.h
+++ b/xpcom/threads/nsProxyRelease.h
@@ -365,9 +365,26 @@ namespace mozilla {
 template <typename T>
 using PtrHolder = nsMainThreadPtrHolder<T>;
 
 template <typename T>
 using PtrHandle = nsMainThreadPtrHandle<T>;
 
 }  // namespace mozilla
 
+class nsCycleCollectionTraversalCallback;
+template <typename T>
+void CycleCollectionNoteChild(nsCycleCollectionTraversalCallback& aCallback,
+                              T* aChild, const char* aName, uint32_t aFlags);
+
+template <typename T>
+inline void ImplCycleCollectionTraverse(
+    nsCycleCollectionTraversalCallback& aCallback,
+    nsMainThreadPtrHandle<T>& aField, const char* aName, uint32_t aFlags = 0) {
+  CycleCollectionNoteChild(aCallback, aField.get(), aName, aFlags);
+}
+
+template <typename T>
+inline void ImplCycleCollectionUnlink(nsMainThreadPtrHandle<T>& aField) {
+  aField = nullptr;
+}
+
 #endif