From 604c32452131f31d57c105e549192d1f71b56f90 Mon Sep 17 00:00:00 2001 From: Gabriele Svelto Date: Mon, 23 May 2022 19:59:57 +0000 Subject: [PATCH] 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 | 6 ++ dom/midi/MIDIAccessManager.cpp | 8 ++ dom/midi/MIDIAccessManager.h | 2 + dom/midi/MIDIManagerParent.cpp | 5 ++ dom/midi/MIDIManagerParent.h | 1 + dom/midi/MIDIPlatformService.h | 3 + dom/midi/PMIDIManager.ipdl | 1 + dom/midi/TestMIDIPlatformService.cpp | 13 +++ dom/midi/TestMIDIPlatformService.h | 3 + dom/midi/midirMIDIPlatformService.cpp | 13 +++ dom/midi/midirMIDIPlatformService.h | 3 + dom/midi/midir_impl/src/lib.rs | 87 ++++++++++++++++++--- dom/midi/tests/browser.ini | 5 ++ dom/midi/tests/browser_refresh_port_list.js | 73 +++++++++++++++++ dom/midi/tests/port_ids_page_1.html | 10 +-- dom/midi/tests/refresh_port_list.html | 49 ++++++++++++ 16 files changed, 266 insertions(+), 16 deletions(-) create mode 100644 dom/midi/tests/browser_refresh_port_list.js create mode 100644 dom/midi/tests/refresh_port_list.html diff --git a/dom/midi/MIDIAccess.cpp b/dom/midi/MIDIAccess.cpp index ed139b3c7c91..2ddb9b46940e 100644 --- a/dom/midi/MIDIAccess.cpp +++ b/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 diff --git a/dom/midi/MIDIAccessManager.cpp b/dom/midi/MIDIAccessManager.cpp index 65c5588cd4b3..dd93d78cfb5e 100644 --- a/dom/midi/MIDIAccessManager.cpp +++ b/dom/midi/MIDIAccessManager.cpp @@ -98,6 +98,8 @@ bool MIDIAccessManager::AddObserver(Observer* 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* aObserver) { } } +void MIDIAccessManager::SendRefresh() { + if (mChild) { + mChild->SendRefresh(); + } +} + void MIDIAccessManager::CreateMIDIAccess(nsPIDOMWindowInner* aWindow, bool aNeedsSysex, Promise* aPromise) { MOZ_ASSERT(aWindow); diff --git a/dom/midi/MIDIAccessManager.h b/dom/midi/MIDIAccessManager.h index 762e89ae52eb..f5f5319af043 100644 --- a/dom/midi/MIDIAccessManager.h +++ b/dom/midi/MIDIAccessManager.h @@ -50,6 +50,8 @@ class MIDIAccessManager final { bool AddObserver(Observer* aObserver); // Removes a device update observer (usually a MIDIAccess object) void RemoveObserver(Observer* aObserver); + // Requests the service to update the list of devices + void SendRefresh(); private: MIDIAccessManager(); diff --git a/dom/midi/MIDIManagerParent.cpp b/dom/midi/MIDIManagerParent.cpp index a0190ff643d1..d170d5733b25 100644 --- a/dom/midi/MIDIManagerParent.cpp +++ b/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); diff --git a/dom/midi/MIDIManagerParent.h b/dom/midi/MIDIManagerParent.h index 048e41b30dd0..2bcfe9b759ab 100644 --- a/dom/midi/MIDIManagerParent.h +++ b/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; diff --git a/dom/midi/MIDIPlatformService.h b/dom/midi/MIDIPlatformService.h index ad55757581d6..f1f0af5d03d7 100644 --- a/dom/midi/MIDIPlatformService.h +++ b/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; diff --git a/dom/midi/PMIDIManager.ipdl b/dom/midi/PMIDIManager.ipdl index 74fe2bce4cdb..f88ec148525a 100644 --- a/dom/midi/PMIDIManager.ipdl +++ b/dom/midi/PMIDIManager.ipdl @@ -13,6 +13,7 @@ async protocol PMIDIManager { manager PBackground; parent: + async Refresh(); async Shutdown(); child: /* diff --git a/dom/midi/TestMIDIPlatformService.cpp b/dom/midi/TestMIDIPlatformService.cpp index 79cb653ff3c3..991eaa87b53e 100644 --- a/dom/midi/TestMIDIPlatformService.cpp +++ b/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(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!"); } diff --git a/dom/midi/TestMIDIPlatformService.h b/dom/midi/TestMIDIPlatformService.h index aa241d9d49b5..02a6a413001f 100644 --- a/dom/midi/TestMIDIPlatformService.h +++ b/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 mClientThread; + // When true calling Refresh() will add new ports. + bool mDoRefresh; // True if server has been brought up already. bool mIsInitialized; }; diff --git a/dom/midi/midirMIDIPlatformService.cpp b/dom/midi/midirMIDIPlatformService.cpp index 8ed24047b99f..6b9622783703 100644 --- a/dom/midi/midirMIDIPlatformService.cpp +++ b/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(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(); diff --git a/dom/midi/midirMIDIPlatformService.h b/dom/midi/midirMIDIPlatformService.h index ac13c06b6a41..ddd663bbafcc 100644 --- a/dom/midi/midirMIDIPlatformService.h +++ b/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); diff --git a/dom/midi/midir_impl/src/lib.rs b/dom/midi/midir_impl/src/lib.rs index 63186c44ffe4..e63277ccaaff 100644 --- a/dom/midi/midir_impl/src/lib.rs +++ b/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, connections: Vec, @@ -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, InitError> { + let input = MidiInput::new("WebMIDI input")?; + let output = MidiOutput::new("WebMIDI output")?; + let mut ports = Vec::::new(); + collect_input_ports(&input, &mut ports); + collect_output_ports(&output, &mut ports); + Ok(ports) +} + impl MidirWrapper { fn new() -> Result { - let input = MidiInput::new("WebMIDI input")?; - let output = MidiOutput::new("WebMIDI output")?; - let mut ports: Vec = Vec::new(); - collect_input_ports(&input, &mut ports); - collect_output_ports(&output, &mut ports); + let ports = collect_ports()?; let connections: Vec = 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++ diff --git a/dom/midi/tests/browser.ini b/dom/midi/tests/browser.ini index daac24c6eb31..345cd2bbcc39 100644 --- a/dom/midi/tests/browser.ini +++ b/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 diff --git a/dom/midi/tests/browser_refresh_port_list.js b/dom/midi/tests/browser_refresh_port_list.js new file mode 100644 index 000000000000..304ac17cf188 --- /dev/null +++ b/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); +}); diff --git a/dom/midi/tests/port_ids_page_1.html b/dom/midi/tests/port_ids_page_1.html index 8c313b04da90..31dadad4a561 100644 --- a/dom/midi/tests/port_ids_page_1.html +++ b/dom/midi/tests/port_ids_page_1.html @@ -7,11 +7,11 @@ diff --git a/dom/midi/tests/refresh_port_list.html b/dom/midi/tests/refresh_port_list.html new file mode 100644 index 000000000000..96e4a7a3092a --- /dev/null +++ b/dom/midi/tests/refresh_port_list.html @@ -0,0 +1,49 @@ + + + +Refresh MIDI port list test + + + + + +