1418522 - Show unmatched candidates on about:webrtc

Status: RESOLVED FIXED

(Core :: WebRTC, defect, P2)

Description

[Michael Froman [:mjf]]

This was supposed to already be in the ICE Stats table on about:webrtc, but the type field was slightly different (local-candidate or remote-candidate) vs the object field name on the page (localcandidate or remotecandidate).

Comment 1

[Michael Froman [:mjf]]

Attachment: Bug 1418522 - fix unmatched ICE candidate handling on about:webrtc.

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

Comment 2

[Nico Grunbaum [:ng, @chew:mozilla.org]]

Comment on attachment 8929660 [details]
Bug 1418522 - fix unmatched ICE candidate handling on about:webrtc.

https://reviewboard.mozilla.org/r/200940/#review206164

LGTM, with one comment.

::: toolkit/content/aboutwebrtc/aboutWebrtc.js:754
(Diff revision 1)
>          }
>          stats.push(stat);
>        }
>      }
>  
> -    for (let c of candidates.values()) {
> +    // add the unmatched candidates to the end of the table

Another option here is something like:
[...candidates.values()].filter(cnd => matched[cnd.id]).forEach(
    cand => stats.push{[cand.type]: this.candidateToString(cand)});
Which reads left to right, just an idea.

Comment 3

[Michael Froman [:mjf]]

Comment on attachment 8929660 [details]
Bug 1418522 - fix unmatched ICE candidate handling on about:webrtc.

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

Comment 4

[Michael Froman [:mjf]]

Comment on attachment 8929660 [details]
Bug 1418522 - fix unmatched ICE candidate handling on about:webrtc.

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

Comment 5

[Nico Grunbaum [:ng, @chew:mozilla.org]]

Comment on attachment 8929660 [details]
Bug 1418522 - fix unmatched ICE candidate handling on about:webrtc.

https://reviewboard.mozilla.org/r/200940/#review206402

Ship it!

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/bdec8ddab580
fix unmatched ICE candidate handling on about:webrtc. r=ng

Comment 7

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Backed out for eslint failure at toolkit/content/aboutwebrtc/aboutWebrtc.js:756:37 | Extra space after computed key 'cand.type':

https://hg.mozilla.org/integration/autoland/rev/9131ecd27dd93ac9b3775719b0892fbe2b39705b

Push with failure: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=bdec8ddab5804081f7795e575d0c2f4e26c8542d&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-resultStatus=retry&filter-resultStatus=usercancel&filter-resultStatus=runnable
Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=146166115&repo=autoland

Comment 8

[Michael Froman [:mjf]]

I've added the lint pre-commit hook to my .hgrc file now. :-)

Comment 9

[Michael Froman [:mjf]]

Comment on attachment 8929660 [details]
Bug 1418522 - fix unmatched ICE candidate handling on about:webrtc.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/200940/diff/3-4/

Comment 10

[Michael Froman [:mjf]]

Comment on attachment 8929660 [details]
Bug 1418522 - fix unmatched ICE candidate handling on about:webrtc.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/200940/diff/4-5/

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/ff6ab6ec5d18
fix unmatched ICE candidate handling on about:webrtc. r=ng

Comment 12

[Tiberius Oros[:tiberius_oros]]

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