Closed
Bug 1019923
Opened 10 years ago
Closed 10 years ago
Data channel test failure logs timeouts after failure
Categories
(Core :: WebRTC, defect)
Core
WebRTC
Tracking
()
RESOLVED
FIXED
mozilla33
People
(Reporter: drno, Assigned: drno)
References
Details
Attachments
(1 file, 1 obsolete file)
5.34 KB,
patch
|
Details | Diff | Splinter Review |
The full log here https://tbpl.mozilla.org/php/getParsedLog.php?id=40966662&tree=Mozilla-Inbound shows that the error case handling is not ideal. It shows that the data channel for pcLocal is open, but then logs an error about pcLocal not switching to open. Additionally it logs the failure for pcRemote to open its data channel twice. And the whole test gets killed by the test framework timeout.
Assignee | ||
Comment 1•10 years ago
|
||
Try run: https://tbpl.mozilla.org/?tree=Try&rev=80553a61ce50
Attachment #8433853 -
Flags: review?(jib)
Comment 2•10 years ago
|
||
Comment on attachment 8433853 [details] [diff] [review] bug_1019923_fix_timeouts.patch Review of attachment 8433853 [details] [diff] [review]: ----------------------------------------------------------------- r=me with nits. ::: dom/media/tests/mochitest/pc.js @@ +1085,3 @@ > onSuccess(); > return; > } Maybe add an is(peer.dataChannels[0].readyState, "connecting", peer + " dataChannels[0] is in state: 'connecting'"); here? @@ +1086,5 @@ > return; > } > > // TODO: drno: convert dataChannels into an object and make > // registerDataChannelOPenEvent a generic function Spotted unrelated typo: OPen @@ +1093,5 @@ > } else { > peer.registerDataChannelOpenEvents(dataChannelConnected); > } > > + if (typeof onFailure === "function") { I would use if (onFailure) here to avoid silently accepting non-functions (that way we hear about it when the code below tries to use it as a function, rather than silently work).
Attachment #8433853 -
Flags: review?(jib) → review+
Assignee | ||
Comment 3•10 years ago
|
||
Addressed NITs from jib. Carrying forward r+=jib. Another try run to be safe, because of the ok(..) I added per jib's comment: https://tbpl.mozilla.org/?tree=Try&rev=1bc833711792
Attachment #8433853 -
Attachment is obsolete: true
Assignee | ||
Comment 4•10 years ago
|
||
I don't see any new problems. Let me know if I need to refresh the patch for the checkin.
Keywords: checkin-needed
Comment 5•10 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/516a5bdb4f16
Flags: in-testsuite-
Keywords: checkin-needed
Comment 6•10 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/516a5bdb4f16
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla33
Comment 7•10 years ago
|
||
https://hg.mozilla.org/releases/mozilla-aurora/rev/b954ca743de1
status-b2g-v2.0:
--- → fixed
status-b2g-v2.1:
--- → fixed
status-firefox31:
--- → wontfix
status-firefox32:
--- → fixed
status-firefox33:
--- → fixed
You need to log in
before you can comment on or make changes to this bug.
Description
•