1529399 - Remove unnecessary wrapper runnable from CreateDirectTaskDrainer() for stable state runnables

Status: RESOLVED FIXED

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

Description

[Karl Tomlinson (:karlt)]

The CreateDirectTaskDrainer() call was added in https://hg.mozilla.org/mozilla-central/rev/02e5708a75fcdd9e8e4b065995f00b29896fb4b5#l9.32 to use an AbstractThread appropriate for the stream (https://bugzilla.mozilla.org/show_bug.cgi?id=1314833#c76) but became unnecessary after separate MSGs for used each DocGroup and the MSG got a single AbstractThread https://hg.mozilla.org/mozilla-central/rev/5c2fea87df16#l9.388

The original DrainDirectTask() call was added for reasons described in https://bugzilla.mozilla.org/show_bug.cgi?id=1144486#c16 which should now be unnecessary since https://hg.mozilla.org/integration/autoland/rev/ed7c9d7a635d

Comment 1

[Karl Tomlinson (:karlt)]

I suspect mixing AbstractThread-style state-mirroring with runnables queued directly for stable-state was never intended to be supported in general. EventTargetWrapper uses the stable-state to run the direct tasks, which is not going to be able to run immediately after every stable state event. I don't see any direct task support in CycleCollectedJSContext::ProcessStableStateQueue(): https://searchfox.org/mozilla-central/rev/93905b660fc99a5d52b683690dd26471daca08c8/xpcom/base/CycleCollectedJSContext.cpp#379

Comment 2

[Karl Tomlinson (:karlt)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=290eea6eec03b05f1e45c220086f82e8aca7893b
https://treeherder.mozilla.org/#/jobs?repo=try&revision=2de2f253cd8bfac58ad10ccb83f7b1c1b47d6287

Comment 3

[Karl Tomlinson (:karlt)]

Attachment: Bug 1529399 Remove unnecessary wrapper runnable from CreateDirectTaskDrainer() for stable state runnables r?padenot

The original DrainDirectTask() call was added for reasons described in https://bugzilla.mozilla.org/show_bug.cgi?id=1144486#c16 which should now be unnecessary since https://hg.mozilla.org/integration/autoland/rev/ed7c9d7a635d

The code being removed here was rework to use an appropriate AbstractThread:
https://hg.mozilla.org/mozilla-central/rev/02e5708a75fcdd9e8e4b065995f00b29896fb4b5#l9.32
https://bugzilla.mozilla.org/show_bug.cgi?id=1314833#c76

Comment 4

[Karl Tomlinson (:karlt)]

Attachment: bug 1529399 remove now-unnecessary aDrainDirectTasks parameter from EventTargetWrapper::Runner constructor r?padenot

Depends on D20604

Comment 5

[Karl Tomlinson (:karlt)]

Attachment: Bug 1529399 replace AutoTaskGuard with equivalent AutoEnter r?padenot

Depends on D20605

Comment 6

[Karl Tomlinson (:karlt)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=de13decc8e61a054b1bef478ce0097daac52f2d0

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/2051c2dbfeda
Remove unnecessary wrapper runnable from CreateDirectTaskDrainer() for stable state runnables r=padenot
https://hg.mozilla.org/integration/autoland/rev/3740242a0860
remove now-unnecessary aDrainDirectTasks parameter from EventTargetWrapper::Runner constructor r=padenot
https://hg.mozilla.org/integration/autoland/rev/bda89c19aff6
replace AutoTaskGuard with equivalent AutoEnter r=padenot

Comment 8

[Noemi Erli[:noemi_erli]]

https://hg.mozilla.org/mozilla-central/rev/2051c2dbfeda
https://hg.mozilla.org/mozilla-central/rev/3740242a0860
https://hg.mozilla.org/mozilla-central/rev/bda89c19aff6