850294 - Create some mochitests for verifying that onXXX handlers fire at the correct time with a local and remote PC

Status: NEW

(Core :: WebRTC, defect, P4)

Description

[Jason Smith [:jsmith]]

Create some mochitests that verify that onXXX handlers fire at the correct time with a local and remote peer connection. Such onXXX handlers that should be analyzed include:

- onicecandidate
- onstatechange
- ongatheringchange
- onicechange
- onclosedconnection
- onconnection
- onopen

Note - we do need to analyze two things with these event handlers - what's feasible to automate right now and what makes sense with this list. I already cut some event handlers that we're either already known to be not supported soon temporarily or are not implemented right now.

Comment 1

[Jason Smith [:jsmith]]

Important because we need to make sure events fire at the *right* time within the large state machine of a peer connection object. Into the ? pool for blocking analysis.

Comment 2

[Henrik Skupin [:whimboo][⌚️UTC+1]]

(In reply to Jason Smith [:jsmith] from comment #0)
> - onclosedconnection
> - onconnection

Given by the IDL those are data connection only events, right? Those will be covered by the tests I'm writing for at the moment.

Comment 3

[Jason Smith [:jsmith]]

(In reply to Henrik Skupin (:whimboo) from comment #2)
> (In reply to Jason Smith [:jsmith] from comment #0)
> > - onclosedconnection
> > - onconnection
> 
> Given by the IDL those are data connection only events, right? Those will be
> covered by the tests I'm writing for at the moment.

Ah. Well, whatever we don't cover should probably encompassed by the mochitests written on this bug. I filed this bug as basically as a "let's get enough tests that at least touch each onXXX handler on a PC." In other words, fill the API coverage gap for whatever it is.

Comment 4

[Nils Ohlmeier [:drno]]

onicechange is covered since checkin fc7cd53d9dd1.

Comment 5

[Nils Ohlmeier [:drno]]

I think the list of events in this ticket is a little outdated.
These are the events defined in RTCPeerConnection.webidl as of today:

* implemented && covered by tests
  - onsignalingstatechange
  - onaddstream
  - oniceconnectionstatechange
  - ondatachannel
* implemented && not covered by tests
  - onicecandidate
  - onconnection
  - onclosedconnection
* stub implementation only && not covered by tests
  - onnegotiationneeded
  - onremovestream
  - onidentityresult
  - onpeeridentity

So it looks like we are missing tests for the second category.

Comment 6

[Nils Ohlmeier [:drno]]

Status update:

* implemented && covered by tests
  - onsignalingstatechange
  - onaddstream
  - oniceconnectionstatechange
  - ondatachannel
* implemented && not covered by tests
  - onicecandidate (will get implemented as part of Bug 850268 - not in 32)
* stub implementation only && not covered by tests
  - onnegotiationneeded
  - onremovestream
  - onidentityresult
  - onpeeridentity
* exist in our webIDL, but on in the WebRTC standard (Bug 1014304)
  - onconnection
  - onclosedconnection

Comment 7

[Maire Reavy [:mreavy]]

Much of the work for this is done.  The remaining work will be completed in Fx 33.

Comment 8

[Bulk Bug Changes for mreavy's org]

Mass change P3->P4 to align with new Mozilla triage process.

Comment 9

[Nils Ohlmeier [:drno]]

I don't have the capacity any more to work on this any time soon.