Bug 1931873 - Use WeakPtr in MIDIAccess, r=gsvelto a=RyanVM

The WeakPtr type simplifies the situation if the `MIDIPort` object
outlives its `MIDIAccess`.

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

dom/midi/MIDIAccess.cpp

@@ -45,6 +45,7 @@ NS_IMPL_CYCLE_COLLECTION_UNLINK_BEGIN_INHERITED(MIDIAccess,
   NS_IMPL_CYCLE_COLLECTION_UNLINK(mOutputMap)
   NS_IMPL_CYCLE_COLLECTION_UNLINK(mAccessPromise)
   tmp->Shutdown();
+  NS_IMPL_CYCLE_COLLECTION_UNLINK_WEAK_PTR
 NS_IMPL_CYCLE_COLLECTION_UNLINK_END
 
 NS_INTERFACE_MAP_BEGIN_CYCLE_COLLECTION(MIDIAccess)
@@ -74,7 +75,6 @@ void MIDIAccess::Shutdown() {
   if (mHasShutdown) {
     return;
   }
-  mDestructionObservers.Broadcast(void_t());
   if (MIDIAccessManager::IsRunning()) {
     MIDIAccessManager::Get()->RemoveObserver(this);
   }
@@ -188,8 +188,6 @@ void MIDIAccess::MaybeCreateMIDIPort(const MIDIPortInfo& aInfo,
     // That is bad.
     MOZ_CRASH("We shouldn't be here!");
   }
-  // Set up port to listen for destruction of this access object.
-  mDestructionObservers.AddObserver(port);
 
   // If we haven't resolved the promise for handing the MIDIAccess object to
   // content, this means we're still populating the list of already connected
@@ -236,10 +234,6 @@ JSObject* MIDIAccess::WrapObject(JSContext* aCx,
   return MIDIAccess_Binding::Wrap(aCx, this, aGivenProto);
 }
 
-void MIDIAccess::RemovePortListener(MIDIAccessDestructionObserver* aObs) {
-  mDestructionObservers.RemoveObserver(aObs);
-}
-
 void MIDIAccess::DisconnectFromOwner() {
   IgnoreKeepAliveIfHasListenersFor(nsGkAtoms::onstatechange);
 

dom/midi/MIDIAccess.h

@@ -10,6 +10,7 @@
 #include "mozilla/Attributes.h"
 #include "mozilla/DOMEventTargetHelper.h"
 #include "mozilla/Observer.h"
+#include "mozilla/WeakPtr.h"
 #include "nsCycleCollectionParticipant.h"
 #include "nsWrapperCache.h"
 
@@ -18,10 +19,6 @@ struct JSContext;
 namespace mozilla {
 class ErrorResult;
 
-// Predeclare void_t here, as including IPCMessageUtils brings in windows.h and
-// causes binding compilation problems.
-struct void_t;
-
 namespace dom {
 
 class MIDIAccessManager;
@@ -35,8 +32,6 @@ class MIDIPortInfo;
 class MIDIPortList;
 class Promise;
 
-using MIDIAccessDestructionObserver = Observer<void_t>;
-
 /**
  * MIDIAccess is the DOM object that is handed to the user upon MIDI permissions
  * being successfully granted. It manages access to MIDI ports, and fires events
@@ -46,7 +41,8 @@ using MIDIAccessDestructionObserver = Observer<void_t>;
  * MIDIAccess objects are managed via MIDIAccessManager.
  */
 class MIDIAccess final : public DOMEventTargetHelper,
-                         public Observer<MIDIPortList> {
+                         public Observer<MIDIPortList>,
+                         public SupportsWeakPtr {
   // Use the Permission Request class in MIDIAccessManager for creating
   // MIDIAccess objects.
   friend class MIDIPermissionRequest;
@@ -72,12 +68,6 @@ class MIDIAccess final : public DOMEventTargetHelper,
   // Observer implementation for receiving port connection updates
   void Notify(const MIDIPortList& aEvent) override;
 
-  // All MIDIPort objects observe destruction of the MIDIAccess object that
-  // created them, as the port object receives disconnection events which then
-  // must be passed up to the MIDIAccess object. If the Port object dies before
-  // the MIDIAccess object, it needs to be removed from the observer list.
-  void RemovePortListener(MIDIAccessDestructionObserver* aObs);
-
   // Fires DOM event on port connection/disconnection
   void FireConnectionEvent(MIDIPort* aPort);
 
@@ -101,8 +91,6 @@ class MIDIAccess final : public DOMEventTargetHelper,
   RefPtr<MIDIInputMap> mInputMap;
   // Stores all known MIDIOutput Ports
   RefPtr<MIDIOutputMap> mOutputMap;
-  // List of MIDIPort observers that need to be updated on destruction.
-  ObserverList<void_t> mDestructionObservers;
   // True if user gave permissions for sysex usage to this object.
   bool mSysexEnabled;
   // Promise created by RequestMIDIAccess call, to be resolved after port

dom/midi/MIDIPort.cpp

@@ -48,10 +48,6 @@ MIDIPort::MIDIPort(nsPIDOMWindowInner* aWindow)
 }
 
 MIDIPort::~MIDIPort() {
-  if (mMIDIAccessParent) {
-    mMIDIAccessParent->RemovePortListener(this);
-    mMIDIAccessParent = nullptr;
-  }
   if (Port()) {
     // If the IPC port channel is still alive at this point, it means we're
     // probably CC'ing this port object. Send the shutdown message to also clean
@@ -191,13 +187,6 @@ already_AddRefed<Promise> MIDIPort::Close(ErrorResult& aError) {
   return p.forget();
 }
 
-void MIDIPort::Notify(const void_t& aVoid) {
-  LOG("MIDIPort::notify MIDIAccess shutting down, dropping reference.");
-  // If we're getting notified, it means the MIDIAccess parent object is dead.
-  // Nullify our copy.
-  mMIDIAccessParent = nullptr;
-}
-
 void MIDIPort::FireStateChangeEvent() {
   if (!GetOwner()) {
     return;  // Ignore changes once we've been disconnected from the owner
@@ -238,8 +227,8 @@ void MIDIPort::FireStateChangeEvent() {
 
   // Fire MIDIAccess events first so that the port is no longer in the port
   // maps.
-  if (mMIDIAccessParent) {
+  if (RefPtr<MIDIAccess> access = mMIDIAccessParent.get()) {
-    mMIDIAccessParent->FireConnectionEvent(this);
+    access->FireConnectionEvent(this);
   }
 
   MIDIConnectionEventInit init;

dom/midi/MIDIPort.h

@@ -28,8 +28,7 @@ class MIDIMessage;
  * and communication.
  *
  */
-class MIDIPort : public DOMEventTargetHelper,
+class MIDIPort : public DOMEventTargetHelper {
-                 public MIDIAccessDestructionObserver {
  public:
   NS_DECL_ISUPPORTS_INHERITED
   NS_DECL_CYCLE_COLLECTION_SCRIPT_HOLDER_CLASS_INHERITED(MIDIPort,
@@ -56,10 +55,6 @@ class MIDIPort : public DOMEventTargetHelper,
   already_AddRefed<Promise> Open(ErrorResult& aError);
   already_AddRefed<Promise> Close(ErrorResult& aError);
 
-  // MIDIPorts observe the death of their parent MIDIAccess object, and delete
-  // their reference accordingly.
-  virtual void Notify(const void_t& aVoid) override;
-
   void FireStateChangeEvent();
 
   virtual void StateChange();
@@ -106,11 +101,8 @@ class MIDIPort : public DOMEventTargetHelper,
 
   // MIDIAccess object that created this MIDIPort object, which we need for
   // firing port connection events. There is a chance this MIDIPort object can
-  // outlive its parent MIDIAccess object, so this is a weak reference that must
+  // outlive its parent MIDIAccess object, so this is a weak pointer.
-  // be handled properly. It is set on construction of the MIDIPort object, and
+  WeakPtr<MIDIAccess> mMIDIAccessParent;
-  // set to null when the parent MIDIAccess object is destroyed, which fires an
-  // notification we observe.
-  MIDIAccess* mMIDIAccessParent;
   // Promise object generated on Open() call, that needs to be resolved once the
   // platform specific Open() function has completed.
   RefPtr<Promise> mOpeningPromise;