Skip to content

Commit cdffe24

Browse files
Bug 1745443 - Keep MIDI objects alive as long as there's some pending activity r=smaug
MIDI ports will be kept alive even without direct JS references to it under the following conditions: * They've not been explicitly closed (they're open or waiting to be) * A statechange listener is present * A midimessage listener is present for input ports only * They've not been disconnected from their owner, when they are we implicitly close them MIDI access objects will be kept alive even without direct JS references to it under the following conditions: * A statechange listener is present * They've not been disconnected from their owner Finally whenever a MIDI port is in use we disable the bfcache on the document. Differential Revision: https://phabricator.services.mozilla.com/D131352
1 parent 13e2971 commit cdffe24

File tree

6 files changed

+111
-16
lines changed

6 files changed

+111
-16
lines changed

dom/midi/MIDIAccess.cpp

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,7 @@ MIDIAccess::MIDIAccess(nsPIDOMWindowInner* aWindow, bool aSysexEnabled,
6363
mHasShutdown(false) {
6464
MOZ_ASSERT(aWindow);
6565
MOZ_ASSERT(aAccessPromise);
66+
KeepAliveIfHasListenersFor(nsGkAtoms::onstatechange);
6667
}
6768

6869
MIDIAccess::~MIDIAccess() { Shutdown(); }
@@ -218,4 +219,10 @@ void MIDIAccess::RemovePortListener(MIDIAccessDestructionObserver* aObs) {
218219
mDestructionObservers.RemoveObserver(aObs);
219220
}
220221

222+
void MIDIAccess::DisconnectFromOwner() {
223+
IgnoreKeepAliveIfHasListenersFor(nsGkAtoms::onstatechange);
224+
225+
DOMEventTargetHelper::DisconnectFromOwner();
226+
}
227+
221228
} // namespace mozilla::dom

dom/midi/MIDIAccess.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,8 @@ class MIDIAccess final : public DOMEventTargetHelper,
8686
void Shutdown();
8787
IMPL_EVENT_HANDLER(statechange);
8888

89+
void DisconnectFromOwner() override;
90+
8991
private:
9092
MIDIAccess(nsPIDOMWindowInner* aWindow, bool aSysexEnabled,
9193
Promise* aAccessPromise);

dom/midi/MIDIInput.cpp

