Bug 1769009 - Refresh the list of MIDI devices both when navigating to a new page and away from an old one r=padenot

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

dom/midi/MIDIAccess.cpp

@@ -205,6 +205,11 @@ void MIDIAccess::MaybeCreateMIDIPort(const MIDIPortInfo& aInfo,
 // request removal from MIDIAccess's maps.
 void MIDIAccess::Notify(const MIDIPortList& aEvent) {
   LOG("MIDIAcess::Notify");
+  if (!GetOwner()) {
+    // Do nothing if we've already been disconnected from the document.
+    return;
+  }
+
   for (const auto& port : aEvent.ports()) {
     // Something went very wrong. Warn and return.
     ErrorResult rv;
@@ -237,6 +242,7 @@ void MIDIAccess::DisconnectFromOwner() {
   IgnoreKeepAliveIfHasListenersFor(nsGkAtoms::onstatechange);
 
   DOMEventTargetHelper::DisconnectFromOwner();
+  MIDIAccessManager::Get()->SendRefresh();
 }
 
 }  // namespace mozilla::dom

dom/midi/MIDIAccessManager.cpp

@@ -98,6 +98,8 @@ bool MIDIAccessManager::AddObserver(Observer<MIDIPortList>* aObserver) {
     // Add a ref to mChild here, that will be deref'd by
     // BackgroundChildImpl::DeallocPMIDIManagerChild on IPC cleanup.
     mChild->SetActorAlive();
+  } else {
+    mChild->SendRefresh();
   }
   return true;
 }
@@ -115,6 +117,12 @@ void MIDIAccessManager::RemoveObserver(Observer<MIDIPortList>* aObserver) {
   }
 }
 
