bugzilla.mozilla.org has resumed normal operation. Attachments prior to 2014 will be unavailable for a few days. This is tracked in Bug 1475801.
Please report any other irregularities here.

Label the EverySecondTelemetryCallback_m runnable

RESOLVED FIXED in Firefox 57

Status

()

Core
WebRTC: Signaling
P1
normal
Rank:
15
RESOLVED FIXED
a year ago
10 months ago

People

(Reporter: billm, Assigned: ng)

Tracking

(Blocks: 1 bug)

unspecified
mozilla57
Points:
---

Firefox Tracking Flags

(firefox57 fixed)

Details

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(1 attachment)

This runnable happens pretty often. It will need to be labeled. It would be nice if we could use SystemGroup here. But it seems possible that we read data that needs to be read when no other runnables are running.

I also wonder if this runnable could be run at idle priority. That would solve the labeling issue and also improve performance.
Rank: 25
Component: WebRTC → WebRTC: Signaling
Priority: -- → P2
Up-ing priority as I learned this is part of the remaining 3% of unlabeled runnables we need to make the concurrent scheduler work effectively for project quantum.
Rank: 25 → 15
Priority: P2 → P1
Nico, can you do this one? If not, I can do it.
Assignee: nobody → na-g
(Assignee)

Comment 3

a year ago
Jan-Ivar, sure.
Status: NEW → UNCONFIRMED
Ever confirmed: false
Gentle ping here :)
(Assignee)

Comment 5

11 months ago
Thanks for the poke.
(Assignee)

Updated

11 months ago
Flags: needinfo?(na-g)
(Assignee)

Comment 6

11 months ago
This looks like it labeled in Bug 1377222.
https://hg.mozilla.org/mozilla-central/rev/99566b93d105#l14.14
Flags: needinfo?(na-g)
(Assignee)

Comment 7

11 months ago
Andrew, could you double check me on comment 6 before I close?
Flags: needinfo?(overholt)
That change just gave it a name. The problem here is the SetTarget call:
http://searchfox.org/mozilla-central/rev/5ce320a16f6998dc2b36e30458131d029d107077/media/webrtc/signaling/src/peerconnection/PeerConnectionCtx.cpp#371

We need the target to be the result of calling EventTargetFor on some SchedulerGroup: either the SystemGroup or a DocGroup or TabGroup. If we use the SystemGroup, that means that the timer code is not allowed to modify any state in a way that would be observable by web content. If we use a DocGroup or TabGroup, that means that the timer can modify state in a way that is observable only by the document or tab in question.

So the main issue here is what EverySecondTelemetryCallback_m is actually doing and whether it modifies state in an observable way.
Flags: needinfo?(overholt)
Status: UNCONFIRMED → NEW
Ever confirmed: true
Comment hidden (mozreview-request)
Is this landable? :)
Flags: needinfo?(na-g)
(Assignee)

Updated

11 months ago
Attachment #8902310 - Flags: review?(wmccloskey)
Attachment #8902310 - Flags: review?(jib)

Comment 13

11 months ago
mozreview-review
Comment on attachment 8902310 [details]
Bug 1381627 - EverySecondTelemetryCallback target

https://reviewboard.mozilla.org/r/173842/#review180180

lgtm.
Attachment #8902310 - Flags: review?(jib) → review+
(Assignee)

Comment 14

11 months ago
(In reply to Andrew Overholt [:overholt] from comment #12)
> Is this landable? :)

Yes, review pending.
(Reporter)

Comment 15

11 months ago
mozreview-review
Comment on attachment 8902310 [details]
Bug 1381627 - EverySecondTelemetryCallback target

https://reviewboard.mozilla.org/r/173842/#review180654
Attachment #8902310 - Flags: review?(wmccloskey) → review+

Comment 16

11 months ago
Pushed by na-g@nostrum.com:
https://hg.mozilla.org/integration/autoland/rev/ac5161ac285d
EverySecondTelemetryCallback target r=billm,jib
https://hg.mozilla.org/mozilla-central/rev/ac5161ac285d
Status: NEW → RESOLVED
Last Resolved: 11 months ago
status-firefox57: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
(Assignee)

Updated

10 months ago
Flags: needinfo?(na-g)
You need to log in before you can comment on or make changes to this bug.