1273070 - Failed to fetch in subworker

Status: RESOLVED FIXED

(Core :: DOM: Workers, defect)

Description

[Thomas Nguyen (:tnguyen)]

When attempting to add some test cases, I found that fetch API in subworker causes a crash in mochitest.
I attached log and my test related to that.

Comment 1

[Thomas Nguyen (:tnguyen)]

Attachment: Call fetch in subworker

Comment 2

[Thomas Nguyen (:tnguyen)]

Attachment: Log

Comment 3

[Ben Kelly [:bkelly, not reviewing]]

Kyle,

WorkerFetchResolve::OnRespondAvailable() gets calls on the main thread:

https://dxr.mozilla.org/mozilla-central/source/dom/fetch/Fetch.cpp#370

It attempts to dispatch a WorkerRunnable back to the original worker that performed the fetch().

It seems, however, that we don't allow main thread to dispatch to a nested worker thread?  We end up triggering this assert:

https://dxr.mozilla.org/mozilla-central/source/dom/workers/WorkerPrivate.cpp#3658

Without a parent we assert main thread, but otherwise we try to assert the parent worker thread.

What is the correct way to structure this?  Do we really have to bounce a runnable through every worker parent?  That seems... complicated.

Comment 4

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

You need to override the Pre/PostDispatch assertions like so http://mxr.mozilla.org/mozilla-central/source/dom/base/WebSocket.cpp?force=1#2576

You also need to make sure that the WorkerPrivate is guaranteed to still be alive at this point.

Comment 5

[Ben Kelly [:bkelly, not reviewing]]

We hold a WorkerFeature for the nested worker.  Is that enough?  Or do we need to hold the parent alive explicitly as well?

Comment 6

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

That is enough.

Comment 7

[Ben Kelly [:bkelly, not reviewing]]

Attachment: P2 Execute fetch() mochitests in nested workers. r=khuey

This patch extends the test framework in dom/tests/mochitest/fetch to execute test cases in nested workers.  This easily triggered the reported assertion.  It also ensures we get a lot of test coverage in nested workers.

Comment 8

[Ben Kelly [:bkelly, not reviewing]]

Attachment: P1 Fix fetch() WorkerRunnable classes to allow it to work with nested Workers. r=khuey

This patch allows us to pass the test, but is not 100% correct yet.  We currently hold a WorkerFeature during body consumption, but not during the actual network request itself.

Comment 9

[Ben Kelly [:bkelly, not reviewing]]

Comment on attachment 8752901 [details] [diff] [review]
P1 Fix fetch() WorkerRunnable classes to allow it to work with nested Workers. r=khuey

Actually, we are holding a WorkerFeature during the network operation via the PromiseWorkerProxy.  So I believe this patch is complete.

I had to fix a number of worker runnables here.  Also, I had to drop the modification of the busy count since that performed the same parent thread assertion.  I think this is ok since we are holding the feature.

Comment 10

[Ben Kelly [:bkelly, not reviewing]]

Comment on attachment 8752894 [details] [diff] [review]
P2 Execute fetch() mochitests in nested workers. r=khuey

Review of attachment 8752894 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/tests/mochitest/fetch/nested_worker_wrapper.js
@@ +13,5 @@
> +  });
> +
> +  worker.addEventListener('error', function(evt) {
> +    self.postMessage({
> +      context: 'NextedWorker',

I fixed this typo locally.

Comment 11

[Ben Kelly [:bkelly, not reviewing]]

This is the try build I did for this:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=721b03d6e55f&selectedJob=20894206

Note, jmaher tells me that the win7 failures are due to clipboard issues with aws test runners.

Comment 12

[Ben Kelly [:bkelly, not reviewing]]

Pulsebot is resting...

remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/5733b66fdedf
remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/00218374a90c

Comment 13

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/5733b66fdedf
https://hg.mozilla.org/mozilla-central/rev/00218374a90c