1414176 - Fix failure WebRTC tests relying on non-comformant Promise handling

Status: RESOLVED FIXED

(Core :: WebRTC, defect, P2)

Description

[Bevis Tseng[:bevis][:btseng](Exited)]

This is a follow-up of the try result in bug 1193394 comment 48.
mda2/mda3 in linux64-debug-build: 
https://treeherder.mozilla.org/logviewer.html#?job_id=141558413&repo=try&lineNumber=12614
https://treeherder.mozilla.org/logviewer.html#?job_id=141558545&repo=try&lineNumber=2529

TEST-UNEXPECTED-FAIL | dom/media/tests/mochitest/test_dataChannel_basicAudio.html | [object DataChannel] is in state: 'open' - got "connecting", expected "open"
TEST-UNEXPECTED-FAIL | dom/media/tests/mochitest/test_dataChannel_basicAudio.html | [object DataChannel] is in state: 'open' - got "connecting", expected "open"
TEST-UNEXPECTED-FAIL | dom/media/tests/mochitest/test_dataChannel_basicAudioVideo.html | [object DataChannel] is in state: 'open' - got "connecting", expected "open"
TEST-UNEXPECTED-FAIL | dom/media/tests/mochitest/test_dataChannel_basicAudioVideoCombined.html | [object DataChannel] is in state: 'open' - got "connecting", expected "open"
TEST-UNEXPECTED-FAIL | dom/media/tests/mochitest/test_dataChannel_basicAudioVideoNoBundle.html | [object DataChannel] is in state: 'open' - got "connecting", expected "open"
TEST-UNEXPECTED-FAIL | dom/media/tests/mochitest/test_dataChannel_basicDataOnly.html | [object DataChannel] is in state: 'open' - got "connecting", expected "open"
TEST-UNEXPECTED-FAIL | dom/media/tests/mochitest/test_dataChannel_basicDataOnly.html | [object DataChannel] is in state: 'open' - got "connecting", expected "open"
TEST-UNEXPECTED-FAIL | dom/media/tests/mochitest/test_dataChannel_basicVideo.html | [object DataChannel] is in state: 'open' - got "connecting", expected "open"
TEST-UNEXPECTED-FAIL | dom/media/tests/mochitest/test_dataChannel_bug1013809.html | [object DataChannel] is in state: 'open' - got "connecting", expected "open"
FAIL | dom/media/tests/mochitest/test_peerConnection_addDataChannel.html | [object DataChannel] is in state: 'open' - got "connecting", expected "open"

Comment 1

[Bevis Tseng[:bevis][:btseng](Exited)]

Attachment: (v1) Patch: Fix failures in WebRTC tests relying on non-comformant Promise handling.

The readyState is changed from 'connecting' to 'open' after the event handler of ondatachannel is returned [1].
However, in the new promise scheduling, the resolved callback will be performed at the return of the outermost JS callback if possible (i.e. 'ondatachannel' callback) [2] instead of the end of current task.
(Please see slides in bug 1193394 comment 47 for further explanation.)

Hence, we invoke the resolve callback of 'ondatachannel' in next task to ensure that the tests can be run in both old/new promising scheduling.

Hi Nils, may I have your review for this change? Thanks!

[1] https://searchfox.org/mozilla-central/rev/be78e6ea9b10b1f5b2b3b013f01d86e1062abb2b/media/webrtc/signaling/src/peerconnection/PeerConnectionImpl.cpp#1427
[2] https://searchfox.org/mozilla-central/rev/be78e6ea9b10b1f5b2b3b013f01d86e1062abb2b/media/webrtc/signaling/src/peerconnection/PeerConnectionImpl.cpp#1426

Comment 2

[Nils Ohlmeier [:drno]]

Comment on attachment 8933241 [details] [diff] [review]
(v1) Patch: Fix failures in WebRTC tests relying on non-comformant Promise handling.

Review of attachment 8933241 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks for trying to track this down, but this patch is only going to make the test accept our implementation wrong behavior here.

The correct behavior is described here http://w3c.github.io/webrtc-pc/#announce-datachannel-open

So the test should stay as is and we should change the implementation instead.

Comment 3

[Bevis Tseng[:bevis][:btseng](Exited)]

