1241508 - WebRTC Telemetry should only be recorded with extended Telemetry on

Status: RESOLVED FIXED

(Toolkit :: Telemetry, defect, P3)

Description

[Georg Fritzsche [:gfritzsche]]

This recording function doesn't check for mCanRecordExtend:
https://dxr.mozilla.org/mozilla-central/rev/6764bc656c1d146962d53710d734c2ac87c2306f/toolkit/components/telemetry/Telemetry.cpp#3651

This means it is always recording, even when extended Telemetry is off and we never use the data.

Comment 1

[Georg Fritzsche [:gfritzsche]]

Nils, is there any reason for this to always record?

Comment 2

[Nils Ohlmeier [:drno]]

Besides that I don't know what extended Telemetry means not, no :-)
My best guess is that I copied the if condition which checks for |sTelemetry| from some other function (which on purpose does not check for |mCanRecordExtended|).

Happy to provide a patch, or should I leave it for someone else as a first bug?

Comment 3

[Tyler Steiman]

I'd love to submit a patch for this if that's ok

Comment 4

[Tyler Steiman]

Attachment: bug1241508_mCanRecordExtended_check.diff

Does this work?

Comment 5

[Nils Ohlmeier [:drno]]

Comment on attachment 8710652 [details] [diff] [review]
bug1241508_mCanRecordExtended_check.diff

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

LGTM

Comment 6

[Tyler Steiman]

Can I go ahead and put checkin-needed or should we wait for Georg?

Comment 7

[Nils Ohlmeier [:drno]]

As I'm the original author of these lines and it's just one line fix, I'm pretty sure Georg doesn't mind if we set checkin-needed :-)

Comment 8

[Pulsebot]

https://hg.mozilla.org/integration/fx-team/rev/a45569eb51d7

Comment 9

[Phil Ringnalda (:philor)]

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