1346913 - Rewrite waitForRtpFlow in mochitests to use new async/await syntax

Status: RESOLVED FIXED

(Core :: WebRTC, enhancement, P3)

Description

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

waitForRtpFlow is unnecessary complicated and it does not always cancel the retry loop. While not leading to failures it does create confusing logging.

It could be simplified significantly by using the new async/await syntax.[0]

[0] https://blog.mozilla.org/webrtc/fiddle-of-the-week-await/

Comment 1

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

Attachment: Bug 1346913 - rewrite waitForRtpFlow using async await;

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

Comment 2

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

Comment on attachment 8846785 [details]
Bug 1346913 - rewrite waitForRtpFlow using async await;

https://reviewboard.mozilla.org/r/119794/#review121784

r=me with nits. Thanks for fixing this. I see the old code was improperly silencing any errors in the setInterval code.

::: dom/media/tests/mochitest/pc.js:1427
(Diff revision 1)
> -   * @returns {Promise}
> -   *        A promise that resolves when media is flowing.
> +   * @returns {StatsReport}
> +   *        A StatsReport object with RTP stats.

An async function still returns a promise.

::: dom/media/tests/mochitest/pc.js:1449
(Diff revision 1)
>             rtp.type + " RTP packets.");
>        return nrPackets > 0;
>      };
>  
>      // Time between stats checks
> -    var retryInterval = 500;
> +    let retryInterval = 500;

const

::: dom/media/tests/mochitest/pc.js:1461
(Diff revision 1)
> -          }
> +      }
> -        });
> -      }, retryInterval);
> -    });
> +      catch (e) {
> +        warn(e);
> +      }

Just remove try/catch here, to let errors escape and kill the test if there's a codig error. We shouldn't proceed in spite of an error here I don't think.

Comment 3

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

Comment on attachment 8846785 [details]
Bug 1346913 - rewrite waitForRtpFlow using async await;

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

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/266bc832c343
rewrite waitForRtpFlow using async await;r=jib

Comment 5

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/266bc832c343