- From: Soares Chen via GitHub <sysbot+gh@w3.org>
- Date: Fri, 23 Jun 2017 09:15:02 +0000
- To: public-webrtc-logs@w3.org
@jan-ivar Thank you for your explanation. Now I understand much better on why the enqueue operation steps are described the way they are. The way I see it is that the main difference the patch in #1397 make is that the synchronous part of the operation also execute asynchronously if the operation queue length is 1. Currently in the spec, there are only two places where operations have synchronous part: - `createAnswer` step 4.1 - `addIceCandidate` step 4 Both of the synchronous parts only concern with returning rejected promise synchronously in case that operation queue length is 1. As a result, the only observable side effect for the special case of length 1 is: 1. When `close()` is called in next synchronous step, promise may or may not resolve depending on operation queue length. 2. When immediately printing returned promise to console, the promise is shown as "rejected" or "pending" depending on operation queue length. I find the second use case of console debugging to be weird. In Chrome inspector one can click an arrow cursor to expand objects including promises, and the expanded attributes show the latest state of the promise, which in this case should be "rejected" even if the initial state is "pending". This seems more like a compensation for other browsers like Firefox that do not have the interactive feature to quickly inspect latest attributes of objects. But Chrome and Firefox also show rejected promises returned from failed cases like `createAnswer()` as pending promise, so I am more confused of which browser that debugging technique is being used. Note that the use case is also limited to implementation-specific debugging because it is impossible to query the current state of promises in JavaScript, which also make this untestable. Let me try to understand your points with some pseudo-tests here. Based on current behavior, enqueue would execute operation synchronously if operation queue length is 1. So: ```javascript // This should fulfill with answer pc.setRemoteDescription(offer); pc.createAnswer().then(() => console.log('pass')) // This should reject // Note that this test fail in current browsers const promise = pc.createAnswer(); pc.close(); promise.catch(() => console.log('pass')); // This should never resolve pc.setRemoteDescription(offer); // increase queue length const promise = pc.createAnswer(); pc.close(); promise.catch(() => console.log('fail')); // This should also never resolve pc.addIceCandidate(...); // rejects const promise = pc.createAnswer(); pc.close(); promise.catch(() => console.log('fail')); // This should show rejected promise in console console.log('rejected promise:', pc.createAnswer()); // This should show pending promise in console // that later have an updated rejected state pc.createAnswer(); // rejects console.log('pending promise:', pc.createAnswer()); // This should not have added track in offer // because createOffer only have async part of operation const promise = pc.createOffer(); pc.addTransceiver('audio'); promise.then(offer => console.log('offer should not have track:', offer); // This should also not have added track in offer pc.setLocalDescription(oldOffer); // increase queue length const promise = pc.createOffer(); pc.addTransceiver('audio'); promise.then(offer => console.log('offer should not have track:', offer); ``` I think we can solve this with a test driven approach. I am away next week. After I'm back I plan to write test cases that match the intended behavior in the spec. -- GitHub Notification of comment by soareschen Please view or discuss this issue at https://github.com/w3c/webrtc-pc/issues/1218#issuecomment-310615500 using your GitHub account
Received on Friday, 23 June 2017 09:15:08 UTC