[presentation-api] Update tests for onconnect, onclose and onterminate #5794
Conversation
Firefox (nightly channel)Testing web-platform-tests at revision a211c01 All results4 tests ran/presentation-api/controlling-ua/PresentationRequest_sandboxing_error.https.html
/presentation-api/controlling-ua/PresentationRequest_sandboxing_success.https.html
/presentation-api/controlling-ua/getAvailability_sandboxing_success.https.html
/presentation-api/controlling-ua/reconnectToPresentation_sandboxing_success.https.html
|
Chrome (unstable channel)Testing web-platform-tests at revision a211c01 All results4 tests ran/presentation-api/controlling-ua/PresentationRequest_sandboxing_error.https.html
/presentation-api/controlling-ua/PresentationRequest_sandboxing_success.https.html
/presentation-api/controlling-ua/getAvailability_sandboxing_success.https.html
/presentation-api/controlling-ua/reconnectToPresentation_sandboxing_success.https.html
|
| connection = c; | ||
| return stash.init(); | ||
| }).then(() => { | ||
| // Step 1: close the presentation connection in "connecting" state |
There was a problem hiding this comment.
Hmm, what guarantees that the presentation connection is still in the "connecting" state here? The previous call to stash.init() can take an arbitrary amount of time. You should probably run that call before calling request.start().
There was a problem hiding this comment.
Oops, that's right. I'll move stash.init() somewhere else.
By the way, while I'm not sure whether it is correct or not, Chrome Canary throws an exception like "InvalidAccessError: PresentationRequest::start() requires user gesture." when request.start() is called within stash.init().then(). This is the reason why I avoided calling stash.init() before request.start().
There was a problem hiding this comment.
I don't know. I believe the behavior is somewhat up to implementations in practice, see end of third bullet point in definition of allowed to show a popup.
It's interesting that Promise.all seems to solve the problem.
| const terminateWatcher = new EventWatcher(t, connection, 'terminate'); | ||
| connection.terminate(); | ||
| return Promise.race([ | ||
| new Promise(resolve => { t.step_timeout(resolve, 1000); }), |
There was a problem hiding this comment.
OK, that's not ideal, but I cannot think of a better way to check the absence of effects.
| eventWatcher = new EventWatcher(t, connection, ['close', 'connect', 'terminate']); | ||
|
|
||
| connection.close(); | ||
| return Promise.all([ |
There was a problem hiding this comment.
You want Promise.race here, the timeout will always occur otherwise!
- `stash.init()` is called before `request.start()` - inappropriate use of `Promise.all` is changed to `Promise.race'
|
I have fixed a couple of bugs in the "onclose" test. Also, I have updated the three tests so that they check both |
This PR upgrades #4330 and fixes #4040 as well as several additional issues.
async_testandstep_funcare replaced withpromise_testandEventWatcher..httpsis added to the file name.oncloseandonterminatetests check behavior in each connection state:connecting,connected,closedandterminated.onterminatetest also checks behavior of another presentation connection in a nested browsing context.onterminatetest now supposes that callingconnection.terminate()must set state of presentation connections inconnectingandconnectedstate toterminatedas defined in the current spec: Allow termination via connections in a connecting state. w3c/presentation-api#427