Don't use lock when accessing ChannelMediaResource::mChannelStatistics

RESOLVED FIXED in Firefox 57

Status

()

enhancement
P3
normal
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: jwwang, Assigned: jwwang)

Tracking

unspecified
mozilla57
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox57 fixed)

Details

Attachments

(4 attachments)

Assignee

Description

2 years ago
Before fixing bug 1395793, we want to simplify the locking model of ChannelMediaResource.
Assignee

Updated

2 years ago
Assignee: nobody → jwwang
Blocks: 1382978
Priority: -- → P3
Comment hidden (mozreview-request)
Assignee

Updated

2 years ago
Attachment #8904817 - Flags: review?(cpearce)
Attachment #8904818 - Flags: review?(cpearce)
Attachment #8904819 - Flags: review?(cpearce)
Assignee

Updated

2 years ago
Attachment #8904817 - Flags: review?(cpearce)
Attachment #8904818 - Flags: review?(cpearce)
Attachment #8904819 - Flags: review?(cpearce)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Assignee

Updated

2 years ago
Attachment #8905089 - Flags: review?(cpearce)
Attachment #8904817 - Flags: review?(cpearce)
Attachment #8904818 - Flags: review?(cpearce)
Attachment #8904819 - Flags: review?(cpearce)

Comment 8

2 years ago
mozreview-review
Comment on attachment 8904817 [details]
Bug 1395802. P2 - assert ChannelMediaResource::GetDownloadRate() runs on the main thread.

https://reviewboard.mozilla.org/r/176570/#review182184
Attachment #8904817 - Flags: review?(cpearce) → review+

Comment 9

2 years ago
mozreview-review
Comment on attachment 8905089 [details]
Bug 1395802. P1 - add AbstractMainThread() to MediaResourceCallback.

https://reviewboard.mozilla.org/r/176902/#review182186
Attachment #8905089 - Flags: review?(cpearce) → review+

Comment 10

2 years ago
mozreview-review
Comment on attachment 8904818 [details]
Bug 1395802. P3 - ensure mChannelStatistics.AddBytes() to happen on the main thread.

https://reviewboard.mozilla.org/r/176572/#review182188
Attachment #8904818 - Flags: review?(cpearce) → review+

Comment 11

2 years ago
mozreview-review
Comment on attachment 8904819 [details]
Bug 1395802. P4 - we don't need lock since mChannelStatistics is always accessed on the main thread.

https://reviewboard.mozilla.org/r/176574/#review182190
Attachment #8904819 - Flags: review?(cpearce) → review+
Assignee

Comment 12

2 years ago
Thanks!

Comment 13

2 years ago
Pushed by jwwang@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/23fd25a4f713
P1 - add AbstractMainThread() to MediaResourceCallback. r=cpearce
https://hg.mozilla.org/integration/autoland/rev/8bfe3fa7b4f1
P2 - assert ChannelMediaResource::GetDownloadRate() runs on the main thread. r=cpearce
https://hg.mozilla.org/integration/autoland/rev/a70f47912fbe
P3 - ensure mChannelStatistics.AddBytes() to happen on the main thread. r=cpearce
https://hg.mozilla.org/integration/autoland/rev/6469a2cb7df6
P4 - we don't need lock since mChannelStatistics is always accessed on the main thread. r=cpearce
Assignee

Updated

2 years ago
Blocks: 1428688
Assignee

Updated

2 years ago
No longer blocks: 1428688
You need to log in before you can comment on or make changes to this bug.