Lines changed: 39 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@
1515
namespace mozilla::dom {
1616

1717
MIDIInput::MIDIInput(nsPIDOMWindowInner* aWindow, MIDIAccess* aMIDIAccessParent)
18-
: MIDIPort(aWindow, aMIDIAccessParent) {}
18+
: MIDIPort(aWindow, aMIDIAccessParent), mKeepAlive(false) {}
1919

2020
// static
2121
MIDIInput* MIDIInput::Create(nsPIDOMWindowInner* aWindow,
@@ -49,14 +49,46 @@ void MIDIInput::Receive(const nsTArray<MIDIMessage>& aMsgs) {
4949
}
5050
}
5151

52-
EventHandlerNonNull* MIDIInput::GetOnmidimessage() {
53-
return GetEventHandler(nsGkAtoms::onmidimessage);
52+
void MIDIInput::StateChange() {
53+
if (mPort->ConnectionState() == MIDIPortConnectionState::Open ||
54+
(mPort->DeviceState() == MIDIPortDeviceState::Connected &&
55+
mPort->ConnectionState() == MIDIPortConnectionState::Pending)) {
56+
KeepAliveOnMidimessage();
57+
} else {
58+
DontKeepAliveOnMidimessage();
59+
}
60+
}
61+
62+
void MIDIInput::EventListenerAdded(nsAtom* aType) {
63+
if (aType == nsGkAtoms::onmidimessage) {
64+
// HACK: the Web MIDI spec states that we should open a port only when
65+
// setting the midimessage event handler but Chrome does it even when
66+
// adding event listeners hence this.
67+
if (mPort->ConnectionState() != MIDIPortConnectionState::Open) {
68+
mPort->SendOpen();
69+
}
70+
}
71+
72+
DOMEventTargetHelper::EventListenerAdded(aType);
73+
}
74+
75+
void MIDIInput::DisconnectFromOwner() {
76+
DontKeepAliveOnMidimessage();
77+
78+
MIDIPort::DisconnectFromOwner();
79+
}
80+
81+
void MIDIInput::KeepAliveOnMidimessage() {
82+
if (!mKeepAlive) {
83+
mKeepAlive = true;
84+
KeepAliveIfHasListenersFor(nsGkAtoms::onmidimessage);
85+
}
5486
}
5587

56-
void MIDIInput::SetOnmidimessage(EventHandlerNonNull* aCallback) {
57-
SetEventHandler(nsGkAtoms::onmidimessage, aCallback);
58-
if (mPort->ConnectionState() != MIDIPortConnectionState::Open) {
59-
mPort->SendOpen();
88+
void MIDIInput::DontKeepAliveOnMidimessage() {
89+
if (mKeepAlive) {
90+
IgnoreKeepAliveIfHasListenersFor(nsGkAtoms::onmidimessage);
91+
mKeepAlive = false;
6092
}
6193
}
6294

dom/midi/MIDIInput.h

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -30,20 +30,23 @@ class MIDIInput final : public MIDIPort {
3030
JSObject* WrapObject(JSContext* aCx,
3131
JS::Handle<JSObject*> aGivenProto) override;
3232

33-
// Since we need to be able to open the port on event handler assignment, we
34-
// can't use IMPL_EVENT_HANDLER. We have to implement the event handler
35-
// functions ourselves.
33+
IMPL_EVENT_HANDLER(midimessage);
3634

37-
// Getter for the event handler callback
38-
EventHandlerNonNull* GetOnmidimessage();
39-
// Setter for the event handler callback
40-
void SetOnmidimessage(EventHandlerNonNull* aCallback);
35+
void StateChange() override;
36+
void EventListenerAdded(nsAtom* aType) override;
37+
void DisconnectFromOwner() override;
4138

4239
private:
4340
MIDIInput(nsPIDOMWindowInner* aWindow, MIDIAccess* aMIDIAccessParent);
4441
// Takes an array of IPC MIDIMessage objects and turns them into
4542
// MIDIMessageEvents, which it then fires.
4643
void Receive(const nsTArray<MIDIMessage>& aMsgs) override;
44+
45+
void KeepAliveOnMidimessage();
46+
void DontKeepAliveOnMidimessage();
47+
48+
// If true this object will be kept alive even without direct JS references
49+
bool mKeepAlive;
4750
};
4851

4952
} // namespace mozilla::dom

dom/midi/MIDIPort.cpp

Lines changed: 44 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,8 @@
1111
#include "mozilla/dom/MIDITypes.h"
1212
#include "mozilla/ipc/PBackgroundChild.h"
1313
#include "mozilla/ipc/BackgroundChild.h"
14+
#include "mozilla/dom/Document.h"
1415
#include "mozilla/dom/Promise.h"
15-
#include "mozilla/dom/MIDITypes.h"
1616
#include "mozilla/Unused.h"
1717
#include "nsISupportsImpl.h" // for MOZ_COUNT_CTOR, MOZ_COUNT_DTOR
1818

@@ -34,9 +34,16 @@ NS_IMPL_ADDREF_INHERITED(MIDIPort, DOMEventTargetHelper)
3434
NS_IMPL_RELEASE_INHERITED(MIDIPort, DOMEventTargetHelper)
3535

3636
MIDIPort::MIDIPort(nsPIDOMWindowInner* aWindow, MIDIAccess* aMIDIAccessParent)
37-
: DOMEventTargetHelper(aWindow), mMIDIAccessParent(aMIDIAccessParent) {
37+
: DOMEventTargetHelper(aWindow),
38+
mMIDIAccessParent(aMIDIAccessParent),
39+
mKeepAlive(false) {
3840
MOZ_ASSERT(aWindow);
3941
MOZ_ASSERT(aMIDIAccessParent);
42+
43+
Document* aDoc = GetOwner()->GetExtantDoc();
44+
if (aDoc) {
45+
aDoc->DisallowBFCaching();
46+
}
4047
}
4148

4249
MIDIPort::~MIDIPort() {
@@ -156,6 +163,8 @@ void MIDIPort::Notify(const void_t& aVoid) {
156163
}
157164

158165
void MIDIPort::FireStateChangeEvent() {
166+
StateChange();
167+
159168
MOZ_ASSERT(mPort);
160169
if (mPort->ConnectionState() == MIDIPortConnectionState::Open ||
161170
mPort->ConnectionState() == MIDIPortConnectionState::Pending) {
@@ -173,10 +182,20 @@ void MIDIPort::FireStateChangeEvent() {
173182
mClosingPromise = nullptr;
174183
}
175184
}
185+
176186
if (mPort->DeviceState() == MIDIPortDeviceState::Connected &&
177187
mPort->ConnectionState() == MIDIPortConnectionState::Pending) {
178188
mPort->SendOpen();
179189
}
190+
191+
if (mPort->ConnectionState() == MIDIPortConnectionState::Open ||
192+
(mPort->DeviceState() == MIDIPortDeviceState::Connected &&
193+
mPort->ConnectionState() == MIDIPortConnectionState::Pending)) {
194+
KeepAliveOnStatechange();
195+
} else {
196+
DontKeepAliveOnStatechange();
197+
}
198+
180199
// Fire MIDIAccess events first so that the port is no longer in the port
181200
// maps.
182201
if (mMIDIAccessParent) {
@@ -190,8 +209,31 @@ void MIDIPort::FireStateChangeEvent() {
190209
DispatchTrustedEvent(event);
191210
}
192211

212+
void MIDIPort::StateChange() {}
213+
193214
void MIDIPort::Receive(const nsTArray<MIDIMessage>& aMsg) {
194215
MOZ_CRASH("We should never get here!");
195216
}
196217

218+
void MIDIPort::DisconnectFromOwner() {
219+
DontKeepAliveOnStatechange();
220+
mPort->SendClose();
221+
222+
DOMEventTargetHelper::DisconnectFromOwner();
223+
}
224+
225+
void MIDIPort::KeepAliveOnStatechange() {
226+
if (!mKeepAlive) {
227+
mKeepAlive = true;
228+
KeepAliveIfHasListenersFor(nsGkAtoms::onstatechange);
229+
}
230+
}
231+
232+
void MIDIPort::DontKeepAliveOnStatechange() {
233+
if (mKeepAlive) {
234+
IgnoreKeepAliveIfHasListenersFor(nsGkAtoms::onstatechange);
235+
mKeepAlive = false;
236+
}
237+
}
238+
197239
} // namespace mozilla::dom

dom/midi/MIDIPort.h

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,18 +63,25 @@ class MIDIPort : public DOMEventTargetHelper,
6363

6464
void FireStateChangeEvent();
6565

66+
virtual void StateChange();
6667
virtual void Receive(const nsTArray<MIDIMessage>& aMsg);
6768

6869
// This object holds a pointer to its corresponding IPC MIDIPortChild actor.
6970
// If the IPC actor is deleted, it cleans itself up via this method.
7071
void UnsetIPCPort();
7172

7273
IMPL_EVENT_HANDLER(statechange)
74+
75+
void DisconnectFromOwner() override;
76+
7377
protected:
7478
// IPC Actor corresponding to this class
7579
RefPtr<MIDIPortChild> mPort;
7680

7781
private:
82+
void KeepAliveOnStatechange();
83+
void DontKeepAliveOnStatechange();
84+
7885
// MIDIAccess object that created this MIDIPort object, which we need for
7986
// firing port connection events. There is a chance this MIDIPort object can
8087
// outlive its parent MIDIAccess object, so this is a weak reference that must
@@ -88,6 +95,8 @@ class MIDIPort : public DOMEventTargetHelper,
8895
// Promise object generated on Close() call, that needs to be resolved once
8996
// the platform specific Close() function has completed.
9097
RefPtr<Promise> mClosingPromise;
98+
// If true this object will be kept alive even without direct JS references
99+
bool mKeepAlive;
91100
};
92101

93102
} // namespace mozilla::dom

0 commit comments

Comments
 (0)