(In reply to Nils Ohlmeier [:drno] from comment #2)
> The correct behavior is described here
> http://w3c.github.io/webrtc-pc/#announce-datachannel-open
> 
> So the test should stay as is and we should change the implementation
> instead.
Oops! Thanks for the clarification.
I'd like to unassign myself until more understanding in WebRTC spec and our implementation.

Comment 4

[Nils Ohlmeier [:drno]]

Bevis: Thanks for finding the problem!

How can we reproduce the this so that we can look into this?

Comment 5

[Bevis Tseng[:bevis][:btseng](Exited)]

It's really appreciated for the help from experts here! :-)

It's easy to reproduce if you apply the updated patch part 1 in bug 1193394 comment 61 and test it locally.
The patch in bug 1193394 can not be landed until all these failures are fixed because it's a fundamental change in microtask scheduling. (There is no easy way to land it and disable it by default after some experiment is done in bug 1420816 comment 9.)
Hence, it will be nice to make the fix work with/without the microtask patch as what has been done in other dependencies of bug 1193394 if possible.
The differences in new/old microtask scheduling is explained in bug 1193394 comment 48 for your reference. (There is a slide in bug 1193394 comment 47 as well.)

Comment 6

[Bevis Tseng[:bevis][:btseng](Exited)]

NI to notify the information in comment 5.

Comment 7

[Bevis Tseng[:bevis][:btseng](Exited)]

After further analysis about the spec and our implementation, it seems that both the test and the implementation are incorrect, IMO. There are 2 issues:
1) In the test, the readyState of the remote channel is expected to be "connecting" instead of "open" if we check during the task of dispatching "datachannel" event, it shall be changed to "open" in another enqueued task mentioned in issue 2):
   See step 7 and step 8 of the enqueued task of "When an underlying data transport is to be announced" in the spec:
>  7. Set channel's [[ReadyState]] slot to *connecting*.
>  8. Fire a *datachannel* event named datachannel with channel at the RTCPeerConnection object.
2) The readyState shall be set to "open" at the same task of "announce an RTCDataChannel as open" [1] instead of the enqueued task of issue 1) if we treat [2] the implementation of the task of issue 1).

Nils, please correct me if my understanding is incorrect.
If my understanding is correct, is there any suggestion to fix them?

[1] http://w3c.github.io/webrtc-pc/#announce-datachannel-open
[2] https://searchfox.org/mozilla-central/rev/be78e6ea9b10b1f5b2b3b013f01d86e1062abb2b/media/webrtc/signaling/src/peerconnection/PeerConnectionImpl.cpp#1458

Comment 8

[Bevis Tseng[:bevis][:btseng](Exited)]

(In reply to Bevis Tseng[:bevis][:btseng] from comment #7)
> 2) The readyState shall be set to "open" at the same task of "announce an
> RTCDataChannel as open" [1] instead of the enqueued task of issue 1) if we
> treat [2] the implementation of the task of issue 1).
Note: the readyState is set to "open" in 
https://searchfox.org/mozilla-central/rev/03877052c151a8f062eea177f684a2743cd7b1d5/netwerk/sctp/datachannel/DataChannel.cpp#3180
However, the "open" event was fired later in DataChannelOnMessageAvailable runnable.
Which is not conformant to the spec that both the updating of readyState to open and the dispatching of "open" event are done at the same task [1].

[1] http://w3c.github.io/webrtc-pc/#announce-datachannel-open
[2] https://searchfox.org/mozilla-central/rev/be78e6ea9b10b1f5b2b3b013f01d86e1062abb2b/media/webrtc/signaling/src/peerconnection/PeerConnectionImpl.cpp#1458

Comment 9

[Bevis Tseng[:bevis][:btseng](Exited)]

Oops, I removed the NI flag unexpectedly.
Set NI again for the issues mentioned in comment 7 and comment 8.

Comment 10

[Hiroyuki Ikezoe (:hiro)]

The tests in comment 0 still failed on the latest try; 
https://treeherder.mozilla.org/#/jobs?repo=try&revision=8ef75c03168fa73749a49a5a6f0e51652c943a1a

Someone should keep an eye on it. Setting ni to :drno for now.

Comment 11

[Hiroyuki Ikezoe (:hiro)]

I think I am getting closer to a fix for this issue.

With the micro task change (bug 1119593), the readyState for remote data channel is still 'connecting' in CREATE_SECOND_DATA_CHANNEL when we check whether the readyState is 'open' in dataChannel.js.

As Bevis commented, Promises in the datachannel event callback is fulfilled before calling NS_DataChannelAppReady() in  NotifyDataChannel_m [1].

