webkitGetUserMedia is implemented as a JS built-in. As such it should not use public promise APIs which may be controlled by user scripts. @then should be used instead
Created attachment 268209 [details] Patch
Comment on attachment 268209 [details] Patch Very nice! r=me.
Comment on attachment 268209 [details] Patch Clearing flags on attachment: 268209 Committed r194554: <http://trac.webkit.org/changeset/194554>
All reviewed patches have been landed. Closing bug.
Good to see this fixed :) FWIW, there are similar bugs in Source/WebCore/Modules//mediastream/RTCPeerConnection.js and RTCPeerConnectionInternals.js. I do not know how testable WebRTC is currently though. On another point, I think the added test could be improved a bit. It is generally good to have a expected.txt containing a simple message like "PASS" or "FAIL". Libraries such as testharness.js or js-test-pre.js/js-test-post.js may help on that. Using console.log(...) may not be js-test best friend. This may trigger more often rebasing of the expected.txt file whenever doing edits on the test. Related specifically to that test, setting "{ audio: false, video: false }" leads to automatic rejection of the promise without any user interaction. The test could be improved as follow: - Add a testResult variable, equal to "PASS" by default - testResult is set to "FAIL" in successCallback and new Promise.prototype.then - In errorCallback (and successCallback), output the value of testResult - Call webkitGetUserMedia with "{ audio: false, video: false }" Since callbacks are asynchronously called, the test needs to be turned into an asynchronous one and ended when either errorCallback or successCallback is called. Note that it is important to handle properly successCallback to ensure the test does not timeout in error case.
Comment on attachment 268209 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=268209&action=review > LayoutTests/ChangeLog:9 > + * streams/webkitGetUserMedia-shadowing-then.html: Added. I forgot this one: the location of the test is probably not appropriate. Could you move it from LayoutTests/streams to LayoutTests/fast/mediastream?