Hi Team, I was going through Blink Commits and noted that it is something we can also merge: Blink Commit - https://chromium.googlesource.com/chromium/blink/+/b06aa901cd3e0e1d1d6587d6b7c9739679c5feed Webkit GitHub - https://github.com/WebKit/WebKit/blob/829dab614cc143c1b8c69f6b2535f44f254ad932/Source/WebCore/Modules/webaudio/ConvolverNode.cpp#L120 We also don't have return after exception. Just wanted to raise this bug since I couldn't find the related bug. Thanks!
Ahmad, it returns an exception currently https://github.com/WebKit/WebKit/blob/06f9978e52a8408ed6c2c8296afb1e7449c2f1ee/Source/WebCore/Modules/webaudio/ConvolverNode.cpp#L120-L121 You can see it was modified here https://github.com/WebKit/WebKit/commit/935f2c7e99aa9f6e03ca7e9449eed91235ceb865#diff-e1c0c01c871ffb6653371a1717b09d4f45b6153e6f1e783c8777a9e4a70b7a1dL127-L130 And then before it was https://github.com/WebKit/WebKit/blame/c996d8b5e859f3dc0090d669eeed82a2370a2aa7/Source/WebCore/Modules/webaudio/ConvolverNode.cpp#L127-L130 with the return. my interpretation: the chromium code was just throwing the exception before instead of returning it. But I'm not an expert. What do you think?
(In reply to Karl Dubost from comment #1) > Ahmad, > > it returns an exception currently > https://github.com/WebKit/WebKit/blob/ > 06f9978e52a8408ed6c2c8296afb1e7449c2f1ee/Source/WebCore/Modules/webaudio/ > ConvolverNode.cpp#L120-L121 > > You can see it was modified here > https://github.com/WebKit/WebKit/commit/ > 935f2c7e99aa9f6e03ca7e9449eed91235ceb865#diff- > e1c0c01c871ffb6653371a1717b09d4f45b6153e6f1e783c8777a9e4a70b7a1dL127-L130 > > > And then before it was > https://github.com/WebKit/WebKit/blame/ > c996d8b5e859f3dc0090d669eeed82a2370a2aa7/Source/WebCore/Modules/webaudio/ > ConvolverNode.cpp#L127-L130 > > with the return. > > > my interpretation: the chromium code was just throwing the exception before > instead of returning it. But I'm not an expert. > > What do you think? If I understand Blink commit then they are adding return because they continued using wrong buffer rate even after throwing exception, while in Webkit code, if I look into from your links, it seems that we use to return earlier because didn't supported the feature by throwing "Not Supported" kind of error message and later when we added supported, we just had the "Exception" message now but we are not returning and continuing with wrong buffer rate but I can be wrong as well but this is my interpretation. Do you anyone who might be familiar with this code to say whether we need to do anything like early return for this or do nothing because Webkit can handle wrong buffer rate smartly as it is.
Maybe Darin knows.
If you look at the test that Blink added in LayoutTests/webaudio/dom-exceptions.html, you'll see that we also have it: ``` audit.define('convolver', (task, should) => { // Convolver buffer rate must match context rate. Create on offline // context so we specify the context rate exactly, in case the test is // run on platforms with different HW sample rates. let oc; should( () => oc = new OfflineAudioContext(1, 44100, 44100), 'oc = new OfflineAudioContext(1, 44100, 44100)') .notThrow(); should(() => conv = oc.createConvolver(), 'conv = oc.createConvolver()') .notThrow(); should(() => conv.buffer = {}, 'conv.buffer = {}').throw(); should( () => conv.buffer = oc.createBuffer(1, 100, 22050), 'conv.buffer = oc.createBuffer(1, 100, 22050)') .throw(); // conv.buffer should be unchanged (null) because the above failed. should(conv.buffer, 'conv.buffer').beEqualTo(null); // THIS CHECK HERE task.done(); }); ``` and that we are passing it: ``` PASS > [convolver] PASS oc = new OfflineAudioContext(1, 44100, 44100) did not throw an exception. PASS conv = oc.createConvolver() did not throw an exception. PASS conv.buffer = {} threw TypeError: "The ConvolverNode.buffer attribute must be an instance of AudioBuffer". PASS conv.buffer = oc.createBuffer(1, 100, 22050) threw NotSupportedError: "Buffer sample rate does not match the context's sample rate". PASS conv.buffer is equal to null. PASS < [convolver] All assertions passed. (total 5 assertions) ```