So, what we need to do is just waiting for 'open' event before proceeding the test just like we do for non-negotiated case.  Before bug 1119593, we waited actually for 'open' [2], but unfortunately it dropped because nonconformant micro task handling wallpapered the issue that we are suffering from now.

Initially I did try to wait for 'open' in expectedDataChannel() something like this;

  expectDataChannel: function(message) {
    this.nextDataChannel = new Promise(resolve => {
      this.ondatachannel = e => {
        ok(e.channel, message);
        e.channel.onopen = () => resolve(e.channel);
      };
    });
  },

But it didn't work since 'open' event didn't appear there.  After some more debugging, I noticed that the open event was received in the ctor of DataChannelWrapper [3].  I don't know why, but I guess that's because the channel in expectedDataChannel is a different one?

Anyway, finally I could get the open event, and waiting for the event worked fine with the micro task change. Here is a try;
https://treeherder.mozilla.org/#/jobs?repo=try&revision=24be81e9f0919b79a64c155f3fc6266a20e5c97e

And, FWIW, here is another try based on bare m-c;
https://treeherder.mozilla.org/#/jobs?repo=try&revision=73a620519f192b428d1084db43651c3eae676c95

I believe I am on the right track, but the way to wait open event is a bit awkward.  I think there is a good way to do it and media guys knows it.

[1] https://hg.mozilla.org/mozilla-central/file/17ade9f88b6e/media/webrtc/signaling/src/peerconnection/PeerConnectionImpl.cpp#l1411
[2] https://hg.mozilla.org/mozilla-central/rev/9be9159b75fd#l3.2781
[3] https://hg.mozilla.org/mozilla-central/file/17ade9f88b6e/dom/media/tests/mochitest/pc.js#l619

Comment 12

[Hiroyuki Ikezoe (:hiro)]

