1865376 - Call the error callback synchronously from the configure error task, and block the message queue processing at the right time

Status: RESOLVED FIXED

(Core :: Audio/Video: Web Codecs, defect)

Description

[Paul Adenot (:padenot)]

Fixes this: https://wpt.live/webcodecs/video-decoder.https.any.html and various other tests that were using the same pattern.

Comment 1

[Paul Adenot (:padenot)]

Attachment: Bug 1865376 - Call the error callback synchronously from the configure error task, and block the message queue processing at the right time. r?chunmin

Two problems:

• This was calling the error callback by queueing another task, which isn't what
  the spec says (we're already in a task at this point, the callback needs to be
  called directly).
• The message queue was blocked too late, so the flush message was being
  processed, and this caused the timeout.

Comment 2

[Paul Adenot (:padenot)]

Attachment: Bug 1865376 - Adjust WPT expectations. r?chunmin

Depends on D193923

Comment 3

[Paul Adenot (:padenot)]

Attachment: Bug 1865376 - Make it clear what closing a decoder with an abort error doesn't call the error callback. r?chunmin

Depends on D193923

Comment 4

[Paul Adenot (:padenot)]

Attachment: Bug 1865376 - Don't tolerate failures in CloseInternal. r?chunmin

Depends on D194138

Comment 5

[Paul Adenot (:padenot)]

Attachment: Bug 1865376 - Introduce QueueATask on DecoderTemplate. r?chunmin

When the spec says "Queue a task to do X and Y", there needs to be exactly a
single task doing X and Y without anything possible in between.

This can probably be observed by spamming e.g. the main thread with tasks, and
seeing that two steps aren't atomic.

Depends on D194139

Comment 6

[Paul Adenot (:padenot)]

Attachment: Bug 1865376 - Use QueueATask to run each step precisely per spec. r?chunmin

Sometimes, the promise and something else need to happen in the same steps, etc.
Generally this removes a bunch of indirection and makes things very clear.

This gets rid of various explicit runnables, preferring simple lambdas, and
generally follows a simple rule: things that are conceptually consequences from
an operation (what we have in the Thenable of our PDM MozPromise) queue a single
task, and that does the steps.

I've removed methods called "ScheduleXXXX(...)", preferring clear calls to
QueueATask with a lambda, so it's clear what is going to run. This usually
runs the synchronous version "XXXXInternal(...)", but not always.

Exceptions are:

• ScheduleDequeueEvent (handled later in this series)
• Calling the output callback (also handled later)

This also removes calls to ProcessMessageQueue that shouldn't have been there.

Depends on D194140

Comment 7

[Paul Adenot (:padenot)]

Attachment: Bug 1865376 - Fix const-correctness around output callback. r?chunmin

Simple compilation fix for the previous patch, but orthogonal so I split it out.

Depends on D194141

Comment 8

[Paul Adenot (:padenot)]

Attachment: Bug 1865376 - Simplify and rename ScheduleDequeueEvent. r?chunmin

This makes it clear that it has some logic.

Depends on D194142

Comment 9

[Paul Adenot (:padenot)]

Attachment: Bug 1865376 - Sprinkle annotations to sidestep issues related to MOZ_CAN_RUN_SCRIPT and lambdas. r?chunmin

Depends on D193924

Comment 10

[Paul Adenot (:padenot)]

Attachment: Bug 1865376 - Format after all the lambda changes. r?chunmin

Depends on D194144

Comment 11

[Paul Adenot (:padenot)]

Attachment: Bug 1865376 - Fix warning: don't copy argument. r?chunmin

Depends on D194145

Comment 12

[Paul Adenot (:padenot)]

Attachment: Bug 1865376 - Don't do meaningful work in VideoDecoder's dtor. r?chunmin

We can't have this happen.

Depends on D194869

Comment 13

[BugBot [:suhaib / :marco]]

The severity field is not set for this bug.
:padenot, could you have a look please?

For more information, please visit BugBot documentation.

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/8b5620d3d4bb
Call the error callback synchronously from the configure error task, and block the message queue processing at the right time. r=chunmin
https://hg.mozilla.org/integration/autoland/rev/0da850525973
Make it clear what closing a decoder with an abort error doesn't call the error callback. r=chunmin
https://hg.mozilla.org/integration/autoland/rev/f69dd666d809
Don't tolerate failures in CloseInternal. r=chunmin
https://hg.mozilla.org/integration/autoland/rev/8c58b7ec90c3
Introduce QueueATask on DecoderTemplate. r=chunmin
https://hg.mozilla.org/integration/autoland/rev/506426f7bfe2
Use QueueATask to run each step precisely per spec. r=chunmin
https://hg.mozilla.org/integration/autoland/rev/9295d457356a
Fix const-correctness around output callback. r=chunmin
https://hg.mozilla.org/integration/autoland/rev/ddf76cb4720b
Simplify and rename ScheduleDequeueEvent. r=chunmin
https://hg.mozilla.org/integration/autoland/rev/9a7b12e28ca1
Adjust WPT expectations. r=chunmin
https://hg.mozilla.org/integration/autoland/rev/c3aa798b2c66
Sprinkle annotations to sidestep issues related to MOZ_CAN_RUN_SCRIPT and lambdas. r=chunmin
https://hg.mozilla.org/integration/autoland/rev/995679e8b389
Format after all the lambda changes. r=chunmin
https://hg.mozilla.org/integration/autoland/rev/9da42c9589eb
Fix warning: don't copy argument. r=chunmin
https://hg.mozilla.org/integration/autoland/rev/ea68e617ec80
Don't do meaningful work in VideoDecoder's dtor. r=chunmin
https://hg.mozilla.org/integration/autoland/rev/51d7fee3bc89
apply code formatting via Lando

Comment 15

[Iulian Moraru]

https://hg.mozilla.org/mozilla-central/rev/8b5620d3d4bb
https://hg.mozilla.org/mozilla-central/rev/0da850525973
https://hg.mozilla.org/mozilla-central/rev/f69dd666d809
https://hg.mozilla.org/mozilla-central/rev/8c58b7ec90c3
https://hg.mozilla.org/mozilla-central/rev/506426f7bfe2
https://hg.mozilla.org/mozilla-central/rev/9295d457356a
https://hg.mozilla.org/mozilla-central/rev/ddf76cb4720b
https://hg.mozilla.org/mozilla-central/rev/9a7b12e28ca1
https://hg.mozilla.org/mozilla-central/rev/c3aa798b2c66
https://hg.mozilla.org/mozilla-central/rev/995679e8b389
https://hg.mozilla.org/mozilla-central/rev/9da42c9589eb
https://hg.mozilla.org/mozilla-central/rev/ea68e617ec80
https://hg.mozilla.org/mozilla-central/rev/51d7fee3bc89

Comment 16

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/7584c2f834c2
1881079: apply code formatting via Lando

Comment 17

[Serban Stanca [:SerbanS]]

https://hg.mozilla.org/mozilla-central/rev/7584c2f834c2