[webaudio-testharness] Migrate cors-check.https.html [chromium/src : main]
0 views
Skip to first unread message
Hongchan Choi (Gerrit)
Oct 8, 2025, 6:06:59 PM (2 days ago) Oct 8
to Saqlain, Chromium LUCI CQ, chromium...@chromium.org, blink-revie...@chromium.org, blink-...@chromium.org, feature-me...@chromium.org
Attention needed from Michael Wilson and Saqlain
Hongchan Choi added 3 comments![]( "Open in Gerrit")
Patchset-level comments
File-level comment, Patchset 4 (Latest):
Hongchan Choi . unresolved
This CL is very similar to https://crrev.com/c/7021577. Restructuring the overall Promise flow will greatly improve the readability of the test.
File third_party/blink/web_tests/external/wpt/webaudio/the-audio-api/the-mediaelementaudiosourcenode-interface/cors-check.https.html
Line 42, Patchset 4 (Latest):
await new Promise((resolve, reject) => {Hongchan Choi . unresolved
Similar to the other CL we discussed:
```suggestion
const recordingPromise = await new Promise((resolve, reject) => {
```
Line 66, Patchset 4 (Latest):
context.resume().then(() => {
// Start playback after resume
const playPromise = audioElement.play();Hongchan Choi . unresolved
```suggestion
await context.resume();
await audioElement.play();
const recordBuffer = await recordingPromise;
// Then do the verification here (L48~L56)
```
Related details
Attention is currently required from:
- Michael Wilson
- Saqlain
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Michael Wilson (Gerrit)
Oct 8, 2025, 7:29:18 PM (2 days ago) Oct 8
to Saqlain, Chromium LUCI CQ, Hongchan Choi, chromium...@chromium.org, blink-revie...@chromium.org, blink-...@chromium.org, feature-me...@chromium.org
Attention needed from Saqlain
Michael Wilson added 1 comment![]( "Open in Gerrit")
Patchset-level comments
Michael Wilson . resolved
Similar to the other CL, I don't have anything to add to Hongchan's comments on the Promise flow restructuring.
Related details
Attention is currently required from:
- Saqlain
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Saqlain (Gerrit)
Oct 9, 2025, 5:03:41 PM (2 days ago) Oct 9
to Chromium LUCI CQ, Hongchan Choi, chromium...@chromium.org, blink-revie...@chromium.org, blink-...@chromium.org, feature-me...@chromium.org
Attention needed from Hongchan Choi
Saqlain added 4 comments![]( "Open in Gerrit")
Patchset-level comments
Saqlain . resolved
PTAL
Hongchan Choi . resolved
This CL is very similar to https://crrev.com/c/7021577. Restructuring the overall Promise flow will greatly improve the readability of the test.
Saqlain
Acknowledged
File third_party/blink/web_tests/external/wpt/webaudio/the-audio-api/the-mediaelementaudiosourcenode-interface/cors-check.https.html
Line 42, Patchset 4 (Latest):
await new Promise((resolve, reject) => {Hongchan Choi . resolved
Similar to the other CL we discussed:
```suggestion
const recordingPromise = await new Promise((resolve, reject) => {
```
Saqlain
Done
Line 66, Patchset 4 (Latest):
context.resume().then(() => {
// Start playback after resume
const playPromise = audioElement.play();Hongchan Choi . resolved
```suggestion
await context.resume();
await audioElement.play();
const recordBuffer = await recordingPromise;
// Then do the verification here (L48~L56)
```
Attention is currently required from:
- Hongchan Choi
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Michael Wilson (Gerrit)
Oct 10, 2025, 6:32:44 PM (7 hours ago) Oct 10
to Saqlain, Chromium LUCI CQ, Hongchan Choi, chromium...@chromium.org, blink-revie...@chromium.org, blink-...@chromium.org, feature-me...@chromium.org
Attention needed from Hongchan Choi and Saqlain
Michael Wilson voted and added 1 comment![]( "Open in Gerrit")
Votes added by Michael Wilson
| Code-Review | +1 |
1 comment
Patchset-level comments
Related details
Attention is currently required from:
- Hongchan Choi
- Saqlain
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Hongchan Choi (Gerrit)
Oct 10, 2025, 6:40:05 PM (7 hours ago) Oct 10
to Saqlain, Chromium LUCI CQ, chromium...@chromium.org, blink-revie...@chromium.org, blink-...@chromium.org, feature-me...@chromium.org
Attention needed from Saqlain
Hongchan Choi added 5 comments![]( "Open in Gerrit")
Patchset-level comments
Hongchan Choi . resolved
LGTM with nits
File third_party/blink/web_tests/external/wpt/webaudio/the-audio-api/the-mediaelementaudiosourcenode-interface/cors-check.https.html
Line 38, Patchset 5 (Latest):
context, `recorder-processor`, {channelCount: 4});Hongchan Choi . unresolved
Similar to the other CL, use the template literal only when substitution is necessary.
Line 44, Patchset 5 (Latest):
if (event.data && event.data.type === `recordfinished`) {Hongchan Choi . unresolved
Ditto.
Line 51, Patchset 5 (Latest):
assert_true(
channelData.some((v) => v !== 0),
`Recorded channel #${i}`);
}Hongchan Choi . unresolved
I think we have implemented `notBeConstantValueOf` in audit-util.js. Can we use that instead?
Line 66, Patchset 5 (Latest):
}, `cross-origin redirect with CORS`);Hongchan Choi . unresolved
Ditto.
Related details
Attention is currently required from:
- Saqlain
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Reply all
Reply to author
Forward
0 new messages