(In reply to Hiroyuki Ikezoe (:hiro) from comment #11)
> I think I am getting closer to a fix for this issue.
> 
> With the micro task change (bug 1119593), the readyState for remote data

Oh no.  I did paste a wrong bug number.  Bug 1193394 is correct.

Comment 13

[Hiroyuki Ikezoe (:hiro)]

I will try to request reviews since we already NI-ed to :drno.

Comment 14

[Hiroyuki Ikezoe (:hiro)]

Attachment: Bug 1414176 - Replace tabs with spaces.

Review commit: https://reviewboard.mozilla.org/r/217094/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/217094/

Comment 15

[Hiroyuki Ikezoe (:hiro)]

Attachment: Bug 1414176 - Wait for the data channel open for non-negotiated case too.

Notifying data channel and making the data channel state open are continuously
done in NotifyDataChannel_m in PeerConnectionImpl.cpp.  But if we check the
data channel state in the callback for notifying data channel, it's 'open' yet.

So to make sure the data channels both for local and remote is open, we have to
wait for 'open' event there.  DataChannelWrapper gives us the functionality to
waiting for the event with timeGuard, it can be useful here.

Review commit: https://reviewboard.mozilla.org/r/217096/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/217096/

Comment 16

[Nils Ohlmeier [:drno]]

Comment on attachment 8947362 [details]
Bug 1414176 - Replace tabs with spaces.

https://reviewboard.mozilla.org/r/217094/#review223134

Comment 17

[Nils Ohlmeier [:drno]]

I apologize for not getting back earlier. I'm pretty swamped right now.

My reading of the spec here https://w3c.github.io/webrtc-pc/#dom-rtcdatachannelstate [1] is that test expectations are in fact correct.
This https://searchfox.org/mozilla-central/rev/eeb7190f9ad6f1a846cd6df09986325b3f2c3117/dom/media/tests/mochitest/dataChannel.js#28 expects the state to be 'connecting' after calling createDataChannel, because at that point the data channel transport is not established and therefore the datachannel is not 'open' right away (that is on the pcLocal side of things).

According to the paragraph above this section https://w3c.github.io/webrtc-pc/#announce-datachannel-open says pcRemote's data channel state initially should be 'connecting' as well. Although the language at [1] sounds different to me (I'll file a spec bug for that). But we don't check the state of that data channel any way in our tests.

Now the big difference is when we call createDataChannel() here https://searchfox.org/mozilla-central/rev/eeb7190f9ad6f1a846cd6df09986325b3f2c3117/dom/media/tests/mochitest/dataChannel.js#71 for the second time my interpretation is that then on both sides of the connection the underlying data transport is ready and therefore the new data channel should start on both sides of the connection in the 'open' state right away.
That is what our tests are checking here https://searchfox.org/mozilla-central/rev/eeb7190f9ad6f1a846cd6df09986325b3f2c3117/dom/media/tests/mochitest/dataChannel.js#74 and what appears to fail.

So sorry :hiro while your patch now waits for the second (are more subsequent) data channel(s) to switch to the 'open' state, I think the tests actually expect the right thing. But out implementation appears to do the wrong thing. We need to not start data channels on established connection in the 'connecting' state but in the 'open' state right away.

@jib: since you are our spec expert, what are your thoughts on my reading of the spec?

Comment 18

[Nils Ohlmeier [:drno]]

Comment on attachment 8947363 [details]
Bug 1414176 - Wait for the data channel open for non-negotiated case too.

https://reviewboard.mozilla.org/r/217096/#review223182

Sorry, while this makes the tests pass for now, I believe it is not the right fix for the underlying problem.
I only could imagine to land this as a stop gap to unblock your bug, and then we need to follow up with a new bug to fix the implementation and the tests.

Comment 19

[Hiroyuki Ikezoe (:hiro)]

Thank you Nils for the clarification!  Now I am starting to bit understand what the problem is.  

So, from the spec <https://w3c.github.io/webrtc-pc/#dom-rtcdatachannelstate>;

open 	The underlying data transport is established and communication is possible. This is the initial state of an RTCDataChannel object dispatched as a part of an RTCDataChannelEvent.

And the |RTCDataChannelEvent| datachannel event that we fire in NotifyDataChannel_m(), right?  So if I understand correctly the spec (and if the spec is correct), we should call NS_DataChannelAppReady() before we call NotifyDataChannel() in NotifyDataChannel_m()?

I did already try it, and it worked with the micro task change, FWIW.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=110eee720095f1b246ea09d5eb3e1ea0ed973652

Comment 20

[Nils Ohlmeier [:drno]]

(In reply to Hiroyuki Ikezoe (:hiro) from comment #19)
> Thank you Nils for the clarification!  Now I am starting to bit understand
> what the problem is.  
> 
> So, from the spec <https://w3c.github.io/webrtc-pc/#dom-rtcdatachannelstate>;
> 
> open 	The underlying data transport is established and communication is
> possible. This is the initial state of an RTCDataChannel object dispatched
> as a part of an RTCDataChannelEvent.
> 
> And the |RTCDataChannelEvent| datachannel event that we fire in
> NotifyDataChannel_m(), right?  So if I understand correctly the spec (and if
> the spec is correct), we should call NS_DataChannelAppReady() before we call
> NotifyDataChannel() in NotifyDataChannel_m()?

That is not really what I had in mind.
 
> I did already try it, and it worked with the micro task change, FWIW.
> 
> https://treeherder.mozilla.org/#/
> jobs?repo=try&revision=110eee720095f1b246ea09d5eb3e1ea0ed973652

But the fact that this appears to change the behavior makes me suspicious if the problem your change exposes is cause by what we recently found in our code base and refer to as queue jumping. This got fixed recently. But Bevis patches in bug 1193394 don't apply cleanly on mozilla-central any more. Could I ask you to rebase you patches and see it that changes anything for you?

Comment 21

[Hiroyuki Ikezoe (:hiro)]

(In reply to Nils Ohlmeier [:drno] from comment #20)
> (In reply to Hiroyuki Ikezoe (:hiro) from comment #19)
> > Thank you Nils for the clarification!  Now I am starting to bit understand
> > what the problem is.  
> > 
> > So, from the spec <https://w3c.github.io/webrtc-pc/#dom-rtcdatachannelstate>;
> > 
> > open 	The underlying data transport is established and communication is
> > possible. This is the initial state of an RTCDataChannel object dispatched
> > as a part of an RTCDataChannelEvent.
> > 
> > And the |RTCDataChannelEvent| datachannel event that we fire in
> > NotifyDataChannel_m(), right?  So if I understand correctly the spec (and if
> > the spec is correct), we should call NS_DataChannelAppReady() before we call
> > NotifyDataChannel() in NotifyDataChannel_m()?
> 
> That is not really what I had in mind.

Oh, OK. WebRTC is pretty hard to track down for me. :)

> > I did already try it, and it worked with the micro task change, FWIW.
> > 
> > https://treeherder.mozilla.org/#/
> > jobs?repo=try&revision=110eee720095f1b246ea09d5eb3e1ea0ed973652
> 
> But the fact that this appears to change the behavior makes me suspicious if
> the problem your change exposes is cause by what we recently found in our
> code base and refer to as queue jumping. This got fixed recently. But Bevis
> patches in bug 1193394 don't apply cleanly on mozilla-central any more.
> Could I ask you to rebase you patches and see it that changes anything for
> you?

Pushed a new try based on fea9e610ec9d.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=6e7f47a0a6af1244bbc3a4e751e16aca8dd96a4e

Thanks!

Comment 22

[Jan-Ivar Bruaroey [:jib] (needinfo? me)]

The spec says ReadyState should be "connecting" after synchronously calling createDataChannel [1]:

  8. Let channel have a [[ReadyState]] internal slot initialized to "connecting".

You can check it here https://jsfiddle.net/jib1/6fr9qoue/

(In reply to Nils Ohlmeier [:drno] from comment #17)
> This
> https://searchfox.org/mozilla-central/rev/
> eeb7190f9ad6f1a846cd6df09986325b3f2c3117/dom/media/tests/mochitest/
> dataChannel.js#28 expects the state to be 'connecting' after calling
> createDataChannel, because at that point the data channel transport is not
> established and therefore the datachannel is not 'open' right away (that is
> on the pcLocal side of things).

Beware our test harness.

This isn't testing createDataChannel, rather a different function PeerConnectionTest.prototype.createDataChannel with different timing characteristics, as it returns a promise (the same-named function in the spec does not).

In the test line 28 you qoute above, we're ignoring the promise, so it ends up working.

> According to the paragraph above this section
> https://w3c.github.io/webrtc-pc/#announce-datachannel-open says pcRemote's
> data channel state initially should be 'connecting' as well. Although the
> language at [1] sounds different to me (I'll file a spec bug for that).

Seems the same to me. Can you clarify the difference you see?

> Now the big difference is when we call createDataChannel() here
> https://searchfox.org/mozilla-central/rev/
> eeb7190f9ad6f1a846cd6df09986325b3f2c3117/dom/media/tests/mochitest/
> dataChannel.js#71 for the second time my interpretation is that then on both
> sides of the connection the underlying data transport is ready and therefore
> the new data channel should start on both sides of the connection in the
> 'open' state right away.

Line 71 says:

  return test.createDataChannel({ }).then(result => {

So here's that promise I mention earlier. This is a test harness function [2] which apparently waits for negotiationneeded and other stuff, from both sides.

> That is what our tests are checking here
> https://searchfox.org/mozilla-central/rev/
> eeb7190f9ad6f1a846cd6df09986325b3f2c3117/dom/media/tests/mochitest/
> dataChannel.js#74 and what appears to fail.

I'm not surprised, but please don't look to the spec for answers, since you're basically testing our test harness here, not a spec API.

IMHO, this is a good example of why wrapping the peer connection API the way our test harness does, is a bad idea. It's confusing, and leads us to thinking we're testing things we're not. 

[1] https://w3c.github.io/webrtc-pc/#dom-rtcpeerconnection-createdatachannel
[2] https://searchfox.org/mozilla-central/source/dom/media/tests/mochitest/pc.js#294-299

Comment 23

[Nils Ohlmeier [:drno]]

I don't think this is about our test framework. While I agree that I overlooked the part where the framework returns a promise and then ignores the status of the promise I think I was not clear enough in trying to point where I see the problem.

This is not about the side which calls createDatachannel(). The problem is on the side which "receives" a new data channel via  RTCDataChannelEvent, so pcRemote in case of our test framework.

I butchered you fiddle to open a second data channel after the first: https://jsfiddle.net/cfaee5co/

Now the spec says "MUST initially be in the connecting state.", which I get for every single data channel on the "receiving" side, independent of the state of the underlying ICE and SCTP transport states.
The spec continuous to say "When the RTCDataChannel object's underlying data transport is ready, the user agent MUST announce the RTCDataChannel as open." Further down https://w3c.github.io/webrtc-pc/#dom-rtcdatachannelstate it says "This is the initial state of an RTCDataChannel object dispatched as a part of an RTCDataChannelEvent."

I don't find it super clear in the spec, but my reading is that when you receive the first data channel via RTCDataChannelEvent it is quite possible that the initial state is going to be 'connecting', assuming the SCTP connection has not been established. But when I wait for the first data channel to switch to the 'open' state and then create another data channel, I would expect the readyState to be 'open' right away on both sides (the side which calles createDatachannel() and the side which received it via RTCDataChannelEvent). And that is clearly not the case, because the data channels we dispatch via RTCDataChannelEvent always start in the 'connecting' state.

Jib what do you think?

Comment 24

[Jan-Ivar Bruaroey [:jib] (needinfo? me)]

I think Firefox is right. I've filed a PR to fix the spec https://github.com/w3c/webrtc-pc/pull/1768.

But assuming Firefox is right (until that PR merges) what exactly are our tests failing over?

Comment 25

[Hiroyuki Ikezoe (:hiro)]

So, the PR got merged.  Would you guys mind taking over this?

Comment 26

[Nils Ohlmeier [:drno]]

Thanks hiro for monitoring the spec issue and the heads up.

Yeah I can take a look at this once I'm back from PTO >= 2018-02-26, now I think I understand pretty clearly what needs to get done here.

If someone has the cycles to do this earlier, then feel free to take it.

Comment 27

[Nils Ohlmeier [:drno]]

Attachment: Bug 1414176: make data channel readyState checks spec compliant

Review commit: https://reviewboard.mozilla.org/r/221134/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/221134/

Comment 28

[Nils Ohlmeier [:drno]]

:hiro could you do me a favor and check if what I just submitted into mozreview works with your promise changes (while I'm gone - that would be big help)?

I think the test expectations are now up to what the spec says now. I'm not sure if we might need to update the implementation to meet the expectations now.

Comment 29

[Hiroyuki Ikezoe (:hiro)]

I just run test_dataChannel_basicAudio.html locally on Linux, it works fine.  Awesome!  Thank you!

Pushed a try for all platforms.
https://treeherder.mozilla.org/#/jobs?repo=try&revision=72942889d42cd7be93443d6fab14e79c63446095

Comment 30

[Hiroyuki Ikezoe (:hiro)]

Comment on attachment 8951841 [details]
Bug 1414176: make data channel readyState checks spec compliant

Jan-Ivar, could you please review this :drno's patch?  We are close to land bug 1193394.

Comment 31

[Jan-Ivar Bruaroey [:jib] (needinfo? me)]

Comment on attachment 8951841 [details]
Bug 1414176: make data channel readyState checks spec compliant

https://reviewboard.mozilla.org/r/221134/#review229728

::: dom/media/tests/mochitest/pc.js:1124
(Diff revision 1)
> +    } else {
> +      is(channel.readyState, "open", "subsequent readyState is 'open'");

Please add a // TODO: Bug 1441723 Update firefox to spec.

Comment 32

[Jan-Ivar Bruaroey [:jib] (needinfo? me)]

Attachment: Bug 1414176: make data channel readyState checks spec compliant

Review commit: https://reviewboard.mozilla.org/r/223696/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/223696/

Comment 33

[Jan-Ivar Bruaroey [:jib] (needinfo? me)]

Comment on attachment 8954606 [details]
Bug 1414176: make data channel readyState checks spec compliant

https://reviewboard.mozilla.org/r/223696/#review229732

Carrying forward r=me for drno's patch.

Comment 34

[Nils Ohlmeier [:drno]]

Comment on attachment 8951841 [details]
Bug 1414176: make data channel readyState checks spec compliant

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/221134/diff/1-2/

Comment 35

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/b5620eb0e2ab
make data channel readyState checks spec compliant r=jib

Comment 36

[Mozilla Autoland]

We're sorry, Autoland could not rebase your commits for you automatically. Please manually rebase your commits and try again.

hg error in cmd: hg rebase -s af5054ecdf8fe9ab1f8b0f277883f5e41c549b99 -d 81cc9bdd0bfc: rebasing 449215:af5054ecdf8f "Bug 1414176: make data channel readyState checks spec compliant r=jib" (tip)
merging dom/media/tests/mochitest/pc.js
warning: conflicts while merging dom/media/tests/mochitest/pc.js! (edit, then use 'hg resolve --mark')
unresolved conflicts (see hg resolve, then hg rebase --continue)

Comment 37

[Noemi Erli[:noemi_erli]]

https://hg.mozilla.org/mozilla-central/rev/b5620eb0e2ab