Skip to content

Commit de24ac6

Browse files
ssz2605ssz2605
authored
fix: guard Transport scheduling against stopped clock and past-time edge cases (#7776)
1 parent 9a87be1 commit de24ac6

5 files changed

Lines changed: 225 additions & 65 deletions

File tree

‎js/__tests__/logo.test.js‎

Lines changed: 169 additions & 61 deletions
Original file line numberDiff line numberDiff line change
@@ -37,47 +37,62 @@ global.Synth = jest.fn().mockImplementation(() => ({
3737
disposeAllInstruments: jest.fn(),
3838
changeInTemperament: false,
3939
recorder: null,
40-
transport: {
41-
get isAvailable() {
42-
return typeof global.Tone !== "undefined" && !!global.Tone.Transport;
43-
},
44-
start() {
45-
if (this.isAvailable) global.Tone.Transport.start();
46-
},
47-
stop() {
48-
if (this.isAvailable) global.Tone.Transport.stop();
49-
},
50-
cancel() {
51-
if (this.isAvailable && typeof global.Tone.Transport.cancel === "function") {
52-
global.Tone.Transport.cancel();
53-
}
54-
},
55-
clear(id) {
56-
if (this.isAvailable && typeof global.Tone.Transport.clear === "function") {
57-
global.Tone.Transport.clear(id);
58-
}
59-
},
60-
schedule(callback, time) {
61-
if (this.isAvailable && typeof global.Tone.Transport.schedule === "function") {
62-
return global.Tone.Transport.schedule(callback, time);
63-
}
64-
return null;
65-
},
66-
get seconds() {
67-
if (this.isAvailable) return global.Tone.Transport.seconds;
68-
return 0;
69-
},
70-
set seconds(v) {
71-
if (this.isAvailable) global.Tone.Transport.seconds = v;
72-
},
73-
getSecondsAtTime(time) {
74-
if (this.isAvailable && typeof global.Tone.Transport.getSecondsAtTime === "function") {
75-
return global.Tone.Transport.getSecondsAtTime(time);
76-
}
77-
return this.seconds;
40+
transport: createTransportMock()
41+
}));
42+
43+
/**
44+
* Returns a transport mock that mirrors the wrapper in synthutils.js.
45+
* Centralised here so both the Synth mock and test-local overrides stay in sync.
46+
*/
47+
const createTransportMock = () => ({
48+
get isAvailable() {
49+
return typeof global.Tone !== "undefined" && !!global.Tone.Transport;
50+
},
51+
get isClockRunning() {
52+
return (
53+
this.isAvailable &&
54+
typeof global.Tone.context !== "undefined" &&
55+
global.Tone.context.state === "running" &&
56+
global.Tone.Transport.state === "started"
57+
);
58+
},
59+
start() {
60+
if (this.isAvailable) global.Tone.Transport.start();
61+
},
62+
stop() {
63+
if (this.isAvailable) global.Tone.Transport.stop();
64+
},
65+
cancel() {
66+
if (this.isAvailable && typeof global.Tone.Transport.cancel === "function") {
67+
global.Tone.Transport.cancel();
68+
}
69+
},
70+
clear(id) {
71+
if (this.isAvailable && typeof global.Tone.Transport.clear === "function") {
72+
global.Tone.Transport.clear(id);
73+
}
74+
},
75+
schedule(callback, time) {
76+
if (this.isAvailable && typeof global.Tone.Transport.schedule === "function") {
77+
return global.Tone.Transport.schedule(callback, time);
7878
}
79+
return null;
80+
},
81+
get seconds() {
82+
if (this.isAvailable) return global.Tone.Transport.seconds;
83+
return 0;
84+
},
85+
set seconds(v) {
86+
if (this.isAvailable) global.Tone.Transport.seconds = v;
87+
},
88+
getSecondsAtTime(time) {
89+
if (this.isAvailable && typeof global.Tone.Transport.getSecondsAtTime === "function") {
90+
return global.Tone.Transport.getSecondsAtTime(time);
91+
}
92+
return this.seconds;
7993
}
80-
}));
94+
});
95+
8196
global.Singer = {
8297
setSynthVolume: jest.fn(),
8398
setMasterVolume: jest.fn(),
@@ -997,26 +1012,7 @@ describe("Logo doStopTurtles", () => {
9971012
stopSound: jest.fn(),
9981013
disposeAllInstruments: jest.fn(),
9991014
recorder: { state: "recording", stop: jest.fn() },
1000-
transport: {
1001-
get isAvailable() {
1002-
return typeof global.Tone !== "undefined" && !!global.Tone.Transport;
1003-
},
1004-
cancel() {
1005-
if (
1006-
this.isAvailable &&
1007-
typeof global.Tone.Transport.cancel === "function"
1008-
) {
1009-
global.Tone.Transport.cancel();
1010-
}
1011-
},
1012-
get seconds() {
1013-
if (this.isAvailable) return global.Tone.Transport.seconds;
1014-
return 0;
1015-
},
1016-
set seconds(v) {
1017-
if (this.isAvailable) global.Tone.Transport.seconds = v;
1018-
}
1019-
}
1015+
transport: createTransportMock()
10201016
};
10211017
});
10221018

