Closed Bug 1411808 Opened 2 years ago Closed 2 years ago

Run MediaCache::Update() off the main thread

Categories

(Core :: Audio/Video: Playback, enhancement, P3)

enhancement

Tracking

()

RESOLVED FIXED
mozilla58
Tracking Status
firefox58 --- fixed

People

(Reporter: jwwang, Assigned: jwwang)

References

Details

Attachments

(3 files)

A step forward to run more MediaCache functions off the main thread.
Assignee: nobody → jwwang
Blocks: 1369263
Priority: -- → P3
Attachment #8924790 - Flags: review?(gsquelart)
Attachment #8924791 - Flags: review?(gsquelart)
Attachment #8924792 - Flags: review?(gsquelart)
Comment on attachment 8924790 [details]
Bug 1411808. P1 - run Update() loops off the main thread.

https://reviewboard.mozilla.org/r/196032/#review201244

::: dom/media/MediaCache.cpp:1177
(Diff revision 1)
> -  NS_ASSERTION(NS_IsMainThread(), "Only call on main thread");
> +  bool onCurrentThread = false;
> +  sThread->IsOnCurrentThread(&onCurrentThread);
> +  MOZ_ASSERT(onCurrentThread);

Can't you just do `MOZ_ASSERT(sThread->IsOnCurrentThread());`?

If not, since MOZ_ASSERT only runs in DEBUG builds, you should add #ifdef DEBUG...#endif around the block, so you don't call IsOnCurrentThread when not needed.
Attachment #8924790 - Flags: review?(gsquelart) → review+
Comment on attachment 8924791 [details]
Bug 1411808. P2 - don't call mClient->IsSuspended() off the main thread in Update().

https://reviewboard.mozilla.org/r/196034/#review201246
Attachment #8924791 - Flags: review?(gsquelart) → review+
Comment on attachment 8924792 [details]
Bug 1411808. P3 - InitAsClone() shouldn't call |mMediaCache->OpenStream(this)| until initialization is done.

https://reviewboard.mozilla.org/r/196036/#review201248
Attachment #8924792 - Flags: review?(gsquelart) → review+
Comment on attachment 8924790 [details]
Bug 1411808. P1 - run Update() loops off the main thread.

https://reviewboard.mozilla.org/r/196032/#review201244

> Can't you just do `MOZ_ASSERT(sThread->IsOnCurrentThread());`?
> 
> If not, since MOZ_ASSERT only runs in DEBUG builds, you should add #ifdef DEBUG...#endif around the block, so you don't call IsOnCurrentThread when not needed.

Thanks! I didn't notice there exists such a function.
Thanks for the reviews!
Pushed by jwwang@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/f26682779a18
P1 - run Update() loops off the main thread. r=gerald
https://hg.mozilla.org/integration/autoland/rev/875d91b29c31
P2 - don't call mClient->IsSuspended() off the main thread in Update(). r=gerald
https://hg.mozilla.org/integration/autoland/rev/efbfeacfdb96
P3 - InitAsClone() shouldn't call |mMediaCache->OpenStream(this)| until initialization is done. r=gerald
You need to log in before you can comment on or make changes to this bug.