1107686 - Remove redundant task check in DataReportingService

Status: RESOLVED FIXED

(Firefox Health Report Graveyard :: Client: Desktop, defect)

Description

[Georg Fritzsche [:gfritzsche]]

Attachment: Remove redundant check

On spot-checking the client id code, i noticed that we have a redundant check there for the task check.
I'm not sure if this actually breaks anything, but its certainly unneeded.

Comment 1

[Gregory Szorc [:gps]]

Comment on attachment 8532218 [details] [diff] [review]
Remove redundant check

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

r+. But this removed check is defense in depth. The new code assumes there is exactly 1 caller of _loadClientID. Software always changes. That assumption may break some day. I think I'd feel better if _loadClientID were implemented as an inline function as part of getClientID.

Comment 2

[Georg Fritzsche [:gfritzsche]]

If you're worried about robustness, i think we should rather make _loadClientID() use the pattern |this._task = Task.spawn(...); return this._task;|.
As is now, doing a task check there just seems broken (i don't think this can run properly).

Comment 3

[Georg Fritzsche [:gfritzsche]]

Attachment: Remove redundant check

Is this still fine with you then?

Comment 4

[Gregory Szorc [:gps]]

Comment on attachment 8533125 [details] [diff] [review]
Remove redundant check

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

Looks good. Thanks!

Comment 5

[Georg Fritzsche [:gfritzsche]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/7365b7551ee3

Comment 6

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/7365b7551ee3