+void MIDIAccessManager::SendRefresh() {
+  if (mChild) {
+    mChild->SendRefresh();
+  }
+}
+
 void MIDIAccessManager::CreateMIDIAccess(nsPIDOMWindowInner* aWindow,
                                          bool aNeedsSysex, Promise* aPromise) {
   MOZ_ASSERT(aWindow);

dom/midi/MIDIAccessManager.h

@@ -50,6 +50,8 @@ class MIDIAccessManager final {
   bool AddObserver(Observer<MIDIPortList>* aObserver);
   // Removes a device update observer (usually a MIDIAccess object)
   void RemoveObserver(Observer<MIDIPortList>* aObserver);
+  // Requests the service to update the list of devices
+  void SendRefresh();
 
  private:
   MIDIAccessManager();

dom/midi/MIDIManagerParent.cpp

@@ -17,6 +17,11 @@ void MIDIManagerParent::Teardown() {
   }
 }
 
+mozilla::ipc::IPCResult MIDIManagerParent::RecvRefresh() {
+  MIDIPlatformService::Get()->Refresh();
+  return IPC_OK();
+}
+
 mozilla::ipc::IPCResult MIDIManagerParent::RecvShutdown() {
   Teardown();
   Unused << Send__delete__(this);

dom/midi/MIDIManagerParent.h

@@ -21,6 +21,7 @@ class MIDIManagerParent final : public PMIDIManagerParent {
  public:
   NS_INLINE_DECL_REFCOUNTING(MIDIManagerParent);
   MIDIManagerParent() = default;
+  mozilla::ipc::IPCResult RecvRefresh();
   mozilla::ipc::IPCResult RecvShutdown();
   void Teardown();
   void ActorDestroy(ActorDestroyReason aWhy) override;

dom/midi/MIDIPlatformService.h

@@ -54,6 +54,9 @@ class MIDIPlatformService {
   // Platform specific init function.
   virtual void Init() = 0;
 
+  // Forces the implementation to refresh the port list.
+  virtual void Refresh() = 0;
+
   // Platform specific MIDI port opening function.
   virtual void Open(MIDIPortParent* aPort) = 0;
 

dom/midi/PMIDIManager.ipdl

@@ -13,6 +13,7 @@ async protocol PMIDIManager
 {
   manager PBackground;
 parent:
+  async Refresh();
   async Shutdown();
 child:
   /*

dom/midi/TestMIDIPlatformService.cpp

@@ -86,6 +86,7 @@ TestMIDIPlatformService::TestMIDIPlatformService()
                                   u"Always Closed MIDI Device Output Port"_ns,
                                   u"Test Manufacturer"_ns, u"1.0.0"_ns,
                                   static_cast<uint32_t>(MIDIPortType::Output)),
+      mDoRefresh(false),
       mIsInitialized(false) {
   AssertIsOnBackgroundThread();
 }
@@ -114,6 +115,13 @@ void TestMIDIPlatformService::Init() {
   NS_DispatchToCurrentThread(r);
 }
 
+void TestMIDIPlatformService::Refresh() {
+  if (mDoRefresh) {
+    AddPortInfo(mStateTestInputPort);
+    mDoRefresh = false;
+  }
+}
+
 void TestMIDIPlatformService::Open(MIDIPortParent* aPort) {
   MOZ_ASSERT(aPort);
   MIDIPortConnectionState s = MIDIPortConnectionState::Open;
@@ -202,6 +210,11 @@ void TestMIDIPlatformService::ProcessMessages(const nsAString& aPortId) {
               mBackgroundThread->Dispatch(r, NS_DISPATCH_NORMAL);
               break;
             }
+            // Causes the next refresh to add new ports to the list
+            case 0x04: {
+              mDoRefresh = true;
+              break;
+            }
             default:
               NS_WARNING("Unknown Test MIDI message received!");
           }

dom/midi/TestMIDIPlatformService.h

@@ -26,6 +26,7 @@ class TestMIDIPlatformService : public MIDIPlatformService {
  public:
   TestMIDIPlatformService();
   virtual void Init() override;
+  virtual void Refresh() override;
   virtual void Open(MIDIPortParent* aPort) override;
   virtual void Stop() override;
   virtual void ScheduleSend(const nsAString& aPort) override;
@@ -53,6 +54,8 @@ class TestMIDIPlatformService : public MIDIPlatformService {
   MIDIPortInfo mAlwaysClosedTestOutputPort;
   // IO Simulation thread. Runs all instances of ProcessMessages().
   nsCOMPtr<nsIThread> mClientThread;
+  // When true calling Refresh() will add new ports.
+  bool mDoRefresh;
   // True if server has been brought up already.
   bool mIsInitialized;
 };

dom/midi/midirMIDIPlatformService.cpp

@@ -11,6 +11,7 @@
 #include "mozilla/dom/MIDIPortParent.h"
 #include "mozilla/dom/MIDIPlatformRunnables.h"
 #include "mozilla/dom/MIDIUtils.h"
+#include "mozilla/dom/midi/midir_impl_ffi_generated.h"
 #include "mozilla/ipc/BackgroundParent.h"
 #include "mozilla/Unused.h"
 #include "nsIThread.h"
@@ -78,6 +79,14 @@ void midirMIDIPlatformService::AddPort(const nsString* aId,
   MIDIPlatformService::Get()->AddPortInfo(port);
 }
 
+// static
+void midirMIDIPlatformService::RemovePort(const nsString* aId,
+                                          const nsString* aName, bool aInput) {
+  MIDIPortType type = aInput ? MIDIPortType::Input : MIDIPortType::Output;
+  MIDIPortInfo port(*aId, *aName, u""_ns, u""_ns, static_cast<uint32_t>(type));
+  MIDIPlatformService::Get()->RemovePortInfo(port);
+}
+
 void midirMIDIPlatformService::Init() {
   if (mImplementation) {
     return;
@@ -117,6 +126,10 @@ void midirMIDIPlatformService::CheckAndReceive(const nsString* aId,
   }
 }
 
+void midirMIDIPlatformService::Refresh() {
+  midir_impl_refresh(mImplementation, AddPort, RemovePort);
+}
+
 void midirMIDIPlatformService::Open(MIDIPortParent* aPort) {
   MOZ_ASSERT(aPort);
   nsString id = aPort->MIDIPortInterface::Id();

dom/midi/midirMIDIPlatformService.h

@@ -26,6 +26,7 @@ class midirMIDIPlatformService : public MIDIPlatformService {
  public:
   midirMIDIPlatformService();
   virtual void Init() override;
+  virtual void Refresh() override;
   virtual void Open(MIDIPortParent* aPort) override;
   virtual void Stop() override;
   virtual void ScheduleSend(const nsAString& aPort) override;
@@ -37,6 +38,8 @@ class midirMIDIPlatformService : public MIDIPlatformService {
   virtual ~midirMIDIPlatformService();
 
   static void AddPort(const nsString* aId, const nsString* aName, bool aInput);
+  static void RemovePort(const nsString* aId, const nsString* aName,
+                         bool aInput);
   static void CheckAndReceive(const nsString* aId, const uint8_t* aData,
                               size_t aLength, const GeckoTimeStamp* aTimeStamp,
                               uint64_t aMicros);

dom/midi/midir_impl/src/lib.rs

@@ -55,6 +55,15 @@ struct MidiPortWrapper {
     open_count: u32,
 }
 
+impl MidiPortWrapper {
+    fn input(self: &MidiPortWrapper) -> bool {
+        match self.port {
+            MidiPort::Input(_) => true,
+            MidiPort::Output(_) => false,
+        }
+    }
+}
+
 pub struct MidirWrapper {
     ports: Vec<MidiPortWrapper>,
     connections: Vec<MidiConnectionWrapper>,
@@ -66,6 +75,43 @@ struct CallbackData {
 }
 
 impl MidirWrapper {
+    fn refresh(
+        self: &mut MidirWrapper,
+        add_callback: unsafe extern "C" fn(id: &nsString, name: &nsString, input: bool),
+        remove_callback: unsafe extern "C" fn(id: &nsString, name: &nsString, input: bool),
+    ) {
+        if let Ok(ports) = collect_ports() {
+            let old_ports = &mut self.ports;
+            let mut i = 0;
+            while i < old_ports.len() {
+                if !ports
+                    .iter()
+                    .any(|p| p.name == old_ports[i].name && p.input() == old_ports[i].input())
+                {
+                    let port = old_ports.remove(i);
+                    let id = nsString::from(&port.id);
+                    let name = nsString::from(&port.name);
+                    unsafe { remove_callback(&id, &name, port.input()) };
+                } else {
+                    i += 1;
+                }
+            }
+
+            for port in ports {
+                if !self
+                    .ports
+                    .iter()
+                    .any(|p| p.name == port.name && p.input() == port.input())
+                {
+                    let id = nsString::from(&port.id);
+                    let name = nsString::from(&port.name);
+                    unsafe { add_callback(&id, &name, port.input()) };
+                    self.ports.push(port);
+                }
+            }
+        }
+    }
+
     fn open_port(
         self: &mut MidirWrapper,
         nsid: &nsString,
@@ -106,22 +152,20 @@ impl MidirWrapper {
                                 data,
                             )
                             .map_err(|_err| ())?;
-                        let connection_wrapper = MidiConnectionWrapper {
+                        MidiConnectionWrapper {
                             id: id.clone(),
                             connection: MidiConnection::Input(connection),
-                        };
-                        connection_wrapper
+                        }
                     }
                     MidiPort::Output(port) => {
                         let output = MidiOutput::new("WebMIDI output").map_err(|_err| ())?;
                         let connection = output
                             .connect(port, "Output connection")
                             .map_err(|_err| ())?;
-                        let connection_wrapper = MidiConnectionWrapper {
+                        MidiConnectionWrapper {
                             connection: MidiConnection::Output(connection),
                             id: id.clone(),
-                        };
-                        connection_wrapper
+                        }
                     }
                 };
 
@@ -175,13 +219,18 @@ impl MidirWrapper {
     }
 }
 
+fn collect_ports() -> Result<Vec<MidiPortWrapper>, InitError> {
+    let input = MidiInput::new("WebMIDI input")?;
+    let output = MidiOutput::new("WebMIDI output")?;
+    let mut ports = Vec::<MidiPortWrapper>::new();
+    collect_input_ports(&input, &mut ports);
+    collect_output_ports(&output, &mut ports);
+    Ok(ports)
+}
+
 impl MidirWrapper {
     fn new() -> Result<MidirWrapper, InitError> {
-        let input = MidiInput::new("WebMIDI input")?;
-        let output = MidiOutput::new("WebMIDI output")?;
-        let mut ports: Vec<MidiPortWrapper> = Vec::new();
-        collect_input_ports(&input, &mut ports);
-        collect_output_ports(&output, &mut ports);
+        let ports = collect_ports()?;
         let connections: Vec<MidiConnectionWrapper> = Vec::new();
         Ok(MidirWrapper { ports, connections })
     }
@@ -210,6 +259,22 @@ pub unsafe extern "C" fn midir_impl_init(
     }
 }
 
+/// Refresh the list of ports.
+///
+/// This function will be exposed to C++
+///
+/// # Safety
+///
+/// `wrapper` must be the pointer returned by [midir_impl_init()].
+#[no_mangle]
+pub unsafe extern "C" fn midir_impl_refresh(
+    wrapper: *mut MidirWrapper,
+    add_callback: unsafe extern "C" fn(id: &nsString, name: &nsString, input: bool),
+    remove_callback: unsafe extern "C" fn(id: &nsString, name: &nsString, input: bool),
+) {
+    (*wrapper).refresh(add_callback, remove_callback)
+}
+
 /// Shutdown midir and free the C++ wrapper.
 ///
 /// This function will be exposed to C++

dom/midi/tests/browser.ini

@@ -10,3 +10,8 @@ run-if = (os != 'android')
 support-files =
   port_ids_page_1.html
   port_ids_page_2.html
+
+[browser_refresh_port_list.js]
+run-if = (os != 'android')
+support-files =
+  refresh_port_list.html

dom/midi/tests/browser_refresh_port_list.js

@@ -0,0 +1,73 @@
+/* Any copyright is dedicated to the Public Domain.
+ * http://creativecommons.org/publicdomain/zero/1.0/ */
+
+const EXAMPLE_ORG_URL = "https://example.org/browser/dom/midi/tests/";
+const PAGE = "refresh_port_list.html";
+
+async function get_access(browser) {
+  return SpecialPowers.spawn(gBrowser.selectedBrowser, [], function() {
+    return content.wrappedJSObject.get_access();
+  });
+}
+
+async function reset_access(browser) {
+  return SpecialPowers.spawn(gBrowser.selectedBrowser, [], function() {
+    return content.wrappedJSObject.reset_access();
+  });
+}
+
+async function get_num_ports(browser) {
+  return SpecialPowers.spawn(gBrowser.selectedBrowser, [], function() {
+    return content.wrappedJSObject.get_num_ports();
+  });
+}
+
+async function add_port(browser) {
+  return SpecialPowers.spawn(gBrowser.selectedBrowser, [], () => {
+    return content.wrappedJSObject.add_port();
+  });
+}
+
+async function remove_port(browser) {
+  return SpecialPowers.spawn(gBrowser.selectedBrowser, [], () => {
+    return content.wrappedJSObject.remove_port();
+  });
+}
+
+async function force_refresh(browser) {
+  return SpecialPowers.spawn(gBrowser.selectedBrowser, [], () => {
+    return content.wrappedJSObject.force_refresh();
+  });
+}
+
+add_task(async function() {
+  gBrowser.selectedTab = BrowserTestUtils.addTab(
+    gBrowser,
+    EXAMPLE_ORG_URL + PAGE
+  );
+  await BrowserTestUtils.browserLoaded(gBrowser.selectedBrowser);
+
+  await get_access(gBrowser.selectedBrowser);
+  let ports_num = await get_num_ports(gBrowser.selectedBrowser);
+  Assert.equal(ports_num, 4, "We start with four ports");
+  await add_port(gBrowser.selectedBrowser);
+  ports_num = await get_num_ports(gBrowser.selectedBrowser);
+  Assert.equal(ports_num, 5, "One port is added manually");
+  // This causes the test service to refresh the ports the next time a refresh
+  // is requested, it will happen after we reload the tab later on and will add
+  // back the port that we're removing on the next line.
+  await force_refresh(gBrowser.selectedBrowser);
+  await remove_port(gBrowser.selectedBrowser);
+  ports_num = await get_num_ports(gBrowser.selectedBrowser);
+  Assert.equal(ports_num, 4, "One port is removed manually");
+
+  const finished = BrowserTestUtils.browserLoaded(gBrowser.selectedBrowser);
+  gBrowser.reloadTab(gBrowser.selectedTab);
+  await finished;
+
+  await get_access(gBrowser.selectedBrowser);
+  let refreshed_ports_num = await get_num_ports(gBrowser.selectedBrowser);
+  Assert.equal(refreshed_ports_num, 5, "One port is added by the refresh");
+
+  gBrowser.removeTab(gBrowser.selectedTab);
+});

dom/midi/tests/port_ids_page_1.html

@@ -7,11 +7,11 @@
 <body>
 <script>
     async function get_first_input_id() {
-    let access = await navigator.requestMIDIAccess({ sysex: false });
-    const inputs = access.inputs.values();
-    const input = inputs.next();
-    return input.value.id;
-}
+      let access = await navigator.requestMIDIAccess({ sysex: false });
+      const inputs = access.inputs.values();
+      const input = inputs.next();
+      return input.value.id;
+  }
 </script>
 </body>
 </html>

dom/midi/tests/refresh_port_list.html

@@ -0,0 +1,49 @@
+<!DOCTYPE html>
+<html>
+<head>
+<title>Refresh MIDI port list test</title>
+<meta http-equiv="Content-Type" content="text/html;charset=utf-8"></meta>
+</head>
+<body>
+<script>
+  var access = null;
+  async function get_access() {
+    access = await navigator.requestMIDIAccess({ sysex: true });
+  }
+
+  async function reset_access() {
+    access = null;
+  }
+
+  async function get_num_ports() {
+    return access.inputs.size + access.outputs.size;
+  }
+
+  async function add_port() {
+    let addPortPromise = new Promise(resolve => {
+      access.addEventListener("statechange", (event) => { dump("***** 1 event.port.name = " + event.port.name + "event.connection = " + event.port.connection + "\n"); if (event.port.connection != "open") { resolve(); } });
+    });
+    const outputs = access.outputs.values();
+    const output = outputs.next().value;
+    output.send([0x90, 0x01, 0x00]);
+    await addPortPromise;
+  }
+
+  async function remove_port() {
+    let removePortPromise = new Promise(resolve => {
+        access.addEventListener("statechange", (event) => { dump("***** 2 event.port.name = " + event.port.name + "event.connection = " + event.port.connection + "\n"); if (event.port.connection != "open") { resolve(); } });
+    });
+    const outputs = access.outputs.values();
+    const output = outputs.next().value;
+    output.send([0x90, 0x02, 0x00]);
+    await removePortPromise;
+  }
+
+  async function force_refresh() {
+    const outputs = access.outputs.values();
+    const output = outputs.next().value;
+    output.send([0x90, 0x04, 0x00]);
+  }
+</script>
+</body>
+</html>