@@ -1275,7 +1271,7 @@ describe("Logo runFromBlock", () => {
12751271
});
12761272

12771273
describe("Transport scheduling", () => {
1278-
test("uses Transport.schedule when available", () => {
1274+
test("uses Transport.schedule when available and clock is running", () => {
12791275
const getSecondsAtTimeMock = jest.fn(t => t + 1);
12801276
let scheduledCallback = null;
12811277
const scheduleSpy = jest.fn((fn, time) => {
@@ -1284,6 +1280,7 @@ describe("Logo runFromBlock", () => {
12841280
});
12851281
global.Tone = {
12861282
...savedTone,
1283+
context: { state: "running" },
12871284
Transport: {
12881285
start: jest.fn(),
12891286
stop: jest.fn(),
@@ -1293,7 +1290,10 @@ describe("Logo runFromBlock", () => {
12931290
get seconds() {
12941291
return 0;
12951292
},
1296-
set seconds(v) {}
1293+
set seconds(v) {},
1294+
get state() {
1295+
return "started";
1296+
}
12971297
}
12981298
};
12991299

@@ -1330,6 +1330,114 @@ describe("Logo runFromBlock", () => {
13301330
expect(timeoutSpy).toHaveBeenCalled();
13311331
expect(logo.runFromBlockNow).toHaveBeenCalledWith(logo, 0, 3, 1, "x");
13321332
});
1333+
1334+
test("falls back to setTimeout when AudioContext is suspended", () => {
1335+
global.Tone = {
1336+
...savedTone,
1337+
context: { state: "suspended" },
1338+
Transport: {
1339+
start: jest.fn(),
1340+
stop: jest.fn(),
1341+
schedule: jest.fn(),
1342+
cancel: jest.fn(),
1343+
getSecondsAtTime: jest.fn(() => 0),
1344+
get seconds() {
1345+
return 5;
1346+
},
1347+
set seconds(v) {},
1348+
get state() {
1349+
return "started";
1350+
}
1351+
}
1352+
};
1353+
timeoutSpy = jest.spyOn(global, "setTimeout").mockImplementation(fn => {
1354+
fn();
1355+
return 5;
1356+
});
1357+
1358+
logo.runFromBlockNow = jest.fn();
1359+
logo.turtleDelay = 0;
1360+
logo.stopTurtle = false;
1361+
turtle0.waitTime = 200;
1362+
turtle0._transportTime = 10;
1363+
1364+
logo.runFromBlock(logo, 0, 3, 1, "x");
1365+
1366+
expect(global.Tone.Transport.schedule).not.toHaveBeenCalled();
1367+
expect(timeoutSpy).toHaveBeenCalled();
1368+
expect(logo.runFromBlockNow).toHaveBeenCalledWith(logo, 0, 3, 1, "x");
1369+
});
1370+
1371+
test("falls back to setTimeout when Transport clock is stopped", () => {
1372+
global.Tone = {
1373+
...savedTone,
1374+
context: { state: "running" },
1375+
Transport: {
1376+
start: jest.fn(),
1377+
stop: jest.fn(),
1378+
schedule: jest.fn(),
1379+
cancel: jest.fn(),
1380+
getSecondsAtTime: jest.fn(() => 0),
1381+
get seconds() {
1382+
return 3;
1383+
},
1384+
set seconds(v) {},
1385+
get state() {
1386+
return "stopped";
1387+
}
1388+
}
1389+
};
1390+
timeoutSpy = jest.spyOn(global, "setTimeout").mockImplementation(fn => {
1391+
fn();
1392+
return 5;
1393+
});
1394+
1395+
logo.runFromBlockNow = jest.fn();
1396+
logo.turtleDelay = 0;
1397+
logo.stopTurtle = false;
1398+
turtle0.waitTime = 200;
1399+
turtle0._transportTime = 10;
1400+
1401+
logo.runFromBlock(logo, 0, 3, 1, "x");
1402+
1403+
expect(global.Tone.Transport.schedule).not.toHaveBeenCalled();
1404+
expect(timeoutSpy).toHaveBeenCalled();
1405+
expect(logo.runFromBlockNow).toHaveBeenCalledWith(logo, 0, 3, 1, "x");
1406+
});
1407+
1408+
test("clamps transportTime to prevent scheduling in the past", () => {
1409+
const scheduleSpy = jest.fn((fn, time) => "evt-clamp");
1410+
global.Tone = {
1411+
...savedTone,
1412+
context: { state: "running" },
1413+
Transport: {
1414+
start: jest.fn(),
1415+
stop: jest.fn(),
1416+
schedule: scheduleSpy,
1417+
cancel: jest.fn(),
1418+
getSecondsAtTime: jest.fn(t => t),
1419+
get seconds() {
1420+
return 20;
1421+
},
1422+
set seconds(v) {},
1423+
get state() {
1424+
return "started";
1425+
}
1426+
}
1427+
};
1428+
1429+
logo.runFromBlockNow = jest.fn();
1430+
logo.turtleDelay = 0;
1431+
logo.stopTurtle = false;
1432+
turtle0.waitTime = 100;
1433+
turtle0._transportTime = 10;
1434+
1435+
logo.runFromBlock(logo, 0, 3, 1, "x");
1436+
1437+
const computedTime = 10 + 100 / 1000;
1438+
expect(computedTime).toBeLessThan(20);
1439+
expect(scheduleSpy).toHaveBeenCalledWith(expect.any(Function), 20);
1440+
});
13331441
});
13341442
});
13351443

‎js/logo.js‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1695,9 +1695,13 @@ class Logo {
16951695
logo.turtleDelay === 0 &&
16961696
delay > 0 &&
16971697
logo.synth.transport.isAvailable &&
1698+
logo.synth.transport.isClockRunning &&
16981699
tur._transportTime !== null
16991700
) {
1700-
const transportTime = tur._transportTime + delay / 1000;
1701+
const transportTime = Math.max(
1702+
tur._transportTime + delay / 1000,
1703+
logo.synth.transport.seconds
1704+
);
17011705
tur.delayParameters = { blk: blk, flow: isflow, arg: receivedArg };
17021706
tur._transportEventId = logo.synth.transport.schedule(audioContextTime => {
17031707
const tur2 = logo.activity.turtles.ithTurtle(turtle);

‎js/utils/__tests__/synthutils.test.js‎

Lines changed: 30 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,8 @@ describe("Utility Functions (logic-only)", () => {
6161
newTone,
6262
preloadProjectSamples,
6363
resolveInstrumentName,
64-
Synth;
64+
Synth,
65+
transport;
6566

6667
const turtle = "turtle1";
6768

@@ -193,6 +194,7 @@ describe("Utility Functions (logic-only)", () => {
193194
newTone = Synth.newTone;
194195
preloadProjectSamples = Synth.preloadProjectSamples;
195196
resolveInstrumentName = Synth.resolveInstrumentName;
197+
transport = Synth.transport;
196198
});
197199

198200
describe("setupRecorder", () => {
@@ -896,6 +898,11 @@ describe("Utility Functions (logic-only)", () => {
896898
});
897899

898900
describe("Tone Transport Controls", () => {
901+
afterEach(() => {
902+
Tone.context.state = "running";
903+
Tone.Transport.state = "started";
904+
});
905+
899906
test("start should call Tone.Transport.start", () => {
900907
const startSpy = jest.spyOn(Tone.Transport, "start");
901908

@@ -931,6 +938,28 @@ describe("Utility Functions (logic-only)", () => {
931938
startSpy.mockRestore();
932939
stopSpy.mockRestore();
933940
});
941+
942+
test("isAvailable returns truthy when Tone.Transport exists", () => {
943+
expect(transport.isAvailable).toBeTruthy();
944+
});
945+
946+
test("isClockRunning returns true when context is running and transport is started", () => {
947+
Tone.context.state = "running";
948+
Tone.Transport.state = "started";
949+
expect(transport.isClockRunning).toBe(true);
950+
});
951+
952+
test("isClockRunning returns false when context is suspended", () => {
953+
Tone.context.state = "suspended";
954+
Tone.Transport.state = "started";
955+
expect(transport.isClockRunning).toBe(false);
956+
});
957+
958+
test("isClockRunning returns false when transport is stopped", () => {
959+
Tone.context.state = "running";
960+
Tone.Transport.state = "stopped";
961+
expect(transport.isClockRunning).toBe(false);
962+
});
934963
});
935964

936965
describe("_createSampleSynth", () => {

‎js/utils/__tests__/tonemock.js‎

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -144,10 +144,12 @@ class PolySynth {
144144
}
145145

146146
class context {
147+
static state = "running";
147148
static resume() {}
148149
}
149150

150151
class Transport {
152+
static _state = "started";
151153
static start() {}
152154
static stop() {}
153155
static schedule() {}
@@ -156,10 +158,19 @@ class Transport {
156158
static getSecondsAtTime() {
157159
return 0;
158160
}
161+
static _seconds = 0;
159162
static get seconds() {
160-
return 0;
163+
return Transport._seconds;
164+
}
165+
static set seconds(value) {
166+
Transport._seconds = value;
167+
}
168+
static get state() {
169+
return Transport._state;
170+
}
171+
static set state(v) {
172+
Transport._state = v;
161173
}
162-
static set seconds(value) {}
163174
}
164175

165176
class ToneAudioBuffer {

‎js/utils/synthutils.js‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -472,6 +472,14 @@ const transport = {
472472
get isAvailable() {
473473
return typeof Tone !== "undefined" && Tone.Transport;
474474
},
475+
get isClockRunning() {
476+
return (
477+
this.isAvailable &&
478+
typeof Tone.context !== "undefined" &&
479+
Tone.context.state === "running" &&
480+
Tone.Transport.state === "started"
481+
);
482+
},
475483
start() {
476484
if (this.isAvailable) Tone.Transport.start();
477485
},

0 commit comments

Comments
 (0)